Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions pkg/middleware/header.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
2 changes: 1 addition & 1 deletion pkg/service/debug/service.go
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
2 changes: 1 addition & 1 deletion services/collaboration/pkg/middleware/accesslog.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()).
Expand Down
1 change: 1 addition & 0 deletions services/collaboration/pkg/server/http/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
2 changes: 1 addition & 1 deletion services/idp/pkg/server/http/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
2 changes: 1 addition & 1 deletion services/invitations/pkg/server/http/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion services/ocs/pkg/server/http/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
100 changes: 100 additions & 0 deletions services/proxy/pkg/command/clientip_test.go
Original file line number Diff line number Diff line change
@@ -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)
})
}
}
19 changes: 18 additions & 1 deletion services/proxy/pkg/command/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
}
}
15 changes: 15 additions & 0 deletions services/proxy/pkg/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -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:"-"`
Expand Down Expand Up @@ -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"`
Expand All @@ -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
Expand Down
5 changes: 5 additions & 0 deletions services/proxy/pkg/config/defaults/defaultconfig.go
Original file line number Diff line number Diff line change
Expand Up @@ -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",
},
Expand Down
10 changes: 10 additions & 0 deletions services/proxy/pkg/config/parser/parse.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down
2 changes: 1 addition & 1 deletion services/proxy/pkg/middleware/accesslog.go
Original file line number Diff line number Diff line change
Expand Up @@ -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).
Expand Down
3 changes: 2 additions & 1 deletion services/proxy/pkg/middleware/context_logger.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package middleware
import (
"net/http"

"github.com/go-chi/chi/v5/middleware"
"github.com/opencloud-eu/opencloud/pkg/log"
)

Expand All @@ -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).
Expand Down
3 changes: 2 additions & 1 deletion services/proxy/pkg/middleware/oidc_auth.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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")
Expand Down
8 changes: 8 additions & 0 deletions services/proxy/pkg/proxy/proxy.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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()
Expand Down
73 changes: 73 additions & 0 deletions services/proxy/pkg/proxy/rewrite_test.go
Original file line number Diff line number Diff line change
@@ -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), "")
}
Loading