Disable local forwarding in netstack mode by default for security reasons

client/Dockerfile-rootless

@@ -9,6 +9,7 @@ USER netbird:netbird
 
 ENV NB_FOREGROUND_MODE=true
 ENV NB_USE_NETSTACK_MODE=true
+ENV NB_ENABLE_NETSTACK_LOCAL_FORWARDING=true
 ENV NB_CONFIG=config.json
 ENV NB_DAEMON_ADDR=unix://netbird.sock
 ENV NB_DISABLE_DNS=true

client/firewall/uspfilter/tracer.go

@@ -310,8 +310,10 @@ func (m *Manager) buildConntrackStateMessage(d *decoder) string {
 }
 
 func (m *Manager) handleLocalDelivery(trace *PacketTrace, packetData []byte, d *decoder, srcIP, dstIP net.IP) bool {
-	if !m.localipmanager.IsLocalIP(dstIP) {
+	if !m.localForwarding {
-		return false
+		trace.AddResult(StageRouting, "Local forwarding disabled", false)
+		trace.AddResult(StageCompleted, "Packet dropped - local forwarding disabled", false)
+		return true
 	}
 
 	trace.AddResult(StageRouting, "Packet destined for local delivery", true)

client/firewall/uspfilter/uspfilter.go

@@ -34,6 +34,10 @@ const (
 
 	// EnvForceUserspaceRouter forces userspace routing even if native routing is available.
 	EnvForceUserspaceRouter = "NB_FORCE_USERSPACE_ROUTER"
+
+	// EnvEnableNetstackLocalForwarding enables forwarding of local traffic to the native stack when running netstack
+	// Leaving this on by default introduces a security risk as sockets on listening on localhost only will be accessible
+	EnvEnableNetstackLocalForwarding = "NB_ENABLE_NETSTACK_LOCAL_FORWARDING"
 )
 
 // RuleSet is a set of rules grouped by a string key
@@ -59,6 +63,8 @@ type Manager struct {
 	stateful bool
 	// indicates whether wireguards runs in netstack mode
 	netstack bool
+	// indicates whether we forward local traffic to the native stack
+	localForwarding bool
 
 	localipmanager *localIPManager
 
@@ -101,7 +107,14 @@ func CreateWithNativeFirewall(iface common.IFaceMapper, nativeFirewall firewall.
 }
 
 func create(iface common.IFaceMapper, nativeFirewall firewall.Manager, disableServerRoutes bool) (*Manager, error) {
-	disableConntrack, _ := strconv.ParseBool(os.Getenv(EnvDisableConntrack))
+	disableConntrack, err := strconv.ParseBool(os.Getenv(EnvDisableConntrack))
+	if err != nil {
+		log.Warnf("failed to parse %s: %v", EnvDisableConntrack, err)
+	}
+	enableLocalForwarding, err := strconv.ParseBool(os.Getenv(EnvEnableNetstackLocalForwarding))
+	if err != nil {
+		log.Warnf("failed to parse %s: %v", EnvEnableNetstackLocalForwarding, err)
+	}
 
 	m := &Manager{
 		decoders: sync.Pool{
@@ -127,6 +140,8 @@ func create(iface common.IFaceMapper, nativeFirewall firewall.Manager, disableSe
 		stateful:       !disableConntrack,
 		logger:         nblog.NewFromLogrus(log.StandardLogger()),
 		netstack:       netstack.IsEnabled(),
+		// default true for non-netstack, for netstack only if explicitly enabled
+		localForwarding: !netstack.IsEnabled() || enableLocalForwarding,
 	}
 
 	if err := m.localipmanager.UpdateLocalIPs(iface); err != nil {
@@ -220,7 +235,7 @@ func (m *Manager) determineRouting(iface common.IFaceMapper, disableServerRoutes
 	}
 
 	// netstack needs the forwarder for local traffic
-	if m.netstack ||
+	if m.netstack && m.localForwarding ||
 		m.routingEnabled && !m.nativeRouter {
 
 		m.initForwarder(iface)
@@ -436,7 +451,7 @@ func (m *Manager) DropOutgoing(packetData []byte) bool {
 
 // DropIncoming filter incoming packets
 func (m *Manager) DropIncoming(packetData []byte) bool {
-	return m.dropFilter(packetData, m.incomingRules)
+	return m.dropFilter(packetData)
 }
 
 // UpdateLocalIPs updates the list of local IPs
@@ -564,7 +579,7 @@ func (m *Manager) trackICMPOutbound(d *decoder, srcIP, dstIP net.IP) {
 
 // dropFilter implements filtering logic for incoming packets.
 // If it returns true, the packet should be dropped.
-func (m *Manager) dropFilter(packetData []byte, rules map[string]RuleSet) bool {
+func (m *Manager) dropFilter(packetData []byte) bool {
 	m.mutex.RLock()
 	defer m.mutex.RUnlock()
 
@@ -588,27 +603,37 @@ func (m *Manager) dropFilter(packetData []byte, rules map[string]RuleSet) bool {
 		return false
 	}
 
-	// Handle local traffic - apply peer ACLs
 	if m.localipmanager.IsLocalIP(dstIP) {
-		if m.peerACLsBlock(srcIP, packetData, rules, d) {
+		return m.handleLocalTraffic(d, srcIP, dstIP, packetData)
-			m.logger.Trace("Dropping local packet: src=%s dst=%s rules=denied",
-				srcIP, dstIP)
-			return true
-		}
-
-		// if running in netstack mode we need to pass this to the forwarder
-		if m.netstack {
-			m.handleNetstackLocalTraffic(packetData)
-			// don't process this packet further
-			return true
-		}
-
-		return false
 	}
 
 	return m.handleRoutedTraffic(d, srcIP, dstIP, packetData)
 }
 
+// handleLocalTraffic handles local traffic.
+// If it returns true, the packet should be dropped.
+func (m *Manager) handleLocalTraffic(d *decoder, srcIP, dstIP net.IP, packetData []byte) bool {
+	if !m.localForwarding {
+		m.logger.Trace("Dropping local packet (local forwarding disabled): src=%s dst=%s", srcIP, dstIP)
+		return true
+	}
+
+	if m.peerACLsBlock(srcIP, packetData, m.incomingRules, d) {
+		m.logger.Trace("Dropping local packet (ACL denied): src=%s dst=%s",
+			srcIP, dstIP)
+		return true
+	}
+
+	// if running in netstack mode we need to pass this to the forwarder
+	if m.netstack {
+		m.handleNetstackLocalTraffic(packetData)
+
+		// don't process this packet further
+		return true
+	}
+
+	return false
+}
 func (m *Manager) handleNetstackLocalTraffic(packetData []byte) {
 	if m.forwarder == nil {
 		return
@@ -619,6 +644,8 @@ func (m *Manager) handleNetstackLocalTraffic(packetData []byte) {
 	}
 }
 
+// handleRoutedTraffic handles routed traffic.
+// If it returns true, the packet should be dropped.
 func (m *Manager) handleRoutedTraffic(d *decoder, srcIP, dstIP net.IP, packetData []byte) bool {
 	// Drop if routing is disabled
 	if !m.routingEnabled {

client/firewall/uspfilter/uspfilter_bench_test.go

@@ -188,7 +188,7 @@ func BenchmarkCoreFiltering(b *testing.B) {
 				// Measure inbound packet processing
 				b.ResetTimer()
 				for i := 0; i < b.N; i++ {
-					manager.dropFilter(inbound, manager.incomingRules)
+					manager.dropFilter(inbound)
 				}
 			})
 		}
@@ -231,7 +231,7 @@ func BenchmarkStateScaling(b *testing.B) {
 
 			b.ResetTimer()
 			for i := 0; i < b.N; i++ {
-				manager.dropFilter(testIn, manager.incomingRules)
+				manager.dropFilter(testIn)
 			}
 		})
 	}
@@ -272,7 +272,7 @@ func BenchmarkEstablishmentOverhead(b *testing.B) {
 
 			b.ResetTimer()
 			for i := 0; i < b.N; i++ {
-				manager.dropFilter(inbound, manager.incomingRules)
+				manager.dropFilter(inbound)
 			}
 		})
 	}
@@ -475,7 +475,7 @@ func BenchmarkRoutedNetworkReturn(b *testing.B) {
 					manager.processOutgoingHooks(syn)
 					// SYN-ACK
 					synack := generateTCPPacketWithFlags(b, dstIP, srcIP, 80, 1024, uint16(conntrack.TCPSyn|conntrack.TCPAck))
-					manager.dropFilter(synack, manager.incomingRules)
+					manager.dropFilter(synack)
 					// ACK
 					ack := generateTCPPacketWithFlags(b, srcIP, dstIP, 1024, 80, uint16(conntrack.TCPAck))
 					manager.processOutgoingHooks(ack)
@@ -484,7 +484,7 @@ func BenchmarkRoutedNetworkReturn(b *testing.B) {
 
 			b.ResetTimer()
 			for i := 0; i < b.N; i++ {
-				manager.dropFilter(inbound, manager.incomingRules)
+				manager.dropFilter(inbound)
 			}
 		})
 	}
@@ -621,7 +621,7 @@ func BenchmarkLongLivedConnections(b *testing.B) {
 				// SYN-ACK
 				synack := generateTCPPacketWithFlags(b, dstIPs[i], srcIPs[i],
 					80, uint16(1024+i), uint16(conntrack.TCPSyn|conntrack.TCPAck))
-				manager.dropFilter(synack, manager.incomingRules)
+				manager.dropFilter(synack)
 
 				// ACK
 				ack := generateTCPPacketWithFlags(b, srcIPs[i], dstIPs[i],
@@ -649,7 +649,7 @@ func BenchmarkLongLivedConnections(b *testing.B) {
 				// First outbound data
 				manager.processOutgoingHooks(outPackets[connIdx])
 				// Then inbound response - this is what we're actually measuring
-				manager.dropFilter(inPackets[connIdx], manager.incomingRules)
+				manager.dropFilter(inPackets[connIdx])
 			}
 		})
 	}
@@ -757,17 +757,17 @@ func BenchmarkShortLivedConnections(b *testing.B) {
 
 				// Connection establishment
 				manager.processOutgoingHooks(p.syn)
-				manager.dropFilter(p.synAck, manager.incomingRules)
+				manager.dropFilter(p.synAck)
 				manager.processOutgoingHooks(p.ack)
 
 				// Data transfer
 				manager.processOutgoingHooks(p.request)
-				manager.dropFilter(p.response, manager.incomingRules)
+				manager.dropFilter(p.response)
 
 				// Connection teardown
 				manager.processOutgoingHooks(p.finClient)
-				manager.dropFilter(p.ackServer, manager.incomingRules)
+				manager.dropFilter(p.ackServer)
-				manager.dropFilter(p.finServer, manager.incomingRules)
+				manager.dropFilter(p.finServer)
 				manager.processOutgoingHooks(p.ackClient)
 			}
 		})
@@ -828,7 +828,7 @@ func BenchmarkParallelLongLivedConnections(b *testing.B) {
 
 				synack := generateTCPPacketWithFlags(b, dstIPs[i], srcIPs[i],
 					80, uint16(1024+i), uint16(conntrack.TCPSyn|conntrack.TCPAck))
-				manager.dropFilter(synack, manager.incomingRules)
+				manager.dropFilter(synack)
 
 				ack := generateTCPPacketWithFlags(b, srcIPs[i], dstIPs[i],
 					uint16(1024+i), 80, uint16(conntrack.TCPAck))
@@ -855,7 +855,7 @@ func BenchmarkParallelLongLivedConnections(b *testing.B) {
 
 					// Simulate bidirectional traffic
 					manager.processOutgoingHooks(outPackets[connIdx])
-					manager.dropFilter(inPackets[connIdx], manager.incomingRules)
+					manager.dropFilter(inPackets[connIdx])
 				}
 			})
 		})
@@ -952,15 +952,15 @@ func BenchmarkParallelShortLivedConnections(b *testing.B) {
 
 					// Full connection lifecycle
 					manager.processOutgoingHooks(p.syn)
-					manager.dropFilter(p.synAck, manager.incomingRules)
+					manager.dropFilter(p.synAck)
 					manager.processOutgoingHooks(p.ack)
 
 					manager.processOutgoingHooks(p.request)
-					manager.dropFilter(p.response, manager.incomingRules)
+					manager.dropFilter(p.response)
 
 					manager.processOutgoingHooks(p.finClient)
-					manager.dropFilter(p.ackServer, manager.incomingRules)
+					manager.dropFilter(p.ackServer)
-					manager.dropFilter(p.finServer, manager.incomingRules)
+					manager.dropFilter(p.finServer)
 					manager.processOutgoingHooks(p.ackClient)
 				}
 			})

