diff --git a/proxy/cmd/proxy/cmd/root.go b/proxy/cmd/proxy/cmd/root.go index ad8e1b7c0..9b180a5c4 100644 --- a/proxy/cmd/proxy/cmd/root.go +++ b/proxy/cmd/proxy/cmd/root.go @@ -18,6 +18,7 @@ import ( "github.com/netbirdio/netbird/client/embed" "github.com/netbirdio/netbird/proxy" nbacme "github.com/netbirdio/netbird/proxy/internal/acme" + "github.com/netbirdio/netbird/trustedproxy" "github.com/netbirdio/netbird/util" ) @@ -209,7 +210,7 @@ func runServer(cmd *cobra.Command, args []string) error { return fmt.Errorf("invalid domain value %q: %w", proxyDomain, err) } - parsedTrustedProxies, err := proxy.ParseTrustedProxies(trustedProxies) + parsedTrustedProxies, err := trustedproxy.Parse(trustedProxies) if err != nil { return fmt.Errorf("invalid --trusted-proxies: %w", err) } diff --git a/proxy/internal/accesslog/logger.go b/proxy/internal/accesslog/logger.go index db868b4e0..d47c71ca4 100644 --- a/proxy/internal/accesslog/logger.go +++ b/proxy/internal/accesslog/logger.go @@ -16,6 +16,7 @@ import ( "github.com/netbirdio/netbird/proxy/auth" "github.com/netbirdio/netbird/proxy/internal/types" "github.com/netbirdio/netbird/shared/management/proto" + "github.com/netbirdio/netbird/trustedproxy" ) const ( @@ -66,7 +67,7 @@ type denyBucket struct { type Logger struct { client gRPCClient logger *log.Logger - trustedProxies []netip.Prefix + trustedProxies *trustedproxy.List usageMux sync.Mutex domainUsage map[string]*domainUsage @@ -82,7 +83,7 @@ type Logger struct { // NewLogger creates a new access log Logger. The trustedProxies parameter // configures which upstream proxy IP ranges are trusted for extracting // the real client IP from X-Forwarded-For headers. -func NewLogger(client gRPCClient, logger *log.Logger, trustedProxies []netip.Prefix) *Logger { +func NewLogger(client gRPCClient, logger *log.Logger, trustedProxies *trustedproxy.List) *Logger { if logger == nil { logger = log.StandardLogger() } diff --git a/proxy/internal/accesslog/requestip.go b/proxy/internal/accesslog/requestip.go index 30c483fd9..71cea85d0 100644 --- a/proxy/internal/accesslog/requestip.go +++ b/proxy/internal/accesslog/requestip.go @@ -4,13 +4,13 @@ import ( "net/http" "net/netip" - "github.com/netbirdio/netbird/proxy/internal/proxy" + "github.com/netbirdio/netbird/trustedproxy" ) // extractSourceIP resolves the real client IP from the request using trusted // proxy configuration. When trustedProxies is non-empty and the direct // connection is from a trusted source, it walks X-Forwarded-For right-to-left // skipping trusted IPs. Otherwise it returns RemoteAddr directly. -func extractSourceIP(r *http.Request, trustedProxies []netip.Prefix) netip.Addr { - return proxy.ResolveClientIP(r.RemoteAddr, r.Header.Get("X-Forwarded-For"), trustedProxies) +func extractSourceIP(r *http.Request, trustedProxies *trustedproxy.List) netip.Addr { + return trustedProxies.ResolveClientIP(r.RemoteAddr, r.Header.Get("X-Forwarded-For")) } diff --git a/proxy/internal/proxy/reverseproxy.go b/proxy/internal/proxy/reverseproxy.go index 835a1c0b2..9150c0329 100644 --- a/proxy/internal/proxy/reverseproxy.go +++ b/proxy/internal/proxy/reverseproxy.go @@ -22,6 +22,7 @@ import ( "github.com/netbirdio/netbird/proxy/internal/roundtrip" "github.com/netbirdio/netbird/proxy/internal/types" "github.com/netbirdio/netbird/proxy/web" + "github.com/netbirdio/netbird/trustedproxy" ) type ReverseProxy struct { @@ -29,10 +30,10 @@ type ReverseProxy struct { // forwardedProto overrides the X-Forwarded-Proto header value. // Valid values: "auto" (detect from TLS), "http", "https". forwardedProto string - // trustedProxies is a list of IP prefixes for trusted upstream proxies. - // When the direct connection comes from a trusted proxy, forwarding - // headers are preserved and appended to instead of being stripped. - trustedProxies []netip.Prefix + // trustedProxies is the set of trusted upstream proxies. When the direct + // connection comes from a trusted proxy, forwarding headers are preserved + // and appended to instead of being stripped. + trustedProxies *trustedproxy.List mappingsMux sync.RWMutex mappings map[string]Mapping logger *log.Logger @@ -63,7 +64,7 @@ func WithMiddlewareManager(m *middleware.Manager) Option { // between requested URLs and targets. // The internal mappings can be modified using the AddMapping // and RemoveMapping functions. -func NewReverseProxy(transport http.RoundTripper, forwardedProto string, trustedProxies []netip.Prefix, logger *log.Logger, opts ...Option) *ReverseProxy { +func NewReverseProxy(transport http.RoundTripper, forwardedProto string, trustedProxies *trustedproxy.List, logger *log.Logger, opts ...Option) *ReverseProxy { if logger == nil { logger = log.StandardLogger() } @@ -527,7 +528,7 @@ func (p *ReverseProxy) isSelfTargetLoop(r *http.Request, target *url.URL) bool { if !types.IsOverlayOrigin(r.Context()) { return false } - srcIP := extractHostIP(r.RemoteAddr) + srcIP := trustedproxy.ExtractHostIP(r.RemoteAddr) if !srcIP.IsValid() { return false } @@ -578,9 +579,9 @@ func (p *ReverseProxy) rewriteFunc(target *url.URL, matchedPath string, passHost stampNetBirdIdentity(r) - clientIP := extractHostIP(r.In.RemoteAddr) + clientIP := trustedproxy.ExtractHostIP(r.In.RemoteAddr) - if isTrustedAddr(clientIP, p.trustedProxies) { + if p.trustedProxies.Contains(clientIP) { p.setTrustedForwardingHeaders(r, clientIP) } else { p.setUntrustedForwardingHeaders(r, clientIP) @@ -664,7 +665,7 @@ func (p *ReverseProxy) setTrustedForwardingHeaders(r *httputil.ProxyRequest, cli if realIP := r.In.Header.Get("X-Real-IP"); realIP != "" { r.Out.Header.Set("X-Real-IP", realIP) } else { - resolved := ResolveClientIP(r.In.RemoteAddr, r.In.Header.Get("X-Forwarded-For"), p.trustedProxies) + resolved := p.trustedProxies.ResolveClientIP(r.In.RemoteAddr, r.In.Header.Get("X-Forwarded-For")) r.Out.Header.Set("X-Real-IP", resolved.String()) } diff --git a/proxy/internal/proxy/reverseproxy_test.go b/proxy/internal/proxy/reverseproxy_test.go index 9bd427056..83afee387 100644 --- a/proxy/internal/proxy/reverseproxy_test.go +++ b/proxy/internal/proxy/reverseproxy_test.go @@ -23,6 +23,7 @@ import ( "github.com/netbirdio/netbird/proxy/internal/roundtrip" "github.com/netbirdio/netbird/proxy/internal/types" "github.com/netbirdio/netbird/proxy/web" + "github.com/netbirdio/netbird/trustedproxy" ) func TestRewriteFunc_HostRewriting(t *testing.T) { @@ -302,7 +303,7 @@ func TestExtractHostIP(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - assert.Equal(t, tt.expected, extractHostIP(tt.remoteAddr)) + assert.Equal(t, tt.expected, trustedproxy.ExtractHostIP(tt.remoteAddr)) }) } } @@ -330,7 +331,7 @@ func TestExtractForwardedPort(t *testing.T) { func TestRewriteFunc_TrustedProxy(t *testing.T) { target, _ := url.Parse("http://backend.internal:8080") - trusted := []netip.Prefix{netip.MustParsePrefix("10.0.0.0/8")} + trusted := trustedproxy.FromPrefixes([]netip.Prefix{netip.MustParsePrefix("10.0.0.0/8")}) t.Run("appends to X-Forwarded-For", func(t *testing.T) { p := &ReverseProxy{forwardedProto: "auto", trustedProxies: trusted} diff --git a/proxy/internal/proxy/trustedproxy.go b/proxy/internal/proxy/trustedproxy.go deleted file mode 100644 index 0fe693f90..000000000 --- a/proxy/internal/proxy/trustedproxy.go +++ /dev/null @@ -1,81 +0,0 @@ -package proxy - -import ( - "net/netip" - "strings" -) - -// IsTrustedProxy checks if the given IP string falls within any of the trusted prefixes. -func IsTrustedProxy(ipStr string, trusted []netip.Prefix) bool { - addr, err := netip.ParseAddr(ipStr) - if err != nil || len(trusted) == 0 { - return false - } - return isTrustedAddr(addr.Unmap(), trusted) -} - -// ResolveClientIP extracts the real client IP from X-Forwarded-For using the trusted proxy list. -// It walks the XFF chain right-to-left, skipping IPs that match trusted prefixes. -// The first untrusted IP is the real client. -// -// If the trusted list is empty or remoteAddr is not trusted, it returns the -// remoteAddr IP directly (ignoring any forwarding headers). -func ResolveClientIP(remoteAddr, xff string, trusted []netip.Prefix) netip.Addr { - remoteIP := extractHostIP(remoteAddr) - - if len(trusted) == 0 || !isTrustedAddr(remoteIP, trusted) { - return remoteIP - } - - if xff == "" { - return remoteIP - } - - parts := strings.Split(xff, ",") - for i := len(parts) - 1; i >= 0; i-- { - ip := strings.TrimSpace(parts[i]) - if ip == "" { - continue - } - addr, err := netip.ParseAddr(ip) - if err != nil { - continue - } - addr = addr.Unmap() - if !isTrustedAddr(addr, trusted) { - return addr - } - } - - // All IPs in XFF are trusted; return the leftmost as best guess. - if first := strings.TrimSpace(parts[0]); first != "" { - if addr, err := netip.ParseAddr(first); err == nil { - return addr.Unmap() - } - } - return remoteIP -} - -// extractHostIP parses the IP from a host:port string and returns it unmapped. -func extractHostIP(hostPort string) netip.Addr { - if ap, err := netip.ParseAddrPort(hostPort); err == nil { - return ap.Addr().Unmap() - } - if addr, err := netip.ParseAddr(hostPort); err == nil { - return addr.Unmap() - } - return netip.Addr{} -} - -// isTrustedAddr checks if the given address falls within any of the trusted prefixes. -func isTrustedAddr(addr netip.Addr, trusted []netip.Prefix) bool { - if !addr.IsValid() { - return false - } - for _, prefix := range trusted { - if prefix.Contains(addr) { - return true - } - } - return false -} diff --git a/proxy/internal/proxy/trustedproxy_test.go b/proxy/internal/proxy/trustedproxy_test.go deleted file mode 100644 index 35ed1f5c2..000000000 --- a/proxy/internal/proxy/trustedproxy_test.go +++ /dev/null @@ -1,129 +0,0 @@ -package proxy - -import ( - "net/netip" - "testing" - - "github.com/stretchr/testify/assert" -) - -func TestIsTrustedProxy(t *testing.T) { - trusted := []netip.Prefix{ - netip.MustParsePrefix("10.0.0.0/8"), - netip.MustParsePrefix("192.168.1.0/24"), - netip.MustParsePrefix("fd00::/8"), - } - - tests := []struct { - name string - ip string - trusted []netip.Prefix - want bool - }{ - {"empty trusted list", "10.0.0.1", nil, false}, - {"IP within /8 prefix", "10.1.2.3", trusted, true}, - {"IP within /24 prefix", "192.168.1.100", trusted, true}, - {"IP outside all prefixes", "203.0.113.50", trusted, false}, - {"boundary IP just outside prefix", "192.168.2.1", trusted, false}, - {"unparsable IP", "not-an-ip", trusted, false}, - {"IPv6 in trusted range", "fd00::1", trusted, true}, - {"IPv6 outside range", "2001:db8::1", trusted, false}, - {"empty string", "", trusted, false}, - } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - assert.Equal(t, tt.want, IsTrustedProxy(tt.ip, tt.trusted)) - }) - } -} - -func TestResolveClientIP(t *testing.T) { - trusted := []netip.Prefix{ - netip.MustParsePrefix("10.0.0.0/8"), - netip.MustParsePrefix("172.16.0.0/12"), - } - - tests := []struct { - name string - remoteAddr string - xff string - trusted []netip.Prefix - want netip.Addr - }{ - { - name: "empty trusted list returns RemoteAddr", - remoteAddr: "203.0.113.50:9999", - xff: "1.2.3.4", - trusted: nil, - want: netip.MustParseAddr("203.0.113.50"), - }, - { - name: "untrusted RemoteAddr ignores XFF", - remoteAddr: "203.0.113.50:9999", - xff: "1.2.3.4, 10.0.0.1", - trusted: trusted, - want: netip.MustParseAddr("203.0.113.50"), - }, - { - name: "trusted RemoteAddr with single client in XFF", - remoteAddr: "10.0.0.1:5000", - xff: "203.0.113.50", - trusted: trusted, - want: netip.MustParseAddr("203.0.113.50"), - }, - { - name: "trusted RemoteAddr walks past trusted entries in XFF", - remoteAddr: "10.0.0.1:5000", - xff: "203.0.113.50, 10.0.0.2, 172.16.0.5", - trusted: trusted, - want: netip.MustParseAddr("203.0.113.50"), - }, - { - name: "trusted RemoteAddr with empty XFF falls back to RemoteAddr", - remoteAddr: "10.0.0.1:5000", - xff: "", - trusted: trusted, - want: netip.MustParseAddr("10.0.0.1"), - }, - { - name: "all XFF IPs trusted returns leftmost", - remoteAddr: "10.0.0.1:5000", - xff: "10.0.0.2, 172.16.0.1, 10.0.0.3", - trusted: trusted, - want: netip.MustParseAddr("10.0.0.2"), - }, - { - name: "XFF with whitespace", - remoteAddr: "10.0.0.1:5000", - xff: " 203.0.113.50 , 10.0.0.2 ", - trusted: trusted, - want: netip.MustParseAddr("203.0.113.50"), - }, - { - name: "XFF with empty segments", - remoteAddr: "10.0.0.1:5000", - xff: "203.0.113.50,,10.0.0.2", - trusted: trusted, - want: netip.MustParseAddr("203.0.113.50"), - }, - { - name: "multi-hop with mixed trust", - remoteAddr: "10.0.0.1:5000", - xff: "8.8.8.8, 203.0.113.50, 172.16.0.1", - trusted: trusted, - want: netip.MustParseAddr("203.0.113.50"), - }, - { - name: "RemoteAddr without port", - remoteAddr: "10.0.0.1", - xff: "203.0.113.50", - trusted: trusted, - want: netip.MustParseAddr("203.0.113.50"), - }, - } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - assert.Equal(t, tt.want, ResolveClientIP(tt.remoteAddr, tt.xff, tt.trusted)) - }) - } -} diff --git a/proxy/lifecycle.go b/proxy/lifecycle.go index 0d4aded9c..f8c74d8b5 100644 --- a/proxy/lifecycle.go +++ b/proxy/lifecycle.go @@ -2,13 +2,13 @@ package proxy import ( "context" - "net/netip" "time" log "github.com/sirupsen/logrus" "github.com/netbirdio/netbird/client/embed" "github.com/netbirdio/netbird/proxy/internal/acme" + "github.com/netbirdio/netbird/trustedproxy" ) // Config bundles every knob the proxy reads at construction time. It mirrors @@ -83,9 +83,9 @@ type Config struct { // ForwardedProto overrides the X-Forwarded-Proto value sent to // backends. Valid values: "auto", "http", "https". ForwardedProto string - // TrustedProxies is a list of IP prefixes for trusted upstream - // proxies that may set forwarding headers. - TrustedProxies []netip.Prefix + // TrustedProxies is the set of trusted upstream proxies that may set + // forwarding headers. + TrustedProxies *trustedproxy.List // WireguardPort is the UDP port for the embedded NetBird tunnel. // Zero asks the OS for a random port. WireguardPort uint16 diff --git a/proxy/proxyprotocol_test.go b/proxy/proxyprotocol_test.go index fe2fe7e2d..9e19314ed 100644 --- a/proxy/proxyprotocol_test.go +++ b/proxy/proxyprotocol_test.go @@ -10,12 +10,14 @@ import ( log "github.com/sirupsen/logrus" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + + "github.com/netbirdio/netbird/trustedproxy" ) func TestWrapProxyProtocol_OverridesRemoteAddr(t *testing.T) { srv := &Server{ Logger: log.StandardLogger(), - TrustedProxies: []netip.Prefix{netip.MustParsePrefix("127.0.0.1/32")}, + TrustedProxies: trustedproxy.FromPrefixes([]netip.Prefix{netip.MustParsePrefix("127.0.0.1/32")}), ProxyProtocol: true, } @@ -66,7 +68,7 @@ func TestWrapProxyProtocol_OverridesRemoteAddr(t *testing.T) { func TestProxyProtocolPolicy_TrustedRequires(t *testing.T) { srv := &Server{ Logger: log.StandardLogger(), - TrustedProxies: []netip.Prefix{netip.MustParsePrefix("10.0.0.0/8")}, + TrustedProxies: trustedproxy.FromPrefixes([]netip.Prefix{netip.MustParsePrefix("10.0.0.0/8")}), } opts := proxyproto.ConnPolicyOptions{ @@ -80,7 +82,7 @@ func TestProxyProtocolPolicy_TrustedRequires(t *testing.T) { func TestProxyProtocolPolicy_UntrustedIgnores(t *testing.T) { srv := &Server{ Logger: log.StandardLogger(), - TrustedProxies: []netip.Prefix{netip.MustParsePrefix("10.0.0.0/8")}, + TrustedProxies: trustedproxy.FromPrefixes([]netip.Prefix{netip.MustParsePrefix("10.0.0.0/8")}), } opts := proxyproto.ConnPolicyOptions{ @@ -94,7 +96,7 @@ func TestProxyProtocolPolicy_UntrustedIgnores(t *testing.T) { func TestProxyProtocolPolicy_InvalidIPRejects(t *testing.T) { srv := &Server{ Logger: log.StandardLogger(), - TrustedProxies: []netip.Prefix{netip.MustParsePrefix("10.0.0.0/8")}, + TrustedProxies: trustedproxy.FromPrefixes([]netip.Prefix{netip.MustParsePrefix("10.0.0.0/8")}), } opts := proxyproto.ConnPolicyOptions{ diff --git a/proxy/server.go b/proxy/server.go index f28d580bd..4f448e4b8 100644 --- a/proxy/server.go +++ b/proxy/server.go @@ -67,6 +67,7 @@ import ( "github.com/netbirdio/netbird/proxy/web" "github.com/netbirdio/netbird/shared/management/domain" "github.com/netbirdio/netbird/shared/management/proto" + "github.com/netbirdio/netbird/trustedproxy" "github.com/netbirdio/netbird/util/embeddedroots" ) @@ -79,19 +80,19 @@ type portRouter struct { type Server struct { ctx context.Context - mgmtClient proto.ProxyServiceClient - proxy *proxy.ReverseProxy - netbird *roundtrip.NetBird - acme *acme.Manager + mgmtClient proto.ProxyServiceClient + proxy *proxy.ReverseProxy + netbird *roundtrip.NetBird + acme *acme.Manager staticCertWatcher *certwatch.Watcher - auth *auth.Middleware - http *http.Server - https *http.Server - debug *http.Server - healthServer *health.Server - healthChecker *health.Checker - meter *proxymetrics.Metrics - accessLog *accesslog.Logger + auth *auth.Middleware + http *http.Server + https *http.Server + debug *http.Server + healthServer *health.Server + healthChecker *health.Checker + meter *proxymetrics.Metrics + accessLog *accesslog.Logger // middlewareManager drives per-target middleware dispatch. Always // constructed during boot; an empty registry produces empty chains and // the reverse-proxy stays on the no-capture fast path. @@ -99,16 +100,16 @@ type Server struct { // middlewareRegistry is the source of registered middleware factories. // Concrete middlewares register themselves through init(). middlewareRegistry *middleware.Registry - mainRouter *nbtcp.Router - mainPort uint16 - udpMu sync.Mutex - udpRelays map[types.ServiceID]*udprelay.Relay - udpRelayWg sync.WaitGroup - portMu sync.RWMutex - portRouters map[uint16]*portRouter - svcPorts map[types.ServiceID][]uint16 - lastMappings map[types.ServiceID]*proto.ProxyMapping - portRouterWg sync.WaitGroup + mainRouter *nbtcp.Router + mainPort uint16 + udpMu sync.Mutex + udpRelays map[types.ServiceID]*udprelay.Relay + udpRelayWg sync.WaitGroup + portMu sync.RWMutex + portRouters map[uint16]*portRouter + svcPorts map[types.ServiceID][]uint16 + lastMappings map[types.ServiceID]*proto.ProxyMapping + portRouterWg sync.WaitGroup // hijackTracker tracks hijacked connections (e.g. WebSocket upgrades) // so they can be closed during graceful shutdown, since http.Server.Shutdown @@ -192,10 +193,10 @@ type Server struct { // ForwardedProto overrides the X-Forwarded-Proto value sent to backends. // Valid values: "auto" (detect from TLS), "http", "https". ForwardedProto string - // TrustedProxies is a list of IP prefixes for trusted upstream proxies. - // When set, forwarding headers from these sources are preserved and - // appended to instead of being stripped. - TrustedProxies []netip.Prefix + // TrustedProxies is the set of trusted upstream proxies. When set, + // forwarding headers from these sources are preserved and appended to + // instead of being stripped. + TrustedProxies *trustedproxy.List // WireguardPort is the port for the NetBird tunnel interface. Use 0 // for a random OS-assigned port. A fixed port only works with // single-account deployments; multiple accounts will fail to bind @@ -718,7 +719,7 @@ func (s *Server) wrapProxyProtocol(ln net.Listener) net.Listener { Listener: ln, ReadHeaderTimeout: proxyProtoHeaderTimeout, } - if len(s.TrustedProxies) > 0 { + if !s.TrustedProxies.Empty() { ppListener.ConnPolicy = s.proxyProtocolPolicy } else { s.Logger.Warn("PROXY protocol enabled without trusted proxies; any source may send PROXY headers") @@ -742,10 +743,8 @@ func (s *Server) proxyProtocolPolicy(opts proxyproto.ConnPolicyOptions) (proxypr addr = addr.Unmap() // called per accept - for _, prefix := range s.TrustedProxies { - if prefix.Contains(addr) { - return proxyproto.REQUIRE, nil - } + if s.TrustedProxies.Contains(addr) { + return proxyproto.REQUIRE, nil } return proxyproto.IGNORE, nil } diff --git a/proxy/trustedproxy.go b/proxy/trustedproxy.go deleted file mode 100644 index 3a1f0ad37..000000000 --- a/proxy/trustedproxy.go +++ /dev/null @@ -1,43 +0,0 @@ -package proxy - -import ( - "fmt" - "net/netip" - "strings" -) - -// ParseTrustedProxies parses a comma-separated list of CIDR prefixes or bare IPs -// into a slice of netip.Prefix values suitable for trusted proxy configuration. -// Bare IPs are converted to single-host prefixes (/32 or /128). -func ParseTrustedProxies(raw string) ([]netip.Prefix, error) { - if raw == "" { - return nil, nil - } - - parts := strings.Split(raw, ",") - prefixes := make([]netip.Prefix, 0, len(parts)) - for _, part := range parts { - part = strings.TrimSpace(part) - if part == "" { - continue - } - - prefix, err := netip.ParsePrefix(part) - if err == nil { - prefixes = append(prefixes, prefix) - continue - } - - addr, addrErr := netip.ParseAddr(part) - if addrErr != nil { - return nil, fmt.Errorf("parse trusted proxy %q: not a valid CIDR or IP: %w", part, addrErr) - } - - bits := 32 - if addr.Is6() { - bits = 128 - } - prefixes = append(prefixes, netip.PrefixFrom(addr, bits)) - } - return prefixes, nil -} diff --git a/proxy/trustedproxy_test.go b/proxy/trustedproxy_test.go deleted file mode 100644 index 974e56863..000000000 --- a/proxy/trustedproxy_test.go +++ /dev/null @@ -1,90 +0,0 @@ -package proxy - -import ( - "net/netip" - "testing" - - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" -) - -func TestParseTrustedProxies(t *testing.T) { - tests := []struct { - name string - raw string - want []netip.Prefix - wantErr bool - }{ - { - name: "empty string returns nil", - raw: "", - want: nil, - }, - { - name: "single CIDR", - raw: "10.0.0.0/8", - want: []netip.Prefix{netip.MustParsePrefix("10.0.0.0/8")}, - }, - { - name: "single bare IPv4", - raw: "1.2.3.4", - want: []netip.Prefix{netip.MustParsePrefix("1.2.3.4/32")}, - }, - { - name: "single bare IPv6", - raw: "::1", - want: []netip.Prefix{netip.MustParsePrefix("::1/128")}, - }, - { - name: "comma-separated CIDRs", - raw: "10.0.0.0/8, 192.168.1.0/24", - want: []netip.Prefix{ - netip.MustParsePrefix("10.0.0.0/8"), - netip.MustParsePrefix("192.168.1.0/24"), - }, - }, - { - name: "mixed CIDRs and bare IPs", - raw: "10.0.0.0/8, 1.2.3.4, fd00::/8", - want: []netip.Prefix{ - netip.MustParsePrefix("10.0.0.0/8"), - netip.MustParsePrefix("1.2.3.4/32"), - netip.MustParsePrefix("fd00::/8"), - }, - }, - { - name: "whitespace around entries", - raw: " 10.0.0.0/8 , 192.168.0.0/16 ", - want: []netip.Prefix{ - netip.MustParsePrefix("10.0.0.0/8"), - netip.MustParsePrefix("192.168.0.0/16"), - }, - }, - { - name: "trailing comma produces no extra entry", - raw: "10.0.0.0/8,", - want: []netip.Prefix{netip.MustParsePrefix("10.0.0.0/8")}, - }, - { - name: "invalid entry", - raw: "not-an-ip", - wantErr: true, - }, - { - name: "partially invalid", - raw: "10.0.0.0/8, garbage", - wantErr: true, - }, - } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - got, err := ParseTrustedProxies(tt.raw) - if tt.wantErr { - require.Error(t, err) - return - } - require.NoError(t, err) - assert.Equal(t, tt.want, got) - }) - } -} diff --git a/relay/cmd/root.go b/relay/cmd/root.go index 4dd1e6236..a64812d4d 100644 --- a/relay/cmd/root.go +++ b/relay/cmd/root.go @@ -24,6 +24,7 @@ import ( "github.com/netbirdio/netbird/shared/metrics" "github.com/netbirdio/netbird/shared/relay/auth" "github.com/netbirdio/netbird/stun" + "github.com/netbirdio/netbird/trustedproxy" "github.com/netbirdio/netbird/util" ) @@ -45,6 +46,9 @@ type Config struct { LogLevel string LogFile string HealthcheckListenAddress string + // TrustedProxies is a comma-separated list of upstream proxy CIDRs/IPs whose + // X-Real-Ip/X-Real-Port headers are trusted. Empty means never trust these headers. + TrustedProxies string // STUN server configuration EnableSTUN bool STUNPorts []int @@ -116,6 +120,7 @@ func init() { rootCmd.PersistentFlags().StringVar(&cobraConfig.LogLevel, "log-level", "info", "log level") rootCmd.PersistentFlags().StringVar(&cobraConfig.LogFile, "log-file", "console", "log file") rootCmd.PersistentFlags().StringVarP(&cobraConfig.HealthcheckListenAddress, "health-listen-address", "H", ":9000", "listen address of healthcheck server") + rootCmd.PersistentFlags().StringVar(&cobraConfig.TrustedProxies, "trusted-proxies", "", "comma-separated list of upstream proxy CIDRs or IPs whose X-Real-Ip/X-Real-Port headers are trusted; leave empty to always use the direct connection address") rootCmd.PersistentFlags().BoolVar(&cobraConfig.EnableSTUN, "enable-stun", false, "enable embedded STUN server") rootCmd.PersistentFlags().IntSliceVar(&cobraConfig.STUNPorts, "stun-ports", []int{3478}, "ports for the embedded STUN server (can be specified multiple times or comma-separated)") rootCmd.PersistentFlags().StringVar(&cobraConfig.STUNLogLevel, "stun-log-level", "info", "log level for STUN server (panic, fatal, error, warn, info, debug, trace)") @@ -155,8 +160,15 @@ func execute(cmd *cobra.Command, args []string) error { return fmt.Errorf("setup metrics: %v", err) } + trustedProxies, err := trustedproxy.Parse(cobraConfig.TrustedProxies) + if err != nil { + log.Debugf("failed to parse trusted proxies: %s", err) + return fmt.Errorf("failed to parse trusted proxies: %s", err) + } + srvListenerCfg := server.ListenerConfig{ - Address: cobraConfig.ListenAddress, + Address: cobraConfig.ListenAddress, + TrustedProxies: trustedProxies, } tlsConfig, tlsSupport, err := handleTLSConfig(cobraConfig) diff --git a/relay/server/listener/ws/listener.go b/relay/server/listener/ws/listener.go index ba175f901..208b9186e 100644 --- a/relay/server/listener/ws/listener.go +++ b/relay/server/listener/ws/listener.go @@ -15,6 +15,7 @@ import ( "github.com/netbirdio/netbird/relay/protocol" relaylistener "github.com/netbirdio/netbird/relay/server/listener" "github.com/netbirdio/netbird/shared/relay" + "github.com/netbirdio/netbird/trustedproxy" ) const ( @@ -27,6 +28,9 @@ type Listener struct { Address string // TLSConfig is the TLS configuration for the server. TLSConfig *tls.Config + // TrustedProxies is the set of upstream proxies whose X-Real-Ip/X-Real-Port + // headers are trusted. Headers from any other immediate peer are ignored. + TrustedProxies *trustedproxy.List server *http.Server acceptFn func(conn relaylistener.Conn) @@ -75,7 +79,7 @@ func (l *Listener) Shutdown(ctx context.Context) error { } func (l *Listener) onAccept(w http.ResponseWriter, r *http.Request) { - connRemoteAddr := remoteAddr(r) + connRemoteAddr := remoteAddr(r, l.TrustedProxies) acceptOptions := &websocket.AcceptOptions{ OriginPatterns: []string{"*"}, @@ -102,9 +106,17 @@ func (l *Listener) onAccept(w http.ResponseWriter, r *http.Request) { l.acceptFn(conn) } -func remoteAddr(r *http.Request) string { - if r.Header.Get("X-Real-Ip") == "" || r.Header.Get("X-Real-Port") == "" { +func remoteAddr(r *http.Request, trustedProxies *trustedproxy.List) string { + realIP := r.Header.Get("X-Real-Ip") + realPort := r.Header.Get("X-Real-Port") + if realIP == "" || realPort == "" { return r.RemoteAddr } - return net.JoinHostPort(r.Header.Get("X-Real-Ip"), r.Header.Get("X-Real-Port")) + + if !trustedProxies.IsTrusted(r.RemoteAddr) { + log.Debugf("ignoring X-Real-Ip header from untrusted peer %s", r.RemoteAddr) + return r.RemoteAddr + } + + return net.JoinHostPort(realIP, realPort) } diff --git a/relay/server/server.go b/relay/server/server.go index 340da55b8..8d303e9e4 100644 --- a/relay/server/server.go +++ b/relay/server/server.go @@ -15,14 +15,17 @@ import ( "github.com/netbirdio/netbird/relay/server/listener/quic" "github.com/netbirdio/netbird/relay/server/listener/ws" quictls "github.com/netbirdio/netbird/shared/relay/tls" + "github.com/netbirdio/netbird/trustedproxy" ) // ListenerConfig is the configuration for the listener. // Address: the address to bind the listener to. It could be an address behind a reverse proxy. // TLSConfig: the TLS configuration for the listener. +// TrustedProxies: upstream proxy prefixes whose forwarding headers (X-Real-Ip/X-Real-Port) are trusted. type ListenerConfig struct { - Address string - TLSConfig *tls.Config + Address string + TLSConfig *tls.Config + TrustedProxies *trustedproxy.List } // Server is the main entry point for the relay server. @@ -62,8 +65,9 @@ func NewServer(config Config) (*Server, error) { // Listen starts the relay server. func (r *Server) Listen(cfg ListenerConfig) error { wSListener := &ws.Listener{ - Address: cfg.Address, - TLSConfig: cfg.TLSConfig, + Address: cfg.Address, + TLSConfig: cfg.TLSConfig, + TrustedProxies: cfg.TrustedProxies, } r.listenerMux.Lock() diff --git a/trustedproxy/trustedproxy.go b/trustedproxy/trustedproxy.go new file mode 100644 index 000000000..70df01d92 --- /dev/null +++ b/trustedproxy/trustedproxy.go @@ -0,0 +1,132 @@ +package trustedproxy + +import ( + "fmt" + "net/netip" + "strings" +) + +// List holds a parsed set of trusted upstream proxy prefixes and answers trust +// questions against it. The zero value (and a nil *List) is a valid, empty list +// that never trusts any address, so callers can use it without a nil check. +type List struct { + prefixes []netip.Prefix +} + +// Parse parses a comma-separated list of CIDR prefixes or bare IPs into a List. +// Bare IPs are converted to single-host prefixes (/32 or /128). An empty input +// yields an empty List that trusts nothing. +func Parse(raw string) (*List, error) { + if raw == "" { + return &List{}, nil + } + + parts := strings.Split(raw, ",") + prefixes := make([]netip.Prefix, 0, len(parts)) + for _, part := range parts { + part = strings.TrimSpace(part) + if part == "" { + continue + } + + prefix, err := netip.ParsePrefix(part) + if err == nil { + prefixes = append(prefixes, prefix) + continue + } + + addr, addrErr := netip.ParseAddr(part) + if addrErr != nil { + return nil, fmt.Errorf("parse trusted proxy %q: not a valid CIDR or IP: %w", part, addrErr) + } + + bits := 32 + if addr.Is6() { + bits = 128 + } + prefixes = append(prefixes, netip.PrefixFrom(addr, bits)) + } + return &List{prefixes: prefixes}, nil +} + +// FromPrefixes wraps an already-parsed set of prefixes in a List. +func FromPrefixes(prefixes []netip.Prefix) *List { + return &List{prefixes: prefixes} +} + +// Empty reports whether the list contains no prefixes. +func (l *List) Empty() bool { + return l == nil || len(l.prefixes) == 0 +} + +// IsTrusted reports whether the given host:port or bare IP falls within the list. +func (l *List) IsTrusted(remoteAddr string) bool { + if l.Empty() { + return false + } + return l.Contains(ExtractHostIP(remoteAddr)) +} + +// Contains reports whether the given address falls within any trusted prefix. +func (l *List) Contains(addr netip.Addr) bool { + if l.Empty() || !addr.IsValid() { + return false + } + for _, prefix := range l.prefixes { + if prefix.Contains(addr) { + return true + } + } + return false +} + +// ResolveClientIP extracts the real client IP from X-Forwarded-For using the +// list. It walks the XFF chain right-to-left, skipping IPs that match trusted +// prefixes; the first untrusted IP is the real client. If the list is empty or +// remoteAddr is not trusted, it returns the remoteAddr IP directly, ignoring any +// forwarding headers. +func (l *List) ResolveClientIP(remoteAddr, xff string) netip.Addr { + remoteIP := ExtractHostIP(remoteAddr) + + if l.Empty() || !l.Contains(remoteIP) { + return remoteIP + } + + if xff == "" { + return remoteIP + } + + parts := strings.Split(xff, ",") + for i := len(parts) - 1; i >= 0; i-- { + ip := strings.TrimSpace(parts[i]) + if ip == "" { + continue + } + addr, err := netip.ParseAddr(ip) + if err != nil { + continue + } + addr = addr.Unmap() + if !l.Contains(addr) { + return addr + } + } + + if first := strings.TrimSpace(parts[0]); first != "" { + if addr, err := netip.ParseAddr(first); err == nil { + return addr.Unmap() + } + } + return remoteIP +} + +// ExtractHostIP parses the IP from a host:port string and returns it unmapped. +func ExtractHostIP(hostPort string) netip.Addr { + if ap, err := netip.ParseAddrPort(hostPort); err == nil { + return ap.Addr().Unmap() + } + if addr, err := netip.ParseAddr(hostPort); err == nil { + return addr.Unmap() + } + return netip.Addr{} +} diff --git a/trustedproxy/trustedproxy_test.go b/trustedproxy/trustedproxy_test.go new file mode 100644 index 000000000..2e702a49c --- /dev/null +++ b/trustedproxy/trustedproxy_test.go @@ -0,0 +1,216 @@ +package trustedproxy + +import ( + "net/netip" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestParse(t *testing.T) { + tests := []struct { + name string + raw string + want []netip.Prefix + wantErr bool + }{ + { + name: "empty string returns empty list", + raw: "", + want: nil, + }, + { + name: "single CIDR", + raw: "10.0.0.0/8", + want: []netip.Prefix{netip.MustParsePrefix("10.0.0.0/8")}, + }, + { + name: "single bare IPv4", + raw: "1.2.3.4", + want: []netip.Prefix{netip.MustParsePrefix("1.2.3.4/32")}, + }, + { + name: "single bare IPv6", + raw: "::1", + want: []netip.Prefix{netip.MustParsePrefix("::1/128")}, + }, + { + name: "comma-separated CIDRs", + raw: "10.0.0.0/8, 192.168.1.0/24", + want: []netip.Prefix{ + netip.MustParsePrefix("10.0.0.0/8"), + netip.MustParsePrefix("192.168.1.0/24"), + }, + }, + { + name: "mixed CIDRs and bare IPs", + raw: "10.0.0.0/8, 1.2.3.4, fd00::/8", + want: []netip.Prefix{ + netip.MustParsePrefix("10.0.0.0/8"), + netip.MustParsePrefix("1.2.3.4/32"), + netip.MustParsePrefix("fd00::/8"), + }, + }, + { + name: "whitespace around entries", + raw: " 10.0.0.0/8 , 192.168.0.0/16 ", + want: []netip.Prefix{ + netip.MustParsePrefix("10.0.0.0/8"), + netip.MustParsePrefix("192.168.0.0/16"), + }, + }, + { + name: "trailing comma produces no extra entry", + raw: "10.0.0.0/8,", + want: []netip.Prefix{netip.MustParsePrefix("10.0.0.0/8")}, + }, + { + name: "invalid entry", + raw: "not-an-ip", + wantErr: true, + }, + { + name: "partially invalid", + raw: "10.0.0.0/8, garbage", + wantErr: true, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, err := Parse(tt.raw) + if tt.wantErr { + require.Error(t, err) + return + } + require.NoError(t, err) + assert.Equal(t, tt.want, got.prefixes) + }) + } +} + +func TestListIsTrusted(t *testing.T) { + list, err := Parse("10.0.0.0/8, 192.168.1.0/24, fd00::/8") + require.NoError(t, err) + + tests := []struct { + name string + addr string + list *List + want bool + }{ + {"nil list", "10.0.0.1", nil, false}, + {"empty list", "10.0.0.1", &List{}, false}, + {"IP within /8 prefix", "10.1.2.3", list, true}, + {"IP within /24 prefix", "192.168.1.100", list, true}, + {"IP outside all prefixes", "203.0.113.50", list, false}, + {"boundary IP just outside prefix", "192.168.2.1", list, false}, + {"unparsable IP", "not-an-ip", list, false}, + {"IPv6 in trusted range", "fd00::1", list, true}, + {"IPv6 outside range", "2001:db8::1", list, false}, + {"empty string", "", list, false}, + {"host:port within prefix", "10.1.2.3:9999", list, true}, + {"host:port outside prefix", "203.0.113.50:9999", list, false}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + assert.Equal(t, tt.want, tt.list.IsTrusted(tt.addr)) + }) + } +} + +func TestListResolveClientIP(t *testing.T) { + trusted, err := Parse("10.0.0.0/8, 172.16.0.0/12") + require.NoError(t, err) + + tests := []struct { + name string + remoteAddr string + xff string + list *List + want netip.Addr + }{ + { + name: "empty list returns RemoteAddr", + remoteAddr: "203.0.113.50:9999", + xff: "1.2.3.4", + list: &List{}, + want: netip.MustParseAddr("203.0.113.50"), + }, + { + name: "nil list returns RemoteAddr", + remoteAddr: "203.0.113.50:9999", + xff: "1.2.3.4", + list: nil, + want: netip.MustParseAddr("203.0.113.50"), + }, + { + name: "untrusted RemoteAddr ignores XFF", + remoteAddr: "203.0.113.50:9999", + xff: "1.2.3.4, 10.0.0.1", + list: trusted, + want: netip.MustParseAddr("203.0.113.50"), + }, + { + name: "trusted RemoteAddr with single client in XFF", + remoteAddr: "10.0.0.1:5000", + xff: "203.0.113.50", + list: trusted, + want: netip.MustParseAddr("203.0.113.50"), + }, + { + name: "trusted RemoteAddr walks past trusted entries in XFF", + remoteAddr: "10.0.0.1:5000", + xff: "203.0.113.50, 10.0.0.2, 172.16.0.5", + list: trusted, + want: netip.MustParseAddr("203.0.113.50"), + }, + { + name: "trusted RemoteAddr with empty XFF falls back to RemoteAddr", + remoteAddr: "10.0.0.1:5000", + xff: "", + list: trusted, + want: netip.MustParseAddr("10.0.0.1"), + }, + { + name: "all XFF IPs trusted returns leftmost", + remoteAddr: "10.0.0.1:5000", + xff: "10.0.0.2, 172.16.0.1, 10.0.0.3", + list: trusted, + want: netip.MustParseAddr("10.0.0.2"), + }, + { + name: "XFF with whitespace", + remoteAddr: "10.0.0.1:5000", + xff: " 203.0.113.50 , 10.0.0.2 ", + list: trusted, + want: netip.MustParseAddr("203.0.113.50"), + }, + { + name: "XFF with empty segments", + remoteAddr: "10.0.0.1:5000", + xff: "203.0.113.50,,10.0.0.2", + list: trusted, + want: netip.MustParseAddr("203.0.113.50"), + }, + { + name: "multi-hop with mixed trust", + remoteAddr: "10.0.0.1:5000", + xff: "8.8.8.8, 203.0.113.50, 172.16.0.1", + list: trusted, + want: netip.MustParseAddr("203.0.113.50"), + }, + { + name: "RemoteAddr without port", + remoteAddr: "10.0.0.1", + xff: "203.0.113.50", + list: trusted, + want: netip.MustParseAddr("203.0.113.50"), + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + assert.Equal(t, tt.want, tt.list.ResolveClientIP(tt.remoteAddr, tt.xff)) + }) + } +}