[management] Only trust forwarded-IP headers from configured trusted peers (#7454)

2026-09-12 17:59:06 +02:00 · 2026-09-11 15:41:52 +03:00
parent b57f0e5608
commit 58114f98fb
5 changed files with 240 additions and 25 deletions
@@ -808,8 +808,9 @@ server:

  # Trust X-Forwarded-* only from the Traefik container's static address. Both
  # keys must stay in step with the ipv4_address pinned in docker-compose.yml:
-  # trustedPeers decides whether forwarded headers are read at all, and leaving
-  # it unset falls back to 0.0.0.0/0.
+  # trustedPeers decides whether forwarded headers are read at all. Leaving it
+  # unset trusts nothing and records Traefik's own address as every peer's
+  # connection IP.
  reverseProxy:
    trustedPeers:
      - "${TRAEFIK_IP}/32"
@@ -153,6 +153,7 @@ check_domain_resolves() {
 #   NETBIRD_TRAEFIK_CERTRESOLVER      external-Traefik cert resolver (type 1)
 #   NETBIRD_BIND_LOCALHOST_ONLY       true/false (default true, types 2-5)
 #   NETBIRD_EXTERNAL_PROXY_NETWORK    docker network to join (types 2-4)
+#   NETBIRD_TRUSTED_PEERS             reverse proxy address management sees (default: built-in Traefik's IP, empty for types 1-5)
 #   NETBIRD_NON_INTERACTIVE           true forces unattended mode even with a TTY

 # tty_available succeeds only when we may prompt: never when the operator has
@@ -459,6 +460,8 @@ initialize_default_values() {
  MANAGEMENT_HOST_PORT="8081"  # Combined server port (management + signal + relay)
  BIND_LOCALHOST_ONLY="true"
  EXTERNAL_PROXY_NETWORK=""
+  TRUSTED_PEERS=""             # Address the reverse proxy connects to management from
+

  # Traefik static IP within the internal bridge network
  TRAEFIK_IP="172.30.0.10"
@@ -519,6 +522,7 @@ apply_agent_network_preset() {
  REVERSE_PROXY_TYPE="0"
  ENABLE_PROXY="true"
  ENABLE_CROWDSEC="false"
+  TRUSTED_PEERS="${NETBIRD_TRUSTED_PEERS:-$TRAEFIK_IP/32}"

  TRAEFIK_ACME_EMAIL=$(resolve NETBIRD_LETSENCRYPT_EMAIL required read_traefik_acme_email)

@@ -573,6 +577,21 @@ configure_reverse_proxy() {
    4) EXTERNAL_PROXY_NETWORK=$(resolve NETBIRD_EXTERNAL_PROXY_NETWORK "" read_proxy_docker_network "Caddy") ;;
    *) ;; # No network prompt for other options
  esac
+
+  # Only the bundled Traefik has an address we know at render time. External proxies
+  # must supply the address their proxy reaches management from.
+  if [[ "$REVERSE_PROXY_TYPE" == "0" ]]; then
+    TRUSTED_PEERS="${NETBIRD_TRUSTED_PEERS:-$TRAEFIK_IP/32}"
+  else
+    TRUSTED_PEERS="${NETBIRD_TRUSTED_PEERS:-}"
+    if [[ -z "$TRUSTED_PEERS" ]]; then
+      echo "" > /dev/stderr
+      echo "Note: reverseProxy.trustedPeers is unset, so NetBird will use the address your" > /dev/stderr
+      echo "proxy connects from as each peer's connection IP. To record real client IPs," > /dev/stderr
+      echo "set NETBIRD_TRUSTED_PEERS to your proxy's address (e.g. 172.20.0.5/32) and re-run." > /dev/stderr
+      echo "" > /dev/stderr
+    fi
+  fi
  return 0
 }

@@ -1033,6 +1052,7 @@ server:
  reverseProxy:
    trustedHTTPProxies:
      - "$TRAEFIK_IP/32"
+$(render_trusted_peers)

  store:
    engine: "sqlite"
@@ -1041,6 +1061,12 @@ EOF
  return 0
 }

+render_trusted_peers() {
+  if [[ -n "$TRUSTED_PEERS" ]]; then
+    printf '    trustedPeers:\n      - "%s"' "$TRUSTED_PEERS"
+  fi
+}
+
 render_dashboard_env() {
  cat <<EOF
 # Endpoints
@@ -1453,6 +1479,9 @@ location ~ ^/(relay|ws-proxy/) {
 # Native gRPC (signal + management)
 location ~ ^/(signalexchange\.SignalExchange|management\.ManagementService)/ {
    grpc_pass grpc://${server_addr};
+    # Overwrite rather than pass through: without this the client's own
+    # x-forwarded-for metadata reaches NetBird as the connection IP.
+    grpc_set_header X-Forwarded-For \$proxy_add_x_forwarded_for;
    grpc_read_timeout 1d;
    grpc_send_timeout 1d;
    grpc_socket_keepalive on;
@@ -34,9 +34,7 @@
    "ReverseProxy": {
        "TrustedHTTPProxies": [],
        "TrustedHTTPProxiesCount": 0,
-        "TrustedPeers": [
-             "0.0.0.0/0"
-        ]
+        "TrustedPeers": []
    },
    "DisableDefaultPolicy": $NETBIRD_MGMT_DISABLE_DEFAULT_POLICY,
    "Datadir": "",
@@ -31,6 +31,7 @@ import (
 	rpservice "github.com/netbirdio/netbird/management/internals/modules/reverseproxy/service"
 	networkmapdb "github.com/netbirdio/netbird/management/internals/network_map_db"
 	networkmapdbfactory "github.com/netbirdio/netbird/management/internals/network_map_db/factory"
+	nbconfig "github.com/netbirdio/netbird/management/internals/server/config"
 	nbgrpc "github.com/netbirdio/netbird/management/internals/shared/grpc"
 	"github.com/netbirdio/netbird/management/server/activity"
 	activitystore "github.com/netbirdio/netbird/management/server/activity/store"
@@ -111,7 +112,8 @@ func (s *BaseServer) NetworkMapStore() *networkmapdb.NetworkMapDBStoreImpl {
 			s.Config.StoreConfig.Engine,
 			s.Config.Datadir,
 			s.IntegratedValidator(),
-			s.SettingsManager())
+			s.SettingsManager(),
+		)
 		// networkmap db store supports postgres and sqlite backends only
 		// for other backends a fallback is used, so NotSupportedStoreEngineError
 		// is not a fatal error
@@ -180,24 +182,7 @@ func (s *BaseServer) RateLimiter() *middleware.APIRateLimiter {

 func (s *BaseServer) GRPCServer() *grpc.Server {
 	return Create(s, func() *grpc.Server {
-		trustedPeers := s.Config.ReverseProxy.TrustedPeers
-		defaultTrustedPeers := []netip.Prefix{netip.MustParsePrefix("0.0.0.0/0"), netip.MustParsePrefix("::/0")}
-		if len(trustedPeers) == 0 || slices.Equal[[]netip.Prefix](trustedPeers, defaultTrustedPeers) {
-			log.WithContext(context.Background()).Warn("TrustedPeers are configured to default value '0.0.0.0/0', '::/0'. This allows connection IP spoofing.")
-			trustedPeers = defaultTrustedPeers
-		}
-		trustedHTTPProxies := s.Config.ReverseProxy.TrustedHTTPProxies
-		trustedProxiesCount := s.Config.ReverseProxy.TrustedHTTPProxiesCount
-		if len(trustedHTTPProxies) > 0 && trustedProxiesCount > 0 {
-			log.WithContext(context.Background()).Warn("TrustedHTTPProxies and TrustedHTTPProxiesCount both are configured. " +
-				"This is not recommended way to extract X-Forwarded-For. Consider using one of these options.")
-		}
-		realipOpts := []realip.Option{
-			realip.WithTrustedPeers(trustedPeers),
-			realip.WithTrustedProxies(trustedHTTPProxies),
-			realip.WithTrustedProxiesCount(trustedProxiesCount),
-			realip.WithHeaders([]string{realip.XForwardedFor, realip.XRealIp}),
-		}
+		realipOpts := realIPOptions(s.Config.ReverseProxy)
 		proxyUnary, proxyStream, proxyAuthClose := nbgrpc.NewProxyAuthInterceptors(s.Store())
 		s.proxyAuthClose = proxyAuthClose
 		gRPCOpts := []grpc.ServerOption{
@@ -333,7 +318,7 @@ func (s *BaseServer) AccessLogsManager() accesslogs.Manager {
 	})
 }

-func loadTLSConfig(certFile string, certKey string) (*tls.Config, error) {
+func loadTLSConfig(certFile, certKey string) (*tls.Config, error) {
 	// Load server's certificate and private key
 	serverCert, err := tls.LoadX509KeyPair(certFile, certKey)
 	if err != nil {
@@ -380,3 +365,34 @@ func streamInterceptor(
 	wrapped.WrappedContext = context.WithValue(ctx, nbContext.RequestIDKey, reqID)
 	return handler(srv, wrapped)
 }
+
+// realIPOptions builds the real-IP middleware options from the reverse proxy config.
+//
+// TrustedPeers controls which transport peers are allowed to supply forwarded-IP
+// headers. If empty, forwarded headers are ignored and the transport peer address
+// is used directly. Operators terminating connections at a reverse proxy should
+// configure TrustedPeers with that proxy's address or network.
+//
+// Only X-Forwarded-For is trusted. X-Real-IP contains a single client-supplied
+// address with no proxy chain to validate, and none of the reverse proxies we ship
+// use it on the gRPC path.
+func realIPOptions(cfg nbconfig.ReverseProxy) []realip.Option {
+	if idx := slices.IndexFunc(cfg.TrustedPeers, func(p netip.Prefix) bool { return p.Bits() == 0 }); idx >= 0 {
+		log.WithContext(context.Background()).Warnf("TrustedPeers contains the default route %s, which trusts "+
+			"X-Forwarded-For from every client and allows connection IP spoofing. Set TrustedPeers to the address "+
+			"of your reverse proxy, or leave it empty to use the connection's source address.", cfg.TrustedPeers[idx])
+	}
+	if cfg.TrustedHTTPProxiesCount > 0 {
+		log.WithContext(context.Background()).Warn(
+			"TrustedHTTPProxiesCount skips X-Forwarded-For entries by position before TrustedHTTPProxies filters by address. " +
+				"An incorrect count may skip the real client IP and produce an incorrect source address.",
+		)
+	}
+
+	return []realip.Option{
+		realip.WithTrustedPeers(cfg.TrustedPeers),
+		realip.WithTrustedProxies(cfg.TrustedHTTPProxies),
+		realip.WithTrustedProxiesCount(cfg.TrustedHTTPProxiesCount),
+		realip.WithHeaders([]string{realip.XForwardedFor}),
+	}
+}
@@ -0,0 +1,171 @@
+package server
+
+import (
+	"context"
+	"io"
+	"net"
+	"net/netip"
+	"testing"
+	"time"
+
+	"github.com/grpc-ecosystem/go-grpc-middleware/v2/interceptors/realip"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+	"google.golang.org/grpc"
+	"google.golang.org/grpc/credentials/insecure"
+	"google.golang.org/grpc/metadata"
+	"google.golang.org/protobuf/types/known/emptypb"
+
+	nbconfig "github.com/netbirdio/netbird/management/internals/server/config"
+)
+
+const (
+	realIPProbeMethod       = "/netbird.test.RealIPProbe/Probe"
+	realIPProbeStreamMethod = "/netbird.test.RealIPProbe/ProbeStream"
+)
+
+// realIPProbe records the real IP the middleware derived for each call.
+type realIPProbe struct {
+	got chan string
+}
+
+func (p *realIPProbe) record(ctx context.Context) {
+	addr, _ := realip.FromContext(ctx)
+	p.got <- addr.String()
+}
+
+func (p *realIPProbe) wait(t *testing.T) string {
+	t.Helper()
+
+	select {
+	case got := <-p.got:
+		return got
+	case <-time.After(5 * time.Second):
+		t.Fatal("timed out waiting for probe")
+		return ""
+	}
+}
+
+func startProbeServer(t *testing.T, cfg nbconfig.ReverseProxy) (*grpc.ClientConn, *realIPProbe) {
+	t.Helper()
+
+	listener, err := net.Listen("tcp", "127.0.0.1:0")
+	require.NoError(t, err)
+
+	probe := &realIPProbe{got: make(chan string, 1)}
+	opts := realIPOptions(cfg)
+	srv := grpc.NewServer(
+		grpc.ChainUnaryInterceptor(realip.UnaryServerInterceptorOpts(opts...)),
+		grpc.ChainStreamInterceptor(realip.StreamServerInterceptorOpts(opts...)),
+	)
+	srv.RegisterService(&grpc.ServiceDesc{
+		ServiceName: "netbird.test.RealIPProbe",
+		HandlerType: (*any)(nil),
+		Methods: []grpc.MethodDesc{{
+			MethodName: "Probe",
+			Handler: func(_ any, ctx context.Context, dec func(any) error, interceptor grpc.UnaryServerInterceptor) (any, error) {
+				req := new(emptypb.Empty)
+				if err := dec(req); err != nil {
+					return nil, err
+				}
+				handler := func(ctx context.Context, _ any) (any, error) {
+					probe.record(ctx)
+					return &emptypb.Empty{}, nil
+				}
+				if interceptor == nil {
+					return handler(ctx, req)
+				}
+				return interceptor(ctx, req, &grpc.UnaryServerInfo{FullMethod: realIPProbeMethod}, handler)
+			},
+		}},
+		Streams: []grpc.StreamDesc{{
+			StreamName:    "ProbeStream",
+			ServerStreams: true,
+			Handler: func(_ any, stream grpc.ServerStream) error {
+				probe.record(stream.Context())
+				return nil
+			},
+		}},
+	}, probe)
+
+	go func() { _ = srv.Serve(listener) }()
+	t.Cleanup(srv.Stop)
+
+	conn, err := grpc.NewClient(listener.Addr().String(), grpc.WithTransportCredentials(insecure.NewCredentials()))
+	require.NoError(t, err)
+	t.Cleanup(func() { _ = conn.Close() })
+
+	return conn, probe
+}
+
+func callUnary(t *testing.T, conn *grpc.ClientConn, probe *realIPProbe, kv ...string) string {
+	t.Helper()
+
+	ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
+	defer cancel()
+	ctx = metadata.AppendToOutgoingContext(ctx, kv...)
+	require.NoError(t, conn.Invoke(ctx, realIPProbeMethod, &emptypb.Empty{}, &emptypb.Empty{}))
+
+	return probe.wait(t)
+}
+
+func callStream(t *testing.T, conn *grpc.ClientConn, probe *realIPProbe, kv ...string) string {
+	t.Helper()
+
+	ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
+	defer cancel()
+	ctx = metadata.AppendToOutgoingContext(ctx, kv...)
+	desc := &grpc.StreamDesc{StreamName: "ProbeStream", ServerStreams: true}
+	stream, err := conn.NewStream(ctx, desc, realIPProbeStreamMethod)
+	require.NoError(t, err)
+	require.NoError(t, stream.CloseSend())
+	require.ErrorIs(t, stream.RecvMsg(&emptypb.Empty{}), io.EOF)
+
+	return probe.wait(t)
+}
+
+func assertRealIP(t *testing.T, cfg nbconfig.ReverseProxy, want string, kv ...string) {
+	t.Helper()
+
+	conn, probe := startProbeServer(t, cfg)
+	t.Run("unary", func(t *testing.T) {
+		assert.Equal(t, want, callUnary(t, conn, probe, kv...))
+	})
+	t.Run("stream", func(t *testing.T) {
+		assert.Equal(t, want, callStream(t, conn, probe, kv...))
+	})
+}
+
+func TestRealIPDefaultIgnoresClientForwardedHeaders(t *testing.T) {
+	assertRealIP(t, nbconfig.ReverseProxy{}, "127.0.0.1",
+		realip.XForwardedFor, "203.0.113.44",
+		realip.XRealIp, "203.0.113.44",
+	)
+}
+
+func TestRealIPUntrustedPeerIgnoresForwardedHeaders(t *testing.T) {
+	cfg := nbconfig.ReverseProxy{TrustedPeers: []netip.Prefix{netip.MustParsePrefix("10.9.8.7/32")}}
+
+	assertRealIP(t, cfg, "127.0.0.1",
+		realip.XForwardedFor, "203.0.113.44",
+		realip.XRealIp, "203.0.113.44",
+	)
+}
+
+func TestRealIPTrustedPeerHonoursForwardedHeaders(t *testing.T) {
+	cfg := nbconfig.ReverseProxy{TrustedPeers: []netip.Prefix{netip.MustParsePrefix("127.0.0.1/32")}}
+
+	assertRealIP(t, cfg, "203.0.113.44",
+		realip.XForwardedFor, "203.0.113.44",
+		realip.XRealIp, "203.0.113.44",
+	)
+}
+
+func TestRealIPIgnoresXRealIPWhenProxyCountIsSet(t *testing.T) {
+	cfg := nbconfig.ReverseProxy{
+		TrustedPeers:            []netip.Prefix{netip.MustParsePrefix("127.0.0.1/32")},
+		TrustedHTTPProxiesCount: 1,
+	}
+
+	assertRealIP(t, cfg, "127.0.0.1", realip.XRealIp, "203.0.113.44")
+}