Remove implicit inbound ssh firewall rules and change default port

Author: lixmal

client/internal/acl/manager.go

@@ -17,7 +17,6 @@ import (
 	nberrors "github.com/netbirdio/netbird/client/errors"
 	firewall "github.com/netbirdio/netbird/client/firewall/manager"
 	"github.com/netbirdio/netbird/client/internal/acl/id"
-	"github.com/netbirdio/netbird/client/ssh"
 	"github.com/netbirdio/netbird/management/domain"
 	mgmProto "github.com/netbirdio/netbird/management/proto"
 )
@@ -86,30 +85,8 @@ func (d *DefaultManager) ApplyFiltering(networkMap *mgmProto.NetworkMap, dnsRout
 }
 
 func (d *DefaultManager) applyPeerACLs(networkMap *mgmProto.NetworkMap) {
-	rules, squashedProtocols := d.squashAcceptRules(networkMap)
+	rules := d.squashAcceptRules(networkMap)
 
-	enableSSH := networkMap.PeerConfig != nil &&
-		networkMap.PeerConfig.SshConfig != nil &&
-		networkMap.PeerConfig.SshConfig.SshEnabled
-	if _, ok := squashedProtocols[mgmProto.RuleProtocol_ALL]; ok {
-		enableSSH = enableSSH && !ok
-	}
-	if _, ok := squashedProtocols[mgmProto.RuleProtocol_TCP]; ok {
-		enableSSH = enableSSH && !ok
-	}
-
-	// if TCP protocol rules not squashed and SSH enabled
-	// we add default firewall rule which accepts connection to any peer
-	// in the network by SSH (TCP 22 port).
-	if enableSSH {
-		rules = append(rules, &mgmProto.FirewallRule{
-			PeerIP:    "0.0.0.0",
-			Direction: mgmProto.RuleDirection_IN,
-			Action:    mgmProto.RuleAction_ACCEPT,
-			Protocol:  mgmProto.RuleProtocol_TCP,
-			Port:      strconv.Itoa(ssh.DefaultSSHPort),
-		})
-	}
 
 	// if we got empty rules list but management not set networkMap.FirewallRulesIsEmpty flag
 	// we have old version of management without rules handling, we should allow all traffic
@@ -373,9 +350,7 @@ func (d *DefaultManager) getPeerRuleID(
 //
 // NOTE: It will not squash two rules for same protocol if one covers all peers in the network,
 // but other has port definitions or has drop policy.
-func (d *DefaultManager) squashAcceptRules(
-	networkMap *mgmProto.NetworkMap,
-) ([]*mgmProto.FirewallRule, map[mgmProto.RuleProtocol]struct{}) {
+ func (d *DefaultManager) squashAcceptRules(networkMap *mgmProto.NetworkMap, ) []*mgmProto.FirewallRule {
 	totalIPs := 0
 	for _, p := range append(networkMap.RemotePeers, networkMap.OfflinePeers...) {
 		for range p.AllowedIps {
@@ -479,11 +454,11 @@ func (d *DefaultManager) squashAcceptRules(
 
 	// if all protocol was squashed everything is allow and we can ignore all other rules
 	if _, ok := squashedProtocols[mgmProto.RuleProtocol_ALL]; ok {
-		return squashedRules, squashedProtocols
+		return squashedRules
 	}
 
 	if len(squashedRules) == 0 {
-		return networkMap.FirewallRules, squashedProtocols
+		return networkMap.FirewallRules
 	}
 
 	var rules []*mgmProto.FirewallRule
@@ -500,7 +475,7 @@ func (d *DefaultManager) squashAcceptRules(
 		rules = append(rules, r)
 	}
 
-	return append(rules, squashedRules...), squashedProtocols
+	return append(rules, squashedRules...)
 }
 
 // getRuleGroupingSelector takes all rule properties except IP address to build selector

client/internal/acl/manager_test.go

@@ -249,7 +249,7 @@ func TestDefaultManagerSquashRules(t *testing.T) {
 	}
 
 	manager := &DefaultManager{}
-	rules, _ := manager.squashAcceptRules(networkMap)
+	rules := manager.squashAcceptRules(networkMap)
 	assert.Equal(t, 2, len(rules))
 
 	r := rules[0]
@@ -326,73 +326,6 @@ func TestDefaultManagerSquashRulesNoAffect(t *testing.T) {
 	}
 
 	manager := &DefaultManager{}
-	rules, _ := manager.squashAcceptRules(networkMap)
+	rules := manager.squashAcceptRules(networkMap)
 	assert.Equal(t, len(networkMap.FirewallRules), len(rules))
 }
-
-func TestDefaultManagerEnableSSHRules(t *testing.T) {
-	networkMap := &mgmProto.NetworkMap{
-		PeerConfig: &mgmProto.PeerConfig{
-			SshConfig: &mgmProto.SSHConfig{
-				SshEnabled: true,
-			},
-		},
-		RemotePeers: []*mgmProto.RemotePeerConfig{
-			{AllowedIps: []string{"10.93.0.1"}},
-			{AllowedIps: []string{"10.93.0.2"}},
-			{AllowedIps: []string{"10.93.0.3"}},
-		},
-		FirewallRules: []*mgmProto.FirewallRule{
-			{
-				PeerIP:    "10.93.0.1",
-				Direction: mgmProto.RuleDirection_IN,
-				Action:    mgmProto.RuleAction_ACCEPT,
-				Protocol:  mgmProto.RuleProtocol_TCP,
-			},
-			{
-				PeerIP:    "10.93.0.2",
-				Direction: mgmProto.RuleDirection_IN,
-				Action:    mgmProto.RuleAction_ACCEPT,
-				Protocol:  mgmProto.RuleProtocol_TCP,
-			},
-			{
-				PeerIP:    "10.93.0.3",
-				Direction: mgmProto.RuleDirection_OUT,
-				Action:    mgmProto.RuleAction_ACCEPT,
-				Protocol:  mgmProto.RuleProtocol_UDP,
-			},
-		},
-	}
-
-	ctrl := gomock.NewController(t)
-	defer ctrl.Finish()
-
-	ifaceMock := mocks.NewMockIFaceMapper(ctrl)
-	ifaceMock.EXPECT().IsUserspaceBind().Return(true).AnyTimes()
-	ifaceMock.EXPECT().SetFilter(gomock.Any())
-	network := netip.MustParsePrefix("172.0.0.1/32")
-
-	ifaceMock.EXPECT().Name().Return("lo").AnyTimes()
-	ifaceMock.EXPECT().Address().Return(wgaddr.Address{
-		IP:      network.Addr(),
-		Network: network,
-	}).AnyTimes()
-	ifaceMock.EXPECT().GetWGDevice().Return(nil).AnyTimes()
-
-	fw, err := firewall.NewFirewall(ifaceMock, nil, flowLogger, false)
-	require.NoError(t, err)
-	defer func() {
-		err = fw.Close(nil)
-		require.NoError(t, err)
-	}()
-
-	acl := NewDefaultManager(fw)
-
-	acl.ApplyFiltering(networkMap, false)
-
-	expectedRules := 3
-	if fw.IsStateful() {
-		expectedRules = 3 // 2 inbound rules + SSH rule
-	}
-	assert.Equal(t, expectedRules, len(acl.peerRulesPairs))
-}

client/ssh/server.go

@@ -18,7 +18,7 @@ import (
 )
 
 // DefaultSSHPort is the default SSH port of the NetBird's embedded SSH server
-const DefaultSSHPort = 44338
+const DefaultSSHPort = 22022
 
 // TerminalTimeout is the timeout for terminal session to be ready
 const TerminalTimeout = 10 * time.Second