diff --git a/infrastructure_files/getting-started-enterprise.sh b/infrastructure_files/getting-started-enterprise.sh index 3f7cf6357..701598a60 100755 --- a/infrastructure_files/getting-started-enterprise.sh +++ b/infrastructure_files/getting-started-enterprise.sh @@ -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" diff --git a/infrastructure_files/getting-started.sh b/infrastructure_files/getting-started.sh index 5efc0181e..afbc5c282 100755 --- a/infrastructure_files/getting-started.sh +++ b/infrastructure_files/getting-started.sh @@ -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 < 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}), + } +} diff --git a/management/internals/server/realip_test.go b/management/internals/server/realip_test.go new file mode 100644 index 000000000..89ac02730 --- /dev/null +++ b/management/internals/server/realip_test.go @@ -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") +}