Accept CIDR ranges in proxy trusted-proxies configuration

The HTTP middleware and proxy-protocol listeners both pinned trust
to exact IPs, which forced re-applying terraform every time AWS
rotated an ALB or NLB ENI. Trusted-proxies entries now accept CIDR
ranges in addition to plain IPs, so callers can trust whole subnets
(where the load balancer ENIs always live) and stop chasing
rotating IPs.

The HTTP middleware splits parsed entries into IPs and IPNets and
checks both. The proxy-protocol listeners switch from
TrustProxyHeaderFrom (IP-only, REJECT) to ConnStrictWhiteListPolicy
(IP or CIDR, REJECT) which preserves the existing reject-on-unknown
semantics.

Signed-off-by: Bryan Frimin <bryan@getprobo.com>

pkg/probod/probod.go

@@ -778,8 +778,12 @@ func (impl *Implm) runApiServer(
 	ctx, span := tracer.Start(ctx, "probod.runApiServer")
 	defer span.End()
 
-	trustedProxies := parseIPs(impl.cfg.Api.ProxyProtocol.TrustedProxies)
+	trustedProxyMiddleware, err := trustedproxy.NewMiddleware(impl.cfg.Api.ProxyProtocol.TrustedProxies)
-	handler = trustedproxy.NewMiddleware(trustedProxies)(handler)
+	if err != nil {
+		span.RecordError(err)
+		return fmt.Errorf("cannot build trusted proxy middleware: %w", err)
+	}
+	handler = trustedProxyMiddleware(handler)
 
 	apiServer := httpserver.NewServer(
 		impl.cfg.Api.Addr,
@@ -799,7 +803,11 @@ func (impl *Implm) runApiServer(
 	}
 
 	if len(impl.cfg.Api.ProxyProtocol.TrustedProxies) > 0 {
-		policy := proxyproto.TrustProxyHeaderFrom(parseIPs(impl.cfg.Api.ProxyProtocol.TrustedProxies)...)
+		policy, err := proxyproto.ConnStrictWhiteListPolicy(impl.cfg.Api.ProxyProtocol.TrustedProxies)
+		if err != nil {
+			span.RecordError(err)
+			return fmt.Errorf("cannot build proxy protocol policy: %w", err)
+		}
 
 		listener = &proxyproto.Listener{
 			Listener:          listener,
@@ -972,7 +980,10 @@ func (impl *Implm) runTrustCenterServer(
 			defer func() { _ = listener.Close() }()
 
 			if len(impl.cfg.TrustCenter.ProxyProtocol.TrustedProxies) > 0 {
-				policy := proxyproto.TrustProxyHeaderFrom(parseIPs(impl.cfg.TrustCenter.ProxyProtocol.TrustedProxies)...)
+				policy, err := proxyproto.ConnStrictWhiteListPolicy(impl.cfg.TrustCenter.ProxyProtocol.TrustedProxies)
+				if err != nil {
+					return fmt.Errorf("cannot build proxy protocol policy: %w", err)
+				}
 
 				listener = &proxyproto.Listener{
 					Listener:          listener,
@@ -1057,7 +1068,10 @@ func (impl *Implm) runTrustCenterServer(
 			defer func() { _ = listener.Close() }()
 
 			if len(impl.cfg.TrustCenter.ProxyProtocol.TrustedProxies) > 0 {
-				policy := proxyproto.TrustProxyHeaderFrom(parseIPs(impl.cfg.TrustCenter.ProxyProtocol.TrustedProxies)...)
+				policy, err := proxyproto.ConnStrictWhiteListPolicy(impl.cfg.TrustCenter.ProxyProtocol.TrustedProxies)
+				if err != nil {
+					return fmt.Errorf("cannot build proxy protocol policy: %w", err)
+				}
 
 				listener = &proxyproto.Listener{
 					Listener:          listener,
@@ -1109,18 +1123,6 @@ func (impl *Implm) runTrustCenterServer(
 	return ctx.Err()
 }
 
-// parseIPs converts a slice of string IP addresses to net.IP.
-// Invalid IPs are skipped.
-func parseIPs(strs []string) []net.IP {
-	ips := make([]net.IP, 0, len(strs))
-	for _, s := range strs {
-		if ip := net.ParseIP(s); ip != nil {
-			ips = append(ips, ip)
-		}
-	}
-	return ips
-}
-
 func oauth2ServerOptions(cfg OAuth2ServerConfig) []oauth2server.Option {
 	var opts []oauth2server.Option
 

pkg/server/trustedproxy/trustedproxy.go

@@ -15,8 +15,10 @@
 package trustedproxy
 
 import (
+	"fmt"
 	"net"
 	"net/http"
+	"strings"
 )
 
 var forwardedHeaders = []string{
@@ -26,22 +28,52 @@ var forwardedHeaders = []string{
 
 // NewMiddleware returns an HTTP middleware that strips forwarded
 // headers from requests that did not originate from one of the given
-// trusted proxy IPs.  When the list is empty every request is treated
+// trusted proxies.  Each entry in trusted may be either a single IP
-// as untrusted and the headers are always removed.
+// address (e.g. "10.0.0.1") or a CIDR range (e.g. "10.0.0.0/24").
-func NewMiddleware(trusted []net.IP) func(http.Handler) http.Handler {
+// When the list is empty every request is treated as untrusted and
+// the headers are always removed.  An error is returned if any entry
+// is neither a valid IP nor a valid CIDR.
+func NewMiddleware(trusted []string) (func(http.Handler) http.Handler, error) {
+	ips, nets, err := parseTrusted(trusted)
+	if err != nil {
+		return nil, err
+	}
+
 	return func(next http.Handler) http.Handler {
 		return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
-			if !isTrusted(r.RemoteAddr, trusted) {
+			if !isTrusted(r.RemoteAddr, ips, nets) {
 				for _, h := range forwardedHeaders {
 					r.Header.Del(h)
 				}
 			}
 			next.ServeHTTP(w, r)
 		})
-	}
+	}, nil
 }
 
-func isTrusted(remoteAddr string, trusted []net.IP) bool {
+func parseTrusted(trusted []string) ([]net.IP, []*net.IPNet, error) {
+	ips := make([]net.IP, 0, len(trusted))
+	nets := make([]*net.IPNet, 0, len(trusted))
+	for _, entry := range trusted {
+		if strings.Contains(entry, "/") {
+			_, ipNet, err := net.ParseCIDR(entry)
+			if err != nil {
+				return nil, nil, fmt.Errorf("cannot parse CIDR %q: %w", entry, err)
+			}
+			nets = append(nets, ipNet)
+			continue
+		}
+
+		ip := net.ParseIP(entry)
+		if ip == nil {
+			return nil, nil, fmt.Errorf("cannot parse IP address %q", entry)
+		}
+		ips = append(ips, ip)
+	}
+	return ips, nets, nil
+}
+
+func isTrusted(remoteAddr string, ips []net.IP, nets []*net.IPNet) bool {
 	host, _, err := net.SplitHostPort(remoteAddr)
 	if err != nil {
 		host = remoteAddr
@@ -52,11 +84,17 @@ func isTrusted(remoteAddr string, trusted []net.IP) bool {
 		return false
 	}
 
-	for _, t := range trusted {
+	for _, t := range ips {
 		if t.Equal(ip) {
 			return true
 		}
 	}
 
+	for _, n := range nets {
+		if n.Contains(ip) {
+			return true
+		}
+	}
+
 	return false
 }

pkg/server/trustedproxy/trustedproxy_test.go

@@ -15,7 +15,6 @@
 package trustedproxy_test
 
 import (
-	"net"
 	"net/http"
 	"net/http/httptest"
 	"testing"
@@ -25,113 +24,159 @@ import (
 	"go.probo.inc/probo/pkg/server/trustedproxy"
 )
 
-func newRequest(remoteAddr string, headers map[string]string) *http.Request {
+func runMiddleware(t *testing.T, trusted []string, remoteAddr string, headers map[string]string) *http.Request {
-	r := httptest.NewRequest(http.MethodGet, "/", nil)
+	t.Helper()
-	r.RemoteAddr = remoteAddr
+
+	middleware, err := trustedproxy.NewMiddleware(trusted)
+	require.NoError(t, err)
+
+	var captured *http.Request
+	handler := middleware(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		captured = r
+	}))
+
+	req := httptest.NewRequest(http.MethodGet, "/", nil)
+	req.RemoteAddr = remoteAddr
 	for k, v := range headers {
-		r.Header.Set(k, v)
+		req.Header.Set(k, v)
 	}
-	return r
+	handler.ServeHTTP(httptest.NewRecorder(), req)
+
+	require.NotNil(t, captured)
+	return captured
 }
 
-func TestNewMiddleware(t *testing.T) {
+func TestNewMiddleware_HeaderHandling(t *testing.T) {
 	t.Parallel()
 
-	t.Run(
+	tests := []struct {
-		"strips forwarded headers from untrusted proxy",
+		name             string
-		func(t *testing.T) {
+		trusted          []string
-			t.Parallel()
+		remoteAddr       string
-
+		expectPreserved  bool
-			trusted := []net.IP{net.ParseIP("10.0.0.1")}
+		forwardedHeaders map[string]string
-			middleware := trustedproxy.NewMiddleware(trusted)
+	}{
-
+		{
-			var captured *http.Request
+			name:            "untrusted proxy strips forwarded headers",
-			handler := middleware(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+			trusted:         []string{"10.0.0.1"},
-				captured = r
+			remoteAddr:      "192.168.1.1:1234",
-			}))
+			expectPreserved: false,
-
-			r := newRequest("192.168.1.1:1234", map[string]string{
-				"X-Forwarded-For": "203.0.113.50",
-				"Forwarded":       "for=198.51.100.17",
-			})
-			handler.ServeHTTP(httptest.NewRecorder(), r)
-
-			require.NotNil(t, captured)
-			assert.Empty(t, captured.Header.Get("X-Forwarded-For"))
-			assert.Empty(t, captured.Header.Get("Forwarded"))
 		},
+		{
+			name:            "trusted proxy preserves forwarded headers",
+			trusted:         []string{"10.0.0.1"},
+			remoteAddr:      "10.0.0.1:1234",
+			expectPreserved: true,
+		},
+		{
+			name:            "empty trusted list strips all forwarded headers",
+			trusted:         nil,
+			remoteAddr:      "10.0.0.1:1234",
+			expectPreserved: false,
+		},
+		{
+			name:            "multiple trusted IPs",
+			trusted:         []string{"10.0.0.1", "10.0.0.2"},
+			remoteAddr:      "10.0.0.2:5678",
+			expectPreserved: true,
+		},
+		{
+			name:            "CIDR range trusts addresses within the range",
+			trusted:         []string{"10.0.0.0/24"},
+			remoteAddr:      "10.0.0.50:1234",
+			expectPreserved: true,
+		},
+		{
+			name:            "CIDR range strips addresses outside the range",
+			trusted:         []string{"10.0.0.0/24"},
+			remoteAddr:      "10.0.1.50:1234",
+			expectPreserved: false,
+		},
+		{
+			name:            "mixed IP and CIDR list trusts plain IP",
+			trusted:         []string{"192.168.1.1", "10.0.0.0/24"},
+			remoteAddr:      "192.168.1.1:1234",
+			expectPreserved: true,
+		},
+		{
+			name:            "mixed IP and CIDR list trusts address in CIDR",
+			trusted:         []string{"192.168.1.1", "10.0.0.0/24"},
+			remoteAddr:      "10.0.0.99:1234",
+			expectPreserved: true,
+		},
+		{
+			name:            "IPv6 CIDR range trusts addresses within the range",
+			trusted:         []string{"fd00::/8"},
+			remoteAddr:      "[fd12:3456::1]:1234",
+			expectPreserved: true,
+		},
+		{
+			name:            "IPv6 CIDR range strips addresses outside the range",
+			trusted:         []string{"fd00::/8"},
+			remoteAddr:      "[2001:db8::1]:1234",
+			expectPreserved: false,
+		},
+	}
+
+	const (
+		xff = "203.0.113.50"
+		fwd = "for=198.51.100.17"
 	)
 
-	t.Run(
+	for _, tc := range tests {
-		"preserves forwarded headers from trusted proxy",
+		t.Run(
-		func(t *testing.T) {
+			tc.name,
-			t.Parallel()
+			func(t *testing.T) {
+				t.Parallel()
 
-			trusted := []net.IP{net.ParseIP("10.0.0.1")}
+				captured := runMiddleware(
-			middleware := trustedproxy.NewMiddleware(trusted)
+					t,
+					tc.trusted,
+					tc.remoteAddr,
+					map[string]string{
+						"X-Forwarded-For": xff,
+						"Forwarded":       fwd,
+					},
+				)
 
-			var captured *http.Request
+				if tc.expectPreserved {
-			handler := middleware(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+					assert.Equal(t, xff, captured.Header.Get("X-Forwarded-For"))
-				captured = r
+					assert.Equal(t, fwd, captured.Header.Get("Forwarded"))
-			}))
+				} else {
-
+					assert.Empty(t, captured.Header.Get("X-Forwarded-For"))
-			r := newRequest("10.0.0.1:1234", map[string]string{
+					assert.Empty(t, captured.Header.Get("Forwarded"))
-				"X-Forwarded-For": "203.0.113.50",
+				}
-				"Forwarded":       "for=198.51.100.17",
+			},
-			})
+		)
-			handler.ServeHTTP(httptest.NewRecorder(), r)
+	}
-
+}
-			require.NotNil(t, captured)
+
-			assert.Equal(t, "203.0.113.50", captured.Header.Get("X-Forwarded-For"))
+func TestNewMiddleware_InvalidInput(t *testing.T) {
-			assert.Equal(t, "for=198.51.100.17", captured.Header.Get("Forwarded"))
+	t.Parallel()
-		},
+
-	)
+	tests := []struct {
-
+		name    string
-	t.Run(
+		trusted []string
-		"empty trusted list strips all forwarded headers",
+	}{
-		func(t *testing.T) {
+		{
-			t.Parallel()
+			name:    "invalid IP",
-
+			trusted: []string{"not-an-ip"},
-			middleware := trustedproxy.NewMiddleware(nil)
+		},
-
+		{
-			var captured *http.Request
+			name:    "invalid CIDR mask",
-			handler := middleware(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+			trusted: []string{"10.0.0.0/99"},
-				captured = r
+		},
-			}))
+	}
-
+
-			r := newRequest("10.0.0.1:1234", map[string]string{
+	for _, tc := range tests {
-				"X-Forwarded-For": "203.0.113.50",
+		t.Run(
-			})
+			tc.name,
-			handler.ServeHTTP(httptest.NewRecorder(), r)
+			func(t *testing.T) {
-
+				t.Parallel()
-			require.NotNil(t, captured)
+
-			assert.Empty(t, captured.Header.Get("X-Forwarded-For"))
+				_, err := trustedproxy.NewMiddleware(tc.trusted)
-		},
+				require.Error(t, err)
-	)
+			},
-
+		)
-	t.Run(
+	}
-		"multiple trusted proxies",
-		func(t *testing.T) {
-			t.Parallel()
-
-			trusted := []net.IP{
-				net.ParseIP("10.0.0.1"),
-				net.ParseIP("10.0.0.2"),
-			}
-			middleware := trustedproxy.NewMiddleware(trusted)
-
-			var captured *http.Request
-			handler := middleware(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
-				captured = r
-			}))
-
-			r := newRequest("10.0.0.2:5678", map[string]string{
-				"X-Forwarded-For": "203.0.113.50",
-			})
-			handler.ServeHTTP(httptest.NewRecorder(), r)
-
-			require.NotNil(t, captured)
-			assert.Equal(t, "203.0.113.50", captured.Header.Get("X-Forwarded-For"))
-		},
-	)
 }