From 17dc93bf01528cde7d1921a35d995337585c6ce1 Mon Sep 17 00:00:00 2001 From: Oscar Sanderson Date: Fri, 2 Oct 2026 13:52:05 +0800 Subject: [PATCH] fix(resource): refuse a request with more than one Authorization header VerifyRequestFromHTTP read the Authorization header with Header.Get, which silently takes the first of several, and net/http doesn't reject repeats. A proxy or gateway in front that acts on another one (the last, or a merged value) would then disagree with the resource server about which credential the request carried. It already refused more than one DPoP header, and the client's backchannel notification endpoint refuses repeated Authorization headers. VerifyRequestFromHTTP now marks such a request, and Verify refuses it with a 400 invalid_request. The Authorization field's doc tells an adapter that fills the struct itself to refuse repeats first. Co-Authored-By: Claude Opus 5.5 --- resource/verify.go | 17 +++++++++++++++-- resource/verify_request_http.go | 6 +++++- resource/verify_test.go | 28 ++++++++++++++++++++++++++++ 3 files changed, 48 insertions(+), 3 deletions(-) diff --git a/resource/verify.go b/resource/verify.go index 9cacf11f..8bc14821 100644 --- a/resource/verify.go +++ b/resource/verify.go @@ -26,8 +26,14 @@ import ( // sender-constrained one way or the other; a bearer token presented // with neither a DPoP proof nor a client certificate is rejected. type VerifyRequest struct { - Method string - URL *url.URL + Method string + URL *url.URL + + // Authorization is the request's one Authorization header. A request + // with more than one is malformed (RFC 9110 §5.3: the field isn't a + // list): VerifyRequestFromHTTP marks it so for Verify to refuse, and + // an adapter filling this field itself refuses it first, rather than + // passing one of them on. A proxy in front might act on another. Authorization string // DPoPProofs is every "DPoP" header value the request carried, in @@ -47,6 +53,10 @@ type VerifyRequest struct { // one). An HTTP adapter reads this straight from the connection's // own TLS state; this package never terminates TLS itself. PeerCertificate *x509.Certificate + + // repeatedAuthorization is VerifyRequestFromHTTP's mark of a request + // with more than one Authorization header. + repeatedAuthorization bool } // AuthorizationContext is what Verify returns for a successfully @@ -191,6 +201,9 @@ func parseAuthorization(req VerifyRequest) (dpopProof, raw string, usedDPoP bool return "", "", true, newError(ErrorInvalidDPoPProof, 401, "multiple DPoP proofs are not permitted", nil) } + if req.repeatedAuthorization { + return "", "", false, newError(ErrorInvalidRequest, 400, "multiple Authorization headers are not permitted", nil) + } if strings.TrimSpace(req.Authorization) == "" { return "", "", false, noCredentials("no Authorization header") } diff --git a/resource/verify_request_http.go b/resource/verify_request_http.go index 3c11346c..a37116ac 100644 --- a/resource/verify_request_http.go +++ b/resource/verify_request_http.go @@ -8,7 +8,9 @@ import ( // VerifyRequestFromHTTP builds the VerifyRequest for r, an incoming // request to the protected resource whose external URL is target: its // method, Authorization header, every DPoP header (DPoPProofsFromHTTP) -// and TLS client certificate (PeerCertificateFromHTTP). Filling the +// and TLS client certificate (PeerCertificateFromHTTP). A request with +// more than one Authorization header is marked for Verify to refuse as +// invalid_request, as it refuses more than one DPoP header. Filling the // struct field by field compiles just as well with a field left out, and // then fails only for the clients that need it — a missing // PeerCertificate refuses every mTLS-bound token, a missing DPoPProofs @@ -34,5 +36,7 @@ func VerifyRequestFromHTTP(r *http.Request, target *url.URL) VerifyRequest { Authorization: r.Header.Get("Authorization"), DPoPProofs: DPoPProofsFromHTTP(r), PeerCertificate: PeerCertificateFromHTTP(r), + + repeatedAuthorization: len(r.Header.Values("Authorization")) > 1, } } diff --git a/resource/verify_test.go b/resource/verify_test.go index 08e35ea4..4a5d675a 100644 --- a/resource/verify_test.go +++ b/resource/verify_test.go @@ -6,6 +6,9 @@ import ( "crypto/elliptic" "crypto/rand" "crypto/sha256" + "errors" + "net/http" + "net/http/httptest" "net/url" "strings" "testing" @@ -572,3 +575,28 @@ func TestErrorAccessorsWithoutCause(t *testing.T) { t.Fatalf("Error() = %q, want %q", rerr.Error(), "resource: invalid_request: method is required") } } + +// TestVerifyRefusesRepeatedAuthorization covers a request with two +// Authorization headers, through VerifyRequestFromHTTP: refused as +// invalid_request rather than verified on the first, while a proxy in +// front might have acted on the other. +func TestVerifyRefusesRepeatedAuthorization(t *testing.T) { + f := newFixture(t) + request := func(authorizations ...string) *http.Request { + r := httptest.NewRequest("GET", f.target.String(), nil) + for _, a := range authorizations { + r.Header.Add("Authorization", a) + } + r.Header.Set("DPoP", f.dpopProof) + return r + } + + if _, err := f.verifier.Verify(context.Background(), resource.VerifyRequestFromHTTP(request("DPoP "+f.accessToken), f.target)); err != nil { + t.Fatalf("Verify(one Authorization header) = %v, want nil", err) + } + _, err := f.verifier.Verify(context.Background(), resource.VerifyRequestFromHTTP(request("DPoP "+f.accessToken, "Bearer other"), f.target)) + var rerr *resource.Error + if !errors.As(err, &rerr) || rerr.Code() != resource.ErrorInvalidRequest || rerr.HTTPStatus() != http.StatusBadRequest { + t.Errorf("Verify(two Authorization headers) = %v, want a 400 invalid_request", err) + } +}