From e6b28493814db8171e03d4d41985f6a2a1078157 Mon Sep 17 00:00:00 2001 From: Chris Lu Date: Thu, 9 Jul 2026 02:34:13 -0700 Subject: [PATCH] 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 --- weed/s3api/auth_proxy_integration_test.go | 44 ++++++-- weed/s3api/auth_signature_v4.go | 116 +++++++++++++++------- weed/s3api/auth_signature_v4_test.go | 106 ++++++++++++++++++++ 3 files changed, 223 insertions(+), 43 deletions(-) diff --git a/weed/s3api/auth_proxy_integration_test.go b/weed/s3api/auth_proxy_integration_test.go index 29d16f6bd..e2474d7ca 100644 --- a/weed/s3api/auth_proxy_integration_test.go +++ b/weed/s3api/auth_proxy_integration_test.go @@ -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) } } diff --git a/weed/s3api/auth_signature_v4.go b/weed/s3api/auth_signature_v4.go index 949e8be47..bd3d2b34e 100644 --- a/weed/s3api/auth_signature_v4.go +++ b/weed/s3api/auth_signature_v4.go @@ -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, "[]") } diff --git a/weed/s3api/auth_signature_v4_test.go b/weed/s3api/auth_signature_v4_test.go index 0fc65f3ee..4be2f97aa 100644 --- a/weed/s3api/auth_signature_v4_test.go +++ b/weed/s3api/auth_signature_v4_test.go @@ -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