From 8efceb676a32b506744ccadd03480e4ff9a8e7d6 Mon Sep 17 00:00:00 2001 From: Gerard Snaauw Date: Mon, 14 Oct 2024 15:57:37 +0200 Subject: [PATCH 1/4] add configurable header to extract client IP --- core/server_config.go | 8 +++ http/cmd/cmd.go | 1 + http/config.go | 6 +- http/echo.go | 4 +- http/echo_test.go | 34 +++++----- http/engine.go | 40 +++++++++-- network/network.go | 1 + network/network_integration_test.go | 1 + network/transport/grpc/config.go | 9 +++ network/transport/grpc/connection_manager.go | 2 +- network/transport/grpc/ip_interceptor.go | 67 +++++++++++++++---- network/transport/grpc/ip_interceptor_test.go | 15 ++++- 12 files changed, 144 insertions(+), 44 deletions(-) diff --git a/core/server_config.go b/core/server_config.go index b9cfc6bfc6..ac7143dd5a 100644 --- a/core/server_config.go +++ b/core/server_config.go @@ -68,9 +68,17 @@ type ServerConfig struct { // LegacyTLS exists to detect usage of deprecated network.{truststorefile,certkeyfile,certfile} parameters. // This can be removed in v6.1+ (can't skip minors in migration). See https://github.com/nuts-foundation/nuts-node/issues/2909 LegacyTLS TLSConfig `koanf:"network"` + // HTTP exists to expose http.clientipheader to the nuts-network layer. + // This header should contaisn the client IP address for logging. Can be removed together with the nuts-network + HTTP HTTPConfig `koanf:"http"` configMap *koanf.Koanf } +// Config is the top-level config struct for HTTP interfaces. +type HTTPConfig struct { + ClientIPHeaderName string `koanf:"clientipheader"` +} + // HTTPClientConfig contains settings for HTTP clients. type HTTPClientConfig struct { // Timeout specifies the timeout for HTTP requests. diff --git a/http/cmd/cmd.go b/http/cmd/cmd.go index 4eedaf8316..3c095aaae1 100644 --- a/http/cmd/cmd.go +++ b/http/cmd/cmd.go @@ -35,6 +35,7 @@ func FlagSet() *pflag.FlagSet { flags.String("http.internal.auth.audience", defs.Internal.Auth.Audience, "Expected audience for JWT tokens (default: hostname)") flags.String("http.internal.auth.authorizedkeyspath", defs.Internal.Auth.AuthorizedKeysPath, "Path to an authorized_keys file for trusted JWT signers") flags.String("http.log", string(defs.Log), fmt.Sprintf("What to log about HTTP requests. Options are '%s', '%s' (log request method, URI, IP and response code), and '%s' (log the request and response body, in addition to the metadata). When debug vebosity is set the authorization headers are also logged when the request is fully logged.", http.LogNothingLevel, http.LogMetadataLevel, http.LogMetadataAndBodyLevel)) + flags.String("http.clientipheader", defs.ClientIPHeaderName, "Case-sensitive HTTP Header that contains the client IP used for audit logs. Supports X-Forwarded-For, X-Real-IP, and custom headers. For custom headers the audit log value will be the exact value of the header.") flags.Int("http.cache.maxbytes", defs.ResponseCacheSize, "HTTP client maximum size of the response cache in bytes. If 0, the HTTP client does not cache responses.") return flags diff --git a/http/config.go b/http/config.go index 4554802dfb..35cd5d7477 100644 --- a/http/config.go +++ b/http/config.go @@ -28,7 +28,8 @@ func DefaultConfig() Config { Public: PublicConfig{ Address: ":8080", }, - ResponseCacheSize: 10 * 1024 * 1024, // 10mb + ResponseCacheSize: 10 * 1024 * 1024, // 10mb + ClientIPHeaderName: "X-Forwarded-For", } } @@ -39,7 +40,8 @@ type Config struct { Public PublicConfig `koanf:"public"` Internal InternalConfig `koanf:"internal"` // ResponseCacheSize is the maximum number of bytes cached by HTTP clients. - ResponseCacheSize int `koanf:"cache.maxbytes"` + ResponseCacheSize int `koanf:"cache.maxbytes"` + ClientIPHeaderName string `koanf:"clientipheader"` } // PublicConfig contains the configuration for outside-facing HTTP endpoints. diff --git a/http/echo.go b/http/echo.go index dd8c07470f..cda8788a9e 100644 --- a/http/echo.go +++ b/http/echo.go @@ -72,7 +72,7 @@ type MultiEcho struct { // results in an error being returned. // If address wasn't used for another bind and thus leads to creating a new Echo server, it returns true. // If an existing Echo server is returned, it returns false. -func (c *MultiEcho) Bind(path string, address string, creatorFn func() (EchoServer, error)) error { +func (c *MultiEcho) Bind(path string, address string, creatorFn func(ipHeader string) (EchoServer, error), ipHeader string) error { if len(address) == 0 { return errors.New("empty address") } @@ -86,7 +86,7 @@ func (c *MultiEcho) Bind(path string, address string, creatorFn func() (EchoServ } c.binds[path] = address if _, addressBound := c.interfaces[address]; !addressBound { - server, err := creatorFn() + server, err := creatorFn(ipHeader) if err != nil { return err } diff --git a/http/echo_test.go b/http/echo_test.go index cb353773b4..1dce7ff22d 100644 --- a/http/echo_test.go +++ b/http/echo_test.go @@ -32,18 +32,18 @@ func Test_MultiEcho_Bind(t *testing.T) { const defaultAddress = ":1323" t.Run("group already bound", func(t *testing.T) { m := NewMultiEcho() - err := m.Bind("", defaultAddress, func() (EchoServer, error) { + err := m.Bind("", defaultAddress, func(ipHeader string) (EchoServer, error) { return echo.New(), nil - }) + }, "header") require.NoError(t, err) - err = m.Bind("", defaultAddress, func() (EchoServer, error) { + err = m.Bind("", defaultAddress, func(ipHeader string) (EchoServer, error) { return echo.New(), nil - }) + }, "header") assert.EqualError(t, err, "http bind already exists: /") }) t.Run("error - group contains subpaths", func(t *testing.T) { m := NewMultiEcho() - err := m.Bind("internal/vdr", defaultAddress, nil) + err := m.Bind("internal/vdr", defaultAddress, nil, "") assert.EqualError(t, err, "bind can't contain subpaths: internal/vdr") }) } @@ -55,9 +55,9 @@ func Test_MultiEcho_Start(t *testing.T) { server.EXPECT().Start(gomock.Any()).Return(errors.New("unable to start")) m := NewMultiEcho() - m.Bind("group2", ":8080", func() (EchoServer, error) { + m.Bind("group2", ":8080", func(ipHeader string) (EchoServer, error) { return server, nil - }) + }, "header") err := m.Start() assert.EqualError(t, err, "unable to start") }) @@ -84,22 +84,22 @@ func Test_MultiEcho(t *testing.T) { // Bind interfaces m := NewMultiEcho() - err := m.Bind(RootPath, defaultAddress, func() (EchoServer, error) { + err := m.Bind(RootPath, defaultAddress, func(ipHeader string) (EchoServer, error) { return defaultServer, nil - }) + }, "header") require.NoError(t, err) - err = m.Bind("internal", "internal:8080", func() (EchoServer, error) { + err = m.Bind("internal", "internal:8080", func(ipHeader string) (EchoServer, error) { return internalServer, nil - }) + }, "header") require.NoError(t, err) - err = m.Bind("public", "public:8080", func() (EchoServer, error) { + err = m.Bind("public", "public:8080", func(ipHeader string) (EchoServer, error) { return publicServer, nil - }) + }, "header") require.NoError(t, err) - err = m.Bind("extra-public", "public:8080", func() (EchoServer, error) { + err = m.Bind("extra-public", "public:8080", func(ipHeader string) (EchoServer, error) { t.Fatal("should not be called!") return nil, nil - }) + }, "header") require.NoError(t, err) m.addFn(http.MethodPost, "/public/pub-endpoint", nil) @@ -129,9 +129,9 @@ func Test_MultiEcho_Methods(t *testing.T) { ) m := NewMultiEcho() - m.Bind(RootPath, ":1323", func() (EchoServer, error) { + m.Bind(RootPath, ":1323", func(ipHeader string) (EchoServer, error) { return defaultServer, nil - }) + }, "header") m.GET("/get", nil) m.POST("/post", nil) m.PUT("/put", nil) diff --git a/http/engine.go b/http/engine.go index d64941bbea..6794a3bd44 100644 --- a/http/engine.go +++ b/http/engine.go @@ -23,6 +23,7 @@ import ( "errors" "fmt" "github.com/nuts-foundation/nuts-node/http/client" + "net" "net/http" "os" "strings" @@ -79,12 +80,12 @@ func (h *Engine) Configure(serverConfig core.ServerConfig) error { h.server = NewMultiEcho() // Public endpoints - if err := h.server.Bind(RootPath, h.config.Public.Address, h.createEchoServer); err != nil { + if err := h.server.Bind(RootPath, h.config.Public.Address, h.createEchoServer, h.config.ClientIPHeaderName); err != nil { return err } // Internal endpoints for _, httpPath := range []string{"/internal", "/status", "/health", "/metrics"} { - if err := h.server.Bind(httpPath, h.config.Internal.Address, h.createEchoServer); err != nil { + if err := h.server.Bind(httpPath, h.config.Internal.Address, h.createEchoServer, h.config.ClientIPHeaderName); err != nil { return err } } @@ -102,7 +103,7 @@ func (h *Engine) configureClient(serverConfig core.ServerConfig) { } } -func (h *Engine) createEchoServer() (EchoServer, error) { +func (h *Engine) createEchoServer(ipHeader string) (EchoServer, error) { echoServer := echo.New() echoServer.HideBanner = true echoServer.HidePort = true @@ -110,8 +111,15 @@ func (h *Engine) createEchoServer() (EchoServer, error) { // ErrorHandler echoServer.HTTPErrorHandler = core.CreateHTTPErrorHandler() - // Reverse proxies must set the X-Forwarded-For header to the original client IP. - echoServer.IPExtractor = echo.ExtractIPFromXFFHeader() + // Extract original client IP from configured header. + switch ipHeader { + case echo.HeaderXForwardedFor: + echoServer.IPExtractor = echo.ExtractIPFromXFFHeader() + case "": + echoServer.IPExtractor = echo.ExtractIPDirect() // use request IP if there is no header + default: + echoServer.IPExtractor = extractIPFromCustomHeader(ipHeader) + } return &echoAdapter{ startFn: echoServer.Start, @@ -253,3 +261,25 @@ func (h Engine) applyAuthMiddleware(echoServer core.EchoRouter, path string, con return nil } + +// extractIPFromCustomHeader extracts an IP address from any custom header. +// If the header is missing or contains an invalid IP, the extractor returns the ip from the request. +// This is an altered version of echo.ExtractIPFromRealIPHeader() that does not check for trusted IPs. +func extractIPFromCustomHeader(ipHeader string) echo.IPExtractor { + extractIP := echo.ExtractIPDirect() + return func(req *http.Request) string { + directIP := extractIP(req) // source address from IPv4/IPv6 packet header + realIP := req.Header.Get(ipHeader) + if realIP == "" { + return directIP + } + + realIP = strings.TrimPrefix(realIP, "[") + realIP = strings.TrimSuffix(realIP, "]") + if rIP := net.ParseIP(realIP); rIP != nil { + return realIP + } + + return directIP + } +} diff --git a/network/network.go b/network/network.go index 0f148d2fea..be8618bdd8 100644 --- a/network/network.go +++ b/network/network.go @@ -295,6 +295,7 @@ func (n *Network) Configure(config core.ServerConfig) error { return fmt.Errorf("failed to open connections store: %w", err) } + grpcOpts = append(grpcOpts, grpc.WithClientIPHeader(config.HTTP.ClientIPHeaderName)) connectionManCfg, err := grpc.NewConfig(n.config.GrpcAddr, n.peerID, grpcOpts...) if err != nil { return err diff --git a/network/network_integration_test.go b/network/network_integration_test.go index 593fe6d535..e1d18e557c 100644 --- a/network/network_integration_test.go +++ b/network/network_integration_test.go @@ -901,6 +901,7 @@ func TestNetworkIntegration_TLSOffloading(t *testing.T) { node1 := startNode(t, "node1", testDirectory, func(serverCfg *core.ServerConfig, cfg *Config) { serverCfg.TLS.Offload = core.OffloadIncomingTLS serverCfg.TLS.ClientCertHeaderName = "client-cert" + serverCfg.HTTP.ClientIPHeaderName = "X-Forwarded-For" }) // Create client (node2) that connects to server node diff --git a/network/transport/grpc/config.go b/network/transport/grpc/config.go index c39aa4f791..5f727d0412 100644 --- a/network/transport/grpc/config.go +++ b/network/transport/grpc/config.go @@ -76,6 +76,13 @@ func WithTLS(clientCertificate tls.Certificate, trustStore *core.TrustStore, pki } } +func WithClientIPHeader(clientIPHeaderName string) ConfigOption { + return func(config *Config) error { + config.clientIPHeaderName = clientIPHeaderName + return nil + } +} + // WithTLSOffloading enables TLS for outgoing connections, but is offloaded for incoming connections. // It MUST be used in conjunction, but after with WithTLS. func WithTLSOffloading(clientCertHeaderName string) ConfigOption { @@ -121,6 +128,8 @@ type Config struct { pkiValidator pki.Validator // clientCertHeaderName specifies the name of the HTTP header that contains the client certificate, if TLS is offloaded. clientCertHeaderName string + // clientIPHeaderName specifies the name of the HTTP header that contains the client IP address. + clientIPHeaderName string // connectionTimeout specifies the time before an outbound connection attempt times out. connectionTimeout time.Duration // listener holds a function to create the net.Listener that is used for inbound connections. diff --git a/network/transport/grpc/connection_manager.go b/network/transport/grpc/connection_manager.go index 12371dd029..f738a56fe4 100644 --- a/network/transport/grpc/connection_manager.go +++ b/network/transport/grpc/connection_manager.go @@ -185,7 +185,7 @@ func newGrpcServer(config Config) (*grpc.Server, error) { } // Chain interceptors. ipInterceptor is added last, so it processes the stream first. - serverInterceptors = append(serverInterceptors, ipInterceptor) + serverInterceptors = append(serverInterceptors, ipInterceptor(config.clientIPHeaderName)) serverOpts = append(serverOpts, grpc.ChainStreamInterceptor(serverInterceptors...)) // Create gRPC server for inbound connectionList and associate it with the protocols diff --git a/network/transport/grpc/ip_interceptor.go b/network/transport/grpc/ip_interceptor.go index 14f0e11157..4d54520f1b 100644 --- a/network/transport/grpc/ip_interceptor.go +++ b/network/transport/grpc/ip_interceptor.go @@ -32,28 +32,39 @@ const headerXForwardedFor = "X-Forwarded-For" // ipInterceptor tries to extract the IP from the X-Forwarded-For header and sets this as the peers address. // No address is set if the header is unavailable. -func ipInterceptor(srv interface{}, serverStream grpc.ServerStream, _ *grpc.StreamServerInfo, handler grpc.StreamHandler) error { - addr := addrFrom(extractIPFromXFFHeader(serverStream)) - if addr == nil { - // Exit without change if there is no X-Forwarded-For in the http header, - // or if no IP could be extracted from the header. - // This will default to the IP found in lvl 4 header. - return handler(srv, serverStream) +func ipInterceptor(ipHeader string) grpc.StreamServerInterceptor { + var extractIPHeader func(serverStream grpc.ServerStream) string + switch ipHeader { + case headerXForwardedFor: + extractIPHeader = extractIPFromXFFHeader + case "": + extractIPHeader = extractPeerIP + default: + extractIPHeader = extractIPFromCustomHeader(ipHeader) } + return func(srv interface{}, serverStream grpc.ServerStream, _ *grpc.StreamServerInfo, handler grpc.StreamHandler) error { + addr := addrFrom(extractIPHeader(serverStream)) + if addr == nil { + // Exit without change if there is no X-Forwarded-For in the http header, + // or if no IP could be extracted from the header. + // This will default to the IP found in lvl 4 header. + return handler(srv, serverStream) + } - peerInfo, _ := peer.FromContext(serverStream.Context()) - if peerInfo == nil { - peerInfo = &peer.Peer{} + peerInfo, _ := peer.FromContext(serverStream.Context()) + if peerInfo == nil { + peerInfo = &peer.Peer{} + } + peerInfo.Addr = addr + ctx := peer.NewContext(serverStream.Context(), peerInfo) + return handler(srv, &wrappedServerStream{ctx: ctx, ServerStream: serverStream}) } - peerInfo.Addr = addr - ctx := peer.NewContext(serverStream.Context(), peerInfo) - return handler(srv, &wrappedServerStream{ctx: ctx, ServerStream: serverStream}) } // extractIPFromXFFHeader tries to retrieve the address from X-Forward-For header. Returns an empty string if non found. // Implementation is based on echo.ExtractIPFromXFFHeader(). func extractIPFromXFFHeader(serverStream grpc.ServerStream) string { - ipUnknown := "" + ipUnknown := extractPeerIP(serverStream) md, ok := metadata.FromIncomingContext(serverStream.Context()) if !ok { return ipUnknown @@ -97,3 +108,31 @@ func addrFrom(ip string) net.Addr { func isInternal(ip net.IP) bool { return ip.IsLoopback() || ip.IsLinkLocalUnicast() || ip.IsPrivate() } + +// extractIPFromCustomHeader extracts an IP address from any custom header. +// If the header is missing or contains an invalid IP, the extractor tries to return the IP in the peer.Peer. +// This is an altered version of echo.ExtractIPFromRealIPHeader() that does not check for trusted IPs. +func extractIPFromCustomHeader(ipHeader string) func(serverStream grpc.ServerStream) string { + return func(serverStream grpc.ServerStream) string { + directIP := extractPeerIP(serverStream) + md, ok := metadata.FromIncomingContext(serverStream.Context()) + if !ok { + return directIP + } + header := md.Get(ipHeader) + if len(header) == 0 { + return directIP + } + + return strings.Join(header, ",") + } +} + +// extractPeerID returns the peer.Peer's Addr if available in the serverStream.Context +func extractPeerIP(serverStream grpc.ServerStream) string { + peer, ok := peer.FromContext(serverStream.Context()) + if !ok { + return "" + } + return peer.Addr.String() +} diff --git a/network/transport/grpc/ip_interceptor_test.go b/network/transport/grpc/ip_interceptor_test.go index a16a3ca21f..c91d85091f 100644 --- a/network/transport/grpc/ip_interceptor_test.go +++ b/network/transport/grpc/ip_interceptor_test.go @@ -79,12 +79,15 @@ func Test_ipInterceptor(t *testing.T) { } for _, tc := range tests { + ran := false serverStream.ctx = contextWithMD(strings.Join(tc.xffIPs, ",")) - _ = ipInterceptor(nil, serverStream, nil, func(srv interface{}, wrappedStream grpc.ServerStream) error { + _ = ipInterceptor(headerXForwardedFor)(nil, serverStream, nil, func(srv interface{}, wrappedStream grpc.ServerStream) error { peerInfo, success = peer.FromContext(wrappedStream.Context()) + ran = true return nil }) + require.True(t, ran, "test logic was not executed") if success { assert.Equal(t, tc.expected.String(), peerInfo.Addr.String()) } else { @@ -93,20 +96,26 @@ func Test_ipInterceptor(t *testing.T) { } }) t.Run("no XXF header", func(t *testing.T) { + ran := false serverStream.ctx = metadata.NewIncomingContext(context.Background(), metadata.New(map[string]string{})) - _ = ipInterceptor(nil, serverStream, nil, func(srv interface{}, wrappedStream grpc.ServerStream) error { + _ = ipInterceptor(headerXForwardedFor)(nil, serverStream, nil, func(srv interface{}, wrappedStream grpc.ServerStream) error { peerInfo, success = peer.FromContext(wrappedStream.Context()) + ran = true return nil }) + require.True(t, ran, "test logic was not executed") assert.False(t, success) assert.Nil(t, peerInfo) }) t.Run("no metadata in context", func(t *testing.T) { + ran := false serverStream.ctx = context.Background() - _ = ipInterceptor(nil, serverStream, nil, func(srv interface{}, wrappedStream grpc.ServerStream) error { + _ = ipInterceptor(headerXForwardedFor)(nil, serverStream, nil, func(srv interface{}, wrappedStream grpc.ServerStream) error { peerInfo, success = peer.FromContext(wrappedStream.Context()) + ran = true return nil }) + require.True(t, ran, "test logic was not executed") assert.False(t, success) assert.Nil(t, peerInfo) }) From d65587503a3b26598844f9c19bf4f5c6d76580f8 Mon Sep 17 00:00:00 2001 From: Gerard Snaauw Date: Wed, 16 Oct 2024 10:07:35 +0200 Subject: [PATCH 2/4] add tests --- docs/pages/deployment/server_options.rst | 1 + http/cmd/cmd.go | 2 +- http/engine.go | 2 +- http/engine_test.go | 20 ++++++++ network/transport/grpc/config.go | 1 + network/transport/grpc/ip_interceptor.go | 33 +++++++------ network/transport/grpc/ip_interceptor_test.go | 47 +++++++++++++++++++ 7 files changed, 87 insertions(+), 19 deletions(-) diff --git a/docs/pages/deployment/server_options.rst b/docs/pages/deployment/server_options.rst index 8afc2d976d..e4c2c47f3f 100755 --- a/docs/pages/deployment/server_options.rst +++ b/docs/pages/deployment/server_options.rst @@ -32,6 +32,7 @@ discovery.definitions.directory ./config/discovery Directory to load Discovery Service Definitions from. If not set, the discovery service will be disabled. If the directory contains JSON files that can't be parsed as service definition, the node will fail to start. discovery.server.ids [] IDs of the Discovery Service for which to act as server. If an ID does not map to a loaded service definition, the node will fail to start. **HTTP** + http.clientipheader X-Forwarded-For Case-sensitive HTTP Header that contains the client IP used for audit logs. For the X-Forwarded-For header only link-local, loopback, and private IPs are excluded. Switch to X-Real-IP or a custom header if you see your own proxy/infra in the logs. http.log metadata What to log about HTTP requests. Options are 'nothing', 'metadata' (log request method, URI, IP and response code), and 'metadata-and-body' (log the request and response body, in addition to the metadata). When debug vebosity is set the authorization headers are also logged when the request is fully logged. http.cache.maxbytes 10485760 HTTP client maximum size of the response cache in bytes. If 0, the HTTP client does not cache responses. http.internal.address 127.0.0.1:8081 Address and port the server will be listening to for internal-facing endpoints. diff --git a/http/cmd/cmd.go b/http/cmd/cmd.go index 3c095aaae1..1721738ebc 100644 --- a/http/cmd/cmd.go +++ b/http/cmd/cmd.go @@ -35,7 +35,7 @@ func FlagSet() *pflag.FlagSet { flags.String("http.internal.auth.audience", defs.Internal.Auth.Audience, "Expected audience for JWT tokens (default: hostname)") flags.String("http.internal.auth.authorizedkeyspath", defs.Internal.Auth.AuthorizedKeysPath, "Path to an authorized_keys file for trusted JWT signers") flags.String("http.log", string(defs.Log), fmt.Sprintf("What to log about HTTP requests. Options are '%s', '%s' (log request method, URI, IP and response code), and '%s' (log the request and response body, in addition to the metadata). When debug vebosity is set the authorization headers are also logged when the request is fully logged.", http.LogNothingLevel, http.LogMetadataLevel, http.LogMetadataAndBodyLevel)) - flags.String("http.clientipheader", defs.ClientIPHeaderName, "Case-sensitive HTTP Header that contains the client IP used for audit logs. Supports X-Forwarded-For, X-Real-IP, and custom headers. For custom headers the audit log value will be the exact value of the header.") + flags.String("http.clientipheader", defs.ClientIPHeaderName, "Case-sensitive HTTP Header that contains the client IP used for audit logs. For the X-Forwarded-For header only link-local, loopback, and private IPs are excluded. Switch to X-Real-IP or a custom header if you see your own proxy/infra in the logs.") flags.Int("http.cache.maxbytes", defs.ResponseCacheSize, "HTTP client maximum size of the response cache in bytes. If 0, the HTTP client does not cache responses.") return flags diff --git a/http/engine.go b/http/engine.go index 6794a3bd44..fa3c1f6739 100644 --- a/http/engine.go +++ b/http/engine.go @@ -116,7 +116,7 @@ func (h *Engine) createEchoServer(ipHeader string) (EchoServer, error) { case echo.HeaderXForwardedFor: echoServer.IPExtractor = echo.ExtractIPFromXFFHeader() case "": - echoServer.IPExtractor = echo.ExtractIPDirect() // use request IP if there is no header + echoServer.IPExtractor = echo.ExtractIPDirect() // sensible fallback; use source address from IPv4/IPv6 packet header if there is no HTTP header. default: echoServer.IPExtractor = extractIPFromCustomHeader(ipHeader) } diff --git a/http/engine_test.go b/http/engine_test.go index f0e38b2361..31d36d5e11 100644 --- a/http/engine_test.go +++ b/http/engine_test.go @@ -131,6 +131,7 @@ func TestEngine_Configure(t *testing.T) { request, _ := http.NewRequest(http.MethodGet, "http://"+engine.config.Internal.Address+securedPath, nil) log.Logger().Infof("requesting %v", request.URL.String()) request.Header.Set("Authorization", "Bearer "+string(serializedToken)) + request.Header.Set(engine.config.ClientIPHeaderName, "1.2.3.4") response, err := http.DefaultClient.Do(request) assert.NoError(t, err) @@ -254,6 +255,7 @@ func TestEngine_LoggingMiddleware(t *testing.T) { t.Run("requestLogger", func(t *testing.T) { engine := New(noop, nil) engine.config.Internal.Address = fmt.Sprintf("localhost:%d", test.FreeTCPPort()) + engine.config.ClientIPHeaderName = "X-Custom-Header" err := engine.Configure(*core.NewServerConfig()) require.NoError(t, err) @@ -283,6 +285,24 @@ func TestEngine_LoggingMiddleware(t *testing.T) { _, _ = http.Get("http://" + engine.config.Internal.Address + "/some-path") assert.Contains(t, output.String(), "HTTP request") }) + t.Run("ip log - custom header", func(t *testing.T) { + // Call to another, registered path is logged + output.Reset() + request, _ := http.NewRequest(http.MethodGet, "http://"+engine.config.Internal.Address+"/some-path", nil) + request.Header.Set(engine.config.ClientIPHeaderName, "1.2.3.4") + _, err = http.DefaultClient.Do(request) + require.NoError(t, err) + assert.Contains(t, output.String(), "remote_ip=1.2.3.4") + }) + t.Run("ip log - custom header missing", func(t *testing.T) { + // Call to another, registered path is logged + output.Reset() + request, _ := http.NewRequest(http.MethodGet, "http://"+engine.config.Internal.Address+"/some-path", nil) + request.Header.Set(engine.config.ClientIPHeaderName, "") + _, err = http.DefaultClient.Do(request) + require.NoError(t, err) + assert.Contains(t, output.String(), "remote_ip=127.0.0.1") + }) }) t.Run("bodyLogger", func(t *testing.T) { engine := New(noop, nil) diff --git a/network/transport/grpc/config.go b/network/transport/grpc/config.go index 5f727d0412..9940e37bda 100644 --- a/network/transport/grpc/config.go +++ b/network/transport/grpc/config.go @@ -76,6 +76,7 @@ func WithTLS(clientCertificate tls.Certificate, trustStore *core.TrustStore, pki } } +// WithClientIPHeader sets the HTTP header that is used to extract the client IP. func WithClientIPHeader(clientIPHeaderName string) ConfigOption { return func(config *Config) error { config.clientIPHeaderName = clientIPHeaderName diff --git a/network/transport/grpc/ip_interceptor.go b/network/transport/grpc/ip_interceptor.go index 4d54520f1b..992d8c1f17 100644 --- a/network/transport/grpc/ip_interceptor.go +++ b/network/transport/grpc/ip_interceptor.go @@ -38,7 +38,9 @@ func ipInterceptor(ipHeader string) grpc.StreamServerInterceptor { case headerXForwardedFor: extractIPHeader = extractIPFromXFFHeader case "": - extractIPHeader = extractPeerIP + extractIPHeader = func(_ grpc.ServerStream) string { + return "" + } default: extractIPHeader = extractIPFromCustomHeader(ipHeader) } @@ -64,7 +66,7 @@ func ipInterceptor(ipHeader string) grpc.StreamServerInterceptor { // extractIPFromXFFHeader tries to retrieve the address from X-Forward-For header. Returns an empty string if non found. // Implementation is based on echo.ExtractIPFromXFFHeader(). func extractIPFromXFFHeader(serverStream grpc.ServerStream) string { - ipUnknown := extractPeerIP(serverStream) + ipUnknown := "" md, ok := metadata.FromIncomingContext(serverStream.Context()) if !ok { return ipUnknown @@ -75,7 +77,7 @@ func extractIPFromXFFHeader(serverStream grpc.ServerStream) string { } ips := strings.Split(strings.Join(xffs, ","), ",") for i := len(ips) - 1; i >= 0; i-- { - ip := net.ParseIP(strings.TrimSpace(ips[i])) + ip := net.ParseIP(trimIP(ips[i])) if ip == nil { // Unable to parse IP; cannot trust entire records return ipUnknown @@ -110,29 +112,26 @@ func isInternal(ip net.IP) bool { } // extractIPFromCustomHeader extracts an IP address from any custom header. -// If the header is missing or contains an invalid IP, the extractor tries to return the IP in the peer.Peer. // This is an altered version of echo.ExtractIPFromRealIPHeader() that does not check for trusted IPs. func extractIPFromCustomHeader(ipHeader string) func(serverStream grpc.ServerStream) string { return func(serverStream grpc.ServerStream) string { - directIP := extractPeerIP(serverStream) + ipUnknown := "" md, ok := metadata.FromIncomingContext(serverStream.Context()) if !ok { - return directIP + return ipUnknown } header := md.Get(ipHeader) - if len(header) == 0 { - return directIP + if len(header) != 1 { + return ipUnknown } - - return strings.Join(header, ",") + return trimIP(header[0]) } } -// extractPeerID returns the peer.Peer's Addr if available in the serverStream.Context -func extractPeerIP(serverStream grpc.ServerStream) string { - peer, ok := peer.FromContext(serverStream.Context()) - if !ok { - return "" - } - return peer.Addr.String() +func trimIP(ip string) string { + ip = strings.TrimSpace(ip) + // trim brackets from IPv6 + ip = strings.TrimPrefix(ip, "[") + ip = strings.TrimSuffix(ip, "]") + return ip } diff --git a/network/transport/grpc/ip_interceptor_test.go b/network/transport/grpc/ip_interceptor_test.go index c91d85091f..65835f41af 100644 --- a/network/transport/grpc/ip_interceptor_test.go +++ b/network/transport/grpc/ip_interceptor_test.go @@ -61,6 +61,7 @@ func Test_ipInterceptor(t *testing.T) { IP: ipAddr.AsSlice(), Zone: ipAddr.Zone(), } + ipv6 = "[" + ipv6 + "]" // trims brackets from ipv6 internalXFFAddr, _ := net.ResolveIPAddr("ip", internalIPs[0]) peerNoAddres := net.Addr(nil) @@ -119,4 +120,50 @@ func Test_ipInterceptor(t *testing.T) { assert.False(t, success) assert.Nil(t, peerInfo) }) + t.Run("custom header", func(t *testing.T) { + header := "X-Custom-Header" + expectedIP := "1.2.3.4" + t.Run("ok", func(t *testing.T) { + ran := false + md := metadata.New(map[string]string{header: expectedIP}) + serverStream.ctx = metadata.NewIncomingContext(context.Background(), md) + _ = ipInterceptor(header)(nil, serverStream, nil, func(srv interface{}, wrappedStream grpc.ServerStream) error { + peerInfo, success = peer.FromContext(wrappedStream.Context()) + ran = true + return nil + }) + + require.True(t, ran, "test logic was not executed") + assert.True(t, success) + assert.Equal(t, expectedIP, peerInfo.Addr.String()) + }) + t.Run("empty header", func(t *testing.T) { + ran := false + md := metadata.New(map[string]string{header: ""}) + serverStream.ctx = metadata.NewIncomingContext(context.Background(), md) + _ = ipInterceptor(header)(nil, serverStream, nil, func(srv interface{}, wrappedStream grpc.ServerStream) error { + peerInfo, success = peer.FromContext(wrappedStream.Context()) + ran = true + return nil + }) + + require.True(t, ran, "test logic was not executed") + assert.False(t, success) + assert.Nil(t, peerInfo) + }) + t.Run("empty header", func(t *testing.T) { + ran := false + md := metadata.New(map[string]string{header: strings.Join([]string{expectedIP, expectedIP}, ",")}) + serverStream.ctx = metadata.NewIncomingContext(context.Background(), md) + _ = ipInterceptor(header)(nil, serverStream, nil, func(srv interface{}, wrappedStream grpc.ServerStream) error { + peerInfo, success = peer.FromContext(wrappedStream.Context()) + ran = true + return nil + }) + + require.True(t, ran, "test logic was not executed") + assert.False(t, success) + assert.Nil(t, peerInfo) + }) + }) } From 0b6e9daaefcf1eff2b7b7ed869fc5ba677140cbf Mon Sep 17 00:00:00 2001 From: Gerard Snaauw Date: Wed, 16 Oct 2024 10:09:34 +0200 Subject: [PATCH 3/4] rename test --- network/transport/grpc/ip_interceptor_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/network/transport/grpc/ip_interceptor_test.go b/network/transport/grpc/ip_interceptor_test.go index 65835f41af..8e6f4a36c3 100644 --- a/network/transport/grpc/ip_interceptor_test.go +++ b/network/transport/grpc/ip_interceptor_test.go @@ -151,7 +151,7 @@ func Test_ipInterceptor(t *testing.T) { assert.False(t, success) assert.Nil(t, peerInfo) }) - t.Run("empty header", func(t *testing.T) { + t.Run("invalid input on custom header", func(t *testing.T) { ran := false md := metadata.New(map[string]string{header: strings.Join([]string{expectedIP, expectedIP}, ",")}) serverStream.ctx = metadata.NewIncomingContext(context.Background(), md) From 94577b79ce6311bf49fdf31c4d7884b5e1ace4de Mon Sep 17 00:00:00 2001 From: Gerard Snaauw Date: Wed, 16 Oct 2024 11:36:40 +0200 Subject: [PATCH 4/4] make cli-docs --- README.rst | 1 + 1 file changed, 1 insertion(+) diff --git a/README.rst b/README.rst index 249ca78627..20748003e1 100644 --- a/README.rst +++ b/README.rst @@ -197,6 +197,7 @@ The following options can be configured on the server: discovery.definitions.directory ./config/discovery Directory to load Discovery Service Definitions from. If not set, the discovery service will be disabled. If the directory contains JSON files that can't be parsed as service definition, the node will fail to start. discovery.server.ids [] IDs of the Discovery Service for which to act as server. If an ID does not map to a loaded service definition, the node will fail to start. **HTTP** + http.clientipheader X-Forwarded-For Case-sensitive HTTP Header that contains the client IP used for audit logs. For the X-Forwarded-For header only link-local, loopback, and private IPs are excluded. Switch to X-Real-IP or a custom header if you see your own proxy/infra in the logs. http.log metadata What to log about HTTP requests. Options are 'nothing', 'metadata' (log request method, URI, IP and response code), and 'metadata-and-body' (log the request and response body, in addition to the metadata). When debug vebosity is set the authorization headers are also logged when the request is fully logged. http.cache.maxbytes 10485760 HTTP client maximum size of the response cache in bytes. If 0, the HTTP client does not cache responses. http.internal.address 127.0.0.1:8081 Address and port the server will be listening to for internal-facing endpoints.