Honor X-Forwarded-For only from a trusted-proxy CIDR (#189)

EnrichContext used to copy the first X-Forwarded-For entry into the
request identity unconditionally. The resulting AttrClientIp drives
client-IP comparisons later in the gateway-access flow, and a direct
caller could set XFF to anything they liked.

Add a small package-level allow-list:

* InitTrustedProxies(cidrs) parses operator-supplied CIDRs once at
  startup. A bad CIDR is fatal, an empty list disables XFF entirely.
* EnrichContext takes the client IP from r.RemoteAddr (host portion)
  and only swaps in the first X-Forwarded-For entry when r.RemoteAddr
  itself sits in a trusted-proxy CIDR. AttrProxies is set from the
  remaining XFF entries on the same condition.

Wire Server.TrustedProxies through configuration.go to web.

cmd/rdpgw/config/configuration.go

@@ -64,6 +64,11 @@ type ServerConfig struct {
 	// forward to loopback, RFC1918, link-local, and IPv6 ULA targets.
 	// Default false.
 	AllowPrivateDestinations bool `koanf:"allowprivatedestinations"`
+	// TrustedProxies is the CIDR allow-list of upstream proxies whose
+	// X-Forwarded-For header is honored when deriving the client IP.
+	// Empty (the default) makes the gateway ignore X-Forwarded-For
+	// entirely and use r.RemoteAddr.
+	TrustedProxies []string `koanf:"trustedproxies"`
 }
 
 type KerberosConfig struct {

cmd/rdpgw/main.go

@@ -101,6 +101,8 @@ func main() {
 		conf.Server.MaxSessionLength,
 	)
 
+	web.InitTrustedProxies(conf.Server.TrustedProxies)
+
 	// configure web backend
 	w := &web.Config{
 		QueryInfo:        security.QueryInfo,

cmd/rdpgw/web/context.go

@@ -1,14 +1,55 @@
 package web
 
 import (
-	"github.com/bolkedebruin/rdpgw/cmd/rdpgw/identity"
-	"github.com/jcmturner/goidentity/v6"
 	"log"
 	"net"
 	"net/http"
 	"strings"
+
+	"github.com/bolkedebruin/rdpgw/cmd/rdpgw/identity"
+	"github.com/jcmturner/goidentity/v6"
 )
 
+// trustedProxyNets is the CIDR allow-list of upstream proxies whose
+// X-Forwarded-For header is honored. Empty (the default) means XFF is
+// ignored entirely and the client IP is taken from r.RemoteAddr.
+var trustedProxyNets []*net.IPNet
+
+// InitTrustedProxies parses the operator-supplied CIDRs once at startup.
+// A bad CIDR is fatal; an empty list disables XFF-derived client-IP
+// attribution.
+func InitTrustedProxies(cidrs []string) {
+	nets := make([]*net.IPNet, 0, len(cidrs))
+	for _, raw := range cidrs {
+		_, n, err := net.ParseCIDR(raw)
+		if err != nil {
+			log.Fatalf("trustedproxies: invalid CIDR %q: %s", raw, err)
+		}
+		nets = append(nets, n)
+	}
+	trustedProxyNets = nets
+}
+
+func remoteIsTrustedProxy(remoteAddr string) bool {
+	if len(trustedProxyNets) == 0 {
+		return false
+	}
+	host, _, err := net.SplitHostPort(remoteAddr)
+	if err != nil {
+		host = remoteAddr
+	}
+	ip := net.ParseIP(host)
+	if ip == nil {
+		return false
+	}
+	for _, n := range trustedProxyNets {
+		if n.Contains(ip) {
+			return true
+		}
+	}
+	return false
+}
+
 func EnrichContext(next http.Handler) http.Handler {
 	return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
 		id, err := GetSessionIdentity(r)
@@ -28,26 +69,30 @@ func EnrichContext(next http.Handler) http.Handler {
 		log.Printf("Identity SessionId: %s, UserName: %s: Authenticated: %t",
 			id.SessionId(), id.UserName(), id.Authenticated())
 
-		h := r.Header.Get("X-Forwarded-For")
-		if h != "" {
-			var proxies []string
-			ips := strings.Split(h, ",")
-			for i := range ips {
-				ips[i] = strings.TrimSpace(ips[i])
-			}
-			clientIp := ips[0]
-			if len(ips) > 1 {
-				proxies = ips[1:]
-			}
-			id.SetAttribute(identity.AttrClientIp, clientIp)
-			id.SetAttribute(identity.AttrProxies, proxies)
+		id.SetAttribute(identity.AttrRemoteAddr, r.RemoteAddr)
+
+		remoteHost, _, err := net.SplitHostPort(r.RemoteAddr)
+		if err != nil {
+			remoteHost = r.RemoteAddr
 		}
 
-		id.SetAttribute(identity.AttrRemoteAddr, r.RemoteAddr)
-		if h == "" {
-			clientIp, _, _ := net.SplitHostPort(r.RemoteAddr)
-			id.SetAttribute(identity.AttrClientIp, clientIp)
+		clientIp := remoteHost
+		var proxies []string
+		if remoteIsTrustedProxy(r.RemoteAddr) {
+			if h := r.Header.Get("X-Forwarded-For"); h != "" {
+				ips := strings.Split(h, ",")
+				for i := range ips {
+					ips[i] = strings.TrimSpace(ips[i])
+				}
+				clientIp = ips[0]
+				if len(ips) > 1 {
+					proxies = ips[1:]
+				}
+			}
 		}
+		id.SetAttribute(identity.AttrClientIp, clientIp)
+		id.SetAttribute(identity.AttrProxies, proxies)
+
 		next.ServeHTTP(w, identity.AddToRequestCtx(id, r))
 	})
 }

cmd/rdpgw/web/context_test.go

@@ -0,0 +1,92 @@
+package web
+
+import (
+	"net/http"
+	"net/http/httptest"
+	"testing"
+
+	"github.com/bolkedebruin/rdpgw/cmd/rdpgw/identity"
+)
+
+// TestEnrichContextXForwardedForRequiresTrustedProxy asserts that the
+// X-Forwarded-For header is only honored when the request arrives from a
+// configured trusted proxy. A direct caller can otherwise set XFF to any
+// value, taking control of the AttrClientIp attribute that downstream
+// session-binding logic compares against.
+func TestEnrichContextXForwardedForRequiresTrustedProxy(t *testing.T) {
+	cases := []struct {
+		name           string
+		trustedProxies []string
+		remoteAddr     string
+		xff            string
+		wantClientIp   string
+	}{
+		{
+			name:           "untrusted remote with no XFF uses RemoteAddr",
+			trustedProxies: nil,
+			remoteAddr:     "198.51.100.7:5678",
+			xff:            "",
+			wantClientIp:   "198.51.100.7",
+		},
+		{
+			name:           "untrusted remote with XFF still uses RemoteAddr",
+			trustedProxies: nil,
+			remoteAddr:     "198.51.100.7:5678",
+			xff:            "10.20.30.40",
+			wantClientIp:   "198.51.100.7",
+		},
+		{
+			name:           "untrusted remote outside allow-list with XFF uses RemoteAddr",
+			trustedProxies: []string{"10.0.0.0/8"},
+			remoteAddr:     "198.51.100.7:5678",
+			xff:            "10.20.30.40",
+			wantClientIp:   "198.51.100.7",
+		},
+		{
+			name:           "trusted remote with XFF honors first XFF entry",
+			trustedProxies: []string{"10.0.0.0/8"},
+			remoteAddr:     "10.1.2.3:5678",
+			xff:            "203.0.113.42, 10.99.0.1",
+			wantClientIp:   "203.0.113.42",
+		},
+		{
+			name:           "trusted remote without XFF uses RemoteAddr",
+			trustedProxies: []string{"10.0.0.0/8"},
+			remoteAddr:     "10.1.2.3:5678",
+			xff:            "",
+			wantClientIp:   "10.1.2.3",
+		},
+	}
+
+	for _, tc := range cases {
+		t.Run(tc.name, func(t *testing.T) {
+			InitTrustedProxies(tc.trustedProxies)
+			t.Cleanup(func() { InitTrustedProxies(nil) })
+
+			var captured identity.Identity
+			next := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+				captured = identity.FromRequestCtx(r)
+				w.WriteHeader(http.StatusOK)
+			})
+			h := EnrichContext(next)
+
+			req := httptest.NewRequest("GET", "/", nil)
+			req.RemoteAddr = tc.remoteAddr
+			if tc.xff != "" {
+				req.Header.Set("X-Forwarded-For", tc.xff)
+			}
+
+			rr := httptest.NewRecorder()
+			h.ServeHTTP(rr, req)
+
+			if captured == nil {
+				t.Fatal("middleware did not store an identity in the request context")
+			}
+			got, _ := captured.GetAttribute(identity.AttrClientIp).(string)
+			if got != tc.wantClientIp {
+				t.Errorf("AttrClientIp = %q, want %q (remoteAddr=%q xff=%q trusted=%v)",
+					got, tc.wantClientIp, tc.remoteAddr, tc.xff, tc.trustedProxies)
+			}
+		})
+	}
+}