authorize: compute "real" IP from X-Forwarded-For

WIP -- this doesn't work correctly yet

Update the authorize log IP address field to match the access log IP
address field.

Add a new method getIPAddress() to compute the "real" client IP address
based on the contents of the X-Forwarded-For header and the
XffNumTrustedHops and SkipXffAppend configuration option.

This is intended to match downstream_remote_address from the access log:
https://www.envoyproxy.io/docs/envoy/latest/api-v3/data/accesslog/v3/accesslog.proto#envoy-v3-api-field-data-accesslog-v3-accesslogcommon-downstream-remote-address

authorize/log.go

@@ -9,6 +9,7 @@ import (
 	"github.com/rs/zerolog"
 
 	"github.com/pomerium/pomerium/authorize/evaluator"
+	"github.com/pomerium/pomerium/config"
 	"github.com/pomerium/pomerium/internal/log"
 	"github.com/pomerium/pomerium/internal/telemetry/requestid"
 	"github.com/pomerium/pomerium/internal/telemetry/trace"
@@ -32,9 +33,10 @@ func (a *Authorize) logAuthorizeCheck(
 	impersonateDetails := a.getImpersonateDetails(ctx, s)
 
 	evt := log.Info(ctx).Str("service", "authorize")
-	fields := a.currentOptions.Load().GetAuthorizeLogFields()
+	currentOptions := a.currentOptions.Load()
+	fields := currentOptions.GetAuthorizeLogFields()
 	for _, field := range fields {
-		evt = populateLogEvent(ctx, field, evt, in, s, u, hdrs, impersonateDetails)
+		evt = populateLogEvent(ctx, field, evt, in, s, u, hdrs, impersonateDetails, currentOptions)
 	}
 	evt = log.HTTPHeaders(evt, fields, hdrs)
 
@@ -152,6 +154,7 @@ func populateLogEvent(
 	u *user.User,
 	hdrs map[string]string,
 	impersonateDetails *impersonateDetails,
+	currentOptions *config.Options,
 ) *zerolog.Event {
 	path, query, _ := strings.Cut(in.GetAttributes().GetRequest().GetHttp().GetPath(), "?")
 
@@ -192,7 +195,7 @@ func populateLogEvent(
 		}
 		return evt
 	case log.AuthorizeLogFieldIP:
-		return evt.Str(string(field), in.GetAttributes().GetSource().GetAddress().GetSocketAddress().GetAddress())
+		return evt.Str(string(field), getIPAddress(in.GetAttributes(), currentOptions))
 	case log.AuthorizeLogFieldMethod:
 		return evt.Str(string(field), in.GetAttributes().GetRequest().GetHttp().GetMethod())
 	case log.AuthorizeLogFieldPath:
@@ -217,3 +220,34 @@ func populateLogEvent(
 		return evt
 	}
 }
+
+func getIPAddress(attributes *envoy_service_auth_v3.AttributeContext, cfg *config.Options) string {
+	socketAddress := attributes.GetSource().GetAddress().GetSocketAddress().GetAddress()
+	xForwardedFor := attributes.GetRequest().GetHttp().GetHeaders()["x-forwarded-for"]
+
+	// If XffNumTrustedHops is zero, we should not trust anything in the
+	// X-Forwarded-For header, so return the actual socket address.
+	// If the X-Forwarded-For header is empty, we should not trust it either.
+	if cfg.XffNumTrustedHops == 0 || xForwardedFor == "" {
+		return socketAddress
+	}
+
+	// Parse the X-Forwarded-For header by splitting on the ',' character.
+	addresses := strings.Split(xForwardedFor, ",")
+
+	// Unless SkipXffAppend is set, the last address in X-Forwarded-For was
+	// appended by Envoy, so it shouldn't count as a separate hop.
+	hops := cfg.XffNumTrustedHops
+	if !cfg.SkipXffAppend {
+		hops++
+	}
+
+	// Fall back to the actual socket address if the X-Forwarded-For header has
+	// fewer entries than expected (or if XffNumTrustedHops was set to exactly
+	// the maximum uint32 value and SkipXffAppend is false).
+	lenAddresses := uint32(len(addresses))
+	if hops > lenAddresses || hops == 0 {
+		return socketAddress
+	}
+	return strings.TrimSpace(addresses[lenAddresses-hops])
+}

authorize/log_test.go

@@ -11,6 +11,7 @@ import (
 	"github.com/rs/zerolog"
 	"github.com/stretchr/testify/assert"
 
+	"github.com/pomerium/pomerium/config"
 	"github.com/pomerium/pomerium/internal/log"
 	"github.com/pomerium/pomerium/internal/telemetry/requestid"
 	"github.com/pomerium/pomerium/pkg/grpc/session"
@@ -62,6 +63,7 @@ func Test_populateLogEvent(t *testing.T) {
 		sessionID: "IMPERSONATE-SESSION-ID",
 		userID:    "IMPERSONATE-USER-ID",
 	}
+	opts := &config.Options{}
 
 	for _, tc := range []struct {
 		field  log.AuthorizeLogField
@@ -93,10 +95,85 @@ func Test_populateLogEvent(t *testing.T) {
 			var buf bytes.Buffer
 			log := zerolog.New(&buf)
 			evt := log.Log()
-			evt = populateLogEvent(ctx, tc.field, evt, checkRequest, tc.s, u, headers, impersonateDetails)
+			evt = populateLogEvent(ctx, tc.field, evt, checkRequest, tc.s, u, headers, impersonateDetails, opts)
 			evt.Send()
 
 			assert.Equal(t, tc.expect, strings.TrimSpace(buf.String()))
 		})
 	}
 }
+
+func Test_getIPAddress(t *testing.T) {
+	t.Parallel()
+
+	cases := []struct {
+		label             string
+		socketIP          string
+		xForwardedFor     string
+		xffNumTrustedHops uint32
+		skipXffAppend     bool
+		expected          string
+	}{
+		{"xff_num_trusted_hops=0",
+			"127.0.0.1", "10.2.2.2, 10.1.1.1, 10.0.0.0", 0, false, "127.0.0.1"},
+		{"xff_num_trusted_hops=1",
+			"127.0.0.1", "10.2.2.2, 10.1.1.1, 10.0.0.0", 1, false, "10.1.1.1"},
+		{"xff_num_trusted_hops=2",
+			"127.0.0.1", "10.2.2.2, 10.1.1.1, 10.0.0.0", 2, false, "10.2.2.2"},
+		{"xff_num_trusted_hops=3",
+			"127.0.0.1", "10.2.2.2, 10.1.1.1, 10.0.0.0", 3, false, "127.0.0.1"},
+		{"xff_num_trusted_hops=4",
+			"127.0.0.1", "10.2.2.2, 10.1.1.1, 10.0.0.0", 4, false, "127.0.0.1"},
+
+		{"xff_num_trusted_hops=0 skip_xff_append",
+			"127.0.0.1", "10.2.2.2, 10.1.1.1", 0, true, "127.0.0.1"},
+		{"xff_num_trusted_hops=1 skip_xff_append",
+			"127.0.0.1", "10.2.2.2, 10.1.1.1", 1, true, "10.1.1.1"},
+		{"xff_num_trusted_hops=2 skip_xff_append",
+			"127.0.0.1", "10.2.2.2, 10.1.1.1", 2, true, "10.2.2.2"},
+		{"xff_num_trusted_hops=3 skip_xff_append",
+			"127.0.0.1", "10.2.2.2, 10.1.1.1", 3, true, "127.0.0.1"},
+		{"xff_num_trusted_hops=4 skip_xff_append",
+			"127.0.0.1", "10.2.2.2, 10.1.1.1", 4, true, "127.0.0.1"},
+
+		{"xff_num_trusted_hops int32 overflow",
+			"127.0.0.1", "10.2.2.2, 10.1.1.1, 10.0.0.0", 0x80000000, false, "127.0.0.1"},
+		{"xff_num_trusted_hops uint32 max",
+			"127.0.0.1", "10.2.2.2, 10.1.1.1, 10.0.0.0", 0xffffffff, false, "127.0.0.1"},
+
+		{"empty X-Forwarded-For",
+			"127.0.0.1", "", 1, false, "127.0.0.1"},
+		{"empty X-Forwarded-For skip_xff_append",
+			"127.0.0.1", "", 1, true, "127.0.0.1"},
+	}
+
+	for i := range cases {
+		c := &cases[i]
+		t.Run(c.label, func(t *testing.T) {
+			attributes := &envoy_service_auth_v3.AttributeContext{
+				Request: &envoy_service_auth_v3.AttributeContext_Request{
+					Http: &envoy_service_auth_v3.AttributeContext_HttpRequest{
+						Headers: map[string]string{
+							"x-forwarded-for": c.xForwardedFor,
+						},
+					},
+				},
+				Source: &envoy_service_auth_v3.AttributeContext_Peer{
+					Address: &envoy_config_core_v3.Address{
+						Address: &envoy_config_core_v3.Address_SocketAddress{
+							SocketAddress: &envoy_config_core_v3.SocketAddress{
+								Address: c.socketIP,
+							},
+						},
+					},
+				},
+			}
+			options := &config.Options{
+				XffNumTrustedHops: c.xffNumTrustedHops,
+				SkipXffAppend:     c.skipXffAppend,
+			}
+			actual := getIPAddress(attributes, options)
+			assert.Equal(t, c.expected, actual)
+		})
+	}
+}