[client,management] Rewrite the SSH feature (#4015)

client/firewall/uspfilter/filter.go

@@ -35,6 +35,12 @@ const (
 	ipTCPHeaderMinSize = 40
 )
 
+// serviceKey represents a protocol/port combination for netstack service registry
+type serviceKey struct {
+	protocol gopacket.LayerType
+	port     uint16
+}
+
 const (
 	// EnvDisableConntrack disables the stateful filter, replies to outbound traffic won't be allowed.
 	EnvDisableConntrack = "NB_DISABLE_CONNTRACK"
@@ -59,12 +65,6 @@ const (
 
 var errNatNotSupported = errors.New("nat not supported with userspace firewall")
 
-// serviceKey represents a protocol/port combination for netstack service registry
-type serviceKey struct {
-	protocol gopacket.LayerType
-	port     uint16
-}
-
 // RuleSet is a set of rules grouped by a string key
 type RuleSet map[string]PeerRule
 

client/firewall/uspfilter/filter_test.go

@@ -22,6 +22,7 @@ import (
 	"github.com/netbirdio/netbird/client/iface/device"
 	"github.com/netbirdio/netbird/client/iface/wgaddr"
 	"github.com/netbirdio/netbird/client/internal/netflow"
+	nftypes "github.com/netbirdio/netbird/client/internal/netflow/types"
 	"github.com/netbirdio/netbird/shared/management/domain"
 )
 
@@ -1114,3 +1115,138 @@ func generateTCPPacketWithFlags(tb testing.TB, srcIP, dstIP net.IP, srcPort, dst
 
 	return buf.Bytes()
 }
+
+func TestShouldForward(t *testing.T) {
+	// Set up test addresses
+	wgIP := netip.MustParseAddr("100.10.0.1")
+	otherIP := netip.MustParseAddr("100.10.0.2")
+
+	// Create test manager with mock interface
+	ifaceMock := &IFaceMock{
+		SetFilterFunc: func(device.PacketFilter) error { return nil },
+	}
+	// Set the mock to return our test WG IP
+	ifaceMock.AddressFunc = func() wgaddr.Address {
+		return wgaddr.Address{IP: wgIP, Network: netip.PrefixFrom(wgIP, 24)}
+	}
+
+	manager, err := Create(ifaceMock, false, flowLogger, nbiface.DefaultMTU)
+	require.NoError(t, err)
+	defer func() {
+		require.NoError(t, manager.Close(nil))
+	}()
+
+	// Helper to create decoder with TCP packet
+	createTCPDecoder := func(dstPort uint16) *decoder {
+		ipv4 := &layers.IPv4{
+			Version:  4,
+			Protocol: layers.IPProtocolTCP,
+			SrcIP:    net.ParseIP("192.168.1.100"),
+			DstIP:    wgIP.AsSlice(),
+		}
+		tcp := &layers.TCP{
+			SrcPort: 54321,
+			DstPort: layers.TCPPort(dstPort),
+		}
+
+		err := tcp.SetNetworkLayerForChecksum(ipv4)
+		require.NoError(t, err)
+
+		buf := gopacket.NewSerializeBuffer()
+		opts := gopacket.SerializeOptions{ComputeChecksums: true, FixLengths: true}
+		err = gopacket.SerializeLayers(buf, opts, ipv4, tcp, gopacket.Payload("test"))
+		require.NoError(t, err)
+
+		d := &decoder{
+			decoded: []gopacket.LayerType{},
+		}
+		d.parser = gopacket.NewDecodingLayerParser(
+			layers.LayerTypeIPv4,
+			&d.eth, &d.ip4, &d.ip6, &d.icmp4, &d.icmp6, &d.tcp, &d.udp,
+		)
+		d.parser.IgnoreUnsupported = true
+
+		err = d.parser.DecodeLayers(buf.Bytes(), &d.decoded)
+		require.NoError(t, err)
+
+		return d
+	}
+
+	tests := []struct {
+		name              string
+		localForwarding   bool
+		netstack          bool
+		dstIP             netip.Addr
+		serviceRegistered bool
+		servicePort       uint16
+		expected          bool
+		description       string
+	}{
+		{
+			name:            "no local forwarding",
+			localForwarding: false,
+			netstack:        true,
+			dstIP:           wgIP,
+			expected:        false,
+			description:     "should never forward when local forwarding disabled",
+		},
+		{
+			name:            "traffic to other local interface",
+			localForwarding: true,
+			netstack:        false,
+			dstIP:           otherIP,
+			expected:        true,
+			description:     "should forward traffic to our other local interfaces (not NetBird IP)",
+		},
+		{
+			name:            "traffic to NetBird IP, no netstack",
+			localForwarding: true,
+			netstack:        false,
+			dstIP:           wgIP,
+			expected:        false,
+			description:     "should send to netstack listeners (final return false path)",
+		},
+		{
+			name:            "traffic to our IP, netstack mode, no service",
+			localForwarding: true,
+			netstack:        true,
+			dstIP:           wgIP,
+			expected:        true,
+			description:     "should forward when in netstack mode with no matching service",
+		},
+		{
+			name:              "traffic to our IP, netstack mode, with service",
+			localForwarding:   true,
+			netstack:          true,
+			dstIP:             wgIP,
+			serviceRegistered: true,
+			servicePort:       22,
+			expected:          false,
+			description:       "should send to netstack listeners when service is registered",
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			// Configure manager
+			manager.localForwarding = tt.localForwarding
+			manager.netstack = tt.netstack
+
+			// Register service if needed
+			if tt.serviceRegistered {
+				manager.RegisterNetstackService(nftypes.TCP, tt.servicePort)
+				defer manager.UnregisterNetstackService(nftypes.TCP, tt.servicePort)
+			}
+
+			// Create decoder for the test
+			decoder := createTCPDecoder(tt.servicePort)
+			if !tt.serviceRegistered {
+				decoder = createTCPDecoder(8080) // Use non-registered port
+			}
+
+			// Test the method
+			result := manager.shouldForward(decoder, tt.dstIP)
+			require.Equal(t, tt.expected, result, tt.description)
+		})
+	}
+}

client/firewall/uspfilter/nat_stateful_test.go

@@ -0,0 +1,85 @@
+package uspfilter
+
+import (
+	"net/netip"
+	"testing"
+
+	"github.com/google/gopacket/layers"
+	"github.com/stretchr/testify/require"
+
+	"github.com/netbirdio/netbird/client/iface"
+	"github.com/netbirdio/netbird/client/iface/device"
+)
+
+// TestPortDNATBasic tests basic port DNAT functionality
+func TestPortDNATBasic(t *testing.T) {
+	manager, err := Create(&IFaceMock{
+		SetFilterFunc: func(device.PacketFilter) error { return nil },
+	}, false, flowLogger, iface.DefaultMTU)
+	require.NoError(t, err)
+	defer func() {
+		require.NoError(t, manager.Close(nil))
+	}()
+
+	// Define peer IPs
+	peerA := netip.MustParseAddr("100.10.0.50")
+	peerB := netip.MustParseAddr("100.10.0.51")
+
+	// Add SSH port redirection rule for peer B (the target)
+	err = manager.addPortRedirection(peerB, layers.LayerTypeTCP, 22, 22022)
+	require.NoError(t, err)
+
+	// Scenario: Peer A connects to Peer B on port 22 (should get NAT)
+	packetAtoB := generateDNATTestPacket(t, peerA, peerB, layers.IPProtocolTCP, 54321, 22)
+	d := parsePacket(t, packetAtoB)
+	translatedAtoB := manager.translateInboundPortDNAT(packetAtoB, d, peerA, peerB)
+	require.True(t, translatedAtoB, "Peer A to Peer B should be translated (NAT applied)")
+
+	// Verify port was translated to 22022
+	d = parsePacket(t, packetAtoB)
+	require.Equal(t, uint16(22022), uint16(d.tcp.DstPort), "Port should be rewritten to 22022")
+
+	// Scenario: Return traffic from Peer B to Peer A should NOT be translated
+	// (prevents double NAT - original port stored in conntrack)
+	returnPacket := generateDNATTestPacket(t, peerB, peerA, layers.IPProtocolTCP, 22022, 54321)
+	d2 := parsePacket(t, returnPacket)
+	translatedReturn := manager.translateInboundPortDNAT(returnPacket, d2, peerB, peerA)
+	require.False(t, translatedReturn, "Return traffic from same IP should not be translated")
+}
+
+// TestPortDNATMultipleRules tests multiple port DNAT rules
+func TestPortDNATMultipleRules(t *testing.T) {
+	manager, err := Create(&IFaceMock{
+		SetFilterFunc: func(device.PacketFilter) error { return nil },
+	}, false, flowLogger, iface.DefaultMTU)
+	require.NoError(t, err)
+	defer func() {
+		require.NoError(t, manager.Close(nil))
+	}()
+
+	// Define peer IPs
+	peerA := netip.MustParseAddr("100.10.0.50")
+	peerB := netip.MustParseAddr("100.10.0.51")
+
+	// Add SSH port redirection rules for both peers
+	err = manager.addPortRedirection(peerA, layers.LayerTypeTCP, 22, 22022)
+	require.NoError(t, err)
+	err = manager.addPortRedirection(peerB, layers.LayerTypeTCP, 22, 22022)
+	require.NoError(t, err)
+
+	// Test traffic to peer B gets translated
+	packetToB := generateDNATTestPacket(t, peerA, peerB, layers.IPProtocolTCP, 54321, 22)
+	d1 := parsePacket(t, packetToB)
+	translatedToB := manager.translateInboundPortDNAT(packetToB, d1, peerA, peerB)
+	require.True(t, translatedToB, "Traffic to peer B should be translated")
+	d1 = parsePacket(t, packetToB)
+	require.Equal(t, uint16(22022), uint16(d1.tcp.DstPort), "Port should be 22022")
+
+	// Test traffic to peer A gets translated
+	packetToA := generateDNATTestPacket(t, peerB, peerA, layers.IPProtocolTCP, 54322, 22)
+	d2 := parsePacket(t, packetToA)
+	translatedToA := manager.translateInboundPortDNAT(packetToA, d2, peerB, peerA)
+	require.True(t, translatedToA, "Traffic to peer A should be translated")
+	d2 = parsePacket(t, packetToA)
+	require.Equal(t, uint16(22022), uint16(d2.tcp.DstPort), "Port should be 22022")
+}