security(s3api): reject unsigned x-amz-* headers in SigV4 requests (#9121)

A presigned URL holder could attach arbitrary x-amz-* headers to a PUT request (e.g. x-amz-tagging, x-amz-acl, x-amz-storage-class, x-amz-server-side-encryption*, x-amz-object-lock-*, x-amz-meta-*, x-amz-website-redirect-location, x-amz-grant-*). Because only the headers declared in SignedHeaders participate in signature verification, the added headers bypass authentication; the PUT handler then persists them into the object's Extended metadata. Match the AWS SigV4 rule: every x-amz-* header present in the request must appear in SignedHeaders. Exempt x-amz-content-sha256 (already tamper-protected via the canonical request's payload-hash line) and, for presigned URLs, the SigV4 protocol parameters that live in the query string (X-Amz-Algorithm/Credential/Date/Expires/SignedHeaders/ Signature) in case they are duplicated as headers. Applies to both header-based and presigned SigV4; non-amz headers are unaffected.
2026-10-01 04:05:54 +00:00 · 2026-04-17 12:20:28 -07:00
parent e8767f42b6
commit 88ac2d0431
2 changed files with 357 additions and 0 deletions
@@ -291,6 +291,14 @@ func (iam *IdentityAccessManagement) verifyV4Signature(r *http.Request, shouldCh
 		return nil, nil, "", nil, errCode
 	}

+	// 5a. Reject requests that carry x-amz-* headers outside of SignedHeaders.
+	// Without this check a presigned-URL holder can inject unsigned headers
+	// (x-amz-tagging, x-amz-acl, x-amz-meta-*, x-amz-storage-class, x-amz-object-lock-*,
+	// x-amz-server-side-encryption*, etc.) that downstream handlers still persist.
+	if errCode = verifySignedHeadersCoverage(r, authInfo.SignedHeaders, authInfo.IsPresigned); errCode != s3err.ErrNone {
+		return nil, nil, "", nil, errCode
+	}
+
 	// 6. Get the query string for the canonical request
 	queryStr := getCanonicalQueryString(r, authInfo.IsPresigned)

@@ -761,6 +769,60 @@ func (iam *IdentityAccessManagement) doesPolicySignatureV4Match(formValues http.
 	return s3err.ErrNone
 }

