Skip to content

[management] Add native ssh port rule on 22 #4810

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Nov 19, 2025
Merged

[management] Add native ssh port rule on 22 #4810

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
ef8468e
add native ssh required port when default is in the rule
mlsmaycon Nov 19, 2025
2533a2b
Add tests for native SSH rule expansion logic
mlsmaycon Nov 19, 2025
39ee515
if native ssh is supported, avoid the extra version check
mlsmaycon Nov 19, 2025
41aef79
Add unit tests for expanded native SSH rule handling
mlsmaycon Nov 19, 2025
76a8d55
Refactor native SSH rule handling
mlsmaycon Nov 19, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
88 changes: 74 additions & 14 deletions management/server/types/account.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,8 +40,20 @@ const (

// firewallRuleMinPortRangesVer defines the minimum peer version that supports port range rules.
firewallRuleMinPortRangesVer = "0.48.0"
// firewallRuleMinNativeSSHVer defines the minimum peer version that supports native SSH features in the firewall rules.
firewallRuleMinNativeSSHVer = "0.60.0"

// nativeSSHPortString defines the default port number as a string used for native SSH connections; this port is used by clients when hijacking ssh connections.
nativeSSHPortString = "22022"
// defaultSSHPortString defines the standard SSH port number as a string, commonly used for default SSH connections.
defaultSSHPortString = "22"
)

type supportedFeatures struct {
nativeSSH bool
portRanges bool
}

type LookupMap map[string]struct{}

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

