Skip to content

Commit 37e5bde

Browse files
jhrozekclaude
andauthored
Strip proxy headers from SigV4 signing clone (#4670)
When requests arrive through a gateway (e.g. ngrok), `X-Forwarded-*` headers get signed by SigV4. Then `httputil.ReverseProxy.SetXForwarded()` rewrites those values, causing AWS to reject with 401 due to signature mismatch. Strip `X-Forwarded-For`/`Host`/`Proto`, `X-Real-Ip`, and `Forwarded` (RFC 7239) from the signing clone before computing the signature. This regressed in 0791876 ("Bump Go to 1.26.0", #4040) which refactored the transparent proxy from `httputil.NewSingleHostReverseProxy` (using `Director`) to `&httputil.ReverseProxy{Rewrite: ...}`. The new `Rewrite` callback calls `pr.SetXForwarded()`, which injects `X-Forwarded-*` headers on the outbound request — something the old `Director`-based approach did not do. Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent b542727 commit 37e5bde

2 files changed

Lines changed: 91 additions & 0 deletions

File tree

pkg/auth/awssts/middleware.go

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -260,6 +260,17 @@ func signRequestForTarget(r *http.Request, signer *requestSigner, creds *aws.Cre
260260
signingReq.URL.Scheme = targetURL.Scheme
261261
signingReq.URL.Host = targetURL.Host
262262
signingReq.Host = targetURL.Host
263+
264+
// Strip headers that upstream gateways inject and that
265+
// httputil.ReverseProxy.SetXForwarded() rewrites after signing.
266+
// Including them in the SigV4 canonical headers produces a
267+
// signature mismatch because the values change in flight.
268+
signingReq.Header.Del("X-Forwarded-For")
269+
signingReq.Header.Del("X-Forwarded-Host")
270+
signingReq.Header.Del("X-Forwarded-Proto")
271+
signingReq.Header.Del("X-Real-Ip")
272+
signingReq.Header.Del("Forwarded") // RFC 7239
273+
263274
if bodyBytes != nil {
264275
signingReq.Body = io.NopCloser(bytes.NewReader(bodyBytes))
265276
signingReq.ContentLength = int64(len(bodyBytes))

pkg/auth/awssts/middleware_test.go

Lines changed: 80 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -303,6 +303,86 @@ func TestMiddlewareFunc_EndToEnd(t *testing.T) {
303303
}
304304
}
305305

306+
// TestMiddlewareFunc_ProxyHeadersExcludedFromSignature verifies that volatile
307+
// proxy-injected headers are stripped from the signing clone so they never
308+
// appear in the SigV4 SignedHeaders field. These headers are rewritten by
309+
// httputil.ReverseProxy.SetXForwarded() after signing, which would
310+
// invalidate the signature if they were included.
311+
func TestMiddlewareFunc_ProxyHeadersExcludedFromSignature(t *testing.T) {
312+
t.Parallel()
313+
314+
expiration := time.Now().Add(time.Hour)
315+
successResponse := &sts.AssumeRoleWithWebIdentityOutput{
316+
Credentials: &ststypes.Credentials{
317+
AccessKeyId: aws.String("AKIATEST"), SecretAccessKey: aws.String("secret"),
318+
SessionToken: aws.String("session"), Expiration: &expiration,
319+
},
320+
}
321+
322+
targetURL, err := url.Parse("https://aws-mcp.us-east-1.api.aws")
323+
require.NoError(t, err)
324+
325+
exchanger := &Exchanger{client: &mockSTSClient{response: successResponse}}
326+
roleMapper, err := NewRoleMapper(&Config{
327+
Region: "us-east-1",
328+
FallbackRoleArn: "arn:aws:iam::123456789012:role/TestRole",
329+
})
330+
require.NoError(t, err)
331+
signer, err := newRequestSigner("us-east-1")
332+
require.NoError(t, err)
333+
334+
middlewareFunc := createAWSStsMiddlewareFunc(exchanger, roleMapper, signer, "sub", 3600, targetURL)
335+
336+
var capturedAuth string
337+
testHandler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
338+
capturedAuth = r.Header.Get("Authorization")
339+
w.WriteHeader(http.StatusOK)
340+
})
341+
342+
req := httptest.NewRequest(http.MethodPost, "http://localhost:8080/mcp/v1", strings.NewReader(`{}`))
343+
req.Header.Set("Authorization", "Bearer test-jwt-token")
344+
req.Header.Set("X-Forwarded-For", "1.2.3.4")
345+
req.Header.Set("X-Forwarded-Host", "proxy.example.com")
346+
req.Header.Set("X-Forwarded-Proto", "https")
347+
req.Header.Set("X-Real-Ip", "10.0.0.1")
348+
req.Header.Set("Forwarded", "for=1.2.3.4")
349+
350+
identity := &auth.Identity{PrincipalInfo: auth.PrincipalInfo{
351+
Subject: "user123",
352+
Claims: map[string]interface{}{"sub": "user123"},
353+
}}
354+
req = req.WithContext(auth.WithIdentity(req.Context(), identity))
355+
356+
rec := httptest.NewRecorder()
357+
middlewareFunc(testHandler).ServeHTTP(rec, req)
358+
359+
require.Equal(t, http.StatusOK, rec.Code)
360+
require.Contains(t, capturedAuth, "SignedHeaders=")
361+
362+
// Extract the SignedHeaders value from the Authorization header.
363+
// Format: AWS4-HMAC-SHA256 Credential=..., SignedHeaders=h1;h2;h3, Signature=...
364+
signedHeadersStart := strings.Index(capturedAuth, "SignedHeaders=")
365+
require.NotEqual(t, -1, signedHeadersStart)
366+
signedHeadersSub := capturedAuth[signedHeadersStart+len("SignedHeaders="):]
367+
signedHeadersEnd := strings.Index(signedHeadersSub, ",")
368+
require.NotEqual(t, -1, signedHeadersEnd)
369+
signedHeaders := signedHeadersSub[:signedHeadersEnd]
370+
371+
excludedHeaders := []string{
372+
"x-forwarded-for",
373+
"x-forwarded-host",
374+
"x-forwarded-proto",
375+
"x-real-ip",
376+
"forwarded",
377+
}
378+
for _, h := range excludedHeaders {
379+
for _, signed := range strings.Split(signedHeaders, ";") {
380+
assert.NotEqual(t, h, signed,
381+
"proxy header %q must not appear in SignedHeaders", h)
382+
}
383+
}
384+
}
385+
306386
// TestMiddlewareFunc_RoleMapperFailure tests that the middleware returns 403
307387
// when the role mapper cannot determine an IAM role for the request.
308388
func TestMiddlewareFunc_RoleMapperFailure(t *testing.T) {

0 commit comments

Comments
 (0)