fix: only trust forwarding headers from trusted reverse proxies

chi's ClientIPFrom* header middlewares trust the header from anyone who can
  reach us. Pick the middleware per request instead: headers are only read
  when the peer is a trusted proxy, everyone else gets their network address.

  - PROXY_TRUSTED_PROXIES, defaults to loopback and private ranges
  - PROXY_CLIENT_IP_HEADER, defaults to X-Forwarded-For, e.g. CF-Connecting-IP
  - drop the strategy setting and xff_trusted_hops
  - pass the client address to the services in X-Forwarded-For instead of
    X-Client-Ip, reva reads it e.g. for OCM
  - same check in collaboration, traefik routes to it directly
  - OC_TRUSTED_PROXIES and OC_CLIENT_IP_HEADER set both services
  - the other services only drop RealIP, none of them reads the address

  Taken from #3864 and https://github.com/opencloud-eu/opencloud/pull/3464#issuecomment-6494799883

# Conflicts:
#	services/proxy/README.md

pkg/middleware/client_ip.go

@@ -0,0 +1,117 @@
+package middleware
+
+import (
+	"fmt"
+	"net"
+	"net/http"
+	"net/netip"
+	"strings"
+
+	chimiddleware "github.com/go-chi/chi/v5/middleware"
+)
+
+// DefaultClientIPHeader is the header a trusted reverse proxy sends the client
+// address in, unless configured otherwise.
+const DefaultClientIPHeader = "X-Forwarded-For"
+
+// DefaultTrustedProxies returns the loopback and the private network ranges.
+// They cover a reverse proxy in the same Docker network or a Kubernetes
+// ingress, while clients connecting from the internet can't set their address.
+func DefaultTrustedProxies() []string {
+	return []string{"127.0.0.0/8", "::1/128", "10.0.0.0/8", "172.16.0.0/12", "192.168.0.0/16", "fc00::/7"}
+}
+
+// ParseTrustedProxies parses IP addresses and CIDR ranges.
+func ParseTrustedProxies(list []string) ([]netip.Prefix, error) {
+	prefixes := make([]netip.Prefix, 0, len(list))
+	for _, s := range list {
+		s = strings.TrimSpace(s)
+		if s == "" {
+			continue
+		}
+		if strings.Contains(s, "/") {
+			p, err := netip.ParsePrefix(s)
+			if err != nil {
+				return nil, fmt.Errorf("invalid trusted proxy %q: %w", s, err)
+			}
+			prefixes = append(prefixes, p.Masked())
+			continue
+		}
+		a, err := netip.ParseAddr(s)
+		if err != nil {
+			return nil, fmt.Errorf("invalid trusted proxy %q: %w", s, err)
+		}
+		a = a.Unmap()
+		prefixes = append(prefixes, netip.PrefixFrom(a, a.BitLen()))
+	}
+	return prefixes, nil
+}
+
+// ClientIP determines the address of the client with chi's ClientIPFrom*
+// middlewares; read it with ClientAddress. None of chi's header based
+// middlewares look at who sent the header, so the header is only read when the
+// request comes from a trusted proxy. Everyone else gets their network
+// address, whatever headers they send.
+//
+// From a trusted proxy the address is taken from header. For X-Forwarded-For
+// it is the first address from the right that is not a trusted proxy, for any
+// other header, e.g. X-Real-IP or CF-Connecting-IP, its value. Without a
+// usable header, e.g. on health checks, it is the address of the proxy.
+func ClientIP(trusted []netip.Prefix, header string) func(http.Handler) http.Handler {
+	cidrs := make([]string, len(trusted))
+	for i, p := range trusted {
+		cidrs[i] = p.String()
+	}
+	fromHeader := chimiddleware.ClientIPFromXFF(cidrs...)
+	if header != "" && http.CanonicalHeaderKey(header) != DefaultClientIPHeader {
+		fromHeader = chimiddleware.ClientIPFromHeader(header)
+	}
+
+	return func(next http.Handler) http.Handler {
+		// the header only overwrites the network address when it has a usable one
+		fromProxy := chimiddleware.ClientIPFromRemoteAddr(fromHeader(next))
+		direct := chimiddleware.ClientIPFromRemoteAddr(next)
+
+		return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+			if peer, ok := parseAddr(r.RemoteAddr); ok && isTrusted(peer, trusted) {
+				fromProxy.ServeHTTP(w, r)
+				return
+			}
+			direct.ServeHTTP(w, r)
+		})
+	}
+}
+
+// ClientAddress returns the client address ClientIP determined, the network
+// address of the peer if it didn't run.
+func ClientAddress(r *http.Request) string {
+	if ip := chimiddleware.GetClientIP(r.Context()); ip != "" {
+		return ip
+	}
+	if host, _, err := net.SplitHostPort(r.RemoteAddr); err == nil {
+		return host
+	}
+	return r.RemoteAddr
+}
+
+// parseAddr reads an address with or without port.
+func parseAddr(s string) (netip.Addr, bool) {
+	s = strings.TrimSpace(s)
+	if host, _, err := net.SplitHostPort(s); err == nil {
+		s = host
+	}
+	a, err := netip.ParseAddr(strings.Trim(s, "[]"))
+	if err != nil {
+		return netip.Addr{}, false
+	}
+	return a.Unmap().WithZone(""), true
+}
+
+func isTrusted(a netip.Addr, trusted []netip.Prefix) bool {
+	for _, p := range trusted {
+		if p.Contains(a) {
+			return true
+		}
+	}
+	return false
+}