client/firewall/uspfilter/uspfilter_test.go

@@ -319,7 +319,7 @@ func TestNotMatchByIP(t *testing.T) {
 
 	ip := net.ParseIP("0.0.0.0")
 	proto := fw.ProtocolUDP
-	direction := fw.RuleDirectionOUT
+	direction := fw.RuleDirectionIN
 	action := fw.ActionAccept
 	comment := "Test rule"
 
@@ -357,7 +357,7 @@ func TestNotMatchByIP(t *testing.T) {
 		return
 	}
 
-	if m.dropFilter(buf.Bytes(), m.outgoingRules) {
+	if m.dropFilter(buf.Bytes()) {
 		t.Errorf("expected packet to be accepted")
 		return
 	}
@@ -669,7 +669,7 @@ func TestStatefulFirewall_UDPTracking(t *testing.T) {
 	for _, cp := range checkPoints {
 		time.Sleep(cp.sleep)
 
-		drop = manager.dropFilter(inboundBuf.Bytes(), manager.incomingRules)
+		drop = manager.dropFilter(inboundBuf.Bytes())
 		require.Equal(t, cp.shouldAllow, !drop, cp.description)
 
 		// If the connection should still be valid, verify it exists
@@ -740,7 +740,7 @@ func TestStatefulFirewall_UDPTracking(t *testing.T) {
 			require.NoError(t, err)
 
 			// Verify the invalid packet is dropped
-			drop = manager.dropFilter(testBuf.Bytes(), manager.incomingRules)
+			drop = manager.dropFilter(testBuf.Bytes())
 			require.True(t, drop, tc.description)
 		})
 	}