fix: enforce SignedHeaders validation for SigV4 requests

aws/signer/internal/v4/header_rules.go

@@ -30,8 +30,12 @@ type MapRule map[string]struct{}
 
 // IsValid for the map Rule satisfies whether it exists in the map
 func (m MapRule) IsValid(value string) bool {
-	_, ok := m[value]
-	return ok
+	for key := range m {
+		if strings.EqualFold(key, value) {
+			return true
+		}
+	}
+	return false
 }
 
 // AllowList is a generic Rule for include listing

aws/signer/internal/v4/headers.go

@@ -4,65 +4,24 @@ package v4
 var IgnoredHeaders = Rules{
 	ExcludeList{
 		MapRule{
-			"Authorization": struct{}{},
-			// some clients use user-agent in signed headers
-			// "User-Agent":      struct{}{},
-			"X-Amzn-Trace-Id": struct{}{},
-			// Expect might appear in signed headers
-			// "Expect":          struct{}{},
+			"Authorization":     struct{}{},
+			"User-Agent":        struct{}{},
+			"X-Amzn-Trace-Id":   struct{}{},
+			"Expect":            struct{}{},
+			"Transfer-Encoding": struct{}{},
 		},
 	},
 }
 
-// RequiredSignedHeaders is a allow list for Build canonical headers.
+// RequiredSignedHeaders are request headers that must be part of SignedHeaders
+// whenever they are present on the request.
 var RequiredSignedHeaders = Rules{
 	AllowList{
 		MapRule{
-			"Cache-Control":                         struct{}{},
-			"Content-Disposition":                   struct{}{},
-			"Content-Encoding":                      struct{}{},
-			"Content-Language":                      struct{}{},
-			"Content-Md5":                           struct{}{},
-			"Content-Type":                          struct{}{},
-			"Expires":                               struct{}{},
-			"If-Match":                              struct{}{},
-			"If-Modified-Since":                     struct{}{},
-			"If-None-Match":                         struct{}{},
-			"If-Unmodified-Since":                   struct{}{},
-			"Range":                                 struct{}{},
-			"X-Amz-Acl":                             struct{}{},
-			"X-Amz-Copy-Source":                     struct{}{},
-			"X-Amz-Copy-Source-If-Match":            struct{}{},
-			"X-Amz-Copy-Source-If-Modified-Since":   struct{}{},
-			"X-Amz-Copy-Source-If-None-Match":       struct{}{},
-			"X-Amz-Copy-Source-If-Unmodified-Since": struct{}{},
-			"X-Amz-Copy-Source-Range":               struct{}{},
-			"X-Amz-Copy-Source-Server-Side-Encryption-Customer-Algorithm": struct{}{},
-			"X-Amz-Copy-Source-Server-Side-Encryption-Customer-Key":       struct{}{},
-			"X-Amz-Copy-Source-Server-Side-Encryption-Customer-Key-Md5":   struct{}{},
-			"X-Amz-Expected-Bucket-Owner":                                 struct{}{},
-			"X-Amz-Grant-Full-control":                                    struct{}{},
-			"X-Amz-Grant-Read":                                            struct{}{},
-			"X-Amz-Grant-Read-Acp":                                        struct{}{},
-			"X-Amz-Grant-Write":                                           struct{}{},
-			"X-Amz-Grant-Write-Acp":                                       struct{}{},
-			"X-Amz-Metadata-Directive":                                    struct{}{},
-			"X-Amz-Mfa":                                                   struct{}{},
-			"X-Amz-Request-Payer":                                         struct{}{},
-			"X-Amz-Server-Side-Encryption":                                struct{}{},
-			"X-Amz-Server-Side-Encryption-Aws-Kms-Key-Id":                 struct{}{},
-			"X-Amz-Server-Side-Encryption-Context":                        struct{}{},
-			"X-Amz-Server-Side-Encryption-Customer-Algorithm":             struct{}{},
-			"X-Amz-Server-Side-Encryption-Customer-Key":                   struct{}{},
-			"X-Amz-Server-Side-Encryption-Customer-Key-Md5":               struct{}{},
-			"X-Amz-Storage-Class":                                         struct{}{},
-			"X-Amz-Website-Redirect-Location":                             struct{}{},
-			"X-Amz-Content-Sha256":                                        struct{}{},
-			"X-Amz-Tagging":                                               struct{}{},
+			"Host": struct{}{},
 		},
 	},
-	Patterns{"X-Amz-Object-Lock-"},
-	Patterns{"X-Amz-Meta-"},
+	Patterns{"X-Amz-"},
 }
 
 // AllowedQueryHoisting is a allowed list for Build query headers. The boolean value

aws/signer/internal/v4/headers_test.go