pkg/middleware/client_ip_test.go

@@ -0,0 +1,95 @@
+package middleware
+
+import (
+	"net/http"
+	"net/http/httptest"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+func TestClientIP(t *testing.T) {
+	trusted, err := ParseTrustedProxies([]string{"10.0.0.0/8", "192.0.2.10", "fd00::/8"})
+	require.NoError(t, err)
+
+	for _, tc := range []struct {
+		name    string
+		header  string
+		remote  string
+		xff     []string
+		xRealIP string
+		want    string
+	}{
+		{name: "direct client, no headers", remote: "203.0.113.7:51234", want: "203.0.113.7"},
+		{name: "direct client spoofs X-Forwarded-For", remote: "203.0.113.7:51234", xff: []string{"198.51.100.1"}, want: "203.0.113.7"},
+		{name: "direct client spoofs the configured header", header: "X-Real-IP", remote: "203.0.113.7:51234", xRealIP: "198.51.100.2", want: "203.0.113.7"},
+		{name: "one trusted proxy", remote: "10.0.0.5:443", xff: []string{"203.0.113.7"}, want: "203.0.113.7"},
+		{name: "client prepends a fake address", remote: "10.0.0.5:443", xff: []string{"198.51.100.1, 203.0.113.7"}, want: "203.0.113.7"},
+		{name: "chain of trusted proxies", remote: "10.0.0.5:443", xff: []string{"203.0.113.7, 192.0.2.10", "10.1.2.3"}, want: "203.0.113.7"},
+		{name: "only trusted addresses", remote: "10.0.0.5:443", xff: []string{"10.9.9.9, 10.1.2.3"}, want: "10.0.0.5"},
+		{name: "broken entry", remote: "10.0.0.5:443", xff: []string{"203.0.113.7, garbage"}, want: "10.0.0.5"},
+		{name: "trusted proxy without a header", remote: "10.0.0.5:443", want: "10.0.0.5"},
+		{name: "X-Real-IP is ignored by default", remote: "10.0.0.5:443", xRealIP: "203.0.113.7", want: "10.0.0.5"},
+		{name: "configured header from a trusted proxy", header: "x-real-ip", remote: "10.0.0.5:443", xff: []string{"198.51.100.1"}, xRealIP: "203.0.113.7", want: "203.0.113.7"},
+		{name: "IPv6 behind an IPv6 proxy", remote: "[fd00::1]:443", xff: []string{"2001:db8::7"}, want: "2001:db8::7"},
+		{name: "IPv4 mapped remote address", remote: "[::ffff:10.0.0.5]:443", xff: []string{"203.0.113.7"}, want: "203.0.113.7"},
+	} {
+		t.Run(tc.name, func(t *testing.T) {
+			r := httptest.NewRequest(http.MethodGet, "/", nil)
+			r.RemoteAddr = tc.remote
+			for _, v := range tc.xff {
+				r.Header.Add("X-Forwarded-For", v)
+			}
+			if tc.xRealIP != "" {
+				r.Header.Set("X-Real-Ip", tc.xRealIP)
+			}
+			header := tc.header
+			if header == "" {
+				header = DefaultClientIPHeader
+			}
+
+			var got, peer string
+			ClientIP(trusted, header)(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+				got = ClientAddress(r)
+				peer = r.RemoteAddr
+			})).ServeHTTP(httptest.NewRecorder(), r)
+
+			assert.Equal(t, tc.want, got)
+			assert.Equal(t, tc.remote, peer, "RemoteAddr stays the peer")
+		})
+	}
+}
+
+func TestClientIPWithoutTrustedProxies(t *testing.T) {
+	r := httptest.NewRequest(http.MethodGet, "/", nil)
+	r.RemoteAddr = "127.0.0.1:443"
+	r.Header.Set("X-Forwarded-For", "203.0.113.7")
+
+	var got string
+	ClientIP(nil, DefaultClientIPHeader)(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		got = ClientAddress(r)
+	})).ServeHTTP(httptest.NewRecorder(), r)
+
+	assert.Equal(t, "127.0.0.1", got)
+}
+
+func TestClientAddressWithoutMiddleware(t *testing.T) {
+	r := httptest.NewRequest(http.MethodGet, "/", nil)
+	r.RemoteAddr = "203.0.113.7:51234"
+	assert.Equal(t, "203.0.113.7", ClientAddress(r))
+}
+
+func TestParseTrustedProxies(t *testing.T) {
+	p, err := ParseTrustedProxies([]string{" 10.0.0.1 ", "172.16.0.0/12", "", "::1"})
+	require.NoError(t, err)
+	assert.Len(t, p, 3)
+
+	_, err = ParseTrustedProxies(DefaultTrustedProxies())
+	require.NoError(t, err)
+
+	_, err = ParseTrustedProxies([]string{"10.0.0.0/33"})
+	assert.Error(t, err)
+	_, err = ParseTrustedProxies([]string{"proxy.example.com"})
+	assert.Error(t, err)
+}

