fix(auth): reject unsigned x-amz-* headers to close CopyObject confused-deputy

A presigned or signed PUT authorized for a single object could be turned
into a server-side copy of any object the signing key can read by adding
an unsigned x-amz-copy-source header, executed as the signer. SigV4
verification only walked the signed-headers list, never the headers that
actually arrived; the meta-header check matched only X-Amz-Meta- and ran
only on the presigned path, so an unsigned x-amz-* header outside the
list was never seen while the router still dispatched the PUT to
CopyObjectHandler.

Reject any x-amz-* request header not covered by the signed headers, on
both the presigned (doesPresignedSignatureMatch) and Authorization-header
(doesSignatureMatch) paths, matching AWS S3. The check tests membership
in the signed set rather than value equality, so a header whose first
value is empty (e.g. {"", "/src/secret"}) cannot slip through.
X-Amz-Content-Sha256 is exempt (payload hash: read from the query for
presigned requests and bound into the string-to-sign for signed ones, so
it is self-protected) and X-Amz-Signature-Age is exempt (an internal
scratch header written after verification, so repeated verification of
the same request stays idempotent). The synthesized X-Amz-Tagging header
in PutObjectTagging is now injected after signature verification.

Tests that previously added x-amz-copy-source and friends after signing
(relying on the vulnerable behavior) now re-sign, mirroring real S3
clients. Adds checkUnsignedHeaders unit cases and TestPresignedVerifyIdempotent.

