Fix SSH proxy stripping shell quoting from forwarded commands

client/firewall/create_linux.go

@@ -6,7 +6,6 @@ import (
 	"errors"
 	"fmt"
 	"os"
-	"strconv"
 
 	"github.com/coreos/go-iptables/iptables"
 	"github.com/google/nftables"
@@ -36,27 +35,20 @@ const SKIP_NFTABLES_ENV = "NB_SKIP_NFTABLES_CHECK"
 type FWType int
 
 func NewFirewall(iface IFaceMapper, stateManager *statemanager.Manager, flowLogger nftypes.FlowLogger, disableServerRoutes bool, mtu uint16) (firewall.Manager, error) {
-	// We run in userspace mode and force userspace firewall was requested. We don't attempt native firewall.
-	if iface.IsUserspaceBind() && forceUserspaceFirewall() {
-		log.Info("forcing userspace firewall")
-		return createUserspaceFirewall(iface, nil, disableServerRoutes, flowLogger, mtu)
-	}
-
-	// Use native firewall for either kernel or userspace, the interface appears identical to netfilter
+	// on the linux system we try to user nftables or iptables
+	// in any case, because we need to allow netbird interface traffic
+	// so we use AllowNetbird traffic from these firewall managers
+	// for the userspace packet filtering firewall
 	fm, err := createNativeFirewall(iface, stateManager, disableServerRoutes, mtu)
 
-	// Kernel cannot fall back to anything else, need to return error
 	if !iface.IsUserspaceBind() {
 		return fm, err
 	}
 
-	// Fall back to the userspace packet filter if native is unavailable
 	if err != nil {
 		log.Warnf("failed to create native firewall: %v. Proceeding with userspace", err)
-		return createUserspaceFirewall(iface, nil, disableServerRoutes, flowLogger, mtu)
 	}
-
-	return fm, nil
+	return createUserspaceFirewall(iface, fm, disableServerRoutes, flowLogger, mtu)
 }
 
 func createNativeFirewall(iface IFaceMapper, stateManager *statemanager.Manager, routes bool, mtu uint16) (firewall.Manager, error) {
@@ -168,17 +160,3 @@ func isIptablesClientAvailable(client *iptables.IPTables) bool {
 	_, err := client.ListChains("filter")
 	return err == nil
 }
-
-func forceUserspaceFirewall() bool {
-	val := os.Getenv(EnvForceUserspaceFirewall)
-	if val == "" {
-		return false
-	}
-
-	force, err := strconv.ParseBool(val)
-	if err != nil {
-		log.Warnf("failed to parse %s: %v", EnvForceUserspaceFirewall, err)
-		return false
-	}
-	return force
-}

client/firewall/iface.go

@@ -7,12 +7,6 @@ import (
 	"github.com/netbirdio/netbird/client/iface/wgaddr"
 )
 
-// EnvForceUserspaceFirewall forces the use of the userspace packet filter even when
-// native iptables/nftables is available. This only applies when the WireGuard interface
-// runs in userspace mode. When set, peer ACLs are handled by USPFilter instead of
-// kernel netfilter rules.
-const EnvForceUserspaceFirewall = "NB_FORCE_USERSPACE_FIREWALL"
-
 // IFaceMapper defines subset methods of interface required for manager
 type IFaceMapper interface {
 	Name() string

client/firewall/iptables/manager_linux.go

@@ -33,6 +33,7 @@ type Manager struct {
 type iFaceMapper interface {
 	Name() string
 	Address() wgaddr.Address
+	IsUserspaceBind() bool
 }
 
 // Create iptables firewall manager
@@ -63,9 +64,10 @@ func Create(wgIface iFaceMapper, mtu uint16) (*Manager, error) {
 func (m *Manager) Init(stateManager *statemanager.Manager) error {
 	state := &ShutdownState{
 		InterfaceState: &InterfaceState{
-			NameStr:   m.wgIface.Name(),
-			WGAddress: m.wgIface.Address(),
-			MTU:       m.router.mtu,
+			NameStr:       m.wgIface.Name(),
+			WGAddress:     m.wgIface.Address(),
+			UserspaceBind: m.wgIface.IsUserspaceBind(),
+			MTU:           m.router.mtu,
 		},
 	}
 	stateManager.RegisterState(state)
@@ -201,10 +203,12 @@ func (m *Manager) Close(stateManager *statemanager.Manager) error {
 	return nberrors.FormatErrorOrNil(merr)
 }
 
-// AllowNetbird allows netbird interface traffic.
-// This is called when USPFilter wraps the native firewall, adding blanket accept
-// rules so that packet filtering is handled in userspace instead of by netfilter.
+// AllowNetbird allows netbird interface traffic
 func (m *Manager) AllowNetbird() error {
+	if !m.wgIface.IsUserspaceBind() {
+		return nil
+	}
+
 	_, err := m.AddPeerFiltering(
 		nil,
 		net.IP{0, 0, 0, 0},

client/firewall/iptables/manager_linux_test.go

@@ -47,6 +47,8 @@ func (i *iFaceMock) Address() wgaddr.Address {
 	panic("AddressFunc is not set")
 }
 
+func (i *iFaceMock) IsUserspaceBind() bool { return false }
+
 func TestIptablesManager(t *testing.T) {
 	ipv4Client, err := iptables.NewWithProtocol(iptables.ProtocolIPv4)
 	require.NoError(t, err)

client/firewall/iptables/state_linux.go

@@ -9,9 +9,10 @@ import (
 )
 
 type InterfaceState struct {
-	NameStr   string         `json:"name"`
-	WGAddress wgaddr.Address `json:"wg_address"`
-	MTU       uint16         `json:"mtu"`
+	NameStr       string         `json:"name"`
+	WGAddress     wgaddr.Address `json:"wg_address"`
+	UserspaceBind bool           `json:"userspace_bind"`
+	MTU           uint16         `json:"mtu"`
 }
 
 func (i *InterfaceState) Name() string {
@@ -22,6 +23,10 @@ func (i *InterfaceState) Address() wgaddr.Address {
 	return i.WGAddress
 }
 
+func (i *InterfaceState) IsUserspaceBind() bool {
+	return i.UserspaceBind
+}
+
 type ShutdownState struct {
 	sync.Mutex
 

client/firewall/nftables/manager_linux.go

@@ -40,6 +40,7 @@ func getTableName() string {
 type iFaceMapper interface {
 	Name() string
 	Address() wgaddr.Address
+	IsUserspaceBind() bool
 }
 
 // Manager of iptables firewall
@@ -105,9 +106,10 @@ func (m *Manager) Init(stateManager *statemanager.Manager) error {
 	// cleanup using Close() without needing to store specific rules.
 	if err := stateManager.UpdateState(&ShutdownState{
 		InterfaceState: &InterfaceState{
-			NameStr:   m.wgIface.Name(),
-			WGAddress: m.wgIface.Address(),
-			MTU:       m.router.mtu,
+			NameStr:       m.wgIface.Name(),
+			WGAddress:     m.wgIface.Address(),
+			UserspaceBind: m.wgIface.IsUserspaceBind(),
+			MTU:           m.router.mtu,
 		},
 	}); err != nil {
 		log.Errorf("failed to update state: %v", err)
@@ -203,10 +205,12 @@ func (m *Manager) RemoveNatRule(pair firewall.RouterPair) error {
 	return m.router.RemoveNatRule(pair)
 }
 
-// AllowNetbird allows netbird interface traffic.
-// This is called when USPFilter wraps the native firewall, adding blanket accept
-// rules so that packet filtering is handled in userspace instead of by netfilter.
+// AllowNetbird allows netbird interface traffic
 func (m *Manager) AllowNetbird() error {
+	if !m.wgIface.IsUserspaceBind() {
+		return nil
+	}
+
 	m.mutex.Lock()
 	defer m.mutex.Unlock()
 

client/firewall/nftables/manager_linux_test.go

@@ -52,6 +52,8 @@ func (i *iFaceMock) Address() wgaddr.Address {
 	panic("AddressFunc is not set")
 }
 
+func (i *iFaceMock) IsUserspaceBind() bool { return false }
+
 func TestNftablesManager(t *testing.T) {
 
 	// just check on the local interface

client/firewall/nftables/state_linux.go

@@ -8,9 +8,10 @@ import (
 )
 
 type InterfaceState struct {
-	NameStr   string         `json:"name"`
-	WGAddress wgaddr.Address `json:"wg_address"`
-	MTU       uint16         `json:"mtu"`
+	NameStr       string         `json:"name"`
+	WGAddress     wgaddr.Address `json:"wg_address"`
+	UserspaceBind bool           `json:"userspace_bind"`
+	MTU           uint16         `json:"mtu"`
 }
 
 func (i *InterfaceState) Name() string {
@@ -21,6 +22,10 @@ func (i *InterfaceState) Address() wgaddr.Address {
 	return i.WGAddress
 }
 
+func (i *InterfaceState) IsUserspaceBind() bool {
+	return i.UserspaceBind
+}
+
 type ShutdownState struct {
 	InterfaceState *InterfaceState `json:"interface_state,omitempty"`
 }

client/internal/acl/manager_test.go

@@ -19,9 +19,6 @@ import (
 var flowLogger = netflow.NewManager(nil, []byte{}, nil).GetLogger()
 
 func TestDefaultManager(t *testing.T) {
-	t.Setenv("NB_WG_KERNEL_DISABLED", "true")
-	t.Setenv(firewall.EnvForceUserspaceFirewall, "true")
-
 	networkMap := &mgmProto.NetworkMap{
 		FirewallRules: []*mgmProto.FirewallRule{
 			{
@@ -138,7 +135,6 @@ func TestDefaultManager(t *testing.T) {
 func TestDefaultManagerStateless(t *testing.T) {
 	// stateless currently only in userspace, so we have to disable kernel
 	t.Setenv("NB_WG_KERNEL_DISABLED", "true")
-	t.Setenv(firewall.EnvForceUserspaceFirewall, "true")
 	t.Setenv("NB_DISABLE_CONNTRACK", "true")
 
 	networkMap := &mgmProto.NetworkMap{
@@ -198,7 +194,6 @@ func TestDefaultManagerStateless(t *testing.T) {
 // This tests the full ACL manager -> uspfilter integration.
 func TestDenyRulesNotAccumulatedOnRepeatedApply(t *testing.T) {
 	t.Setenv("NB_WG_KERNEL_DISABLED", "true")
-	t.Setenv(firewall.EnvForceUserspaceFirewall, "true")
 
 	networkMap := &mgmProto.NetworkMap{
 		FirewallRules: []*mgmProto.FirewallRule{
@@ -263,7 +258,6 @@ func TestDenyRulesNotAccumulatedOnRepeatedApply(t *testing.T) {
 // up when they're removed from the network map in a subsequent update.
 func TestDenyRulesCleanedUpOnRemoval(t *testing.T) {
 	t.Setenv("NB_WG_KERNEL_DISABLED", "true")
-	t.Setenv(firewall.EnvForceUserspaceFirewall, "true")
 
 	ctrl := gomock.NewController(t)
 	defer ctrl.Finish()
@@ -345,7 +339,6 @@ func TestDenyRulesCleanedUpOnRemoval(t *testing.T) {
 // one added without leaking.
 func TestRuleUpdateChangingAction(t *testing.T) {
 	t.Setenv("NB_WG_KERNEL_DISABLED", "true")
-	t.Setenv(firewall.EnvForceUserspaceFirewall, "true")
 
 	ctrl := gomock.NewController(t)
 	defer ctrl.Finish()

client/ssh/proxy/proxy.go

@@ -141,7 +141,7 @@ func (p *SSHProxy) runProxySSHServer(jwtToken string) error {
 
 func (p *SSHProxy) handleSSHSession(session ssh.Session) {
 	ptyReq, winCh, isPty := session.Pty()
-	hasCommand := len(session.Command()) > 0
+	hasCommand := session.RawCommand() != ""
 
 	sshClient, err := p.getOrCreateBackendClient(session.Context(), session.User())
 	if err != nil {
@@ -180,7 +180,7 @@ func (p *SSHProxy) handleSSHSession(session ssh.Session) {
 	}
 
 	if hasCommand {
-		if err := serverSession.Run(strings.Join(session.Command(), " ")); err != nil {
+		if err := serverSession.Run(session.RawCommand()); err != nil {
 			log.Debugf("run command: %v", err)
 			p.handleProxyExitCode(session, err)
 		}

client/ssh/proxy/proxy_test.go

@@ -1,6 +1,7 @@
 package proxy
 
 import (
+	"bytes"
 	"context"
 	"crypto/rand"
 	"crypto/rsa"
@@ -245,6 +246,191 @@ func TestSSHProxy_Connect(t *testing.T) {
 	cancel()
 }
 
+// TestSSHProxy_CommandQuoting verifies that the proxy preserves shell quoting
+// when forwarding commands to the backend. This is critical for tools like
+// Ansible that send commands such as:
+//
+//	/bin/sh -c '( umask 77 && mkdir -p ... ) && sleep 0'
+//
+// The single quotes must be preserved so the backend shell receives the
+// subshell expression as a single argument to -c.
+func TestSSHProxy_CommandQuoting(t *testing.T) {
+	if testing.Short() {
+		t.Skip("Skipping integration test in short mode")
+	}
+
+	sshClient, cleanup := setupProxySSHClient(t)
+	defer cleanup()
+
+	// These commands simulate what the SSH protocol delivers as exec payloads.
+	// When a user types: ssh host '/bin/sh -c "( echo hello )"'
+	// the local shell strips the outer single quotes, and the SSH exec request
+	// contains the raw string: /bin/sh -c "( echo hello )"
+	//
+	// The proxy must forward this string verbatim. Using session.Command()
+	// (shlex.Split + strings.Join) strips the inner double quotes, breaking
+	// the command on the backend.
+	tests := []struct {
+		name    string
+		command string
+		expect  string
+	}{
+		{
+			name:    "subshell_in_double_quotes",
+			command: `/bin/sh -c "( echo from-subshell ) && echo outer"`,
+			expect:  "from-subshell\nouter\n",
+		},
+		{
+			name:    "printf_with_special_chars",
+			command: `/bin/sh -c "printf '%s\n' 'hello world'"`,
+			expect:  "hello world\n",
+		},
+		{
+			name:    "nested_command_substitution",
+			command: `/bin/sh -c "echo $(echo nested)"`,
+			expect:  "nested\n",
+		},
+	}
+
+	for _, tc := range tests {
+		t.Run(tc.name, func(t *testing.T) {
+			session, err := sshClient.NewSession()
+			require.NoError(t, err)
+			defer func() { _ = session.Close() }()
+
+			var stderrBuf bytes.Buffer
+			session.Stderr = &stderrBuf
+
+			outputCh := make(chan []byte, 1)
+			errCh := make(chan error, 1)
+			go func() {
+				output, err := session.Output(tc.command)
+				outputCh <- output
+				errCh <- err
+			}()
+
+			select {
+			case output := <-outputCh:
+				err := <-errCh
+				if stderrBuf.Len() > 0 {
+					t.Logf("stderr: %s", stderrBuf.String())
+				}
+				require.NoError(t, err, "command should succeed: %s", tc.command)
+				assert.Equal(t, tc.expect, string(output), "output mismatch for: %s", tc.command)
+			case <-time.After(5 * time.Second):
+				t.Fatalf("command timed out: %s", tc.command)
+			}
+		})
+	}
+}
+
+// setupProxySSHClient creates a full proxy test environment and returns
+// an SSH client connected through the proxy to a backend NetBird SSH server.
+func setupProxySSHClient(t *testing.T) (*cryptossh.Client, func()) {
+	t.Helper()
+
+	const (
+		issuer   = "https://test-issuer.example.com"
+		audience = "test-audience"
+	)
+
+	jwksServer, privateKey, jwksURL := setupJWKSServer(t)
+
+	hostKey, err := nbssh.GeneratePrivateKey(nbssh.ED25519)
+	require.NoError(t, err)
+	hostPubKey, err := nbssh.GeneratePublicKey(hostKey)
+	require.NoError(t, err)
+
+	serverConfig := &server.Config{
+		HostKeyPEM: hostKey,
+		JWT: &server.JWTConfig{
+			Issuer:       issuer,
+			Audiences:    []string{audience},
+			KeysLocation: jwksURL,
+		},
+	}
+	sshServer := server.New(serverConfig)
+	sshServer.SetAllowRootLogin(true)
+
+	testUsername := testutil.GetTestUsername(t)
+	testJWTUser := "test-username"
+	testUserHash, err := sshuserhash.HashUserID(testJWTUser)
+	require.NoError(t, err)
+
+	authConfig := &sshauth.Config{
+		UserIDClaim:     sshauth.DefaultUserIDClaim,
+		AuthorizedUsers: []sshuserhash.UserIDHash{testUserHash},
+		MachineUsers: map[string][]uint32{
+			testUsername: {0},
+		},
+	}
+	sshServer.UpdateSSHAuth(authConfig)
+
+	sshServerAddr := server.StartTestServer(t, sshServer)
+
+	mockDaemon := startMockDaemon(t)
+
+	host, portStr, err := net.SplitHostPort(sshServerAddr)
+	require.NoError(t, err)
+	port, err := strconv.Atoi(portStr)
+	require.NoError(t, err)
+
+	mockDaemon.setHostKey(host, hostPubKey)
+
+	validToken := generateValidJWT(t, privateKey, issuer, audience, testJWTUser)
+	mockDaemon.setJWTToken(validToken)
+
+	proxyInstance, err := New(mockDaemon.addr, host, port, io.Discard, nil)
+	require.NoError(t, err)
+
+	origStdin := os.Stdin
+	origStdout := os.Stdout
+
+	stdinReader, stdinWriter, err := os.Pipe()
+	require.NoError(t, err)
+	stdoutReader, stdoutWriter, err := os.Pipe()
+	require.NoError(t, err)
+
+	os.Stdin = stdinReader
+	os.Stdout = stdoutWriter
+
+	clientConn, proxyConn := net.Pipe()
+
+	go func() { _, _ = io.Copy(stdinWriter, proxyConn) }()
+	go func() { _, _ = io.Copy(proxyConn, stdoutReader) }()
+
+	ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second)
+
+	go func() {
+		_ = proxyInstance.Connect(ctx)
+	}()
+
+	sshConfig := &cryptossh.ClientConfig{
+		User:            testutil.GetTestUsername(t),
+		Auth:            []cryptossh.AuthMethod{},
+		HostKeyCallback: cryptossh.InsecureIgnoreHostKey(),
+		Timeout:         5 * time.Second,
+	}
+
+	sshClientConn, chans, reqs, err := cryptossh.NewClientConn(clientConn, "test", sshConfig)
+	require.NoError(t, err)
+
+	client := cryptossh.NewClient(sshClientConn, chans, reqs)
+
+	cleanupFn := func() {
+		_ = client.Close()
+		_ = clientConn.Close()
+		cancel()
+		os.Stdin = origStdin
+		os.Stdout = origStdout
+		_ = sshServer.Stop()
+		mockDaemon.stop()
+		jwksServer.Close()
+	}
+
+	return client, cleanupFn
+}
+
 type mockDaemonServer struct {
 	proto.UnimplementedDaemonServiceServer
 	hostKeys map[string][]byte

client/ssh/server/session_handlers.go

@@ -60,7 +60,7 @@ func (s *Server) sessionHandler(session ssh.Session) {
 	}
 
 	ptyReq, winCh, isPty := session.Pty()
-	hasCommand := len(session.Command()) > 0
+	hasCommand := session.RawCommand() != ""
 
 	if isPty && !hasCommand {
 		// ssh <host> - PTY interactive session (login)