+// sigV4PayloadHashHeader is x-amz-content-sha256. It participates in the
+// canonical request via the payload-hash line regardless of whether it is
+// listed in SignedHeaders, so tampering with it still breaks the signature.
+// It is therefore exempted from the "all x-amz-* headers must be signed" rule
+// for both header-based and presigned requests.
+const sigV4PayloadHashHeader = "x-amz-content-sha256"
+
+// presignedSigV4ProtocolHeaders lists SigV4 protocol parameters that are
+// conveyed in the presigned URL's query string. Some clients and proxies echo
+// them as request headers; those duplicates do not influence signature
+// computation or object persistence and are exempted from the
+// "all x-amz-* headers must be signed" rule for presigned requests only.
+var presignedSigV4ProtocolHeaders = map[string]struct{}{
+	"x-amz-algorithm":     {},
+	"x-amz-credential":    {},
+	"x-amz-date":          {},
+	"x-amz-expires":       {},
+	"x-amz-signedheaders": {},
+	"x-amz-signature":     {},
+}
+
+// verifySignedHeadersCoverage rejects requests that carry x-amz-* headers
+// outside of the SignedHeaders list. AWS SigV4 requires every x-amz-* header
+// present in the request to be covered by the signature; without this check a
+// presigned-URL holder can add unsigned headers (ACL, tagging, storage class,
+// user metadata, encryption options, object lock) that the server still
+// persists to object metadata.
+func verifySignedHeadersCoverage(r *http.Request, signedHeaders []string, isPresigned bool) s3err.ErrorCode {
+	signedSet := make(map[string]struct{}, len(signedHeaders))
+	for _, h := range signedHeaders {
+		signedSet[strings.ToLower(h)] = struct{}{}
+	}
+	for name := range r.Header {
+		lower := strings.ToLower(name)
+		if !strings.HasPrefix(lower, "x-amz-") {
+			continue
+		}
+		if _, ok := signedSet[lower]; ok {
+			continue
+		}
+		if lower == sigV4PayloadHashHeader {
+			continue
+		}
+		if isPresigned {
+			if _, exempt := presignedSigV4ProtocolHeaders[lower]; exempt {
+				continue
+			}
+		}
+		glog.V(2).Infof("reject %s %s: unsigned header %q not in SignedHeaders", r.Method, r.URL.Path, name)
+		return s3err.ErrSignatureDoesNotMatch
+	}
+	return s3err.ErrNone
+}
+
 // Verify if extracted signed headers are not properly signed.
 func extractSignedHeaders(signedHeaders []string, r *http.Request, externalHost string) (http.Header, s3err.ErrorCode) {
 	reqHeaders := r.Header
@@ -0,0 +1,295 @@
+package s3api
+
+import (
+	"fmt"
+	"net/http"
+	"sort"
+	"strings"
+	"testing"
+	"time"
+
+	"github.com/seaweedfs/seaweedfs/weed/s3api/s3err"
+)
+
+// TestVerifySignedHeadersCoverage_Unit exercises the helper directly.
+func TestVerifySignedHeadersCoverage_Unit(t *testing.T) {
+	tests := []struct {
+		name          string
+		headers       map[string]string
+		signedHeaders []string
+		isPresigned   bool
+		want          s3err.ErrorCode
+	}{
+		{
+			name:          "no x-amz headers is fine",
+			headers:       map[string]string{"Content-Type": "text/plain"},
+			signedHeaders: []string{"host"},
+			want:          s3err.ErrNone,
+		},
+		{
+			name: "signed x-amz header is accepted",
+			headers: map[string]string{
+				"X-Amz-Date":    "20250101T000000Z",
+				"X-Amz-Tagging": "a=b",
+			},
+			signedHeaders: []string{"host", "x-amz-date", "x-amz-tagging"},
+			want:          s3err.ErrNone,
+		},
+		{
+			name: "unsigned x-amz-tagging is rejected (header-based)",
+			headers: map[string]string{
+				"X-Amz-Date":    "20250101T000000Z",
+				"X-Amz-Tagging": "secret=pwn",
+			},
+			signedHeaders: []string{"host", "x-amz-date"},
+			want:          s3err.ErrSignatureDoesNotMatch,
+		},
+		{
+			name: "unsigned x-amz-meta-* is rejected",
+			headers: map[string]string{
+				"X-Amz-Meta-Owner": "attacker",
+			},
+			signedHeaders: []string{"host"},
+			want:          s3err.ErrSignatureDoesNotMatch,
+		},
+		{
+			name: "unsigned x-amz-acl is rejected",
+			headers: map[string]string{
+				"X-Amz-Acl": "public-read",
+			},
+			signedHeaders: []string{"host"},
+			want:          s3err.ErrSignatureDoesNotMatch,
+		},
+		{
+			name: "unsigned x-amz-storage-class is rejected",
+			headers: map[string]string{
+				"X-Amz-Storage-Class": "GLACIER",
+			},
+			signedHeaders: []string{"host"},
+			want:          s3err.ErrSignatureDoesNotMatch,
+		},
+		{
+			name: "unsigned x-amz-server-side-encryption is rejected",
+			headers: map[string]string{
+				"X-Amz-Server-Side-Encryption": "AES256",
+			},
+			signedHeaders: []string{"host"},
+			want:          s3err.ErrSignatureDoesNotMatch,
+		},
+		{
+			name: "unsigned x-amz-object-lock-retain-until-date is rejected",
+			headers: map[string]string{
+				"X-Amz-Object-Lock-Retain-Until-Date": "2099-01-01T00:00:00Z",
+			},
+			signedHeaders: []string{"host"},
+			want:          s3err.ErrSignatureDoesNotMatch,
+		},
+		{
+			name: "unsigned x-amz-security-token is rejected (even presigned)",
+			headers: map[string]string{
+				"X-Amz-Security-Token": "attacker-token",
+			},
+			signedHeaders: []string{"host"},
+			isPresigned:   true,
+			want:          s3err.ErrSignatureDoesNotMatch,
+		},
+		{
+			name: "x-amz-content-sha256 is always exempt (presigned)",
+			headers: map[string]string{
+				"X-Amz-Content-Sha256": "UNSIGNED-PAYLOAD",
+			},
+			signedHeaders: []string{"host"},
+			isPresigned:   true,
+			want:          s3err.ErrNone,
+		},
+		{
+			name: "x-amz-content-sha256 is always exempt (header-based)",
+			headers: map[string]string{
+				"X-Amz-Content-Sha256": "e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855",
+			},
+			signedHeaders: []string{"host"},
+			isPresigned:   false,
+			want:          s3err.ErrNone,
+		},
+		{
+			name: "presigned exempts sigv4 query params echoed as headers",
+			headers: map[string]string{
+				"X-Amz-Algorithm":     "AWS4-HMAC-SHA256",
+				"X-Amz-Credential":    "AKIA/20250101/us-east-1/s3/aws4_request",
+				"X-Amz-Date":          "20250101T000000Z",
+				"X-Amz-Expires":       "3600",
+				"X-Amz-SignedHeaders": "host",
+				"X-Amz-Signature":     "deadbeef",
+			},
+			signedHeaders: []string{"host"},
+			isPresigned:   true,
+			want:          s3err.ErrNone,
+		},
+		{
+			name: "header-based does NOT exempt x-amz-date when unsigned",
+			headers: map[string]string{
+				"X-Amz-Date": "20250101T000000Z",
+			},
+			signedHeaders: []string{"host"},
+			isPresigned:   false,
+			want:          s3err.ErrSignatureDoesNotMatch,
+		},
+		{
+			name: "signed headers match is case-insensitive",
+			headers: map[string]string{
+				"X-Amz-Tagging": "a=b",
+			},
+			signedHeaders: []string{"HOST", "X-Amz-Tagging"},
+			want:          s3err.ErrNone,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			req, err := http.NewRequest(http.MethodPut, "http://example.com/bucket/key", nil)
+			if err != nil {
+				t.Fatalf("NewRequest: %v", err)
+			}
+			for k, v := range tt.headers {
+				req.Header.Set(k, v)
+			}
+			got := verifySignedHeadersCoverage(req, tt.signedHeaders, tt.isPresigned)
+			if got != tt.want {
+				t.Fatalf("verifySignedHeadersCoverage = %v, want %v", got, tt.want)
+			}
+		})
+	}
+}
+
+// TestPresignedPutRejectsUnsignedTagging exercises the full verification path
+// through reqSignatureV4Verify for a presigned URL that signs only `host` but
+// whose caller attached an x-amz-tagging header after-the-fact.
+func TestPresignedPutRejectsUnsignedTagging(t *testing.T) {
+	iam := newTestIAM()
+
+	req, err := newTestRequest(http.MethodPut, "http://127.0.0.1:9000/bucket/key", 0, nil)
+	if err != nil {
+		t.Fatalf("newTestRequest: %v", err)
+	}
+	if err := preSignV4(iam, req, "AKIAIOSFODNN7EXAMPLE", "wJalrXUtnFEMI/K7MDENG/bPxRfiCYEXAMPLEKEY", 600); err != nil {
+		t.Fatalf("preSignV4: %v", err)
+	}
+	// Attacker appends an unsigned header after the URL was signed.
+	req.Header.Set("X-Amz-Tagging", "classification=public")
+
+	_, errCode := iam.reqSignatureV4Verify(req)
+	if errCode != s3err.ErrSignatureDoesNotMatch {
+		t.Fatalf("expected ErrSignatureDoesNotMatch for unsigned x-amz-tagging, got %v", errCode)
+	}
+}
+
+// TestPresignedPutAcceptsSignedTagging confirms that a presigned URL whose
+// SignedHeaders list covers x-amz-tagging still validates successfully.
+func TestPresignedPutAcceptsSignedTagging(t *testing.T) {
+	iam := newTestIAM()
+
+	req, err := newTestRequest(http.MethodPut, "http://127.0.0.1:9000/bucket/key", 0, nil)
+	if err != nil {
+		t.Fatalf("newTestRequest: %v", err)
+	}
+	req.Header.Set("X-Amz-Tagging", "classification=public")
+	if err := preSignV4WithHeaders(iam, req, "AKIAIOSFODNN7EXAMPLE", "wJalrXUtnFEMI/K7MDENG/bPxRfiCYEXAMPLEKEY", 600, []string{"host", "x-amz-tagging"}); err != nil {
+		t.Fatalf("preSignV4WithHeaders: %v", err)
+	}
+
+	_, errCode := iam.reqSignatureV4Verify(req)
+	if errCode != s3err.ErrNone {
+		t.Fatalf("expected ErrNone for signed x-amz-tagging, got %v", errCode)
+	}
+}
+
+// preSignV4WithHeaders is a test helper that builds a presigned URL whose
+// SignedHeaders list covers the specified headers (which must already be set
+// on the request), then computes the signature over those headers and
+// attaches X-Amz-Signature.
+func preSignV4WithHeaders(_ *IdentityAccessManagement, req *http.Request, accessKey, secretKey string, expires int64, signedHeaders []string) error {
+	now := time.Now().UTC()
+	dateStr := now.Format(iso8601Format)
+
+	scope := fmt.Sprintf("%s/%s/%s/%s", now.Format(yyyymmdd), "us-east-1", "s3", "aws4_request")
+	credential := fmt.Sprintf("%s/%s", accessKey, scope)
+
+	normalized := make([]string, 0, len(signedHeaders))
+	for _, h := range signedHeaders {
+		normalized = append(normalized, strings.ToLower(h))
+	}
+	sort.Strings(normalized)
+
+	query := req.URL.Query()
+	query.Set("X-Amz-Algorithm", signV4Algorithm)
+	query.Set("X-Amz-Credential", credential)
+	query.Set("X-Amz-Date", dateStr)
+	query.Set("X-Amz-Expires", fmt.Sprintf("%d", expires))
+	query.Set("X-Amz-SignedHeaders", strings.Join(normalized, ";"))
+	req.URL.RawQuery = query.Encode()
+
+	hashedPayload := query.Get("X-Amz-Content-Sha256")
+	if hashedPayload == "" {
+		hashedPayload = unsignedPayload
+	}
+
+	extracted := make(http.Header)
+	for _, h := range normalized {
+		if h == "host" {
+			extracted[h] = []string{req.Host}
+			continue
+		}
+		if values, ok := req.Header[http.CanonicalHeaderKey(h)]; ok {
+			extracted[h] = values
+		}
+	}
+
+	canonicalRequest := getCanonicalRequest(extracted, hashedPayload, req.URL.RawQuery, req.URL.Path, req.Method)
+	stringToSign := getStringToSign(canonicalRequest, now, scope)
+	signingKey := getSigningKey(secretKey, now.Format(yyyymmdd), "us-east-1", "s3")
+	signature := getSignature(signingKey, stringToSign)
+
+	query.Set("X-Amz-Signature", signature)
+	req.URL.RawQuery = query.Encode()
+	return nil
+}
+
+// TestPresignedPutRejectsUnsignedMetadataHeaders checks a spread of
+// security-relevant headers that the PUT handler persists.
+func TestPresignedPutRejectsUnsignedMetadataHeaders(t *testing.T) {
+	dangerous := []struct {
+		name   string
+		header string
+		value  string
+	}{
+		{"acl", "X-Amz-Acl", "public-read"},
+		{"user-metadata", "X-Amz-Meta-Owner", "attacker"},
+		{"storage-class", "X-Amz-Storage-Class", "GLACIER"},
+		{"server-side-encryption", "X-Amz-Server-Side-Encryption", "AES256"},
+		{"sse-kms-key-id", "X-Amz-Server-Side-Encryption-Aws-Kms-Key-Id", "kms-key"},
+		{"object-lock-mode", "X-Amz-Object-Lock-Mode", "GOVERNANCE"},
+		{"object-lock-retain-until", "X-Amz-Object-Lock-Retain-Until-Date", "2099-01-01T00:00:00Z"},
+		{"website-redirect-location", "X-Amz-Website-Redirect-Location", "https://attacker.example/"},
+		{"grant-full-control", "X-Amz-Grant-Full-Control", "id=attacker"},
+	}
+
+	for _, c := range dangerous {
+		t.Run(c.name, func(t *testing.T) {
+			iam := newTestIAM()
+
+			req, err := newTestRequest(http.MethodPut, "http://127.0.0.1:9000/bucket/key", 0, nil)
+			if err != nil {
+				t.Fatalf("newTestRequest: %v", err)
+			}
+			if err := preSignV4(iam, req, "AKIAIOSFODNN7EXAMPLE", "wJalrXUtnFEMI/K7MDENG/bPxRfiCYEXAMPLEKEY", 600); err != nil {
+				t.Fatalf("preSignV4: %v", err)
+			}
+			req.Header.Set(c.header, c.value)
+
+			_, errCode := iam.reqSignatureV4Verify(req)
+			if errCode != s3err.ErrSignatureDoesNotMatch {
+				t.Fatalf("expected rejection for unsigned %s, got %v", c.header, errCode)
+			}
+		})
+	}
+}