@@ -17,7 +17,19 @@ func TestAllowedQueryHoisting(t *testing.T) {
 		},
 		"another header": {
 			Header:      "X-Amz-SomeOtherHeader",
-			ExpectHoist: true,
+			ExpectHoist: false,
+		},
+		"lowercase amz header": {
+			Header:      "x-amz-someotherheader",
+			ExpectHoist: false,
+		},
+		"mixed case amz header": {
+			Header:      "x-AmZ-someotherheader",
+			ExpectHoist: false,
+		},
+		"non-amz content header": {
+			Header:      "Content-Type",
+			ExpectHoist: false,
 		},
 		"non X-AMZ header": {
 			Header:      "X-SomeOtherHeader",
@@ -34,19 +46,91 @@ func TestAllowedQueryHoisting(t *testing.T) {
 	}
 }
 
+func TestRequiredSignedHeaders(t *testing.T) {
+	cases := map[string]struct {
+		Header         string
+		ExpectRequired bool
+	}{
+		"known content header": {
+			Header:         "Content-Type",
+			ExpectRequired: false,
+		},
+		"known content header lowercase": {
+			Header:         "content-type",
+			ExpectRequired: false,
+		},
+		"known conditional header": {
+			Header:         "If-Match",
+			ExpectRequired: false,
+		},
+		"range header": {
+			Header:         "Range",
+			ExpectRequired: false,
+		},
+		"content md5 header": {
+			Header:         "Content-Md5",
+			ExpectRequired: false,
+		},
+		"arbitrary amz header": {
+			Header:         "X-Amz-SomeOtherHeader",
+			ExpectRequired: true,
+		},
+		"arbitrary amz header lowercase": {
+			Header:         "x-amz-someotherheader",
+			ExpectRequired: true,
+		},
+		"object-lock amz header": {
+			Header:         "X-Amz-Object-Lock-Mode",
+			ExpectRequired: true,
+		},
+		"metadata amz header": {
+			Header:         "X-Amz-Meta-SomeName",
+			ExpectRequired: true,
+		},
+		"non-amz custom header": {
+			Header:         "X-SomeOtherHeader",
+			ExpectRequired: false,
+		},
+	}
+
+	for name, c := range cases {
+		t.Run(name, func(t *testing.T) {
+			if e, a := c.ExpectRequired, RequiredSignedHeaders.IsValid(c.Header); e != a {
+				t.Errorf("expect required %v, was %v", e, a)
+			}
+		})
+	}
+}
+
 func TestIgnoredHeaders(t *testing.T) {
 	cases := map[string]struct {
 		Header        string
 		ExpectIgnored bool
 	}{
 		"expect": {
 			Header:        "Expect",
-			ExpectIgnored: false,
+			ExpectIgnored: true,
+		},
+		"user-agent": {
+			Header:        "User-Agent",
+			ExpectIgnored: true,
+		},
+		"transfer-encoding": {
+			Header:        "Transfer-Encoding",
+			ExpectIgnored: true,
 		},
 		"authorization": {
 			Header:        "Authorization",
 			ExpectIgnored: true,
 		},
+		"authorization lowercase": {
+			Header:        "authorization",
+			ExpectIgnored: true,
+		},
+		"trace id lowercase": {
+			Header:        "x-amzn-trace-id",
+			ExpectIgnored: true,
+		},
 		"X-AMZ header": {
 			Header:        "X-Amz-Content-Sha256",
 			ExpectIgnored: false,

aws/signer/v4/header_rules.go

@@ -0,0 +1,14 @@
+package v4
+
+import v4Internal "github.com/versity/versitygw/aws/signer/internal/v4"
+
+// IsRequiredSignedHeader reports whether a header must be signed when it is
+// present on an incoming request.
+func IsRequiredSignedHeader(header string) bool {
+	return v4Internal.RequiredSignedHeaders.IsValid(header)
+}
+
+// IsIgnoredHeader reports whether a header is normally excluded from signing.
+func IsIgnoredHeader(header string) bool {
+	return !v4Internal.IgnoredHeaders.IsValid(header)
+}

aws/signer/v4/v4.go

@@ -443,7 +443,7 @@ func (s *httpSigner) buildCanonicalHeaders(host string, rule v4Internal.Rule, he
 	}
 
 	for k, v := range header {
-		if !rule.IsValid(k) {
+		if !s.shouldSignHeader(k, rule) {
 			continue // ignored header
 		}
 		if strings.EqualFold(k, contentLengthHeader) {
@@ -493,6 +493,18 @@ func (s *httpSigner) buildCanonicalHeaders(host string, rule v4Internal.Rule, he
 	return signed, signedHeaders, canonicalHeadersStr
 }
 
+func (s *httpSigner) shouldSignHeader(header string, rule v4Internal.Rule) bool {
+	if rule.IsValid(header) {
+		return true
+	}
+	if strings.EqualFold(header, authorizationHeader) {
+		return false
+	}
+	return slices.ContainsFunc(s.SignedHdrs, func(signedHeader string) bool {
+		return strings.EqualFold(signedHeader, header)
+	})
+}
+
 func (s *httpSigner) buildCanonicalString(method, uri, query, signedHeaders, canonicalHeaders string) string {
 	return strings.Join([]string{
 		method,

aws/signer/v4/v4_test.go

@@ -71,10 +71,9 @@ func TestPresignRequest(t *testing.T) {
 	}
 
 	expectedDate := "19700101T000000Z"
-	expectedHeaders := "content-length;content-type;host;x-amz-meta-other-header;x-amz-meta-other-header_with_underscore"
-	expectedSig := "122f0b9e091e4ba84286097e2b3404a1f1f4c4aad479adda95b7dff0ccbe5581"
+	expectedHeaders := "content-length;content-type;host;x-amz-meta-other-header;x-amz-meta-other-header_with_underscore;x-amz-target"
+	expectedSig := "266528f4c66b4b20807f199141c606c7aa81dd793592b4c6f8dc301c05691e54"
 	expectedCred := "AKID/19700101/us-east-1/dynamodb/aws4_request"
-	expectedTarget := "prefix.Operation"
 
 	q, err := url.ParseQuery(signed[strings.Index(signed, "?"):])
 	if err != nil {
@@ -96,8 +95,8 @@ func TestPresignRequest(t *testing.T) {
 	if a := q.Get("X-Amz-Meta-Other-Header"); len(a) != 0 {
 		t.Errorf("expect %v to be empty", a)
 	}
-	if e, a := expectedTarget, q.Get("X-Amz-Target"); e != a {
-		t.Errorf("expect %v, got %v", e, a)
+	if a := q.Get("X-Amz-Target"); len(a) != 0 {
+		t.Errorf("expect X-Amz-Target to be empty, got %v", a)
 	}
 
 	for h := range strings.SplitSeq(expectedHeaders, ";") {
@@ -129,10 +128,9 @@ func TestPresignBodyWithArrayRequest(t *testing.T) {
 	}
 
 	expectedDate := "19700101T000000Z"
-	expectedHeaders := "content-length;content-type;host;x-amz-meta-other-header;x-amz-meta-other-header_with_underscore"
-	expectedSig := "e3ac55addee8711b76c6d608d762cff285fe8b627a057f8b5ec9268cf82c08b1"
+	expectedHeaders := "content-length;content-type;host;x-amz-meta-other-header;x-amz-meta-other-header_with_underscore;x-amz-target"
+	expectedSig := "f8a1f60771366686c04045b64ae1381d302c83d67d84a02567926000e3e653c4"
 	expectedCred := "AKID/19700101/us-east-1/dynamodb/aws4_request"
-	expectedTarget := "prefix.Operation"
 
 	if e, a := expectedSig, q.Get("X-Amz-Signature"); e != a {
 		t.Errorf("expect %v, got %v", e, a)
@@ -149,8 +147,8 @@ func TestPresignBodyWithArrayRequest(t *testing.T) {
 	if a := q.Get("X-Amz-Meta-Other-Header"); len(a) != 0 {
 		t.Errorf("expect %v to be empty, was not", a)
 	}
-	if e, a := expectedTarget, q.Get("X-Amz-Target"); e != a {
-		t.Errorf("expect %v, got %v", e, a)
+	if a := q.Get("X-Amz-Target"); len(a) != 0 {
+		t.Errorf("expect X-Amz-Target to be empty, got %v", a)
 	}
 
 	for h := range strings.SplitSeq(expectedHeaders, ";") {

s3api/utils/auth-reader.go

@@ -59,7 +59,7 @@ func CheckValidSignature(ctx *fiber.Ctx, auth AuthData, secret, checksum string,
 	// Create a new http request instance from fasthttp request
 	req, err := createHttpRequestFromCtx(ctx, signedHdrs, contentLen)
 	if err != nil {
-		return "", fmt.Errorf("create http request from context: %w", err)
+		return "", err
 	}
 
 	signer := v4.NewSigner()

s3api/utils/presign-auth-reader.go

@@ -54,7 +54,7 @@ func CheckPresignedSignature(ctx *fiber.Ctx, auth AuthData, secret string) error
 	// Create a new http request instance from fasthttp request
 	req, err := createPresignedHttpRequestFromCtx(ctx, signedHdrs, contentLength)
 	if err != nil {
-		return fmt.Errorf("create http request from context: %w", err)
+		return err
 	}
 
 	date, _ := time.Parse(iso8601Format, auth.Date)