Skip to content

Commit 01442fe

Browse files
committed
Add expiry validation for S3 gateway requests
This change adds validation for the `X-Amz-Expires` parameter in presigned URLs and enforces the 15-minute time window for header-based authentication as per the AWS S3 spec. Changes: - Parse `X-Amz-Expires` and validate within allowed range - Check expiration time for presigned URLs and header-based authenticated requests - Add an explicit check for all required parameters for presigned URLs Fixes #9599
1 parent a33e082 commit 01442fe

File tree

3 files changed

+394
-4
lines changed

3 files changed

+394
-4
lines changed

pkg/gateway/sig/sig.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -99,6 +99,8 @@ func (c *chainedAuthenticator) Parse() (SigContext, error) {
9999
if err == nil {
100100
c.chosen = method
101101
return sigContext, nil
102+
} else if !errors.Is(err, ErrHeaderMalformed) {
103+
return nil, err
102104
}
103105
}
104106
return nil, gwErrors.ErrMissingFields

pkg/gateway/sig/v4.go

Lines changed: 100 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,14 @@ const (
3333
v4scopeTerminator = "aws4_request"
3434
v4timeFormat = "20060102T150405Z"
3535
v4shortTimeFormat = "20060102"
36+
maxExpires = 604800
37+
38+
v4AmzAlgorithm = "X-Amz-Algorithm"
39+
v4AmzCredential = "X-Amz-Credential"
40+
v4AmzSignature = "X-Amz-Signature"
41+
v4AmzDate = "X-Amz-Date"
42+
v4AmzSignedHeaders = "X-Amz-SignedHeaders"
43+
v4AmzExpires = "X-Amz-Expires"
3644
)
3745

3846
var (
@@ -43,12 +51,14 @@ var (
4351
type V4Auth struct {
4452
AccessKeyID string
4553
Date string
54+
Expires int64
4655
Region string
4756
Service string
4857
SignedHeaders []string
4958
SignedHeadersString string
5059
Signature string
5160
ChecksumAlgorithm string
61+
IsPresigned bool
5262
}
5363

5464
func (a V4Auth) GetAccessKeyID() string {
@@ -61,6 +71,40 @@ func splitHeaders(headers string) []string {
6171
return headerValues
6272
}
6373

74+
func parseExpires(expiresStr string) (int64, error) {
75+
expires, err := strconv.ParseInt(expiresStr, 10, 64)
76+
if err != nil {
77+
// handles both empty string and non-int string
78+
return 0, errors.ErrMalformedExpires
79+
}
80+
if expires < 0 {
81+
return 0, errors.ErrNegativeExpires
82+
}
83+
if expires > maxExpires {
84+
return 0, errors.ErrMaximumExpires
85+
}
86+
return expires, nil
87+
}
88+
89+
func hasRequiredParams(values url.Values) bool {
90+
var requiredPresignedParams = []string{
91+
v4AmzAlgorithm,
92+
v4AmzCredential,
93+
v4AmzSignature,
94+
v4AmzDate,
95+
v4AmzSignedHeaders,
96+
v4AmzExpires,
97+
}
98+
99+
for _, param := range requiredPresignedParams {
100+
if _, ok := values[param]; !ok {
101+
return false
102+
}
103+
}
104+
105+
return true
106+
}
107+
64108
func ParseV4AuthContext(r *http.Request) (V4Auth, error) {
65109
var ctx V4Auth
66110

@@ -88,16 +132,22 @@ func ParseV4AuthContext(r *http.Request) (V4Auth, error) {
88132
signatureHeaders := result["SignatureHeaders"]
89133
ctx.SignedHeaders = splitHeaders(signatureHeaders)
90134
ctx.SignedHeadersString = signatureHeaders
135+
ctx.IsPresigned = false
91136
return ctx, nil
92137
}
93138

94-
// otherwise, see if we have all the required query parameters
95139
query := r.URL.Query()
96-
algorithm := query.Get("X-Amz-Algorithm")
140+
// otherwise, see if we have all the required query parameters
141+
if !hasRequiredParams(query) {
142+
return ctx, errors.ErrInvalidQueryParams
143+
}
144+
ctx.IsPresigned = true
145+
146+
algorithm := query.Get(v4AmzAlgorithm)
97147
if len(algorithm) == 0 || !strings.EqualFold(algorithm, V4authHeaderPrefix) {
98148
return ctx, errors.ErrInvalidQuerySignatureAlgo
99149
}
100-
credentialScope := query.Get("X-Amz-Credential")
150+
credentialScope := query.Get(v4AmzCredential)
101151
if len(credentialScope) == 0 {
102152
return ctx, errors.ErrMissingCredTag
103153
}
@@ -116,7 +166,13 @@ func ParseV4AuthContext(r *http.Request) (V4Auth, error) {
116166
ctx.Region = credsResult["Region"]
117167
ctx.Service = credsResult["Service"]
118168

119-
ctx.SignedHeadersString = query.Get("X-Amz-SignedHeaders")
169+
expires, err := parseExpires(query.Get(v4AmzExpires))
170+
if err != nil {
171+
return ctx, err
172+
}
173+
ctx.Expires = expires
174+
175+
ctx.SignedHeadersString = query.Get(v4AmzSignedHeaders)
120176
headers := splitHeaders(ctx.SignedHeadersString)
121177
ctx.SignedHeaders = headers
122178
ctx.Signature = query.Get(v4SignatureHeader)
@@ -144,6 +200,11 @@ func V4Verify(auth V4Auth, credentials *model.Credential, r *http.Request) error
144200
return errors.ErrSignatureDoesNotMatch
145201
}
146202

203+
// check that the request hasn't expired
204+
if err := ctx.verifyExpiration(); err != nil {
205+
return err
206+
}
207+
147208
// wrap body with verifier
148209
reader, err := ctx.reader(r.Body, credentials)
149210
if err != nil {
@@ -293,6 +354,41 @@ func (ctx *verificationCtx) getAmzDate() (string, error) {
293354
return amzDate, nil
294355
}
295356

357+
func (ctx *verificationCtx) verifyExpiration() error {
358+
// Get and validate the request timestamp
359+
amzDate, err := ctx.getAmzDate()
360+
if err != nil {
361+
return err
362+
}
363+
364+
requestTime, err := time.Parse(v4timeFormat, amzDate)
365+
if err != nil {
366+
return err
367+
}
368+
369+
now := time.Now().UTC()
370+
timeDiff := now.Sub(requestTime)
371+
372+
// Check for requests from the future and allow small clock skew
373+
if timeDiff < 0 && timeDiff.Abs() > 5*time.Minute {
374+
return errors.ErrRequestNotReadyYet
375+
}
376+
377+
if ctx.AuthValue.IsPresigned {
378+
expirationTime := requestTime.Add(time.Duration(ctx.AuthValue.Expires) * time.Second)
379+
if now.After(expirationTime) {
380+
return errors.ErrExpiredPresignRequest
381+
}
382+
} else {
383+
// Header based auth requests are valid for 15 minutes
384+
if timeDiff.Abs() > 15*time.Minute {
385+
return errors.ErrExpiredPresignRequest
386+
}
387+
}
388+
389+
return nil
390+
}
391+
296392
func sign(key []byte, msg string) []byte {
297393
h := hmac.New(sha256.New, key)
298394
_, _ = h.Write([]byte(msg))

0 commit comments

Comments
 (0)