Skip to content

Commit

Permalink
Extract client ip using custom header (#3490)
Browse files Browse the repository at this point in the history
* add configurable header to extract client IP

* add tests

* rename test

* make cli-docs
  • Loading branch information
gerardsn authored Oct 16, 2024
1 parent e9f86ee commit afcebd1
Show file tree
Hide file tree
Showing 15 changed files with 213 additions and 44 deletions.
1 change: 1 addition & 0 deletions README.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
8 changes: 8 additions & 0 deletions core/server_config.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
1 change: 1 addition & 0 deletions docs/pages/deployment/server_options.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
1 change: 1 addition & 0 deletions http/cmd/cmd.go
Original file line number Diff line number Diff line change
Expand Up @@ -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. 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
Expand Down
6 changes: 4 additions & 2 deletions http/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -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",
}
}

Expand All @@ -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.
Expand Down
4 changes: 2 additions & 2 deletions http/echo.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
}
Expand All @@ -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
}
Expand Down
34 changes: 17 additions & 17 deletions http/echo_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
})
}
Expand All @@ -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")
})
Expand All @@ -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)
Expand Down Expand Up @@ -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)
Expand Down
40 changes: 35 additions & 5 deletions http/engine.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import (
"errors"
"fmt"
"github.com/nuts-foundation/nuts-node/http/client"
"net"
"net/http"
"os"
"strings"
Expand Down Expand Up @@ -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
}
}
Expand All @@ -102,16 +103,23 @@ 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

// 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() // sensible fallback; use source address from IPv4/IPv6 packet header if there is no HTTP header.
default:
echoServer.IPExtractor = extractIPFromCustomHeader(ipHeader)
}

return &echoAdapter{
startFn: echoServer.Start,
Expand Down Expand Up @@ -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
}
}
Loading

0 comments on commit afcebd1

Please sign in to comment.