pkg/middleware/header.go

@@ -10,10 +10,6 @@ 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) {

pkg/service/debug/service.go

@@ -73,7 +73,6 @@ func NewService(opts ...Option) *http.Server {
 			return baseCtx
 		},
 		Handler: alice.New(
-			chimiddleware.ClientIPFromHeader(middleware.DefaultClientIPHeader),
 			chimiddleware.RequestID,
 			middleware.NoCache,
 			middleware.Cors(

services/collaboration/README.md

@@ -59,6 +59,10 @@ There are a few variables that you need to set:
 
 The application can be customized further by changing the `COLLABORATION_APP_*` options to better describe the application.
 
+## Client Address
+
+The reverse proxy routes the WOPI requests directly to the collaboration service, not through the proxy service. The collaboration service therefore determines the client address in its access log itself, with `COLLABORATION_HTTP_TRUSTED_PROXIES` and `COLLABORATION_HTTP_CLIENT_IP_HEADER`. They work like `PROXY_TRUSTED_PROXIES` and `PROXY_CLIENT_IP_HEADER`, see [Trusted Reverse Proxies](../proxy/README.md#trusted-reverse-proxies) in the proxy service.
+
 ## Storing
 
 The `collaboration` service persists information via the configured store in `COLLABORATION_STORE`. Possible stores are:

services/collaboration/pkg/config/defaults/defaultconfig.go

@@ -5,6 +5,7 @@ import (
 	"time"
 
 	"github.com/opencloud-eu/opencloud/pkg/config/defaults"
+	"github.com/opencloud-eu/opencloud/pkg/middleware"
 	"github.com/opencloud-eu/opencloud/pkg/shared"
 	"github.com/opencloud-eu/opencloud/pkg/structs"
 	"github.com/opencloud-eu/opencloud/services/collaboration/pkg/config"
@@ -52,8 +53,10 @@ func DefaultConfig() *config.Config {
 			Namespace: "eu.opencloud.api",
 		},
 		HTTP: config.HTTP{
-			Addr:      "127.0.0.1:9300",
-			Namespace: "eu.opencloud.web",
+			Addr:           "127.0.0.1:9300",
+			Namespace:      "eu.opencloud.web",
+			TrustedProxies: middleware.DefaultTrustedProxies(),
+			ClientIPHeader: middleware.DefaultClientIPHeader,
 		},
 		Debug: config.Debug{
 			Addr:   "127.0.0.1:9304",

services/collaboration/pkg/config/http.go

@@ -6,7 +6,9 @@ import (
 
 // HTTP defines the available http configuration.
 type HTTP struct {
-	Addr      string                `yaml:"addr" env:"COLLABORATION_HTTP_ADDR" desc:"The bind address of the HTTP service." introductionVersion:"1.0.0"`
-	Namespace string                `yaml:"-"`
-	TLS       shared.HTTPServiceTLS `yaml:"tls"`
+	Addr           string                `yaml:"addr" env:"COLLABORATION_HTTP_ADDR" desc:"The bind address of the HTTP service." introductionVersion:"1.0.0"`
+	Namespace      string                `yaml:"-"`
+	TLS            shared.HTTPServiceTLS `yaml:"tls"`
+	TrustedProxies []string              `yaml:"trusted_proxies" env:"OC_TRUSTED_PROXIES;COLLABORATION_HTTP_TRUSTED_PROXIES" desc:"IP addresses and CIDR ranges of the reverse proxies in front of the collaboration service. The client address in the access log is only taken from the header set in COLLABORATION_HTTP_CLIENT_IP_HEADER if the request comes from one of these. Defaults to the loopback and private network ranges. See the Environment Variable Types description for more details." introductionVersion:"%%NEXT%%"`
+	ClientIPHeader string                `yaml:"client_ip_header" env:"OC_CLIENT_IP_HEADER;COLLABORATION_HTTP_CLIENT_IP_HEADER" desc:"The header a trusted reverse proxy sends the client address in, e.g. X-Real-IP or CF-Connecting-IP. X-Forwarded-For is read from the right, the first address that is not a trusted proxy is the client." introductionVersion:"%%NEXT%%"`
 }

services/collaboration/pkg/config/parser/parse.go

@@ -8,6 +8,7 @@ import (
 	occfg "github.com/opencloud-eu/opencloud/pkg/config"
 	ocdefaults "github.com/opencloud-eu/opencloud/pkg/config/defaults"
 	"github.com/opencloud-eu/opencloud/pkg/config/envdecode"
+	"github.com/opencloud-eu/opencloud/pkg/middleware"
 	"github.com/opencloud-eu/opencloud/pkg/shared"
 	"github.com/opencloud-eu/opencloud/services/collaboration/pkg/config"
 	"github.com/opencloud-eu/opencloud/services/collaboration/pkg/config/defaults"
@@ -43,6 +44,9 @@ func Validate(cfg *config.Config) error {
 	if cfg.Wopi.Secret == "" {
 		return shared.MissingWOPISecretError(cfg.Service.Name)
 	}
+	if _, err := middleware.ParseTrustedProxies(cfg.HTTP.TrustedProxies); err != nil {
+		return fmt.Errorf("invalid COLLABORATION_HTTP_TRUSTED_PROXIES in service %s: %w", cfg.Service.Name, err)
+	}
 	url, err := url.Parse(cfg.Wopi.WopiSrc)
 	if err != nil {
 		return fmt.Errorf("The WOPI Src has not been set properly in your config for %s. "+

services/collaboration/pkg/middleware/accesslog.go

@@ -6,6 +6,7 @@ import (
 
 	"github.com/go-chi/chi/v5/middleware"
 	"github.com/opencloud-eu/opencloud/pkg/log"
+	ocmiddleware "github.com/opencloud-eu/opencloud/pkg/middleware"
 	"go.opentelemetry.io/otel/trace"
 )
 
@@ -25,7 +26,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", middleware.GetClientIP(r.Context())).
+				Str("remote-addr", ocmiddleware.ClientAddress(r)).
 				Str("method", r.Method).
 				Str("wopi-action", r.Header.Get("X-WOPI-Override")).
 				Int("status", wrap.Status()).

services/collaboration/pkg/server/http/server.go

@@ -39,8 +39,12 @@ func Server(opts ...Option) (http.Service, error) {
 		return http.Service{}, fmt.Errorf("could not initialize http service: %w", err)
 	}
 
+	// validated by the config parser
+	trustedProxies, _ := middleware.ParseTrustedProxies(options.Config.HTTP.TrustedProxies)
+
 	middlewares := []func(stdhttp.Handler) stdhttp.Handler{
-		chimiddleware.ClientIPFromHeader(middleware.DefaultClientIPHeader),
+		// Traefik routes to us directly, not through the proxy service
+		middleware.ClientIP(trustedProxies, options.Config.HTTP.ClientIPHeader),
 		chimiddleware.RequestID,
 		middleware.Version(
 			options.Config.Service.Name,

services/idp/pkg/server/http/server.go

@@ -57,7 +57,6 @@ func Server(opts ...Option) (http.Service, error) {
 		svc.Logger(options.Logger),
 		svc.Config(options.Config),
 		svc.Middleware(
-			chimiddleware.ClientIPFromHeader(middleware.DefaultClientIPHeader),
 			chimiddleware.RequestID,
 			middleware.TraceContext,
 			middleware.NoCache,

services/invitations/pkg/server/http/server.go

@@ -43,7 +43,6 @@ func Server(opts ...Option) (ohttp.Service, error) {
 
 	mux := chi.NewMux()
 
-	mux.Use(chimiddleware.ClientIPFromHeader(middleware.DefaultClientIPHeader))
 	mux.Use(chimiddleware.RequestID)
 	mux.Use(middleware.TraceContext)
 	mux.Use(middleware.NoCache)

services/ocs/pkg/server/http/server.go

@@ -53,7 +53,6 @@ func Server(opts ...Option) (http.Service, error) {
 		svc.Logger(options.Logger),
 		svc.Config(options.Config),
 		svc.Middleware(
-			chimiddleware.ClientIPFromHeader(middleware.DefaultClientIPHeader),
 			chimiddleware.RequestID,
 			middleware.NoCache,
 			middleware.Cors(

services/proxy/README.md

@@ -300,6 +300,27 @@ In a production deployment, you want to have basic authentication (`PROXY_ENABLE
 Configure `PROXY_OIDC_AUDIENCES` as described in [OIDC Access Token Audiences](#oidc-access-token-audiences).
 Enabling this check is strongly recommended for production deployments.
 
+Check `PROXY_TRUSTED_PROXIES` as described in [Trusted Reverse Proxies](#trusted-reverse-proxies) if your reverse proxy is not in a private network.
+
+### Trusted Reverse Proxies
+
+Behind a reverse proxy, the proxy service sees the address of the reverse proxy, the address of the client comes in the `X-Forwarded-For` header. Any client can send this header, so the proxy service only uses it when the request comes from a trusted reverse proxy. The client address is used in the access log.
+
+`PROXY_TRUSTED_PROXIES` takes a list of IP addresses and CIDR ranges. The default trusts the loopback and the private network ranges: `127.0.0.0/8`, `::1/128`, `10.0.0.0/8`, `172.16.0.0/12`, `192.168.0.0/16` and `fc00::/7`. This works for a reverse proxy in the same Docker network or a Kubernetes ingress.
+
+Change the setting in these cases:
+
+- Your reverse proxy reaches OpenCloud from a public address, e.g. Cloudflare, a load balancer of a cloud provider or a proxy on another server. With the default its headers are ignored, every request then has the address of the reverse proxy. Add the addresses of your reverse proxy, for Cloudflare its published IP ranges.
+- Clients can reach OpenCloud from a private network without passing your reverse proxy. With the default they can set their address themselves. Set the addresses of your reverse proxies only.
+
+`PROXY_TRUSTED_PROXIES` replaces the default, list the loopback and private ranges again if you still need them.
+
+`X-Forwarded-For` is read from the right: the first address that is not a trusted proxy is the client, addresses a client added in front of it are ignored. If your reverse proxy sends the client address in another header, e.g. `X-Real-IP` or `CF-Connecting-IP`, set `PROXY_CLIENT_IP_HEADER`. A request from a trusted proxy without a usable header gets the address of the proxy.
+
+The proxy service passes the client address on to the other services in `X-Forwarded-For`.
+
+`OC_TRUSTED_PROXIES` and `OC_CLIENT_IP_HEADER` set both values for the proxy and the collaboration service.
+
 ### Content Security Policy
 
 For OpenCloud, external resources like an IDP (e.g. Keycloak) or when using web office documents or web apps, require defining a CSP. If not defined, the referenced services will not work.

services/proxy/pkg/command/clientip_test.go

@@ -1,100 +0,0 @@
-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)
-		})
-	}
-}

services/proxy/pkg/command/server.go

@@ -384,13 +384,16 @@ func loadMiddlewares(logger log.Logger, cfg *config.Config,
 		URLVerifier:        signURLVerifier,
 	})
 
+	// validated by the config parser
+	trustedProxies, _ := pkgmiddleware.ParseTrustedProxies(cfg.ClientIP.TrustedProxies)
+
 	cspConfig, err := middleware.LoadCSPConfig(cfg)
 	if err != nil {
 		logger.Fatal().Err(err).Msg("Failed to load CSP configuration.")
 	}
 
 	return alice.New(
-		clientIPMiddleware(cfg),
+		pkgmiddleware.ClientIP(trustedProxies, cfg.ClientIP.Header),
 		chimiddleware.RequestID,
 
 		// 1. Logging & Tracing first
@@ -460,20 +463,3 @@ func loadMiddlewares(logger log.Logger, cfg *config.Config,
 		),
 	), nil
 }
-
-// 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
-	}
-}

services/proxy/pkg/config/config.go

@@ -98,12 +98,10 @@ var (
 	RouteTypes = []RouteType{QueryRoute, RegexRoute, PrefixRoute}
 )
 
-// ClientIP configures how the proxy resolves the real client IP address.
+// ClientIP configures how the proxy determines the address of the client.
 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%"`
+	TrustedProxies []string `yaml:"trusted_proxies" env:"OC_TRUSTED_PROXIES;PROXY_TRUSTED_PROXIES" desc:"IP addresses and CIDR ranges of the reverse proxies in front of OpenCloud. The client address is only taken from the header set in PROXY_CLIENT_IP_HEADER if the request comes from one of these. Defaults to the loopback and private network ranges. Set it to the addresses of your reverse proxies if clients can reach OpenCloud from a private network. See the Environment Variable Types description for more details." introductionVersion:"%%NEXT%%"`
+	Header         string   `yaml:"header" env:"OC_CLIENT_IP_HEADER;PROXY_CLIENT_IP_HEADER" desc:"The header a trusted reverse proxy sends the client address in, e.g. X-Real-IP or CF-Connecting-IP. X-Forwarded-For is read from the right, the first address that is not a trusted proxy is the client." introductionVersion:"%%NEXT%%"`
 }
 
 // AuthMiddleware configures the proxy http auth middleware.
@@ -122,11 +120,6 @@ 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

services/proxy/pkg/config/defaults/defaultconfig.go

@@ -8,6 +8,7 @@ import (
 	"time"
 
 	"github.com/opencloud-eu/opencloud/pkg/config/defaults"
+	"github.com/opencloud-eu/opencloud/pkg/middleware"
 	"github.com/opencloud-eu/opencloud/pkg/shared"
 	"github.com/opencloud-eu/opencloud/pkg/structs"
 	"github.com/opencloud-eu/opencloud/services/proxy/pkg/config"
@@ -47,9 +48,8 @@ func DefaultConfig() *config.Config {
 			},
 		},
 		ClientIP: config.ClientIP{
-			Strategy:    config.ClientIPStrategyRemoteAddr,
-			Header:      "X-Real-IP",
-			TrustedHops: 1, // this might need to be adjusted
+			TrustedProxies: middleware.DefaultTrustedProxies(),
+			Header:         middleware.DefaultClientIPHeader,
 		},
 		Service: config.Service{
 			Name: "proxy",

services/proxy/pkg/config/parser/parse.go

@@ -6,6 +6,7 @@ import (
 	"strings"
 
 	occfg "github.com/opencloud-eu/opencloud/pkg/config"
+	"github.com/opencloud-eu/opencloud/pkg/middleware"
 	"github.com/opencloud-eu/opencloud/pkg/shared"
 	"github.com/opencloud-eu/opencloud/services/proxy/pkg/config"
 	"github.com/opencloud-eu/opencloud/services/proxy/pkg/config/defaults"
@@ -66,14 +67,8 @@ 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 _, err := middleware.ParseTrustedProxies(cfg.ClientIP.TrustedProxies); err != nil {
+		return fmt.Errorf("invalid PROXY_TRUSTED_PROXIES in service %s: %w", cfg.Service.Name, err)
 	}
 
 	if cfg.ServiceAccount.ServiceAccountID == "" {

services/proxy/pkg/middleware/accesslog.go

@@ -6,6 +6,7 @@ import (
 
 	"github.com/go-chi/chi/v5/middleware"
 	"github.com/opencloud-eu/opencloud/pkg/log"
+	ocmiddleware "github.com/opencloud-eu/opencloud/pkg/middleware"
 	"go.opentelemetry.io/otel/trace"
 )
 
@@ -25,7 +26,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", middleware.GetClientIP(r.Context())).
+				Str("remote-addr", ocmiddleware.ClientAddress(r)).
 				Str("method", r.Method).
 				Int("status", wrap.Status()).
 				Str("path", r.URL.Path).

services/proxy/pkg/middleware/context_logger.go

@@ -3,8 +3,8 @@ package middleware
 import (
 	"net/http"
 
-	"github.com/go-chi/chi/v5/middleware"
 	"github.com/opencloud-eu/opencloud/pkg/log"
+	ocmiddleware "github.com/opencloud-eu/opencloud/pkg/middleware"
 )
 
 // ContextLogger is a middleware to use a logger associated with the request's
@@ -13,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", middleware.GetClientIP(r.Context())).
+				Str("remoteAddr", ocmiddleware.ClientAddress(r)).
 				Str(log.RequestIDString, r.Header.Get("X-Request-ID")).
 				Str("proto", r.Proto).
 				Str("method", r.Method).

services/proxy/pkg/middleware/oidc_auth.go

@@ -15,8 +15,8 @@ import (
 	"golang.org/x/crypto/sha3"
 	"golang.org/x/oauth2"
 
-	"github.com/go-chi/chi/v5/middleware"
 	"github.com/opencloud-eu/opencloud/pkg/log"
+	ocmiddleware "github.com/opencloud-eu/opencloud/pkg/middleware"
 	"github.com/opencloud-eu/opencloud/pkg/oidc"
 	"github.com/opencloud-eu/opencloud/services/proxy/pkg/config"
 	bcl "github.com/opencloud-eu/opencloud/services/proxy/pkg/staticroutes/backchannellogout"
@@ -236,7 +236,7 @@ func (m *OIDCAuthenticator) Authenticate(r *http.Request) AuthenticationResult {
 			Str("authenticator", "oidc").
 			Str("path", r.URL.Path).
 			Str("user_agent", r.UserAgent()).
-			Str("client.address", middleware.GetClientIP(r.Context())).
+			Str("client.address", ocmiddleware.ClientAddress(r)).
 			Str("network.peer.address", host).
 			Str("network.peer.port", port).
 			Msg("failed to authenticate the request")

services/proxy/pkg/proxy/proxy.go

@@ -17,7 +17,6 @@ import (
 	"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"
 )
 
@@ -45,20 +44,18 @@ 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()
-			return
+		} else {
+			// Fallback to standard service routing logic
+			ri := router.ContextRoutingInfo(r.In.Context())
+			ri.Rewrite()(r)
 		}
 
-		// Fallback to standard service routing logic
-		ri := router.ContextRoutingInfo(r.In.Context())
-		ri.Rewrite()(r)
+		// SetXForwarded uses the peer, which is the reverse proxy in front of us.
+		// Tell the services who the client is, reva reads it e.g. for OCM.
+		r.Out.Header.Set("X-Forwarded-For", pkgmiddleware.ClientAddress(r.In))
 	}
 
 	rp.ErrorHandler = func(rw http.ResponseWriter, req *http.Request, err error) {

services/proxy/pkg/proxy/rewrite_test.go

@@ -9,65 +9,56 @@ import (
 	"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 {
+func rewrite(t *testing.T, in *http.Request) *http.Request {
+	t.Helper()
+	rp := newTestProxy(testConfig(nil), 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)
-
+	in = in.WithContext(context.WithValue(in.Context(), middleware.DatagatewaySkipRoutingKey, true))
 	pr := &httputil.ProxyRequest{
-		In:  withCtx,
-		Out: withCtx.Clone(context.Background()),
+		In:  in,
+		Out: in.Clone(context.Background()),
 	}
 	rp.Rewrite(pr)
-
-	assert.Equal(t, pr.Out.Header.Get(pkgmiddleware.DefaultClientIPHeader), "192.0.2.1")
+	return pr.Out
 }
 
-// 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)
+// TestRewriteForwardsClientIP verifies that the services get the client
+// address in X-Forwarded-For, not the reverse proxy in front of us.
+func TestRewriteForwardsClientIP(t *testing.T) {
+	trusted, err := pkgmiddleware.ParseTrustedProxies([]string{"10.0.0.0/8"})
+	assert.NilError(t, err)
 
-	rp := newTestProxy(cfg, func(req *http.Request) *http.Response {
-		return &http.Response{StatusCode: 200, Body: io.NopCloser(bytes.NewBufferString("OK")), Header: make(http.Header)}
-	})
+	in := httptest.NewRequest(http.MethodGet, "/", nil)
+	in.RemoteAddr = "10.0.0.5:443"
+	in.Header.Set("X-Forwarded-For", "198.51.100.1, 203.0.113.7")
 
-	inReq := httptest.NewRequest(http.MethodGet, "/", nil)
-	inReq.RemoteAddr = "192.0.2.1:1234"
+	var withCtx *http.Request
+	pkgmiddleware.ClientIP(trusted, pkgmiddleware.DefaultClientIPHeader)(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		withCtx = r
+	})).ServeHTTP(httptest.NewRecorder(), in)
 
-	ctx := context.WithValue(inReq.Context(), middleware.DatagatewaySkipRoutingKey, true)
-	inReq = inReq.WithContext(ctx)
+	out := rewrite(t, withCtx)
 
-	pr := &httputil.ProxyRequest{
-		In:  inReq,
-		Out: inReq.Clone(context.Background()),
-	}
-	rp.Rewrite(pr)
-
-	assert.Equal(t, pr.Out.Header.Get(pkgmiddleware.DefaultClientIPHeader), "")
+	assert.Equal(t, out.Header.Get("X-Forwarded-For"), "203.0.113.7")
+}
+
+// TestRewriteForwardsPeerWithoutClientIP verifies the peer is forwarded when
+// the client IP middleware did not run, and a header sent by the client is
+// replaced.
+func TestRewriteForwardsPeerWithoutClientIP(t *testing.T) {
+	in := httptest.NewRequest(http.MethodGet, "/", nil)
+	in.RemoteAddr = "192.0.2.1:1234"
+	in.Header.Set("X-Forwarded-For", "203.0.113.9")
+
+	out := rewrite(t, in)
+
+	assert.Equal(t, out.Header.Get("X-Forwarded-For"), "192.0.2.1")
 }

services/settings/pkg/server/http/server.go

@@ -42,7 +42,6 @@ func Server(opts ...Option) (ohttp.Service, error) {
 
 	mux := chi.NewMux()
 
-	mux.Use(chimiddleware.ClientIPFromHeader(middleware.DefaultClientIPHeader))
 	mux.Use(chimiddleware.RequestID)
 	mux.Use(middleware.NoCache)
 	mux.Use(middleware.Cors(

services/thumbnails/pkg/server/http/server.go

@@ -37,7 +37,6 @@ func Server(opts ...Option) (http.Service, error) {
 		svc.Logger(options.Logger),
 		svc.Config(options.Config),
 		svc.Middleware(
-			middleware.ClientIPFromHeader(opencloudmiddleware.DefaultClientIPHeader),
 			middleware.RequestID,
 			opencloudmiddleware.Cors(
 				cors.Logger(options.Logger),

services/web/pkg/server/http/server.go

@@ -118,7 +118,6 @@ func Server(opts ...Option) (http.Service, error) {
 		svc.Config(options.Config),
 		svc.GatewaySelector(gatewaySelector),
 		svc.Middleware(
-			chimiddleware.ClientIPFromHeader(middleware.DefaultClientIPHeader),
 			chimiddleware.RequestID,
 			chimiddleware.Compress(5),
 			middleware.NoCache,

services/webdav/pkg/server/http/server.go

@@ -38,7 +38,6 @@ func Server(opts ...Option) (http.Service, error) {
 		svc.Logger(options.Logger),
 		svc.Config(options.Config),
 		svc.Middleware(
-			chimiddleware.ClientIPFromHeader(middleware.DefaultClientIPHeader),
 			chimiddleware.RequestID,
 			middleware.NoCache,
 			middleware.Cors(

services/webfinger/pkg/server/http/server.go

@@ -45,7 +45,6 @@ func Server(opts ...Option) (ohttp.Service, error) {
 
 	mux := chi.NewMux()
 
-	mux.Use(chimiddleware.ClientIPFromHeader(middleware.DefaultClientIPHeader))
 	mux.Use(chimiddleware.RequestID)
 	mux.Use(middleware.TraceContext)
 	mux.Use(middleware.NoCache)