diff --git a/pkg/middleware/header.go b/pkg/middleware/header.go index 0f50a4e413..00ad140553 100644 --- a/pkg/middleware/header.go +++ b/pkg/middleware/header.go @@ -10,6 +10,10 @@ import ( rscors "github.com/rs/cors" ) +// DefaultClientIPHeader is the header the proxy uses to propagate the resolved +// client IP to the downstream services. +const DefaultClientIPHeader = "X-Client-Ip" + // NoCache writes required cache headers to all requests. func NoCache(next http.Handler) http.Handler { return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { diff --git a/pkg/service/debug/service.go b/pkg/service/debug/service.go index 026f2e90ec..7cb3223cf5 100644 --- a/pkg/service/debug/service.go +++ b/pkg/service/debug/service.go @@ -73,7 +73,7 @@ func NewService(opts ...Option) *http.Server { return baseCtx }, Handler: alice.New( - chimiddleware.RealIP, + chimiddleware.ClientIPFromHeader(middleware.DefaultClientIPHeader), chimiddleware.RequestID, middleware.NoCache, middleware.Cors( diff --git a/services/collaboration/pkg/middleware/accesslog.go b/services/collaboration/pkg/middleware/accesslog.go index 7b765ad24e..69ddbc0872 100644 --- a/services/collaboration/pkg/middleware/accesslog.go +++ b/services/collaboration/pkg/middleware/accesslog.go @@ -25,7 +25,7 @@ func AccessLog(logger log.Logger) func(http.Handler) http.Handler { Str("proto", r.Proto). Str(log.RequestIDString, requestID). Str("traceid", spanContext.TraceID().String()). - Str("remote-addr", r.RemoteAddr). + Str("remote-addr", middleware.GetClientIP(r.Context())). Str("method", r.Method). Str("wopi-action", r.Header.Get("X-WOPI-Override")). Int("status", wrap.Status()). diff --git a/services/collaboration/pkg/server/http/server.go b/services/collaboration/pkg/server/http/server.go index 328763a209..17603a8ffb 100644 --- a/services/collaboration/pkg/server/http/server.go +++ b/services/collaboration/pkg/server/http/server.go @@ -40,6 +40,7 @@ func Server(opts ...Option) (http.Service, error) { } middlewares := []func(stdhttp.Handler) stdhttp.Handler{ + chimiddleware.ClientIPFromHeader(middleware.DefaultClientIPHeader), chimiddleware.RequestID, middleware.Version( options.Config.Service.Name, diff --git a/services/idp/pkg/server/http/server.go b/services/idp/pkg/server/http/server.go index 4a74785943..a782a52446 100644 --- a/services/idp/pkg/server/http/server.go +++ b/services/idp/pkg/server/http/server.go @@ -57,7 +57,7 @@ func Server(opts ...Option) (http.Service, error) { svc.Logger(options.Logger), svc.Config(options.Config), svc.Middleware( - chimiddleware.RealIP, + chimiddleware.ClientIPFromHeader(middleware.DefaultClientIPHeader), chimiddleware.RequestID, middleware.TraceContext, middleware.NoCache, diff --git a/services/invitations/pkg/server/http/server.go b/services/invitations/pkg/server/http/server.go index cc60b27747..5955d8c8dc 100644 --- a/services/invitations/pkg/server/http/server.go +++ b/services/invitations/pkg/server/http/server.go @@ -43,7 +43,7 @@ func Server(opts ...Option) (ohttp.Service, error) { mux := chi.NewMux() - mux.Use(chimiddleware.RealIP) + mux.Use(chimiddleware.ClientIPFromHeader(middleware.DefaultClientIPHeader)) mux.Use(chimiddleware.RequestID) mux.Use(middleware.TraceContext) mux.Use(middleware.NoCache) diff --git a/services/ocs/pkg/server/http/server.go b/services/ocs/pkg/server/http/server.go index 74c42b43c4..8fe5ea70e7 100644 --- a/services/ocs/pkg/server/http/server.go +++ b/services/ocs/pkg/server/http/server.go @@ -53,7 +53,7 @@ func Server(opts ...Option) (http.Service, error) { svc.Logger(options.Logger), svc.Config(options.Config), svc.Middleware( - chimiddleware.RealIP, + chimiddleware.ClientIPFromHeader(middleware.DefaultClientIPHeader), chimiddleware.RequestID, middleware.NoCache, middleware.Cors( diff --git a/services/proxy/pkg/command/clientip_test.go b/services/proxy/pkg/command/clientip_test.go new file mode 100644 index 0000000000..08b4aa4635 --- /dev/null +++ b/services/proxy/pkg/command/clientip_test.go @@ -0,0 +1,100 @@ +package command + +import ( + "net/http" + "net/http/httptest" + "testing" + + chimiddleware "github.com/go-chi/chi/v5/middleware" + + "github.com/opencloud-eu/opencloud/services/proxy/pkg/config" + "gotest.tools/v3/assert" +) + +// TestClientIPMiddleware verifies that clientIPMiddleware maps the configured +// strategy to the right chi ClientIPFrom* middleware and that the resolved IP +// matches the expected behaviour for each supported deployment case. +func TestClientIPMiddleware(t *testing.T) { + tests := []struct { + name string + strategy string + cfg config.ClientIP + remoteAddr string + headers map[string]string + expected string + }{ + { + name: "remote_addr direct exposure", + strategy: "remote_addr", + remoteAddr: "192.0.2.1:1234", + expected: "192.0.2.1", + }, + { + name: "remote_addr ignores spoofed xff", + strategy: "remote_addr", + remoteAddr: "192.0.2.1:1234", + headers: map[string]string{"X-Forwarded-For": "203.0.113.9"}, + expected: "192.0.2.1", + }, + { + name: "header strategy reads trusted header", + strategy: "header", + cfg: config.ClientIP{Header: "X-Real-IP"}, + remoteAddr: "192.0.2.1:1234", + headers: map[string]string{"X-Real-IP": "198.51.100.7"}, + expected: "198.51.100.7", + }, + { + name: "xff rightmost entry wins", + strategy: "xff", + remoteAddr: "192.0.2.1:1234", + headers: map[string]string{"X-Forwarded-For": "198.51.100.10, 203.0.113.5"}, + expected: "203.0.113.5", + }, + { + name: "xff skips trusted prefixes", + strategy: "xff", + cfg: config.ClientIP{TrustedPrefixes: []string{"10.0.0.0/8"}}, + remoteAddr: "10.0.0.5:1234", + headers: map[string]string{"X-Forwarded-For": "198.51.100.10, 10.0.0.5"}, + expected: "198.51.100.10", + }, + { + name: "xff_trusted_hops reads nth hop", + strategy: "xff_trusted_hops", + cfg: config.ClientIP{TrustedHops: 1}, + remoteAddr: "10.0.0.5:1234", + headers: map[string]string{"X-Forwarded-For": "198.51.100.10, 10.0.0.5"}, + expected: "10.0.0.5", + }, + { + name: "unknown strategy falls back to remote_addr", + strategy: "something_else", + remoteAddr: "192.0.2.1:1234", + headers: map[string]string{"X-Forwarded-For": "203.0.113.9"}, + expected: "192.0.2.1", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + cfg := tt.cfg + cfg.Strategy = tt.strategy + + var got string + handler := clientIPMiddleware(&config.Config{ClientIP: cfg})(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + got = chimiddleware.GetClientIP(r.Context()) + })) + + req := httptest.NewRequest(http.MethodGet, "/", nil) + req.RemoteAddr = tt.remoteAddr + for k, v := range tt.headers { + req.Header.Set(k, v) + } + + handler.ServeHTTP(httptest.NewRecorder(), req) + + assert.Equal(t, got, tt.expected) + }) + } +} diff --git a/services/proxy/pkg/command/server.go b/services/proxy/pkg/command/server.go index 2a9a5cc0ba..509bd40637 100644 --- a/services/proxy/pkg/command/server.go +++ b/services/proxy/pkg/command/server.go @@ -341,7 +341,7 @@ func loadMiddlewares(logger log.Logger, cfg *config.Config, } return alice.New( - chimiddleware.RealIP, + clientIPMiddleware(cfg), chimiddleware.RequestID, // 1. Logging & Tracing first @@ -411,3 +411,20 @@ func loadMiddlewares(logger log.Logger, cfg *config.Config, ), ) } + +// clientIPMiddleware returns the chi ClientIPFrom* middleware matching the +// configured client IP strategy. +func clientIPMiddleware(cfg *config.Config) func(http.Handler) http.Handler { + switch cfg.ClientIP.Strategy { + case config.ClientIPStrategyHeader: + return chimiddleware.ClientIPFromHeader(cfg.ClientIP.Header) + case config.ClientIPStrategyXFF: + return chimiddleware.ClientIPFromXFF(cfg.ClientIP.TrustedPrefixes...) + case config.ClientIPStrategyXFFTrustedHops: + return chimiddleware.ClientIPFromXFFTrustedProxies(cfg.ClientIP.TrustedHops) + case config.ClientIPStrategyRemoteAddr: + fallthrough + default: + return chimiddleware.ClientIPFromRemoteAddr + } +} diff --git a/services/proxy/pkg/config/config.go b/services/proxy/pkg/config/config.go index 05344049a6..a3bb86706a 100644 --- a/services/proxy/pkg/config/config.go +++ b/services/proxy/pkg/config/config.go @@ -19,6 +19,8 @@ type Config struct { HTTP HTTP `yaml:"http"` + ClientIP ClientIP `yaml:"client_ip"` + Reva *shared.Reva `yaml:"reva"` GRPCClientTLS *shared.GRPCClientTLS `yaml:"grpc_client_tls"` GrpcClient client.Client `yaml:"-"` @@ -95,6 +97,14 @@ var ( RouteTypes = []RouteType{QueryRoute, RegexRoute, PrefixRoute} ) +// ClientIP configures how the proxy resolves the real client IP address. +type ClientIP struct { + Strategy string `yaml:"strategy" env:"PROXY_CLIENT_IP_STRATEGY" desc:"Determines how the proxy resolves the real client IP. Supported values: remote_addr, header, xff, xff_trusted_hops." introductionVersion:"%NEXT%"` + Header string `yaml:"header" env:"PROXY_CLIENT_IP_HEADER" desc:"The header name to use for the 'header' strategy, e.g. X-Real-IP or CF-Connecting-IP." introductionVersion:"%NEXT%"` + TrustedPrefixes []string `yaml:"trusted_prefixes" env:"PROXY_CLIENT_IP_TRUSTED_PREFIXES" desc:"CIDRs of trusted proxies used by the 'xff' strategy." introductionVersion:"%NEXT%"` + TrustedHops int `yaml:"trusted_hops" env:"PROXY_CLIENT_IP_TRUSTED_HOPS" desc:"Number of trusted proxy hops used by the 'xff_trusted_hops' strategy." introductionVersion:"%NEXT%"` +} + // AuthMiddleware configures the proxy http auth middleware. type AuthMiddleware struct { CredentialsByUserAgent map[string]string `yaml:"credentials_by_user_agent"` @@ -111,6 +121,11 @@ const ( AccessTokenVerificationJWT = "jwt" // tdb: // AccessTokenVerificationIntrospect = "introspect" + + ClientIPStrategyRemoteAddr = "remote_addr" + ClientIPStrategyHeader = "header" + ClientIPStrategyXFF = "xff" + ClientIPStrategyXFFTrustedHops = "xff_trusted_hops" ) // OIDC is the config for the OpenID-Connect middleware. If set the proxy will try to authenticate every request diff --git a/services/proxy/pkg/config/defaults/defaultconfig.go b/services/proxy/pkg/config/defaults/defaultconfig.go index 3614e1f0a8..bf4ae339d4 100644 --- a/services/proxy/pkg/config/defaults/defaultconfig.go +++ b/services/proxy/pkg/config/defaults/defaultconfig.go @@ -46,6 +46,11 @@ func DefaultConfig() *config.Config { ExpectContinueTimeout: 1 * time.Second, }, }, + ClientIP: config.ClientIP{ + Strategy: config.ClientIPStrategyRemoteAddr, + Header: "X-Real-IP", + TrustedHops: 1, // this might need to be adjusted + }, Service: config.Service{ Name: "proxy", }, diff --git a/services/proxy/pkg/config/parser/parse.go b/services/proxy/pkg/config/parser/parse.go index 7a8be8c7a8..f85fddbe5a 100644 --- a/services/proxy/pkg/config/parser/parse.go +++ b/services/proxy/pkg/config/parser/parse.go @@ -57,6 +57,16 @@ func Validate(cfg *config.Config) error { ) } + if cfg.ClientIP.Strategy != config.ClientIPStrategyRemoteAddr && cfg.ClientIP.Strategy != config.ClientIPStrategyHeader && + cfg.ClientIP.Strategy != config.ClientIPStrategyXFF && cfg.ClientIP.Strategy != config.ClientIPStrategyXFFTrustedHops { + return fmt.Errorf( + "Invalid value '%s' for 'client_ip.strategy' in service %s. Possible values are: '%s', '%s', '%s' or '%s'.", + cfg.ClientIP.Strategy, cfg.Service.Name, + config.ClientIPStrategyRemoteAddr, config.ClientIPStrategyHeader, + config.ClientIPStrategyXFF, config.ClientIPStrategyXFFTrustedHops, + ) + } + if cfg.ServiceAccount.ServiceAccountID == "" { return shared.MissingServiceAccountID(cfg.Service.Name) } diff --git a/services/proxy/pkg/middleware/accesslog.go b/services/proxy/pkg/middleware/accesslog.go index 23c58f4d24..316271d4f9 100644 --- a/services/proxy/pkg/middleware/accesslog.go +++ b/services/proxy/pkg/middleware/accesslog.go @@ -25,7 +25,7 @@ func AccessLog(logger log.Logger) func(http.Handler) http.Handler { Str("proto", r.Proto). Str(log.RequestIDString, requestID). Str("traceid", spanContext.TraceID().String()). - Str("remote-addr", r.RemoteAddr). + Str("remote-addr", middleware.GetClientIP(r.Context())). Str("method", r.Method). Int("status", wrap.Status()). Str("path", r.URL.Path). diff --git a/services/proxy/pkg/middleware/context_logger.go b/services/proxy/pkg/middleware/context_logger.go index 5f8fd326d8..b323d0a111 100644 --- a/services/proxy/pkg/middleware/context_logger.go +++ b/services/proxy/pkg/middleware/context_logger.go @@ -3,6 +3,7 @@ package middleware import ( "net/http" + "github.com/go-chi/chi/v5/middleware" "github.com/opencloud-eu/opencloud/pkg/log" ) @@ -12,7 +13,7 @@ func ContextLogger(logger log.Logger) func(http.Handler) http.Handler { return func(next http.Handler) http.Handler { return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { ctx := logger.With(). - Str("remoteAddr", r.RemoteAddr). + Str("remoteAddr", middleware.GetClientIP(r.Context())). Str(log.RequestIDString, r.Header.Get("X-Request-ID")). Str("proto", r.Proto). Str("method", r.Method). diff --git a/services/proxy/pkg/middleware/oidc_auth.go b/services/proxy/pkg/middleware/oidc_auth.go index b80f7fcb7a..059a486e50 100644 --- a/services/proxy/pkg/middleware/oidc_auth.go +++ b/services/proxy/pkg/middleware/oidc_auth.go @@ -14,6 +14,7 @@ import ( "golang.org/x/crypto/sha3" "golang.org/x/oauth2" + "github.com/go-chi/chi/v5/middleware" "github.com/opencloud-eu/opencloud/pkg/log" "github.com/opencloud-eu/opencloud/pkg/oidc" "github.com/opencloud-eu/opencloud/services/proxy/pkg/config" @@ -217,7 +218,7 @@ func (m *OIDCAuthenticator) Authenticate(r *http.Request) (*http.Request, bool) Str("authenticator", "oidc"). Str("path", r.URL.Path). Str("user_agent", r.UserAgent()). - Str("client.address", r.Header.Get("X-Forwarded-For")). + Str("client.address", middleware.GetClientIP(r.Context())). Str("network.peer.address", host). Str("network.peer.port", port). Msg("failed to authenticate the request") diff --git a/services/proxy/pkg/proxy/proxy.go b/services/proxy/pkg/proxy/proxy.go index 98f340b4b7..692a923ad0 100644 --- a/services/proxy/pkg/proxy/proxy.go +++ b/services/proxy/pkg/proxy/proxy.go @@ -16,6 +16,9 @@ import ( "github.com/opencloud-eu/opencloud/services/proxy/pkg/proxy/policy" "github.com/opencloud-eu/opencloud/services/proxy/pkg/router" "github.com/rs/zerolog" + + chimiddleware "github.com/go-chi/chi/v5/middleware" + pkgmiddleware "github.com/opencloud-eu/opencloud/pkg/middleware" ) // MultiHostReverseProxy extends "httputil" to support multiple hosts with different policies @@ -42,6 +45,11 @@ func NewMultiHostReverseProxy(opts ...Option) (*MultiHostReverseProxy, error) { } rp.Rewrite = func(r *httputil.ProxyRequest) { + // Set the resolved client IP to header so the downstream services can use it + if clientIP := chimiddleware.GetClientIP(r.In.Context()); clientIP != "" { + r.Out.Header.Set(pkgmiddleware.DefaultClientIPHeader, clientIP) + } + // Check if datagateway middleware already handled this request if skip, _ := r.In.Context().Value(middleware.DatagatewaySkipRoutingKey).(bool); skip { r.SetXForwarded() diff --git a/services/proxy/pkg/proxy/rewrite_test.go b/services/proxy/pkg/proxy/rewrite_test.go new file mode 100644 index 0000000000..f494834a9e --- /dev/null +++ b/services/proxy/pkg/proxy/rewrite_test.go @@ -0,0 +1,73 @@ +package proxy + +import ( + "bytes" + "context" + "io" + "net/http" + "net/http/httptest" + "net/http/httputil" + "testing" + + chimiddleware "github.com/go-chi/chi/v5/middleware" + + pkgmiddleware "github.com/opencloud-eu/opencloud/pkg/middleware" + "github.com/opencloud-eu/opencloud/services/proxy/pkg/middleware" + "gotest.tools/v3/assert" +) + +// TestRewriteForwardsClientIP verifies that the proxy overwrites the forwarded +// IP header on the outbound request with the client IP resolved by the +// ClientIPFrom* middleware, so downstream services can trust it. +func TestRewriteForwardsClientIP(t *testing.T) { + cfg := testConfig(nil) + + rp := newTestProxy(cfg, func(req *http.Request) *http.Response { + return &http.Response{StatusCode: 200, Body: io.NopCloser(bytes.NewBufferString("OK")), Header: make(http.Header)} + }) + + inReq := httptest.NewRequest(http.MethodGet, "/", nil) + inReq.RemoteAddr = "192.0.2.1:1234" + + // Resolve the client IP into the request context via a chi middleware. + var withCtx *http.Request + chimiddleware.ClientIPFromRemoteAddr(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + withCtx = r + })).ServeHTTP(httptest.NewRecorder(), inReq) + + // Take the datagateway skip path so Rewrite does not need routing info. + ctx := context.WithValue(withCtx.Context(), middleware.DatagatewaySkipRoutingKey, true) + withCtx = withCtx.WithContext(ctx) + + pr := &httputil.ProxyRequest{ + In: withCtx, + Out: withCtx.Clone(context.Background()), + } + rp.Rewrite(pr) + + assert.Equal(t, pr.Out.Header.Get(pkgmiddleware.DefaultClientIPHeader), "192.0.2.1") +} + +// TestRewriteDoesNotForwardEmptyClientIP verifies the header is left unset when +// the client IP could not be resolved (no middleware populated the context). +func TestRewriteDoesNotForwardEmptyClientIP(t *testing.T) { + cfg := testConfig(nil) + + rp := newTestProxy(cfg, func(req *http.Request) *http.Response { + return &http.Response{StatusCode: 200, Body: io.NopCloser(bytes.NewBufferString("OK")), Header: make(http.Header)} + }) + + inReq := httptest.NewRequest(http.MethodGet, "/", nil) + inReq.RemoteAddr = "192.0.2.1:1234" + + ctx := context.WithValue(inReq.Context(), middleware.DatagatewaySkipRoutingKey, true) + inReq = inReq.WithContext(ctx) + + pr := &httputil.ProxyRequest{ + In: inReq, + Out: inReq.Clone(context.Background()), + } + rp.Rewrite(pr) + + assert.Equal(t, pr.Out.Header.Get(pkgmiddleware.DefaultClientIPHeader), "") +} diff --git a/services/settings/pkg/server/http/server.go b/services/settings/pkg/server/http/server.go index 6c5b85c115..abd083f948 100644 --- a/services/settings/pkg/server/http/server.go +++ b/services/settings/pkg/server/http/server.go @@ -42,7 +42,7 @@ func Server(opts ...Option) (ohttp.Service, error) { mux := chi.NewMux() - mux.Use(chimiddleware.RealIP) + mux.Use(chimiddleware.ClientIPFromHeader(middleware.DefaultClientIPHeader)) mux.Use(chimiddleware.RequestID) mux.Use(middleware.NoCache) mux.Use(middleware.Cors( diff --git a/services/thumbnails/pkg/server/http/server.go b/services/thumbnails/pkg/server/http/server.go index d9c2a1c1fa..978945f0a7 100644 --- a/services/thumbnails/pkg/server/http/server.go +++ b/services/thumbnails/pkg/server/http/server.go @@ -38,7 +38,7 @@ func Server(opts ...Option) (http.Service, error) { svc.Logger(options.Logger), svc.Config(options.Config), svc.Middleware( - middleware.RealIP, + middleware.ClientIPFromHeader(opencloudmiddleware.DefaultClientIPHeader), middleware.RequestID, opencloudmiddleware.Cors( cors.Logger(options.Logger), diff --git a/services/web/pkg/server/http/server.go b/services/web/pkg/server/http/server.go index 0b9621c798..d8a0c9d3be 100644 --- a/services/web/pkg/server/http/server.go +++ b/services/web/pkg/server/http/server.go @@ -118,7 +118,7 @@ func Server(opts ...Option) (http.Service, error) { svc.Config(options.Config), svc.GatewaySelector(gatewaySelector), svc.Middleware( - chimiddleware.RealIP, + chimiddleware.ClientIPFromHeader(middleware.DefaultClientIPHeader), chimiddleware.RequestID, chimiddleware.Compress(5), middleware.NoCache, diff --git a/services/webdav/pkg/server/http/server.go b/services/webdav/pkg/server/http/server.go index 4e7c794f24..eddebf90fa 100644 --- a/services/webdav/pkg/server/http/server.go +++ b/services/webdav/pkg/server/http/server.go @@ -38,7 +38,7 @@ func Server(opts ...Option) (http.Service, error) { svc.Logger(options.Logger), svc.Config(options.Config), svc.Middleware( - chimiddleware.RealIP, + chimiddleware.ClientIPFromHeader(middleware.DefaultClientIPHeader), chimiddleware.RequestID, middleware.NoCache, middleware.Cors( diff --git a/services/webfinger/pkg/server/http/server.go b/services/webfinger/pkg/server/http/server.go index dbe91b0ca4..32d84ad945 100644 --- a/services/webfinger/pkg/server/http/server.go +++ b/services/webfinger/pkg/server/http/server.go @@ -45,7 +45,7 @@ func Server(opts ...Option) (ohttp.Service, error) { mux := chi.NewMux() - mux.Use(chimiddleware.RealIP) + mux.Use(chimiddleware.ClientIPFromHeader(middleware.DefaultClientIPHeader)) mux.Use(chimiddleware.RequestID) mux.Use(middleware.TraceContext) mux.Use(middleware.NoCache)