// expandPortsAndRanges expands Ports and PortRanges of a rule into individual firewall rules
func expandPortsAndRanges(base FirewallRule, rule *PolicyRule, peer *nbpeer.Peer) []*FirewallRule {
features := peerSupportedFirewallFeatures(peer.Meta.WtVersion)

var expanded []*FirewallRule

if len(rule.Ports) > 0 {
for _, port := range rule.Ports {
fr := base
fr.Port = port
expanded = append(expanded, &fr)
}
return expanded
for _, port := range rule.Ports {
fr := base
fr.Port = port
expanded = append(expanded, &fr)
}

supportPortRanges := peerSupportsPortRanges(peer.Meta.WtVersion)
for _, portRange := range rule.PortRanges {
// prefer PolicyRule.Ports
if len(rule.Ports) > 0 {
break
}
fr := base

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

if shouldCheckRulesForNativeSSH(features.nativeSSH, rule, peer) {
expanded = addNativeSSHRule(base, expanded)
}

return expanded
}

// peerSupportsPortRanges checks if the peer version supports port ranges.
func peerSupportsPortRanges(peerVer string) bool {
// addNativeSSHRule adds a native SSH rule (port 22022) to the expanded rules if the base rule has port 22 configured.
func addNativeSSHRule(base FirewallRule, expanded []*FirewallRule) []*FirewallRule {
shouldAdd := false
for _, fr := range expanded {
if isPortInRule(nativeSSHPortString, 22022, fr) {
return expanded
}
if isPortInRule(defaultSSHPortString, 22, fr) {
shouldAdd = true
}
}
if !shouldAdd {
return expanded
}

fr := base
fr.Port = nativeSSHPortString
return append(expanded, &fr)
}

func isPortInRule(portString string, portInt uint16, rule *FirewallRule) bool {
return rule.Port == portString || (rule.PortRange.Start <= portInt && portInt <= rule.PortRange.End)
}

// shouldCheckRulesForNativeSSH determines whether specific policy rules should be checked for native SSH support.
// While users can add the nativeSSHPortString, we look for cases when they used port 22 and based on SSH enabled
// in both management and client, we indicate to add the native port.
func shouldCheckRulesForNativeSSH(supportsNative bool, rule *PolicyRule, peer *nbpeer.Peer) bool {
return supportsNative && peer.SSHEnabled && peer.Meta.Flags.ServerSSHAllowed && rule.Protocol == PolicyRuleProtocolTCP
}
Comment on lines +1725 to +1730
Copy link
Contributor

@coderabbitai coderabbitai bot Nov 19, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

Consider including PolicyRuleProtocolALL in protocol check.

The function currently only checks for TCP protocol, but SSH operates over TCP and policy rules with Protocol=ALL should also support native SSH since ALL encompasses TCP.

Should the condition be:

rule.Protocol == PolicyRuleProtocolTCP || rule.Protocol == PolicyRuleProtocolALL

This would ensure native SSH rules are added for ALL-protocol policies.

🏁 Script executed:

#!/bin/bash
# Search for how PolicyRuleProtocolALL is defined and used
ast-grep --pattern 'PolicyRuleProtocolALL = PolicyRuleProtocolType($_)'

# Search for tests covering native SSH with ALL protocol
rg -n -C3 'PolicyRuleProtocolALL.*SSH|SSH.*PolicyRuleProtocolALL' management/server/types/

Length of output: 0

🏁 Script executed:

#!/bin/bash
# Find protocol constant definitions
rg -n "PolicyRuleProtocol" management/server/types/account.go | head -20

# Find shouldCheckRulesForNativeSSH usage/calls
rg -n "shouldCheckRulesForNativeSSH" management/server/

# Look for protocol handling patterns in the same file
rg -n "rule\.Protocol ==" management/server/types/account.go | head -10

Length of output: 750

🏁 Script executed:

#!/bin/bash
# Find where PolicyRuleProtocol constants are defined
rg -n "const.*PolicyRuleProtocol|type.*PolicyRuleProtocol" management/server/types/

# Find protocol check patterns to see how ALL is typically handled
rg -n "Protocol.*ALL|ALL.*Protocol" management/server/types/account.go -B2 -A2

# Get context around line 1694 to see how shouldCheckRulesForNativeSSH is used
sed -n '1690,1700p' management/server/types/account.go

Length of output: 1176

🏁 Script executed:

#!/bin/bash
# Find constant definitions for PolicyRuleProtocol types
cat management/server/types/policyrule.go | grep -A 20 "type PolicyRuleProtocolType"

# Search for protocol matching logic to understand how ALL is handled
rg -n "Protocol.*==" management/server/types/ -A 1 -B 1 | head -40

# Check if there are utility functions for protocol matching
rg -n "func.*Protocol|Protocol.*Match" management/server/types/account.go

Length of output: 1506

🏁 Script executed:

#!/bin/bash
# Find const block with protocol values
rg -n "const.*=" management/server/types/policyrule.go | head -20

# Look for ALL protocol definition
rg -n "PolicyRuleProtocolALL|PolicyRuleProtocolTCP" management/server/types/policyrule.go

# Check test for native SSH to understand expected behavior
rg -n -B5 -A5 "addNativeSSHRule|nativeSSH" management/server/types/account.go | head -50

Length of output: 1718

🏁 Script executed:

#!/bin/bash
# Get full policyrule.go file to find const blocks
cat management/server/types/policyrule.go | head -100

# Look for protocol constant definitions throughout types package
rg -n "PolicyRuleProtocol.*=" management/server/types/ | grep -v "//"

Length of output: 3148

🏁 Script executed:

#!/bin/bash
# Search for tests covering native SSH scenarios
rg -n "nativeSSH|addNativeSSHRule" management/server/types/ --type go -g "*test*"

# Check if there are any tests with PolicyRuleProtocolALL in context of SSH
rg -n "PolicyRuleProtocolALL" management/server/types/ --type go -g "*test*" -B3 -A3

Length of output: 1452

Include PolicyRuleProtocolALL in protocol check for native SSH support.

The function currently only checks rule.Protocol == PolicyRuleProtocolTCP, but policies with Protocol = PolicyRuleProtocolALL should also support native SSH since "all" encompasses TCP. This causes native SSH rules to be silently skipped for all-protocol policies.

The fix should be:

rule.Protocol == PolicyRuleProtocolTCP || rule.Protocol == PolicyRuleProtocolALL

This ensures native SSH port expansion (22 → 22022) works for policies with protocol="all", which is used in production scenarios (as seen in test fixtures).

🤖 Prompt for AI Agents 
In management/server/types/account.go around lines 1725 to 1730, the protocol
check in shouldCheckRulesForNativeSSH only allows PolicyRuleProtocolTCP so rules
with Protocol set to PolicyRuleProtocolALL are skipped; update the boolean
condition to accept both TCP and ALL (i.e., treat ALL as encompassing TCP) so
native SSH port expansion runs for all-protocol policies, ensuring the function
returns true when supportsNative is true, peer SSH is enabled, ServerSSHAllowed
flag is set, and the rule protocol is either TCP or ALL.


// peerSupportedFirewallFeatures checks if the peer version supports port ranges.
func peerSupportedFirewallFeatures(peerVer string) supportedFeatures {
if strings.Contains(peerVer, "dev") {
return true
return supportedFeatures{true, true}
}

var features supportedFeatures

meetMinVer, err := posture.MeetsMinVersion(firewallRuleMinNativeSSHVer, peerVer)
features.nativeSSH = err == nil && meetMinVer

if features.nativeSSH {
features.portRanges = true
} else {
meetMinVer, err = posture.MeetsMinVersion(firewallRuleMinPortRangesVer, peerVer)
features.portRanges = err == nil && meetMinVer
}

meetMinVer, err := posture.MeetsMinVersion(firewallRuleMinPortRangesVer, peerVer)
return err == nil && meetMinVer
return features
}

// filterZoneRecordsForPeers filters DNS records to only include peers to connect.
Expand Down
266 changes: 266 additions & 0 deletions management/server/types/account_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -839,6 +839,272 @@ func Test_NetworksNetMapGenShouldExcludeOtherRouters(t *testing.T) {
assert.Len(t, sourcePeers, 2, "expected source peers don't match")
}

func Test_ExpandPortsAndRanges_SSHRuleExpansion(t *testing.T) {
tests := []struct {
name string
peer *nbpeer.Peer
rule *PolicyRule
base FirewallRule
expectedPorts []string
}{
{
name: "adds port 22022 when SSH enabled on modern peer with port 22",
peer: &nbpeer.Peer{
ID: "peer1",
SSHEnabled: true,
Meta: nbpeer.PeerSystemMeta{
WtVersion: "0.60.0",
Flags: nbpeer.Flags{ServerSSHAllowed: true},
},
},
rule: &PolicyRule{
Protocol: PolicyRuleProtocolTCP,
Ports: []string{"22"},
},
base: FirewallRule{PeerIP: "10.0.0.1", Direction: 0, Action: "accept", Protocol: "tcp"},
expectedPorts: []string{"22", "22022"},
},
{
name: "adds port 22022 once when port 22 is duplicated within policy",
peer: &nbpeer.Peer{
ID: "peer1",
SSHEnabled: true,
Meta: nbpeer.PeerSystemMeta{
WtVersion: "0.60.0",
Flags: nbpeer.Flags{ServerSSHAllowed: true},
},
},
rule: &PolicyRule{
Protocol: PolicyRuleProtocolTCP,
Ports: []string{"22", "80", "22"},
},
base: FirewallRule{PeerIP: "10.0.0.1", Direction: 0, Action: "accept", Protocol: "tcp"},
expectedPorts: []string{"22", "80", "22", "22022"},
},
{
name: "does not add 22022 for peer with old version",
peer: &nbpeer.Peer{
ID: "peer1",
SSHEnabled: true,
Meta: nbpeer.PeerSystemMeta{
WtVersion: "0.59.0",
Flags: nbpeer.Flags{ServerSSHAllowed: true},
},
},
rule: &PolicyRule{
Protocol: PolicyRuleProtocolTCP,
Ports: []string{"22"},
},
base: FirewallRule{PeerIP: "10.0.0.1", Direction: 0, Action: "accept", Protocol: "tcp"},
expectedPorts: []string{"22"},
},
{
name: "does not add 22022 when SSHEnabled is false",
peer: &nbpeer.Peer{
ID: "peer1",
SSHEnabled: false,
Meta: nbpeer.PeerSystemMeta{
WtVersion: "0.60.0",
Flags: nbpeer.Flags{ServerSSHAllowed: true},
},
},
rule: &PolicyRule{
Protocol: PolicyRuleProtocolTCP,
Ports: []string{"22"},
},
base: FirewallRule{PeerIP: "10.0.0.1", Direction: 0, Action: "accept", Protocol: "tcp"},
expectedPorts: []string{"22"},
},
{
name: "does not add 22022 when ServerSSHAllowed is false",
peer: &nbpeer.Peer{
ID: "peer1",
SSHEnabled: true,
Meta: nbpeer.PeerSystemMeta{
WtVersion: "0.60.0",
Flags: nbpeer.Flags{ServerSSHAllowed: false},
},
},
rule: &PolicyRule{
Protocol: PolicyRuleProtocolTCP,
Ports: []string{"22"},
},
base: FirewallRule{PeerIP: "10.0.0.1", Direction: 0, Action: "accept", Protocol: "tcp"},
expectedPorts: []string{"22"},
},
{
name: "does not add 22022 for UDP protocol",
peer: &nbpeer.Peer{
ID: "peer1",
SSHEnabled: true,
Meta: nbpeer.PeerSystemMeta{
WtVersion: "0.60.0",
Flags: nbpeer.Flags{ServerSSHAllowed: true},
},
},
rule: &PolicyRule{
Protocol: PolicyRuleProtocolUDP,
Ports: []string{"22"},
},
base: FirewallRule{PeerIP: "10.0.0.1", Direction: 0, Action: "accept", Protocol: "udp"},
expectedPorts: []string{"22"},
},
{
name: "does not add 22022 when port 22 not in rule",
peer: &nbpeer.Peer{
ID: "peer1",
SSHEnabled: true,
Meta: nbpeer.PeerSystemMeta{
WtVersion: "0.60.0",
Flags: nbpeer.Flags{ServerSSHAllowed: true},
},
},
rule: &PolicyRule{
Protocol: PolicyRuleProtocolTCP,
Ports: []string{"80", "443"},
},
base: FirewallRule{PeerIP: "10.0.0.1", Direction: 0, Action: "accept", Protocol: "tcp"},
expectedPorts: []string{"80", "443"},
},
{
name: "does not duplicate 22022 when already present",
peer: &nbpeer.Peer{
ID: "peer1",
SSHEnabled: true,
Meta: nbpeer.PeerSystemMeta{
WtVersion: "0.60.0",
Flags: nbpeer.Flags{ServerSSHAllowed: true},
},
},
rule: &PolicyRule{
Protocol: PolicyRuleProtocolTCP,
Ports: []string{"22", "22022"},
},
base: FirewallRule{PeerIP: "10.0.0.1", Direction: 0, Action: "accept", Protocol: "tcp"},
expectedPorts: []string{"22", "22022"},
},
{
name: "does not duplicate 22022 when already within a port range",
peer: &nbpeer.Peer{
ID: "peer1",
SSHEnabled: true,
Meta: nbpeer.PeerSystemMeta{
WtVersion: "0.60.0",
Flags: nbpeer.Flags{ServerSSHAllowed: true},
},
},
rule: &PolicyRule{
Protocol: PolicyRuleProtocolTCP,
PortRanges: []RulePortRange{{Start: 20, End: 32000}},
},
base: FirewallRule{PeerIP: "10.0.0.1", Direction: 0, Action: "accept", Protocol: "tcp"},
expectedPorts: []string{"20-32000"},
},
{
name: "adds 22022 when port 22 in port range",
peer: &nbpeer.Peer{
ID: "peer1",
SSHEnabled: true,
Meta: nbpeer.PeerSystemMeta{
WtVersion: "0.60.0",
Flags: nbpeer.Flags{ServerSSHAllowed: true},
},
},
rule: &PolicyRule{
Protocol: PolicyRuleProtocolTCP,
PortRanges: []RulePortRange{{Start: 20, End: 25}},
},
base: FirewallRule{PeerIP: "10.0.0.1", Direction: 0, Action: "accept", Protocol: "tcp"},
expectedPorts: []string{"20-25", "22022"},
},
{
name: "adds single 22022 once when port 22 in multiple port ranges",
peer: &nbpeer.Peer{
ID: "peer1",
SSHEnabled: true,
Meta: nbpeer.PeerSystemMeta{
WtVersion: "0.60.0",
Flags: nbpeer.Flags{ServerSSHAllowed: true},
},
},
rule: &PolicyRule{
Protocol: PolicyRuleProtocolTCP,
PortRanges: []RulePortRange{{Start: 20, End: 25}, {Start: 10, End: 100}},
},
base: FirewallRule{PeerIP: "10.0.0.1", Direction: 0, Action: "accept", Protocol: "tcp"},
expectedPorts: []string{"20-25", "10-100", "22022"},
},
{
name: "dev suffix version supports all features",
peer: &nbpeer.Peer{
ID: "peer1",
SSHEnabled: true,
Meta: nbpeer.PeerSystemMeta{
WtVersion: "0.50.0-dev",
Flags: nbpeer.Flags{ServerSSHAllowed: true},
},
},
rule: &PolicyRule{
Protocol: PolicyRuleProtocolTCP,
Ports: []string{"22"},
},
base: FirewallRule{PeerIP: "10.0.0.1", Direction: 0, Action: "accept", Protocol: "tcp"},
expectedPorts: []string{"22", "22022"},
},
{
name: "dev suffix version supports all features",
peer: &nbpeer.Peer{
ID: "peer1",
SSHEnabled: true,
Meta: nbpeer.PeerSystemMeta{
WtVersion: "dev",
Flags: nbpeer.Flags{ServerSSHAllowed: true},
},
},
rule: &PolicyRule{
Protocol: PolicyRuleProtocolTCP,
Ports: []string{"22"},
},
base: FirewallRule{PeerIP: "10.0.0.1", Direction: 0, Action: "accept", Protocol: "tcp"},
expectedPorts: []string{"22", "22022"},
},
{
name: "development suffix version supports all features",
peer: &nbpeer.Peer{
ID: "peer1",
SSHEnabled: true,
Meta: nbpeer.PeerSystemMeta{
WtVersion: "development",
Flags: nbpeer.Flags{ServerSSHAllowed: true},
},
},
rule: &PolicyRule{
Protocol: PolicyRuleProtocolTCP,
Ports: []string{"22"},
},
base: FirewallRule{PeerIP: "10.0.0.1", Direction: 0, Action: "accept", Protocol: "tcp"},
expectedPorts: []string{"22", "22022"},
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
result := expandPortsAndRanges(tt.base, tt.rule, tt.peer)

var ports []string
for _, fr := range result {
if fr.Port != "" {
ports = append(ports, fr.Port)
} else if fr.PortRange.Start > 0 {
ports = append(ports, fmt.Sprintf("%d-%d", fr.PortRange.Start, fr.PortRange.End))
}
}

assert.Equal(t, tt.expectedPorts, ports, "expanded ports should match expected")
})
}
}

func Test_FilterZoneRecordsForPeers(t *testing.T) {
tests := []struct {
name string
Expand Down
Loading