Skip to content

Commit 68f56b7

Browse files
mlsmayconmlsmaycon
authored
[management] Add native ssh port rule on 22 (#4810)
Implements feature-aware firewall rule expansion: derives peer-supported features (native SSH, portRanges) from peer version, prefers explicit Ports over PortRanges when expanding, conditionally appends a native SSH (22022) rule when policy and peer support allow, and adds helpers plus tests for SSH expansion behavior.
1 parent 3351b38 commit 68f56b7

File tree

2 files changed

+340
-14
lines changed

2 files changed

+340
-14
lines changed

‎management/server/types/account.go‎

Lines changed: 74 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -40,8 +40,20 @@ const (
4040

4141
// firewallRuleMinPortRangesVer defines the minimum peer version that supports port range rules.
4242
firewallRuleMinPortRangesVer = "0.48.0"
43+
// firewallRuleMinNativeSSHVer defines the minimum peer version that supports native SSH features in the firewall rules.
44+
firewallRuleMinNativeSSHVer = "0.60.0"
45+
46+
// nativeSSHPortString defines the default port number as a string used for native SSH connections; this port is used by clients when hijacking ssh connections.
47+
nativeSSHPortString = "22022"
48+
// defaultSSHPortString defines the standard SSH port number as a string, commonly used for default SSH connections.
49+
defaultSSHPortString = "22"
4350
)
4451

52+
type supportedFeatures struct {
53+
nativeSSH bool
54+
portRanges bool
55+
}
56+
4557
type LookupMap map[string]struct{}
4658

4759
// AccountMeta is a struct that contains a stripped down version of the Account object.
@@ -1650,22 +1662,24 @@ func (a *Account) AddAllGroup(disableDefaultPolicy bool) error {
16501662

16511663
// expandPortsAndRanges expands Ports and PortRanges of a rule into individual firewall rules
16521664
func expandPortsAndRanges(base FirewallRule, rule *PolicyRule, peer *nbpeer.Peer) []*FirewallRule {
1665+
features := peerSupportedFirewallFeatures(peer.Meta.WtVersion)
1666+
16531667
var expanded []*FirewallRule
16541668

1655-
if len(rule.Ports) > 0 {
1656-
for _, port := range rule.Ports {
1657-
fr := base
1658-
fr.Port = port
1659-
expanded = append(expanded, &fr)
1660-
}
1661-
return expanded
1669+
for _, port := range rule.Ports {
1670+
fr := base
1671+
fr.Port = port
1672+
expanded = append(expanded, &fr)
16621673
}
16631674

1664-
supportPortRanges := peerSupportsPortRanges(peer.Meta.WtVersion)
16651675
for _, portRange := range rule.PortRanges {
1676+
// prefer PolicyRule.Ports
1677+
if len(rule.Ports) > 0 {
1678+
break
1679+
}
16661680
fr := base
16671681

1668-
if supportPortRanges {
1682+
if features.portRanges {
16691683
fr.PortRange = portRange
16701684
} else {
16711685
// Peer doesn't support port ranges, only allow single-port ranges
@@ -1677,17 +1691,63 @@ func expandPortsAndRanges(base FirewallRule, rule *PolicyRule, peer *nbpeer.Peer
16771691
expanded = append(expanded, &fr)
16781692
}
16791693

1694+
if shouldCheckRulesForNativeSSH(features.nativeSSH, rule, peer) {
1695+
expanded = addNativeSSHRule(base, expanded)
1696+
}
1697+
16801698
return expanded
16811699
}
16821700

1683-
// peerSupportsPortRanges checks if the peer version supports port ranges.
1684-
func peerSupportsPortRanges(peerVer string) bool {
1701+
// addNativeSSHRule adds a native SSH rule (port 22022) to the expanded rules if the base rule has port 22 configured.
1702+
func addNativeSSHRule(base FirewallRule, expanded []*FirewallRule) []*FirewallRule {
1703+
shouldAdd := false
1704+
for _, fr := range expanded {
1705+
if isPortInRule(nativeSSHPortString, 22022, fr) {
1706+
return expanded
1707+
}
1708+
if isPortInRule(defaultSSHPortString, 22, fr) {
1709+
shouldAdd = true
1710+
}
1711+
}
1712+
if !shouldAdd {
1713+
return expanded
1714+
}
1715+
1716+
fr := base
1717+
fr.Port = nativeSSHPortString
1718+
return append(expanded, &fr)
1719+
}
1720+
1721+
func isPortInRule(portString string, portInt uint16, rule *FirewallRule) bool {
1722+
return rule.Port == portString || (rule.PortRange.Start <= portInt && portInt <= rule.PortRange.End)
1723+
}
1724+
1725+
// shouldCheckRulesForNativeSSH determines whether specific policy rules should be checked for native SSH support.
1726+
// While users can add the nativeSSHPortString, we look for cases when they used port 22 and based on SSH enabled
1727+
// in both management and client, we indicate to add the native port.
1728+
func shouldCheckRulesForNativeSSH(supportsNative bool, rule *PolicyRule, peer *nbpeer.Peer) bool {
1729+
return supportsNative && peer.SSHEnabled && peer.Meta.Flags.ServerSSHAllowed && rule.Protocol == PolicyRuleProtocolTCP
1730+
}
1731+
1732+
// peerSupportedFirewallFeatures checks if the peer version supports port ranges.
1733+
func peerSupportedFirewallFeatures(peerVer string) supportedFeatures {
16851734
if strings.Contains(peerVer, "dev") {
1686-
return true
1735+
return supportedFeatures{true, true}
1736+
}
1737+
1738+
var features supportedFeatures
1739+
1740+
meetMinVer, err := posture.MeetsMinVersion(firewallRuleMinNativeSSHVer, peerVer)
1741+
features.nativeSSH = err == nil && meetMinVer
1742+
1743+
if features.nativeSSH {
1744+
features.portRanges = true
1745+
} else {
1746+
meetMinVer, err = posture.MeetsMinVersion(firewallRuleMinPortRangesVer, peerVer)
1747+
features.portRanges = err == nil && meetMinVer
16871748
}
16881749

1689-
meetMinVer, err := posture.MeetsMinVersion(firewallRuleMinPortRangesVer, peerVer)
1690-
return err == nil && meetMinVer
1750+
return features
16911751
}
16921752

16931753
// filterZoneRecordsForPeers filters DNS records to only include peers to connect.

‎management/server/types/account_test.go‎

Lines changed: 266 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -839,6 +839,272 @@ func Test_NetworksNetMapGenShouldExcludeOtherRouters(t *testing.T) {
839839
assert.Len(t, sourcePeers, 2, "expected source peers don't match")
840840
}
841841

842+
func Test_ExpandPortsAndRanges_SSHRuleExpansion(t *testing.T) {
843+
tests := []struct {
844+
name string
845+
peer *nbpeer.Peer
846+
rule *PolicyRule
847+
base FirewallRule
848+
expectedPorts []string
849+
}{
850+
{
851+
name: "adds port 22022 when SSH enabled on modern peer with port 22",
852+
peer: &nbpeer.Peer{
853+
ID: "peer1",
854+
SSHEnabled: true,
855+
Meta: nbpeer.PeerSystemMeta{
856+
WtVersion: "0.60.0",
857+
Flags: nbpeer.Flags{ServerSSHAllowed: true},
858+
},
859+
},
860+
rule: &PolicyRule{
861+
Protocol: PolicyRuleProtocolTCP,
862+
Ports: []string{"22"},
863+
},
864+
base: FirewallRule{PeerIP: "10.0.0.1", Direction: 0, Action: "accept", Protocol: "tcp"},
865+
expectedPorts: []string{"22", "22022"},
866+
},
867+
{
868+
name: "adds port 22022 once when port 22 is duplicated within policy",
869+
peer: &nbpeer.Peer{
870+
ID: "peer1",
871+
SSHEnabled: true,
872+
Meta: nbpeer.PeerSystemMeta{
873+
WtVersion: "0.60.0",
874+
Flags: nbpeer.Flags{ServerSSHAllowed: true},
875+
},
876+
},
877+
rule: &PolicyRule{
878+
Protocol: PolicyRuleProtocolTCP,
879+
Ports: []string{"22", "80", "22"},
880+
},
881+
base: FirewallRule{PeerIP: "10.0.0.1", Direction: 0, Action: "accept", Protocol: "tcp"},
882+
expectedPorts: []string{"22", "80", "22", "22022"},
883+
},
884+
{
885+
name: "does not add 22022 for peer with old version",
886+
peer: &nbpeer.Peer{
887+
ID: "peer1",
888+
SSHEnabled: true,
889+
Meta: nbpeer.PeerSystemMeta{
890+
WtVersion: "0.59.0",
891+
Flags: nbpeer.Flags{ServerSSHAllowed: true},
892+
},
893+
},
894+
rule: &PolicyRule{
895+
Protocol: PolicyRuleProtocolTCP,
896+
Ports: []string{"22"},
897+
},
898+
base: FirewallRule{PeerIP: "10.0.0.1", Direction: 0, Action: "accept", Protocol: "tcp"},
899+
expectedPorts: []string{"22"},
900+
},
901+
{
902+
name: "does not add 22022 when SSHEnabled is false",
903+
peer: &nbpeer.Peer{
904+
ID: "peer1",
905+
SSHEnabled: false,
906+
Meta: nbpeer.PeerSystemMeta{
907+
WtVersion: "0.60.0",
908+
Flags: nbpeer.Flags{ServerSSHAllowed: true},
909+
},
910+
},
911+
rule: &PolicyRule{
912+
Protocol: PolicyRuleProtocolTCP,
913+
Ports: []string{"22"},
914+
},
915+
base: FirewallRule{PeerIP: "10.0.0.1", Direction: 0, Action: "accept", Protocol: "tcp"},
916+
expectedPorts: []string{"22"},
917+
},
918+
{
919+
name: "does not add 22022 when ServerSSHAllowed is false",
920+
peer: &nbpeer.Peer{
921+
ID: "peer1",
922+
SSHEnabled: true,
923+
Meta: nbpeer.PeerSystemMeta{
924+
WtVersion: "0.60.0",
925+
Flags: nbpeer.Flags{ServerSSHAllowed: false},
926+
},
927+
},
928+
rule: &PolicyRule{
929+
Protocol: PolicyRuleProtocolTCP,
930+
Ports: []string{"22"},
931+
},
932+
base: FirewallRule{PeerIP: "10.0.0.1", Direction: 0, Action: "accept", Protocol: "tcp"},
933+
expectedPorts: []string{"22"},
934+
},
935+
{
936+
name: "does not add 22022 for UDP protocol",
937+
peer: &nbpeer.Peer{
938+
ID: "peer1",
939+
SSHEnabled: true,
940+
Meta: nbpeer.PeerSystemMeta{
941+
WtVersion: "0.60.0",
942+
Flags: nbpeer.Flags{ServerSSHAllowed: true},
943+
},
944+
},
945+
rule: &PolicyRule{
946+
Protocol: PolicyRuleProtocolUDP,
947+
Ports: []string{"22"},
948+
},
949+
base: FirewallRule{PeerIP: "10.0.0.1", Direction: 0, Action: "accept", Protocol: "udp"},
950+
expectedPorts: []string{"22"},
951+
},
952+
{
953+
name: "does not add 22022 when port 22 not in rule",
954+
peer: &nbpeer.Peer{
955+
ID: "peer1",
956+
SSHEnabled: true,
957+
Meta: nbpeer.PeerSystemMeta{
958+
WtVersion: "0.60.0",
959+
Flags: nbpeer.Flags{ServerSSHAllowed: true},
960+
},
961+
},
962+
rule: &PolicyRule{
963+
Protocol: PolicyRuleProtocolTCP,
964+
Ports: []string{"80", "443"},
965+
},
966+
base: FirewallRule{PeerIP: "10.0.0.1", Direction: 0, Action: "accept", Protocol: "tcp"},
967+
expectedPorts: []string{"80", "443"},
968+
},
969+
{
970+
name: "does not duplicate 22022 when already present",
971+
peer: &nbpeer.Peer{
972+
ID: "peer1",
973+
SSHEnabled: true,
974+
Meta: nbpeer.PeerSystemMeta{
975+
WtVersion: "0.60.0",
976+
Flags: nbpeer.Flags{ServerSSHAllowed: true},
977+
},
978+
},
979+
rule: &PolicyRule{
980+
Protocol: PolicyRuleProtocolTCP,
981+
Ports: []string{"22", "22022"},
982+
},
983+
base: FirewallRule{PeerIP: "10.0.0.1", Direction: 0, Action: "accept", Protocol: "tcp"},
984+
expectedPorts: []string{"22", "22022"},
985+
},
986+
{
987+
name: "does not duplicate 22022 when already within a port range",
988+
peer: &nbpeer.Peer{
989+
ID: "peer1",
990+
SSHEnabled: true,
991+
Meta: nbpeer.PeerSystemMeta{
992+
WtVersion: "0.60.0",
993+
Flags: nbpeer.Flags{ServerSSHAllowed: true},
994+
},
995+
},
996+
rule: &PolicyRule{
997+
Protocol: PolicyRuleProtocolTCP,
998+
PortRanges: []RulePortRange{{Start: 20, End: 32000}},
999+
},
1000+
base: FirewallRule{PeerIP: "10.0.0.1", Direction: 0, Action: "accept", Protocol: "tcp"},
1001+
expectedPorts: []string{"20-32000"},
1002+
},
1003+
{
1004+
name: "adds 22022 when port 22 in port range",
1005+
peer: &nbpeer.Peer{
1006+
ID: "peer1",
1007+
SSHEnabled: true,
1008+
Meta: nbpeer.PeerSystemMeta{
1009+
WtVersion: "0.60.0",
1010+
Flags: nbpeer.Flags{ServerSSHAllowed: true},
1011+
},
1012+
},
1013+
rule: &PolicyRule{
1014+
Protocol: PolicyRuleProtocolTCP,
1015+
PortRanges: []RulePortRange{{Start: 20, End: 25}},
1016+
},
1017+
base: FirewallRule{PeerIP: "10.0.0.1", Direction: 0, Action: "accept", Protocol: "tcp"},
1018+
expectedPorts: []string{"20-25", "22022"},
1019+
},
1020+
{
1021+
name: "adds single 22022 once when port 22 in multiple port ranges",
1022+
peer: &nbpeer.Peer{
1023+
ID: "peer1",
1024+
SSHEnabled: true,
1025+
Meta: nbpeer.PeerSystemMeta{
1026+
WtVersion: "0.60.0",
1027+
Flags: nbpeer.Flags{ServerSSHAllowed: true},
1028+
},
1029+
},
1030+
rule: &PolicyRule{
1031+
Protocol: PolicyRuleProtocolTCP,
1032+
PortRanges: []RulePortRange{{Start: 20, End: 25}, {Start: 10, End: 100}},
1033+
},
1034+
base: FirewallRule{PeerIP: "10.0.0.1", Direction: 0, Action: "accept", Protocol: "tcp"},
1035+
expectedPorts: []string{"20-25", "10-100", "22022"},
1036+
},
1037+
{
1038+
name: "dev suffix version supports all features",
1039+
peer: &nbpeer.Peer{
1040+
ID: "peer1",
1041+
SSHEnabled: true,
1042+
Meta: nbpeer.PeerSystemMeta{
1043+
WtVersion: "0.50.0-dev",
1044+
Flags: nbpeer.Flags{ServerSSHAllowed: true},
1045+
},
1046+
},
1047+
rule: &PolicyRule{
1048+
Protocol: PolicyRuleProtocolTCP,
1049+
Ports: []string{"22"},
1050+
},
1051+
base: FirewallRule{PeerIP: "10.0.0.1", Direction: 0, Action: "accept", Protocol: "tcp"},
1052+
expectedPorts: []string{"22", "22022"},
1053+
},
1054+
{
1055+
name: "dev suffix version supports all features",
1056+
peer: &nbpeer.Peer{
1057+
ID: "peer1",
1058+
SSHEnabled: true,
1059+
Meta: nbpeer.PeerSystemMeta{
1060+
WtVersion: "dev",
1061+
Flags: nbpeer.Flags{ServerSSHAllowed: true},
1062+
},
1063+
},
1064+
rule: &PolicyRule{
1065+
Protocol: PolicyRuleProtocolTCP,
1066+
Ports: []string{"22"},
1067+
},
1068+
base: FirewallRule{PeerIP: "10.0.0.1", Direction: 0, Action: "accept", Protocol: "tcp"},
1069+
expectedPorts: []string{"22", "22022"},
1070+
},
1071+
{
1072+
name: "development suffix version supports all features",
1073+
peer: &nbpeer.Peer{
1074+
ID: "peer1",
1075+
SSHEnabled: true,
1076+
Meta: nbpeer.PeerSystemMeta{
1077+
WtVersion: "development",
1078+
Flags: nbpeer.Flags{ServerSSHAllowed: true},
1079+
},
1080+
},
1081+
rule: &PolicyRule{
1082+
Protocol: PolicyRuleProtocolTCP,
1083+
Ports: []string{"22"},
1084+
},
1085+
base: FirewallRule{PeerIP: "10.0.0.1", Direction: 0, Action: "accept", Protocol: "tcp"},
1086+
expectedPorts: []string{"22", "22022"},
1087+
},
1088+
}
1089+
1090+
for _, tt := range tests {
1091+
t.Run(tt.name, func(t *testing.T) {
1092+
result := expandPortsAndRanges(tt.base, tt.rule, tt.peer)
1093+
1094+
var ports []string
1095+
for _, fr := range result {
1096+
if fr.Port != "" {
1097+
ports = append(ports, fr.Port)
1098+
} else if fr.PortRange.Start > 0 {
1099+
ports = append(ports, fmt.Sprintf("%d-%d", fr.PortRange.Start, fr.PortRange.End))
1100+
}
1101+
}
1102+
1103+
assert.Equal(t, tt.expectedPorts, ports, "expanded ports should match expected")
1104+
})
1105+
}
1106+
}
1107+
8421108
func Test_FilterZoneRecordsForPeers(t *testing.T) {
8431109
tests := []struct {
8441110
name string

0 commit comments

Comments
 (0)