fix(auth): align signed request and policy condition semantics

Signed-off-by: Feng Ruohang <rh@vonng.com>

cmd/bucket-policy.go

@@ -255,13 +255,32 @@ func getConditionValuesWithTags(r *http.Request, lc string, cred auth.Credential
 	}
 
 	cloneHeader := r.Header.Clone()
-	signatureAge := cloneHeader.Get(xhttp.AmzSignatureAge)
+
-	cloneHeader.Del(xhttp.AmzSignatureAge)
+	// s3:signatureAge is derived from the presigned X-Amz-Date rather than from
-	// The presigned V4 verifier overwrites this internal scratch header after
+	// anything the verifier writes back: PutObject and UploadPart authorize
-	// validating the signature. Ignore a value supplied on every other request
+	// before they verify the signature, so a post-verification value is not yet
-	// type, where it would otherwise synthesize s3:signatureAge.
+	// available on the first evaluation. The date is bound by the signature
-	if authType == authTypePresigned && signatureAge != "" {
+	// (doesPresignedSignatureMatch rebuilds and compares it), so a forged date
-		args["signatureAge"] = []string{signatureAge}
+	// only changes the authorization outcome of a request that then fails
+	// verification. A date that does not parse leaves the key absent; the
+	// verifier rejects the request as ErrMalformedPresignedDate.
+	if authType == authTypePresigned {
+		if signedDate, err := time.Parse(iso8601Format, r.Form.Get(xhttp.AmzDate)); err == nil {
+			args["signatureAge"] = []string{strconv.FormatInt(currTime.Sub(signedDate).Milliseconds(), 10)}
+		}
+	}
+
+	// s3:x-amz-content-sha256 must name the payload hash the request is actually
+	// verified and enforced against, and only one such value. Presence of the
+	// header controls whether the key exists at all (AWS documents that the
+	// query-string form does not populate it), but the value comes from the same
+	// selection getContentSha256Cksum makes for verification: the presigned query
+	// value takes precedence over the header, and a repeated header contributes
+	// only its first value. Exposing every raw header value instead let a
+	// request satisfy a policy with a value the verifier never checked.
+	if _, ok := cloneHeader[xhttp.AmzContentSha256]; ok {
+		args[xhttp.AmzContentSha256] = []string{getContentSha256Cksum(r, serviceS3)}
+		cloneHeader.Del(xhttp.AmzContentSha256)
 	}
 
 	userTags := cloneHeader.Get(xhttp.AmzObjectTagging)

cmd/bucket-policy_test.go

@@ -23,8 +23,10 @@ import (
 	"net/url"
 	"os"
 	"slices"
+	"strconv"
 	"strings"
 	"testing"
+	"time"
 
 	"github.com/minio/minio/internal/auth"
 	"github.com/minio/minio/internal/handlers"
@@ -579,8 +581,20 @@ func TestGetConditionValuesRejectsAbsentInternalKeys(t *testing.T) {
 	}
 }
 
+// s3:signatureAge is derived from the presigned X-Amz-Date, which the signature
+// binds. A client header under the former scratch name must never supply it on
+// any auth type, and a presign whose date is missing or malformed leaves the
+// key absent (the verifier then rejects the request).
 func TestGetConditionValuesOnlyAcceptsPresignedSignatureAge(t *testing.T) {
 	const signatureAgeHeader = "x-amz-signature-age"
+	signedDate := UTCNow().Add(-90 * time.Second)
+	presignQuery := func(date string) string {
+		q := url.Values{xhttp.AmzCredential: {"access/20260803/us-east-1/s3/aws4_request"}}
+		if date != "" {
+			q.Set(xhttp.AmzDate, date)
+		}
+		return "http://minio.local/bkt/obj?" + q.Encode()
+	}
 
 	for _, tc := range []struct {
 		name    string
@@ -602,19 +616,33 @@ func TestGetConditionValuesOnlyAcceptsPresignedSignatureAge(t *testing.T) {
 			},
 		},
 		{
-			name: "presigned verifier value",
+			name:    "presigned client header without date",
-			target: "http://minio.local/bkt/obj?" + url.Values{
+			target:  presignQuery(""),
-				xhttp.AmzCredential: {"access/20260803/us-east-1/s3/aws4_request"},
+			headers: map[string]string{signatureAgeHeader: "250"},
-			}.Encode(),
+		},
+		{
+			name:   "presigned malformed date",
+			target: presignQuery("yesterday"),
+		},
+		{
+			name:    "presigned signed date",
+			target:  presignQuery(signedDate.Format(iso8601Format)),
 			headers: map[string]string{signatureAgeHeader: "250"},
 			want:    true,
 		},
 	} {
 		t.Run(tc.name, func(t *testing.T) {
 			got := condValuesForRequest(t, tc.target, tc.headers)
-			_, ok := got["signatureAge"]
+			v, ok := got["signatureAge"]
 			if ok != tc.want {
-				t.Fatalf("signatureAge presence: expected %v, got %v", tc.want, got["signatureAge"])
+				t.Fatalf("signatureAge presence: expected %v, got %v", tc.want, v)
+			}
+			if !tc.want {
+				return
+			}
+			age, err := strconv.ParseInt(strings.Join(v, ""), 10, 64)
+			if err != nil || age < (90*time.Second).Milliseconds() || age > (2*time.Minute).Milliseconds() {
+				t.Fatalf("signatureAge = %v, want about 90s derived from X-Amz-Date rather than the client header", v)
 			}
 		})
 	}

cmd/signature-v4-boundary_test.go

@@ -0,0 +1,324 @@
+// Copyright (c) 2026 PGSTY
+// SPDX-License-Identifier: AGPL-3.0-only
+
+package cmd
+
+import (
+	"bytes"
+	"encoding/xml"
+	"fmt"
+	"net/http"
+	"net/http/httptest"
+	"os"
+	"slices"
+	"strconv"
+	"strings"
+	"testing"
+	"time"
+
+	"github.com/minio/madmin-go/v3"
+	"github.com/minio/minio/internal/auth"
+	xhttp "github.com/minio/minio/internal/http"
+	"github.com/pgsty/silo-pkg/v3/policy"
+)
+
+// forgedSignatureAgeHeader is the name of the scratch header the presigned
+// verifier once wrote s3:signatureAge through. Production code no longer knows
+// it; a client that sends it is sending an ordinary unsigned x-amz-* header.
+const forgedSignatureAgeHeader = "X-Amz-Signature-Age"
+
+// Presign at a chosen time, including exactly the supplied operation headers.
+func presignBoundaryRequest(t *testing.T, r *http.Request, date time.Time, signedHeaders []string, cred auth.Credentials) {
+	t.Helper()
+	query := r.URL.Query()
+	query.Del(xhttp.AmzSignature)
+	query.Set(xhttp.AmzAlgorithm, signV4Algorithm)
+	query.Set(xhttp.AmzDate, date.Format(iso8601Format))
+	query.Set(xhttp.AmzExpires, "3600")
+	query.Set(xhttp.AmzSignedHeaders, strings.Join(signedHeaders, ";"))
+	query.Set(xhttp.AmzCredential, cred.AccessKey+"/"+getScope(date, globalSite.Region()))
+	r.Form = query
+	headers, code := extractSignedHeaders(signedHeaders, r)
+	if code != ErrNone {
+		t.Fatal(niceError(code))
+	}
+	canonical := getCanonicalRequest(headers, getContentSha256Cksum(r, serviceS3), query.Encode(), r.URL.Path, r.Method)
+	key := getSigningKey(cred.SecretKey, date, globalSite.Region(), serviceS3)
+	query.Set(xhttp.AmzSignature, getSignature(key, getStringToSign(canonical, date, getScope(date, globalSite.Region()))))
+	r.URL.RawQuery = query.Encode()
+	r.Form = query
+}
+
+func setupSignatureBoundaryTest(t *testing.T) {
+	t.Helper()
+	obj, fsDir, err := prepareFS(t.Context())
+	if err != nil {
+		t.Fatal(err)
+	}
+	t.Cleanup(func() { os.RemoveAll(fsDir) })
+	if err = newTestConfig(globalMinioDefaultRegion, obj); err != nil {
+		t.Fatal(err)
+	}
+}
+
+// Canonicalization comma-joins repeated header fields, so a signature over one
+// x-amz-copy-source value containing a comma also covers the same text split
+// into two fields, while the copy handlers act on Header.Get alone. A single
+// value may contain a literal or percent-encoded comma; a repeated header is
+// rejected at the shared SigV4 boundary for both signed and presigned requests.
+func TestV4CopySourceMultiplicity(t *testing.T) {
+	setupSignatureBoundaryTest(t)
+	for _, presigned := range []bool{false, true} {
+		for _, source := range []string{"/source/allowed,tail", "/source/allowed%2Ctail"} {
+			t.Run(fmt.Sprintf("presigned=%v/%s", presigned, source), func(t *testing.T) {
+				r := httptest.NewRequest(http.MethodPut, "http://minio.local/destination/object", nil)
+				r.Header.Set(xhttp.AmzContentSha256, emptySHA256)
+				r.Header.Set(xhttp.AmzCopySource, source)
+				if presigned {
+					presignBoundaryRequest(t, r, UTCNow(), []string{"host", "x-amz-copy-source"}, globalActiveCred)
+				} else {
+					if err := signRequestV4(r, globalActiveCred.AccessKey, globalActiveCred.SecretKey); err != nil {
+						t.Fatal(err)
+					}
+					r.Form = r.URL.Query()
+				}
+				if code := reqSignatureV4Verify(r, globalSite.Region(), serviceS3); code != ErrNone {
+					t.Fatalf("a single source key containing a comma must remain valid: %s", niceError(code))
+				}
+				if source != "/source/allowed,tail" {
+					return
+				}
+				r.Header[xhttp.AmzCopySource] = []string{"/source/allowed", "tail"}
+				if code := reqSignatureV4Verify(r, globalSite.Region(), serviceS3); code != ErrInvalidCopySource {
+					t.Fatalf("split copy source: got %s, want %s; handler would copy %q",
+						niceError(code), niceError(ErrInvalidCopySource), r.Header.Get(xhttp.AmzCopySource))
+				}
+			})
+		}
+	}
+}
+
+// PutObject and UploadPart authorize before they verify the signature, so
+// s3:signatureAge must come from the signed X-Amz-Date on the first policy
+// evaluation. A client header under the former scratch name must neither
+// supply the value nor survive verification, and verifying the same request
+// twice must give the same answer.
+func TestGetConditionValuesPresignedAgeFromDate(t *testing.T) {
+	setupSignatureBoundaryTest(t)
+	for _, tc := range []struct {
+		name, header string
+		age          time.Duration
+		wantVerify   APIErrorCode
+	}{
+		{name: "old", age: 10 * time.Minute},
+		{name: "old forged 0", age: 10 * time.Minute, header: "0", wantVerify: ErrUnsignedHeaders},
+		{name: "old forged 1", age: 10 * time.Minute, header: "1", wantVerify: ErrUnsignedHeaders},
+		// A signer slightly ahead of the server stays within globalMaxSkewTime.
+		{name: "future within skew", age: -2 * time.Minute},
+	} {
+		t.Run(tc.name, func(t *testing.T) {
+			r := httptest.NewRequest(http.MethodPut, "http://minio.local/bucket/object", nil)
+			presignBoundaryRequest(t, r, UTCNow().Add(-tc.age), []string{"host"}, globalActiveCred)
+			if tc.header != "" {
+				r.Header.Set(forgedSignatureAgeHeader, tc.header)
+			}
+			values := getConditionValues(r, "", globalActiveCred)
+			age, err := strconv.ParseInt(strings.Join(values["signatureAge"], ""), 10, 64)
+			lo, hi := (tc.age - time.Minute).Milliseconds(), (tc.age + time.Minute).Milliseconds()
+			if err != nil || age < lo || age > hi {
+				t.Errorf("pre-verification policy got age %v; want the age of the signed date (about %d ms)", values["signatureAge"], tc.age.Milliseconds())
+			}
+			code := reqSignatureV4Verify(r, globalSite.Region(), serviceS3)
+			if code != tc.wantVerify {
+				t.Fatalf("verification: got %s, want %s", niceError(code), niceError(tc.wantVerify))
+			}
+			if again := reqSignatureV4Verify(r, globalSite.Region(), serviceS3); again != code {
+				t.Fatalf("second verification changed the outcome: %s -> %s", niceError(code), niceError(again))
+			}
+		})
+	}
+}
+
+// The s3:x-amz-content-sha256 policy value must be the single payload hash the
+// request is verified and enforced against. Header presence decides whether the
+// key exists at all; the value is the one getContentSha256Cksum selects, so a
+// presigned query value wins over the header and a repeated header contributes
+// only its first value. None of these requests is rejected at the protocol
+// level; the policy simply sees what verification bound.
+func TestGetConditionValuesPayloadHashMatchesVerifiedValue(t *testing.T) {
+	setupSignatureBoundaryTest(t)
+	hashA, hashB := getSHA256Hash([]byte("a")), getSHA256Hash([]byte("b"))
+	for _, tc := range []struct {
+		name      string
+		presigned bool
+		query     string
+		header    []string
+		want      []string
+	}{
+		{name: "signed header", header: []string{hashA}, want: []string{hashA}},
+		{name: "signed absent"},
+		{name: "signed present empty", header: []string{""}, want: []string{""}},
+		{name: "signed duplicate header", header: []string{hashA, hashB}, want: []string{hashA}},
+		{name: "signed streaming with second value", header: []string{streamingContentSHA256, hashA}, want: []string{streamingContentSHA256}},
+		{name: "presigned query only", presigned: true, query: hashA},
+		{name: "presigned header only", presigned: true, header: []string{hashA}, want: []string{hashA}},
+		{name: "presigned matching query and header", presigned: true, query: hashA, header: []string{hashA}, want: []string{hashA}},
+		{name: "presigned unsigned query with forged header", presigned: true, query: unsignedPayload, header: []string{hashA}, want: []string{unsignedPayload}},
+		{name: "presigned duplicate header", presigned: true, header: []string{hashA, hashB}, want: []string{hashA}},
+		{name: "presigned query with empty header", presigned: true, query: hashA, header: []string{""}, want: []string{hashA}},
+	} {
+		t.Run(tc.name, func(t *testing.T) {
+			target := "http://minio.local/bucket/object"
+			if tc.query != "" {
+				target += "?" + xhttp.AmzContentSha256 + "=" + tc.query
+			}
+			r := httptest.NewRequest(http.MethodPut, target, nil)
+			if tc.header != nil {
+				r.Header[xhttp.AmzContentSha256] = tc.header
+			}
+			if tc.presigned {
+				presignBoundaryRequest(t, r, UTCNow(), []string{"host"}, globalActiveCred)
+			} else {
+				r.Header.Set(xhttp.Authorization, signV4Algorithm+" Credential=x/20260910/us-east-1/s3/aws4_request, SignedHeaders=host, Signature=x")
+				r.Form = r.URL.Query()
+			}
+			got, ok := getConditionValues(r, "", globalActiveCred)[xhttp.AmzContentSha256]
+			if ok != (tc.want != nil) || !slices.Equal(got, tc.want) {
+				t.Fatalf("policy value = %v (present=%v), want %v (present=%v)", got, ok, tc.want, tc.want != nil)
+			}
+			if tc.presigned {
+				if code := reqSignatureV4Verify(r, globalSite.Region(), serviceS3); code != ErrNone {
+					t.Fatalf("the presigned request must still verify: %s", niceError(code))
+				}
+			}
+		})
+	}
+}
+
+func newSignatureBoundaryUser(t *testing.T, bucket, statements string) auth.Credentials {
+	t.Helper()
+	cred, err := auth.GetNewCredentials()
+	if err != nil {
+		t.Fatal(err)
+	}
+	if _, err := globalIAMSys.CreateUser(t.Context(), cred.AccessKey, madmin.AddOrUpdateUserReq{SecretKey: cred.SecretKey, Status: madmin.AccountEnabled}); err != nil {
+		t.Fatal(err)
+	}
+	p, err := policy.ParseConfig(strings.NewReader(fmt.Sprintf(`{"Version":"2012-10-17","Statement":[{"Effect":"Allow","Action":"s3:PutObject","Resource":"arn:aws:s3:::%s/*"},%s]}`, bucket, statements)))
+	if err != nil {
+		t.Fatal(err)
+	}
+	name := "signature-boundary-" + mustGetUUID()
+	if _, err := globalIAMSys.SetPolicy(t.Context(), name, *p); err != nil {
+		t.Fatal(err)
+	}
+	if _, err := globalIAMSys.PolicyDBSet(t.Context(), cred.AccessKey, name, regUser, false); err != nil {
+		t.Fatal(err)
+	}
+	return cred
+}
+
+func TestAPIPresignedSignatureAgeBeforeAuthorization(t *testing.T) {
+	defer DetectTestLeak(t)()
+	ExecObjectLayerAPITest(ExecObjectLayerAPITestArgs{
+		t:         t,
+		endpoints: []string{"NewMultipart", "PutObjectPart", "PutObject"},
+		objAPITest: func(obj ObjectLayer, instanceType, bucket string, router http.Handler, root auth.Credentials, t *testing.T) {
+			user := newSignatureBoundaryUser(t, bucket, fmt.Sprintf(`{"Effect":"Deny","Action":"s3:PutObject","Resource":"arn:aws:s3:::%s/*","Condition":{"NumericGreaterThan":{"s3:signatureAge":"60000"}}}`, bucket))
+			initReq, err := newTestSignedRequestV4(http.MethodPost, getNewMultipartURL("", bucket, "multipart"), 0, nil, root.AccessKey, root.SecretKey, nil)
+			if err != nil {
+				t.Fatal(err)
+			}
+			initRec := httptest.NewRecorder()
+			router.ServeHTTP(initRec, initReq)
+			var upload InitiateMultipartUploadResponse
+			if initRec.Code != http.StatusOK || xml.Unmarshal(initRec.Body.Bytes(), &upload) != nil {
+				t.Fatalf("multipart initiation: %d %s", initRec.Code, initRec.Body.String())
+			}
+			for _, operation := range []string{"put", "part"} {
+				for _, tc := range []struct {
+					name, header string
+					age          time.Duration
+					want         int
+				}{
+					{name: "fresh", want: http.StatusOK},
+					{name: "old without header", age: 10 * time.Minute, want: http.StatusForbidden},
+					{name: "old forged header", age: 10 * time.Minute, header: "0", want: http.StatusForbidden},
+					// Policy allows a fresh signature; the unsigned header then fails
+					// verification with the existing ErrUnsignedHeaders (HTTP 400).
+					{name: "fresh with unsigned header", header: "0", want: http.StatusBadRequest},
+				} {
+					t.Run(instanceType+"/"+operation+"/"+tc.name, func(t *testing.T) {
+						target := getPutObjectURL("", bucket, "put-"+strings.ReplaceAll(tc.name, " ", "-"))
+						if operation == "part" {
+							target = getPutObjectPartURL("", bucket, "multipart", upload.UploadID, "1")
+						}
+						r, err := newTestRequest(http.MethodPut, target, 4, bytes.NewReader([]byte("body")))
+						if err != nil {
+							t.Fatal(err)
+						}
+						r.Header.Del(xhttp.AmzContentSha256)
+						presignBoundaryRequest(t, r, UTCNow().Add(-tc.age), []string{"host"}, user)
+						if tc.header != "" {
+							r.Header.Set(forgedSignatureAgeHeader, tc.header)
+						}
+						rec := httptest.NewRecorder()
+						router.ServeHTTP(rec, r)
+						if rec.Code != tc.want {
+							t.Errorf("got %d %s, want %d", rec.Code, rec.Body.String(), tc.want)
+						}
+					})
+				}
+			}
+		},
+	})
+}
+
+func TestAPIPayloadHashPolicyMatchesVerifiedValue(t *testing.T) {
+	defer DetectTestLeak(t)()
+	ExecObjectLayerAPITest(ExecObjectLayerAPITestArgs{
+		t:         t,
+		endpoints: []string{"PutObject"},
+		objAPITest: func(_ ObjectLayer, instanceType, bucket string, router http.Handler, _ auth.Credentials, t *testing.T) {
+			allowedHash := getSHA256Hash([]byte("expected"))
+			user := newSignatureBoundaryUser(t, bucket, fmt.Sprintf(`{"Effect":"Deny","Action":"s3:PutObject","Resource":"arn:aws:s3:::%s/*","Condition":{"StringNotEquals":{"s3:x-amz-content-sha256":"%s"}}}`, bucket, allowedHash))
+			for _, kind := range []string{"signed control", "presigned control", "unsigned query forged header", "signed duplicate header", "presigned duplicate header"} {
+				t.Run(instanceType+"/"+kind, func(t *testing.T) {
+					control := strings.HasSuffix(kind, "control")
+					body := "modified"
+					if control {
+						body = "expected"
+					}
+					r, err := newTestRequest(http.MethodPut, getPutObjectURL("", bucket, strings.ReplaceAll(kind, " ", "-")), int64(len(body)), strings.NewReader(body))
+					if err != nil {
+						t.Fatal(err)
+					}
+					if strings.Contains(kind, "duplicate") {
+						r.Header.Add(xhttp.AmzContentSha256, allowedHash)
+					}
+					if kind == "unsigned query forged header" {
+						q := r.URL.Query()
+						q.Set(xhttp.AmzContentSha256, unsignedPayload)
+						r.URL.RawQuery = q.Encode()
+						r.Header.Set(xhttp.AmzContentSha256, allowedHash)
+					}
+					if strings.HasPrefix(kind, "signed") {
+						if err := signRequestV4(r, user.AccessKey, user.SecretKey); err != nil {
+							t.Fatal(err)
+						}
+					} else {
+						presignBoundaryRequest(t, r, UTCNow(), []string{"host"}, user)
+					}
+					rec := httptest.NewRecorder()
+					router.ServeHTTP(rec, r)
+					if control && rec.Code != http.StatusOK {
+						t.Errorf("control rejected: %d %s", rec.Code, rec.Body.String())
+					}
+					if !control && rec.Code < http.StatusBadRequest {
+						t.Errorf("payload-hash policy bypass returned %d %s", rec.Code, rec.Body.String())
+					}
+				})
+			}
+		},
+	})
+}

cmd/signature-v4-utils.go

@@ -211,7 +211,16 @@ func extractSignedHeaders(signedHeaders []string, r *http.Request) (http.Header,
 		// `host` will not be found in the headers, can be found in r.Host.
 		// but its always necessary that the list of signed headers containing host in it.
 		val, ok := reqHeaders[http.CanonicalHeaderKey(header)]
-		if !ok {
+		if ok {
+			// Canonicalization comma-joins repeated header fields, so a signature
+			// over the single value "/src/a,tail" also verifies a request carrying
+			// ["/src/a", "tail"]. The copy handlers read only Header.Get, so that
+			// rewrite would copy a different source than the one signed. Reject a
+			// repeated x-amz-copy-source; a single value may still contain commas.
+			if len(val) > 1 && strings.EqualFold(header, strings.ToLower(xhttp.AmzCopySource)) {
+				return nil, ErrInvalidCopySource
+			}
+		} else {
 			// try to set headers from Query String
 			val, ok = reqQueries[header]
 		}
@@ -274,9 +283,8 @@ func signV4TrimAll(input string) string {
 // 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
+// x-amz-* headers (e.g. x-amz-tagging derived from a request body) are set
-// post-verification x-amz-signature-age scratch header) are set after signature
+// after signature verification and therefore never reach this walk.
-// 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 {
@@ -294,14 +302,6 @@ func checkUnsignedHeaders(signedHeadersMap http.Header, r *http.Request) APIErro
 		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

cmd/signature-v4-utils_test.go

@@ -469,15 +469,16 @@ func TestCheckUnsignedHeaders(t *testing.T) {
 		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,
+	// X-Amz-Signature-Age was once a scratch header the presigned verifier wrote
-	// written after this check. Exempting it keeps verification idempotent when
+	// back and this check exempted. s3:signatureAge is now derived from the
-	// the same request object is verified more than once.
+	// signed date, so a client that sends the header is sending an ordinary
+	// unsigned x-amz-* header and must be rejected like any other.
 	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")
+	r.Header.Set("X-Amz-Signature-Age", "1234")
-	if errCode = checkUnsignedHeaders(signedHeadersMap, r); errCode != ErrNone {
+	if errCode = checkUnsignedHeaders(signedHeadersMap, r); errCode != ErrUnsignedHeaders {
-		t.Fatalf("internal x-amz-signature-age must be exempt: expected %d, got %d", ErrNone, errCode)
+		t.Fatalf("unsigned x-amz-signature-age must be rejected: expected %d, got %d", ErrUnsignedHeaders, errCode)
 	}
 }

cmd/signature-v4.go

@@ -336,8 +336,6 @@ func doesPresignedSignatureMatch(hashedPayload string, r *http.Request, region s
 		return ErrSignatureDoesNotMatch
 	}
 
-	r.Header.Set(xhttp.AmzSignatureAge, strconv.FormatInt(UTCNow().Sub(pSignValues.Date).Milliseconds(), 10))
-
 	return ErrNone
 }
 

cmd/signature-v4_test.go

@@ -25,8 +25,6 @@ import (
 	"os"
 	"testing"
 	"time"
-
-	xhttp "github.com/minio/minio/internal/http"
 )
 
 func niceError(code APIErrorCode) string {
@@ -317,11 +315,11 @@ func TestDoesPresignedSignatureMatch(t *testing.T) {
 }
 
 // TestPresignedVerifyIdempotent guards against a regression where verifying the
-// same presigned request twice began to fail. doesPresignedSignatureMatch
+// same presigned request twice began to fail. The verifier must not write
-// writes an internal x-amz-signature-age header after validating the signature;
+// anything back to the request (it once recorded an internal
-// the unsigned-header check must exempt that scratch header (and an unsigned
+// x-amz-signature-age header that the unsigned-header check then had to
-// x-amz-content-sha256 the client may carry) so a second verification of the
+// exempt), and an unsigned x-amz-content-sha256 the client may carry stays
-// same *http.Request still succeeds.
+// exempt, so a second verification of the same *http.Request still succeeds.
 func TestPresignedVerifyIdempotent(t *testing.T) {
 	ctx, cancel := context.WithCancel(t.Context())
 	defer cancel()
@@ -350,7 +348,9 @@ func TestPresignedVerifyIdempotent(t *testing.T) {
 		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)",
+		t.Fatalf("second verification of the same request: expected ErrNone, got %s", niceError(got))
-			niceError(got), req.Header.Get(xhttp.AmzSignatureAge))
+	}
+	if _, ok := req.Header["X-Amz-Signature-Age"]; ok {
+		t.Fatal("the verifier wrote a scratch header back to the request")
 	}
 }

internal/http/headers.go

@@ -132,11 +132,6 @@ 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"