s3: verify SigV4 against each plausible reverse-proxy host (#10284)

* s3: verify SigV4 against each plausible reverse-proxy host A portless X-Forwarded-Host leaves the client's true port ambiguous: a proxy that kept the Host header makes the backend Host port right, one that rewrote it makes X-Forwarded-Port right, and a client on the scheme's default port signed no port at all. The verifier bet on the backend Host port whenever the hostnames matched, so nginx-style $host/$server_port forwarding got SignatureDoesNotMatch whenever the proxy and backend share a hostname. Try each plausible host value in likelihood order instead of guessing one. * s3: unbracket IPv6 X-Forwarded-Host before matching the request host net.SplitHostPort strips brackets from the request host, so a bracketed portless X-Forwarded-Host like [::1] never matched and lost its port candidate. * s3: cover unbracketed IPv6 forwarded-host candidates; compare with slices.Equal
2026-09-19 06:31:54 +00:00 · 2026-07-09 02:34:13 -07:00
parent 95023af489
commit e6b2849381
3 changed files with 223 additions and 43 deletions
@@ -4,6 +4,7 @@ import (
 	"context"
 	"crypto/sha256"
 	"fmt"
+	"net"
 	"net/http"
 	"net/http/httptest"
 	"net/http/httputil"
@@ -46,12 +47,13 @@ func TestReverseProxySignatureVerification(t *testing.T) {
 }`

 	tests := []struct {
-		name              string
-		externalUrl       string // s3.externalUrl config for the backend
-		clientScheme      string // scheme the client uses for signing
-		clientHost        string // host the client signs against
-		proxyForwardsHost bool   // whether proxy sets X-Forwarded-Host
-		expectSuccess     bool
+		name                  string
+		externalUrl           string // s3.externalUrl config for the backend
+		clientScheme          string // scheme the client uses for signing
+		clientHost            string // host the client signs against
+		proxyForwardsHost     bool   // whether proxy sets X-Forwarded-Host
+		portlessForwardedHost bool   // nginx-style $host / $server_port: hostname in X-Forwarded-Host, port in X-Forwarded-Port
+		expectSuccess         bool
 	}{
 		{
 			name:              "non-standard port, externalUrl matches proxy address",
@@ -101,6 +103,26 @@ func TestReverseProxySignatureVerification(t *testing.T) {
 			proxyForwardsHost: false,
 			expectSuccess:     false,
 		},
+		{
+			// The backend Host (127.0.0.1:ephemeral) shares the forwarded hostname, so the
+			// backend port must not shadow X-Forwarded-Port.
+			name:                  "nginx $host/$server_port forwarding, backend on same hostname",
+			externalUrl:           "",
+			clientScheme:          "http",
+			clientHost:            "127.0.0.1:9000",
+			proxyForwardsHost:     true,
+			portlessForwardedHost: true,
+			expectSuccess:         true,
+		},
+		{
+			name:                  "portless X-Forwarded-Host on default port, backend on same hostname",
+			externalUrl:           "",
+			clientScheme:          "http",
+			clientHost:            "127.0.0.1",
+			proxyForwardsHost:     true,
+			portlessForwardedHost: true,
+			expectSuccess:         true,
+		},
 		{
 			name:              "proxy without X-Forwarded-Host, externalUrl saves the day",
 			externalUrl:       "http://api.example.com:9000",
@@ -136,6 +158,7 @@ func TestReverseProxySignatureVerification(t *testing.T) {
 			proxy := httputil.NewSingleHostReverseProxy(backendURL)

 			forwardsHost := tt.proxyForwardsHost
+			portlessForwardedHost := tt.portlessForwardedHost
 			originalDirector := proxy.Director
 			proxy.Director = func(req *http.Request) {
 				originalHost := req.Host
@@ -148,7 +171,14 @@ func TestReverseProxySignatureVerification(t *testing.T) {
 				// (nginx proxy_pass and Kong both do this by default)
 				req.Host = backendURL.Host
 				if forwardsHost {
-					req.Header.Set("X-Forwarded-Host", originalHost)
+					forwardedHost := originalHost
+					if portlessForwardedHost {
+						if h, p, splitErr := net.SplitHostPort(originalHost); splitErr == nil {
+							forwardedHost = h
+							req.Header.Set("X-Forwarded-Port", p)
+						}
+					}
+					req.Header.Set("X-Forwarded-Host", forwardedHost)
 					req.Header.Set("X-Forwarded-Proto", originalScheme)
 				}
 			}
@@ -29,6 +29,7 @@ import (
 	"net/http"
 	"net/url"
 	"regexp"
+	"slices"
 	"sort"
 	"strconv"
 	"strings"
@@ -314,36 +315,57 @@ func (iam *IdentityAccessManagement) verifyV4Signature(r *http.Request, shouldCh
 		)
 	}

-	// 8. Verify the signature, trying with X-Forwarded-Prefix first
+	// 8. Verify the signature for each plausible host value: when X-Forwarded-Host carries
+	// no port, the client may have signed the Host header the proxy kept or the forwarded
+	// host and port, and the headers alone cannot tell which.
 	pathForSignature := r.URL.EscapedPath()
 	if pathForSignature == "" {
 		pathForSignature = r.URL.Path
 	}
-	if forwardedPrefix := r.Header.Get("X-Forwarded-Prefix"); forwardedPrefix != "" {
-		cleanedPath := buildPathWithForwardedPrefix(forwardedPrefix, pathForSignature)
-		calculatedSignature, errCode = verify(cleanedPath)
+	forwardedPrefix := r.Header.Get("X-Forwarded-Prefix")
+	for i, hostCandidate := range extractHostHeaderCandidates(r, iam.externalHost) {
+		if i > 0 && !replaceSignedHostHeader(extractedSignedHeaders, hostCandidate) {
+			break
+		}
+
+		// 9. Verify with the X-Forwarded-Prefix path first
+		if forwardedPrefix != "" {
+			cleanedPath := buildPathWithForwardedPrefix(forwardedPrefix, pathForSignature)
+			calculatedSignature, errCode = verify(cleanedPath)
+			if errCode == s3err.ErrNone {
+				return identity, cred, calculatedSignature, authInfo, s3err.ErrNone
+			}
+		}
+
+		// 10. Verify with the original path
+		calculatedSignature, errCode = verify(pathForSignature)
 		if errCode == s3err.ErrNone {
 			return identity, cred, calculatedSignature, authInfo, s3err.ErrNone
 		}
-	}

-	// 9. Verify with the original path
-	calculatedSignature, errCode = verify(pathForSignature)
-	if errCode == s3err.ErrNone {
-		return identity, cred, calculatedSignature, authInfo, s3err.ErrNone
-	}
-
-	// 10. Retry with decoded path if signature used raw path encoding
-	if decodedPath, decodeErr := url.PathUnescape(pathForSignature); decodeErr == nil && decodedPath != pathForSignature {
-		calculatedSignature, errCode = verify(decodedPath)
-		if errCode == s3err.ErrNone {
-			return identity, cred, calculatedSignature, authInfo, s3err.ErrNone
+		// 11. Retry with decoded path if signature used raw path encoding
+		if decodedPath, decodeErr := url.PathUnescape(pathForSignature); decodeErr == nil && decodedPath != pathForSignature {
+			calculatedSignature, errCode = verify(decodedPath)
+			if errCode == s3err.ErrNone {
+				return identity, cred, calculatedSignature, authInfo, s3err.ErrNone
+			}
 		}
 	}

 	return nil, nil, "", nil, errCode
 }

+func replaceSignedHostHeader(headers http.Header, host string) bool {
+	replaced := false
+	for name := range headers {
+		if strings.EqualFold(name, "host") {
+			headers[name] = []string{host}
+			replaced = true
+		}
+	}
+	return replaced
+}
+
 // validateSTSSessionToken validates an STS session token and extracts temporary credentials
 func (iam *IdentityAccessManagement) validateSTSSessionToken(r *http.Request, sessionToken string, accessKey string) (*Identity, *Credential, s3err.ErrorCode) {
 	// Check if IAM integration is available
@@ -860,13 +882,20 @@ func extractSignedHeaders(signedHeaders []string, r *http.Request, externalHost
 	return extractedSignedHeaders, s3err.ErrNone
 }

-// extractHostHeader returns the value of host header to use for signature verification.
-// When externalHost is set (from s3.externalUrl), it is returned directly.
-// Otherwise, the host is reconstructed from X-Forwarded-* headers or the request Host,
-// with default port stripping to match AWS SDK SanitizeHostForHeader behavior.
+// extractHostHeader returns the most likely host header value for signature verification.
 func extractHostHeader(r *http.Request, externalHost string) string {
+	return extractHostHeaderCandidates(r, externalHost)[0]
+}
+
+// extractHostHeaderCandidates returns the host values the client may have signed, most
+// likely first. When externalHost is set (from s3.externalUrl), it is the only candidate.
+// Otherwise, the host is reconstructed from X-Forwarded-* headers or the request Host.
+// When X-Forwarded-Host carries no port, the true client port is ambiguous: a proxy that
+// kept the Host header makes the r.Host port right, one that rewrote it makes
+// X-Forwarded-Port right, and a client on the scheme's default port signed no port at all.
+func extractHostHeaderCandidates(r *http.Request, externalHost string) []string {
 	if externalHost != "" {
-		return externalHost
+		return []string{externalHost}
 	}

 	forwardedHost := r.Header.Get("X-Forwarded-Host")
@@ -895,7 +924,8 @@ func extractHostHeader(r *http.Request, externalHost string) string {
 		scheme = forwardedProto
 	}

-	var host, port string
+	var host string
+	var ports []string
 	if forwardedHost != "" {
 		// X-Forwarded-Host can be a comma-separated list of hosts when there are multiple proxies.
 		// Use only the first host in the list and trim spaces for robustness.
@@ -908,14 +938,16 @@ func extractHostHeader(r *http.Request, externalHost string) string {
 		// If the host itself contains a port, it should take precedence
 		if h, p, err := net.SplitHostPort(host); err == nil {
 			host = h
-			port = p
+			ports = []string{p}
 		} else {
-			// If X-Forwarded-Host has no port, try to get port from r.Host if hostnames match
-			if rh, rp, err := net.SplitHostPort(r.Host); err == nil && rh == host {
-				port = rp
-			} else if forwardedPort != "" {
-				port = forwardedPort
+			// SplitHostPort unbrackets IPv6 hosts, so unbracket the forwarded host to match
+			if rh, rp, err := net.SplitHostPort(r.Host); err == nil && rh == strings.Trim(host, "[]") {
+				ports = append(ports, rp)
 			}
+			if forwardedPort != "" {
+				ports = append(ports, forwardedPort)
+			}
+			ports = append(ports, "")
 		}
 	} else {
 		host = r.Host
@@ -927,14 +959,29 @@ func extractHostHeader(r *http.Request, externalHost string) string {
 		// Otherwise, if X-Forwarded-Port is set, use it.
 		if h, p, err := net.SplitHostPort(host); err == nil {
 			host = h
-			port = p
-		} else if forwardedPort != "" {
-			port = forwardedPort
+			ports = []string{p}
+		} else {
+			if forwardedPort != "" {
+				ports = append(ports, forwardedPort)
+			}
+			ports = append(ports, "")
 		}
 	}

-	// Strip default ports based on scheme to match AWS SDK SanitizeHostForHeader behavior.
-	// The AWS SDK strips port 80 for HTTP and port 443 for HTTPS before signing.
+	var candidates []string
+	for _, port := range ports {
+		candidate := joinSignedHost(host, port, scheme)
+		if !slices.Contains(candidates, candidate) {
+			candidates = append(candidates, candidate)
+		}
+	}
+	return candidates
+}
+
+// joinSignedHost renders host:port the way AWS SDKs sign it: default ports are stripped
+// to match SanitizeHostForHeader, and bare IPv6 addresses lose their brackets.
+// Reference: https://github.com/aws/aws-sdk-go-v2/blob/main/aws/signer/internal/v4/host.go
+func joinSignedHost(host, port, scheme string) string {
 	if port != "" && !isDefaultPort(scheme, port) {
 		// Strip existing brackets before calling JoinHostPort, which automatically adds
 		// brackets for IPv6 addresses. This prevents double-bracketing like [[::1]]:8080.
@@ -942,9 +989,6 @@ func extractHostHeader(r *http.Request, externalHost string) string {
 		return net.JoinHostPort(host, port)
 	}

-	// Default port was stripped, or no port present.
-	// For IPv6 addresses, strip brackets to match AWS SDK behavior.
-	// Reference: https://github.com/aws/aws-sdk-go-v2/blob/main/aws/signer/internal/v4/host.go
 	if strings.Contains(host, ":") {
 		return strings.Trim(host, "[]")
 	}
@@ -6,6 +6,7 @@ import (
 	"encoding/hex"
 	"fmt"
 	"net/http"
+	"slices"
 	"testing"
 	"time"

@@ -521,6 +522,111 @@ func TestExtractHostHeader(t *testing.T) {
 	}
 }

+// TestExtractHostHeaderCandidates tests the alternate host values tried during verification
+// when the true client-facing host is ambiguous behind a reverse proxy.
+func TestExtractHostHeaderCandidates(t *testing.T) {
+	tests := []struct {
+		name           string
+		hostHeader     string
+		forwardedHost  string
+		forwardedPort  string
+		forwardedProto string
+		externalHost   string
+		expected       []string
+	}{
+		{
+			name:         "externalHost is the only candidate",
+			hostHeader:   "backend:8333",
+			externalHost: "api.example.com:9000",
+			expected:     []string{"api.example.com:9000"},
+		},
+		{
+			name:          "X-Forwarded-Host with port is trusted as-is",
+			hostHeader:    "backend:8333",
+			forwardedHost: "example.com:9000",
+			forwardedPort: "443",
+			expected:      []string{"example.com:9000"},
+		},
+		{
+			name:       "plain Host with port is the only candidate",
+			hostHeader: "example.com:8080",
+			expected:   []string{"example.com:8080"},
+		},
+		{
+			name:           "portless X-Forwarded-Host, hostnames match: r.Host port first, then X-Forwarded-Port, then bare",
+			hostHeader:     "example.com:8333",
+			forwardedHost:  "example.com",
+			forwardedPort:  "9000",
+			forwardedProto: "http",
+			expected:       []string{"example.com:8333", "example.com:9000", "example.com"},
+		},
+		{
+			name:          "portless X-Forwarded-Host, hostnames match, no X-Forwarded-Port: bare host as fallback",
+			hostHeader:    "example.com:8333",
+			forwardedHost: "example.com",
+			expected:      []string{"example.com:8333", "example.com"},
+		},
+		{
+			name:          "portless X-Forwarded-Host, hostnames differ: X-Forwarded-Port, then bare",
+			hostHeader:    "backend:8333",
+			forwardedHost: "example.com",
+			forwardedPort: "9000",
+			expected:      []string{"example.com:9000", "example.com"},
+		},
+		{
+			name:           "default X-Forwarded-Port collapses into the bare candidate",
+			hostHeader:     "example.com:8333",
+			forwardedHost:  "example.com",
+			forwardedPort:  "443",
+			forwardedProto: "https",
+			expected:       []string{"example.com:8333", "example.com"},
+		},
+		{
+			name:          "portless Host with X-Forwarded-Port: forwarded port, then bare",
+			hostHeader:    "example.com",
+			forwardedPort: "9000",
+			expected:      []string{"example.com:9000", "example.com"},
+		},
+		{
+			name:          "bracketed portless IPv6 X-Forwarded-Host matches the request host",
+			hostHeader:    "[::1]:8333",
+			forwardedHost: "[::1]",
+			expected:      []string{"[::1]:8333", "::1"},
+		},
+		{
+			name:          "unbracketed portless IPv6 X-Forwarded-Host with X-Forwarded-Port",
+			hostHeader:    "backend:8333",
+			forwardedHost: "::1",
+			forwardedPort: "8080",
+			expected:      []string{"[::1]:8080", "::1"},
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			req, err := http.NewRequest("GET", "http://"+tt.hostHeader+"/bucket/object", nil)
+			if err != nil {
+				t.Fatalf("Failed to create request: %v", err)
+			}
+			req.Host = tt.hostHeader
+			if tt.forwardedHost != "" {
+				req.Header.Set("X-Forwarded-Host", tt.forwardedHost)
+			}
+			if tt.forwardedPort != "" {
+				req.Header.Set("X-Forwarded-Port", tt.forwardedPort)
+			}
+			if tt.forwardedProto != "" {
+				req.Header.Set("X-Forwarded-Proto", tt.forwardedProto)
+			}
+
+			result := extractHostHeaderCandidates(req, tt.externalHost)
+			if !slices.Equal(result, tt.expected) {
+				t.Errorf("extractHostHeaderCandidates() = %v, want %v", result, tt.expected)
+			}
+		})
+	}
+}
+
 func TestExtractSignedHeadersCase(t *testing.T) {
 	tests := []struct {
 		name        string