Reported by Oren Yomtov. Inherited unchanged from upstream minio/minio.
Tracked as SN-2026-011.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Signed-off-by: Feng Ruohang <rh@vonng.com>
This commit is contained in:
Feng Ruohang
2026-09-10 09:57:01 +08:00
parent 8a2fe9b7a0
commit 1233254309
12 changed files with 231 additions and 24 deletions
+2 -2
View File
@@ -255,8 +255,8 @@ func getConditionValuesWithTags(r *http.Request, lc string, cred auth.Credential
}
cloneHeader := r.Header.Clone()
signatureAge := cloneHeader.Get("x-amz-signature-age")
cloneHeader.Del("x-amz-signature-age")
signatureAge := cloneHeader.Get(xhttp.AmzSignatureAge)
cloneHeader.Del(xhttp.AmzSignatureAge)
// The presigned V4 verifier overwrites this internal scratch header after
// validating the signature. Ignore a value supplied on every other request
// type, where it would otherwise synthesize s3:signatureAge.
@@ -157,6 +157,13 @@ func copyPartWithoutChecksumHTTP(t *testing.T, apiRouter http.Handler, creds aut
if sourceRange != "" {
req.Header.Set(xhttp.AmzCopySourceRange, sourceRange)
}
// Re-sign so the copy-source x-amz-* headers are covered by the signature,
// as real S3 clients send them; the verifier rejects unsigned x-amz-*.
if creds.AccessKey != "" && creds.SecretKey != "" {
if err := signRequestV4(req, creds.AccessKey, creds.SecretKey); err != nil {
t.Fatalf("failed to re-sign UploadPartCopy request: %v", err)
}
}
rec := httptest.NewRecorder()
apiRouter.ServeHTTP(rec, req)
if rec.Code != http.StatusOK {
+7
View File
@@ -62,6 +62,13 @@ func copyChecksumRequest(t *testing.T, apiRouter http.Handler, credentials auth.
t.Fatalf("failed to build CopyObject request: %v", err)
}
req.Header.Set(xhttp.AmzCopySource, SlashSeparator+pathJoin(bucket, source))
// Re-sign so x-amz-copy-source is covered by the signature, as real S3
// clients send it; the verifier rejects unsigned x-amz-* headers.
if credentials.AccessKey != "" && credentials.SecretKey != "" {
if err := signRequestV4(req, credentials.AccessKey, credentials.SecretKey); err != nil {
t.Fatalf("failed to re-sign CopyObject request: %v", err)
}
}
rec := httptest.NewRecorder()
apiRouter.ServeHTTP(rec, req)
return rec
+7
View File
@@ -197,6 +197,13 @@ func federatedCopyRequest(t *testing.T, apiRouter http.Handler, credentials auth
t.Fatalf("failed to build federated CopyObject request: %v", err)
}
req.Header.Set(xhttp.AmzCopySource, SlashSeparator+pathJoin(srcBucket, srcObject))
// Re-sign so x-amz-copy-source is covered by the signature, as real S3
// clients send it; the verifier rejects unsigned x-amz-* headers.
if credentials.AccessKey != "" && credentials.SecretKey != "" {
if err := signRequestV4(req, credentials.AccessKey, credentials.SecretKey); err != nil {
t.Fatalf("failed to re-sign federated CopyObject request: %v", err)
}
}
rec := httptest.NewRecorder()
apiRouter.ServeHTTP(rec, req)
return rec
+6 -3
View File
@@ -3658,9 +3658,6 @@ func (api objectAPIHandlers) PutObjectTaggingHandler(w http.ResponseWriter, r *h
}
tagsStr := tags.String()
// Set this such that authorization policies can be applied on the object tags.
r.Header.Set(xhttp.AmzObjectTagging, tagsStr)
logger.GetReqInfo(ctx).BucketName = bucket
logger.GetReqInfo(ctx).ObjectName = object
if s3Error := authenticateRequest(ctx, r, policy.PutObjectTaggingAction); s3Error != ErrNone {
@@ -3668,6 +3665,12 @@ func (api objectAPIHandlers) PutObjectTaggingHandler(w http.ResponseWriter, r *h
return
}
// Set this such that authorization policies can be applied on the object
// tags. This is derived from the request body, so it must be injected only
// after signature verification: the SigV4 verifier now rejects unsigned
// x-amz-* request headers, and this synthesized header is never signed.
r.Header.Set(xhttp.AmzObjectTagging, tagsStr)
opts, err := getOpts(ctx, r, bucket, object)
if err != nil {
writeErrorResponse(ctx, w, toAPIError(ctx, err), r.URL)
+25
View File
@@ -1799,6 +1799,12 @@ func testAPICopyObjectPartHandlerSanity(obj ObjectLayer, instanceType, bucketNam
req.Header.Set("X-Amz-Copy-Source", url.QueryEscape(pathJoin(bucketName, objectName)))
req.Header.Set("X-Amz-Copy-Source-Range", fmt.Sprintf("bytes=%d-%d", a, b))
// Re-sign so the copy-source x-amz-* headers are covered by the
// signature, as real clients do; the verifier rejects unsigned x-amz-*.
if err = signRequestV4(req, credentials.AccessKey, credentials.SecretKey); err != nil {
t.Fatalf("Test failed to re-sign HTTP request for copy object part: <ERROR> %v", err)
}
// Since `apiRouter` satisfies `http.Handler` it has a ServeHTTP to execute the logic of the handler.
// Call the ServeHTTP to execute the handler, `func (api objectAPIHandlers) CopyObjectHandler` handles the request.
a = globalMinPartSize + 1
@@ -2200,6 +2206,15 @@ func testAPICopyObjectPartHandler(obj ObjectLayer, instanceType, bucketName stri
}
}
// Re-sign so the copy-source x-amz-* headers set above are covered by
// the signature, as real clients do; the verifier rejects unsigned
// x-amz-* headers.
if testCase.accessKey != "" && testCase.secretKey != "" {
if err = signRequestV4(req, testCase.accessKey, testCase.secretKey); err != nil {
t.Fatalf("Test %d: Failed to re-sign HTTP request for copy Object: <ERROR> %v", i+1, err)
}
}
// Since `apiRouter` satisfies `http.Handler` it has a ServeHTTP to execute the logic of the handler.
// Call the ServeHTTP to execute the handler, `func (api objectAPIHandlers) CopyObjectHandler` handles the request.
apiRouter.ServeHTTP(rec, req)
@@ -2626,6 +2641,16 @@ func testAPICopyObjectHandler(obj ObjectLayer, instanceType, bucketName string,
if testCase.metadataGarbage {
req.Header.Set("X-Amz-Metadata-Directive", "Unknown")
}
// The x-amz-copy-source and related x-amz-* headers set above must be
// part of the SigV4 signature, exactly as real S3 clients send them.
// Re-sign now that they are present; the verifier rejects unsigned
// x-amz-* headers (an unsigned x-amz-copy-source could otherwise turn a
// PUT grant into a server-side copy).
if testCase.accessKey != "" && testCase.secretKey != "" {
if err = signRequestV4(req, testCase.accessKey, testCase.secretKey); err != nil {
t.Fatalf("Test %d: Failed to re-sign HTTP request for copy Object: <ERROR> %v", i, err)
}
}
// Since `apiRouter` satisfies `http.Handler` it has a ServeHTTP to execute the logic of the handler.
// Call the ServeHTTP to execute the handler, `func (api objectAPIHandlers) CopyObjectHandler` handles the request.
apiRouter.ServeHTTP(rec, req)
+5
View File
@@ -165,6 +165,11 @@ func testAPIZeroByteSSECAuthenticatesKey(obj ObjectLayer, instanceType, bucketNa
t.Fatal(err)
}
req.Header.Set(xhttp.AmzCopySource, SlashSeparator+pathJoin(bucketName, object))
// Re-sign so x-amz-copy-source is covered by the signature, as real S3
// clients send it; the verifier rejects unsigned x-amz-* headers.
if err = signRequestV4(req, credentials.AccessKey, credentials.SecretKey); err != nil {
t.Fatalf("%s: failed to re-sign UploadPartCopy request: %v", instanceType, err)
}
rec = httptest.NewRecorder()
apiRouter.ServeHTTP(rec, req)
if rec.Code != http.StatusForbidden {
+43 -8
View File
@@ -264,14 +264,49 @@ func signV4TrimAll(input string) string {
return strings.Join(strings.Fields(input), " ")
}
// checkMetaHeaders will check if the metadata from header/url is the same with the one from signed headers
func checkMetaHeaders(signedHeadersMap http.Header, r *http.Request) APIErrorCode {
// check values from http header
for k, val := range r.Header {
if stringsHasPrefixFold(k, "X-Amz-Meta-") {
if signedHeadersMap.Get(k) == val[0] {
continue
}
// checkUnsignedHeaders rejects any x-amz-* request header that is not covered by
// the SigV4 signed-headers list. AWS S3 requires every x-amz-* header to be
// signed and returns AccessDenied ("There were headers present in the request
// which were not signed") otherwise. Enforcing the same here prevents an
// unsigned x-amz-* header (for example x-amz-copy-source) from changing the
// semantics of an already-signed or presigned request: without this check a
// presigned PUT grant could be turned into a server-side copy that reads any
// object the signing key can reach.
//
// Only headers actually sent by the client are inspected. Server-synthesized
// x-amz-* headers (e.g. x-amz-tagging derived from a request body, or the
// post-verification x-amz-signature-age scratch header) are set after signature
// verification and therefore never reach this walk.
func checkUnsignedHeaders(signedHeadersMap http.Header, r *http.Request) APIErrorCode {
// check headers that arrived on the request
for k := range r.Header {
if !stringsHasPrefixFold(k, "X-Amz-") {
continue
}
// X-Amz-Content-Sha256 carries the payload hash, not an operation or
// authorization input, and is handled specially: for presigned requests
// it is read from the query string (getContentSha256Cksum) and any
// header copy is ignored, while for signed requests it is bound into the
// string-to-sign as the payload hash, so a tampered value fails
// signature verification regardless of the signed-headers list. Some
// clients send it as an unsigned header, so exempt it to preserve
// compatibility without weakening the operation-header protection.
if strings.EqualFold(k, xhttp.AmzContentSha256) {
continue
}
// X-Amz-Signature-Age is an internal scratch header written by the
// presigned verifier itself, after this check, purely so bucket-policy
// evaluation can expose s3:signatureAge. It is never sent or signed by a
// client, and exempting it keeps signature verification idempotent when
// the same request is verified more than once.
if strings.EqualFold(k, xhttp.AmzSignatureAge) {
continue
}
// The header must be a member of the signed-headers list. Testing
// membership (not value equality) is essential: an unsigned header whose
// first value is empty would otherwise compare equal to the empty string
// returned for an absent key and slip through.
if _, ok := signedHeadersMap[http.CanonicalHeaderKey(k)]; !ok {
return ErrUnsignedHeaders
}
}
+69 -6
View File
@@ -363,8 +363,8 @@ func TestGetContentSha256Cksum(t *testing.T) {
}
}
// Test TestCheckMetaHeaders tests the logic of checkMetaHeaders() function
func TestCheckMetaHeaders(t *testing.T) {
// Test TestCheckUnsignedHeaders tests the logic of checkUnsignedHeaders() function
func TestCheckUnsignedHeaders(t *testing.T) {
signedHeadersMap := map[string][]string{
"X-Amz-Meta-Test": {"test"},
"X-Amz-Meta-Extension": {"png"},
@@ -384,7 +384,7 @@ func TestCheckMetaHeaders(t *testing.T) {
inputHeader.Set("X-Amz-Meta-Extension", expectedMetaExtension)
inputHeader.Set("X-Amz-Meta-Name", expectedMetaName)
// calling the function being tested.
errCode := checkMetaHeaders(signedHeadersMap, r)
errCode := checkUnsignedHeaders(signedHeadersMap, r)
if errCode != ErrNone {
t.Fatalf("Expected the APIErrorCode to be %d, but got %d", ErrNone, errCode)
}
@@ -392,7 +392,7 @@ func TestCheckMetaHeaders(t *testing.T) {
// Add new metadata in inputHeader
inputHeader.Set("X-Amz-Meta-Clone", "fail")
// calling the function being tested.
errCode = checkMetaHeaders(signedHeadersMap, r)
errCode = checkUnsignedHeaders(signedHeadersMap, r)
if errCode != ErrUnsignedHeaders {
t.Fatalf("Expected the APIErrorCode to be %d, but got %d", ErrUnsignedHeaders, errCode)
}
@@ -400,7 +400,7 @@ func TestCheckMetaHeaders(t *testing.T) {
// Delete extra metadata from header to don't affect other test
inputHeader.Del("X-Amz-Meta-Clone")
// calling the function being tested.
errCode = checkMetaHeaders(signedHeadersMap, r)
errCode = checkUnsignedHeaders(signedHeadersMap, r)
if errCode != ErrNone {
t.Fatalf("Expected the APIErrorCode to be %d, but got %d", ErrNone, errCode)
}
@@ -413,8 +413,71 @@ func TestCheckMetaHeaders(t *testing.T) {
r.ParseForm()
// calling the function being tested.
errCode = checkMetaHeaders(signedHeadersMap, r)
errCode = checkUnsignedHeaders(signedHeadersMap, r)
if errCode != ErrNone {
t.Fatalf("Expected the APIErrorCode to be %d, but got %d", ErrNone, errCode)
}
// Regression for the unsigned x-amz-copy-source coverage gap: an x-amz-*
// header outside the signed-headers list (here x-amz-copy-source, which the
// router uses to select CopyObjectHandler) must be rejected. Previously only
// x-amz-meta-* headers were inspected, so this header slipped through and a
// presigned/authorized PUT could be turned into a server-side copy.
r, err = http.NewRequest(http.MethodPut, "http://play.min.io:9000", nil)
if err != nil {
t.Fatal("Unable to create http.Request :", err)
}
r.Header.Set("X-Amz-Copy-Source", "/src/secret.txt")
if errCode = checkUnsignedHeaders(signedHeadersMap, r); errCode != ErrUnsignedHeaders {
t.Fatalf("unsigned x-amz-copy-source: expected %d, got %d", ErrUnsignedHeaders, errCode)
}
// When the same header is part of the signed-headers list with a matching
// value it is allowed through, exactly as for x-amz-meta-*.
signedWithCopy := http.Header{}
for k, v := range signedHeadersMap {
signedWithCopy[k] = v
}
signedWithCopy.Set("X-Amz-Copy-Source", "/src/secret.txt")
if errCode = checkUnsignedHeaders(signedWithCopy, r); errCode != ErrNone {
t.Fatalf("signed x-amz-copy-source: expected %d, got %d", ErrNone, errCode)
}
// Membership, not value equality: an unsigned x-amz-* header whose first
// value is empty must still be rejected. A value-equality check would
// compare "" against the empty string returned for an absent signed header
// and wrongly let it through, so a multi-value header like
// {"", "/src/secret.txt"} could smuggle an unsigned copy-source.
r, err = http.NewRequest(http.MethodPut, "http://play.min.io:9000", nil)
if err != nil {
t.Fatal("Unable to create http.Request :", err)
}
r.Header["X-Amz-Copy-Source"] = []string{"", "/src/secret.txt"}
if errCode = checkUnsignedHeaders(signedHeadersMap, r); errCode != ErrUnsignedHeaders {
t.Fatalf("empty-first unsigned x-amz-copy-source: expected %d, got %d", ErrUnsignedHeaders, errCode)
}
// X-Amz-Content-Sha256 is exempt: it carries the payload hash (handled from
// the query for presigned and bound into the string-to-sign for signed
// requests), so it is allowed even when it is not in the signed-headers map.
r, err = http.NewRequest(http.MethodPut, "http://play.min.io:9000", nil)
if err != nil {
t.Fatal("Unable to create http.Request :", err)
}
r.Header.Set(xhttp.AmzContentSha256, unsignedPayload)
if errCode = checkUnsignedHeaders(signedHeadersMap, r); errCode != ErrNone {
t.Fatalf("unsigned x-amz-content-sha256 must be exempt: expected %d, got %d", ErrNone, errCode)
}
// X-Amz-Signature-Age is the presigned verifier's own scratch header,
// written after this check. Exempting it keeps verification idempotent when
// the same request object is verified more than once.
r, err = http.NewRequest(http.MethodPut, "http://play.min.io:9000", nil)
if err != nil {
t.Fatal("Unable to create http.Request :", err)
}
r.Header.Set(xhttp.AmzSignatureAge, "1234")
if errCode = checkUnsignedHeaders(signedHeadersMap, r); errCode != ErrNone {
t.Fatalf("internal x-amz-signature-age must be exempt: expected %d, got %d", ErrNone, errCode)
}
}
+14 -5
View File
@@ -229,10 +229,11 @@ func doesPresignedSignatureMatch(hashedPayload string, r *http.Request, region s
return errCode
}
// Check if the metadata headers are equal with signedheaders
errMetaCode := checkMetaHeaders(extractedSignedHeaders, r)
if errMetaCode != ErrNone {
return errMetaCode
// Reject any x-amz-* header that the client did not sign. Without this an
// unsigned header (e.g. x-amz-copy-source) could alter the request that the
// presigned URL actually authorized.
if errUnsigned := checkUnsignedHeaders(extractedSignedHeaders, r); errUnsigned != ErrNone {
return errUnsigned
}
// If the host which signed the request is slightly ahead in time (by less than globalMaxSkewTime) the
@@ -335,7 +336,7 @@ func doesPresignedSignatureMatch(hashedPayload string, r *http.Request, region s
return ErrSignatureDoesNotMatch
}
r.Header.Set("x-amz-signature-age", strconv.FormatInt(UTCNow().Sub(pSignValues.Date).Milliseconds(), 10))
r.Header.Set(xhttp.AmzSignatureAge, strconv.FormatInt(UTCNow().Sub(pSignValues.Date).Milliseconds(), 10))
return ErrNone
}
@@ -363,6 +364,14 @@ func doesSignatureMatch(hashedPayload string, r *http.Request, region string, st
return errCode
}
// Reject any x-amz-* header that the client did not sign. The Authorization
// header path shares extractSignedHeaders with the presigned path but, prior
// to this, never inspected the headers that actually arrived, so an unsigned
// x-amz-copy-source could redirect a signed PUT into a server-side copy.
if errUnsigned := checkUnsignedHeaders(extractedSignedHeaders, r); errUnsigned != ErrNone {
return errUnsigned
}
cred, _, s3Err := checkKeyValid(r, signV4Values.Credential.accessKey)
if s3Err != ErrNone {
return s3Err
+41
View File
@@ -25,6 +25,8 @@ import (
"os"
"testing"
"time"
xhttp "github.com/minio/minio/internal/http"
)
func niceError(code APIErrorCode) string {
@@ -313,3 +315,42 @@ func TestDoesPresignedSignatureMatch(t *testing.T) {
}
}
}
// TestPresignedVerifyIdempotent guards against a regression where verifying the
// same presigned request twice began to fail. doesPresignedSignatureMatch
// writes an internal x-amz-signature-age header after validating the signature;
// the unsigned-header check must exempt that scratch header (and an unsigned
// x-amz-content-sha256 the client may carry) so a second verification of the
// same *http.Request still succeeds.
func TestPresignedVerifyIdempotent(t *testing.T) {
ctx, cancel := context.WithCancel(t.Context())
defer cancel()
obj, fsDir, err := prepareFS(ctx)
if err != nil {
t.Fatal(err)
}
defer os.RemoveAll(fsDir)
if err = newTestConfig(globalMinioDefaultRegion, obj); err != nil {
t.Fatal(err)
}
req, err := newTestRequest(http.MethodGet, "http://127.0.0.1:9000/bucket/object", 0, nil)
if err != nil {
t.Fatal(err)
}
if err = preSignV4(req, globalActiveCred.AccessKey, globalActiveCred.SecretKey, int64(10*60)); err != nil {
t.Fatal(err)
}
if err = req.ParseForm(); err != nil {
t.Fatal(err)
}
if got := reqSignatureV4Verify(req, globalSite.Region(), serviceS3); got != ErrNone {
t.Fatalf("first verification: expected ErrNone, got %s", niceError(got))
}
if got := reqSignatureV4Verify(req, globalSite.Region(), serviceS3); got != ErrNone {
t.Fatalf("second verification of the same request: expected ErrNone, got %s (x-amz-signature-age=%q)",
niceError(got), req.Header.Get(xhttp.AmzSignatureAge))
}
}
+5
View File
@@ -132,6 +132,11 @@ const (
AmzMaxParts = "X-Amz-Max-Parts"
AmzPartNumberMarker = "X-Amz-Part-Number-Marker"
// AmzSignatureAge is an internal scratch header the presigned verifier
// writes after validating the signature so that bucket-policy evaluation can
// expose s3:signatureAge. It is never sent or signed by a client.
AmzSignatureAge = "X-Amz-Signature-Age"
// Constants used for GetObjectAttributes and GetObjectVersionAttributes
AmzObjectAttributes = "X-Amz-Object-Attributes"