From 88ac2d04319dc31812ccb1d5c14919d4b4865791 Mon Sep 17 00:00:00 2001 From: Chris Lu Date: Fri, 17 Apr 2026 12:20:28 -0700 Subject: [PATCH] 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. --- weed/s3api/auth_signature_v4.go | 62 ++++ ...auth_signature_v4_unsigned_headers_test.go | 295 ++++++++++++++++++ 2 files changed, 357 insertions(+) create mode 100644 weed/s3api/auth_signature_v4_unsigned_headers_test.go diff --git a/weed/s3api/auth_signature_v4.go b/weed/s3api/auth_signature_v4.go index cf8860e9a..7d3705257 100644 --- a/weed/s3api/auth_signature_v4.go +++ b/weed/s3api/auth_signature_v4.go @@ -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 diff --git a/weed/s3api/auth_signature_v4_unsigned_headers_test.go b/weed/s3api/auth_signature_v4_unsigned_headers_test.go new file mode 100644 index 000000000..a0397fad3 --- /dev/null +++ b/weed/s3api/auth_signature_v4_unsigned_headers_test.go @@ -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) + } + }) + } +}