Skip to content

Commit 1f3d9f7

Browse files
committed
Fix v4 auth to allow fallback to other auth methods
This change ensures that when a request doesn't have the v4 algorithm URL parameter (`x-amz-algorithm`), the chained authenticator tries the next auth method instead of failing. Previously, requests using other signature versions would fail because v4 parsing returned and error that blocked the chain.
1 parent 45143f8 commit 1f3d9f7

File tree

3 files changed

+68
-38
lines changed

3 files changed

+68
-38
lines changed

pkg/gateway/sig/sig.go

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,8 @@ import (
1515
)
1616

1717
var (
18-
ErrHeaderMalformed = errors.New("header malformed")
18+
ErrHeaderMalformed = errors.New("header malformed")
19+
ErrBadAuthorizationFormat = errors.New("authorization format not supported by this authenticator")
1920

2021
// reservedObjectNames - if object matches reserved string, no need to encode them
2122
reservedObjectNames = regexp.MustCompile("^[a-zA-Z0-9-_.~/]+$")
@@ -99,7 +100,10 @@ func (c *chainedAuthenticator) Parse() (SigContext, error) {
99100
if err == nil {
100101
c.chosen = method
101102
return sigContext, nil
102-
} else if !errors.Is(err, ErrHeaderMalformed) {
103+
} else if !errors.Is(err, ErrHeaderMalformed) && !errors.Is(err, ErrBadAuthorizationFormat) {
104+
// ErrHeaderMalformed and ErrBadAuthorizationFormat indicate "wrong auth format, try next method".
105+
// All other errors mean the request matched this method's format but failed validation,
106+
// so return immediately without trying remaining methods.
103107
return nil, err
104108
}
105109
}

pkg/gateway/sig/v4.go

Lines changed: 23 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ const (
3333
v4scopeTerminator = "aws4_request"
3434
v4timeFormat = "20060102T150405Z"
3535
v4shortTimeFormat = "20060102"
36-
maxExpires = 604800
36+
AmzPresignMaxExpires = 7 * 24 * time.Hour // 7 days or 604800 seconds
3737

3838
v4AmzAlgorithm = "X-Amz-Algorithm"
3939
//nolint:gosec
@@ -81,29 +81,36 @@ func parseExpires(expiresStr string) (int64, error) {
8181
if expires < 0 {
8282
return 0, errors.ErrNegativeExpires
8383
}
84-
if expires > maxExpires {
84+
if expires > int64(AmzPresignMaxExpires.Seconds()) {
8585
return 0, errors.ErrMaximumExpires
8686
}
8787
return expires, nil
8888
}
8989

90-
func hasRequiredParams(values url.Values) bool {
91-
var requiredPresignedParams = []string{
92-
v4AmzAlgorithm,
90+
func isV4PresignedRequest(query url.Values) error {
91+
algorithm, ok := query[v4AmzAlgorithm]
92+
if !ok {
93+
// Not a V4 presigned request, try next auth method
94+
return ErrBadAuthorizationFormat
95+
}
96+
if len(algorithm) == 0 || !strings.EqualFold(algorithm[0], V4authHeaderPrefix) {
97+
return errors.ErrInvalidQuerySignatureAlgo
98+
}
99+
100+
requiredParams := []string{
93101
v4AmzCredential,
94102
v4AmzSignature,
95103
v4AmzDate,
96104
v4AmzSignedHeaders,
97105
v4AmzExpires,
98106
}
99-
100-
for _, param := range requiredPresignedParams {
101-
if _, ok := values[param]; !ok {
102-
return false
107+
for _, param := range requiredParams {
108+
if _, ok := query[param]; !ok {
109+
return errors.ErrMissingFields
103110
}
104111
}
105112

106-
return true
113+
return nil
107114
}
108115

109116
func ParseV4AuthContext(r *http.Request) (V4Auth, error) {
@@ -138,16 +145,13 @@ func ParseV4AuthContext(r *http.Request) (V4Auth, error) {
138145
}
139146

140147
query := r.URL.Query()
141-
// otherwise, see if we have all the required query parameters
142-
if !hasRequiredParams(query) {
143-
return ctx, errors.ErrInvalidQueryParams
144-
}
145-
ctx.IsPresigned = true
146148

147-
algorithm := query.Get(v4AmzAlgorithm)
148-
if len(algorithm) == 0 || !strings.EqualFold(algorithm, V4authHeaderPrefix) {
149-
return ctx, errors.ErrInvalidQuerySignatureAlgo
149+
// If we couldn't find the auth header, try to parse the request as a presigned URL
150+
if err := isV4PresignedRequest(query); err != nil {
151+
return ctx, err
150152
}
153+
154+
ctx.IsPresigned = true
151155
credentialScope := query.Get(v4AmzCredential)
152156
if len(credentialScope) == 0 {
153157
return ctx, errors.ErrMissingCredTag
@@ -357,9 +361,9 @@ func (ctx *verificationCtx) getAmzDate() (string, error) {
357361

358362
func (ctx *verificationCtx) verifyExpiration() error {
359363
if !ctx.AuthValue.IsPresigned {
360-
// TODO: we currently don't have handling for this
361364
return nil
362365
}
366+
363367
// Get and validate the request timestamp
364368
amzDate, err := ctx.getAmzDate()
365369
if err != nil {

pkg/gateway/sig/v4_expiration_internal_test.go

Lines changed: 39 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -183,14 +183,14 @@ func TestParseExpires(t *testing.T) {
183183
}
184184
}
185185

186-
func TestHasRequiredParams(t *testing.T) {
186+
func TestIsV4PresignedRequest(t *testing.T) {
187187
testCases := []struct {
188-
name string
189-
query url.Values
190-
expected bool
188+
name string
189+
query url.Values
190+
expectedError error
191191
}{
192192
{
193-
name: "all required params present",
193+
name: "valid V4 presigned request",
194194
query: url.Values{
195195
v4AmzAlgorithm: {"AWS4-HMAC-SHA256"},
196196
v4AmzCredential: {"AKIAIOSFODNN7EXAMPLE/20230101/us-east-1/s3/aws4_request"},
@@ -199,43 +199,65 @@ func TestHasRequiredParams(t *testing.T) {
199199
v4AmzSignedHeaders: {"host"},
200200
v4AmzExpires: {"3600"},
201201
},
202-
expected: true,
202+
expectedError: nil,
203203
},
204204
{
205-
name: "missing a parameter",
205+
name: "missing algorithm - not a V4 request",
206206
query: url.Values{
207207
v4AmzCredential: {"AKIAIOSFODNN7EXAMPLE/20230101/us-east-1/s3/aws4_request"},
208208
v4AmzSignature: {"abcd1234"},
209209
v4AmzDate: {"20230101T120000Z"},
210210
v4AmzSignedHeaders: {"host"},
211211
v4AmzExpires: {"3600"},
212212
},
213-
expected: false,
213+
expectedError: ErrBadAuthorizationFormat,
214+
},
215+
{
216+
name: "invalid algorithm",
217+
query: url.Values{
218+
v4AmzAlgorithm: {"AWS4-HMAC-SHA512"},
219+
v4AmzCredential: {"AKIAIOSFODNN7EXAMPLE/20230101/us-east-1/s3/aws4_request"},
220+
v4AmzSignature: {"abcd1234"},
221+
v4AmzDate: {"20230101T120000Z"},
222+
v4AmzSignedHeaders: {"host"},
223+
v4AmzExpires: {"3600"},
224+
},
225+
expectedError: errors.ErrInvalidQuerySignatureAlgo,
226+
},
227+
{
228+
name: "missing required param (credential)",
229+
query: url.Values{
230+
v4AmzAlgorithm: {"AWS4-HMAC-SHA256"},
231+
v4AmzSignature: {"abcd1234"},
232+
v4AmzDate: {"20230101T120000Z"},
233+
v4AmzSignedHeaders: {"host"},
234+
v4AmzExpires: {"3600"},
235+
},
236+
expectedError: errors.ErrMissingFields,
214237
},
215238
{
216-
name: "parameter is present but empty",
239+
name: "missing required param (expires)",
217240
query: url.Values{
218241
v4AmzAlgorithm: {"AWS4-HMAC-SHA256"},
219242
v4AmzCredential: {"AKIAIOSFODNN7EXAMPLE/20230101/us-east-1/s3/aws4_request"},
220243
v4AmzSignature: {"abcd1234"},
221244
v4AmzDate: {"20230101T120000Z"},
222245
v4AmzSignedHeaders: {"host"},
223-
v4AmzExpires: {""},
224246
},
225-
expected: true, // Key exists in map, even if empty
247+
expectedError: errors.ErrMissingFields,
226248
},
227249
{
228-
name: "empty query params",
229-
query: url.Values{},
230-
expected: false,
250+
name: "empty query params - not a V4 request",
251+
query: url.Values{},
252+
expectedError: ErrBadAuthorizationFormat,
231253
},
232254
}
233255

234256
for _, tc := range testCases {
235257
t.Run(tc.name, func(t *testing.T) {
236-
result := hasRequiredParams(tc.query)
237-
if result != tc.expected {
238-
t.Errorf("expected %v, got %v", tc.expected, result)
258+
err := isV4PresignedRequest(tc.query)
259+
if err != tc.expectedError {
260+
t.Errorf("expected error %v, got %v", tc.expectedError, err)
239261
}
240262
})
241263
}

0 commit comments

Comments
 (0)