From 0c656d1755f95c01fdf673a6cc37de6941197887 Mon Sep 17 00:00:00 2001 From: Ke Wang Date: Wed, 10 Jul 2024 17:52:31 +0800 Subject: [PATCH] feat(auth): add authentication expiration check in signature-v4 --- signature/signature-errors.go | 12 ++++ signature/signature-v4.go | 25 +++++++- signature/signature-v4_test.go | 114 +++++++++++++++++++++++++++++++++ 3 files changed, 150 insertions(+), 1 deletion(-) diff --git a/signature/signature-errors.go b/signature/signature-errors.go index 9397a6e..7362bb2 100644 --- a/signature/signature-errors.go +++ b/signature/signature-errors.go @@ -39,8 +39,10 @@ const ( errUnsignedHeaders errMissingDateHeader errMalformedDate + errMalformedExpires errUnsupportAlgorithm errSignatureDoesNotMatch + errExpiredRequest // ErrNone is None(err=nil) ErrNone @@ -109,6 +111,11 @@ var errorCodes = errorCodeMap{ Description: "Invalid date format header, expected to be in ISO8601, RFC1123 or RFC1123Z time format.", HTTPStatusCode: http.StatusBadRequest, }, + errMalformedExpires: { + Code: "MalformedExpires", + Description: "Invalid X-Amz-Expires, expected to be a number", + HTTPStatusCode: http.StatusBadRequest, + }, errUnsupportAlgorithm: { Code: "UnsupportedAlgorithm", Description: "Encountered an unsupported algorithm.", @@ -119,6 +126,11 @@ var errorCodes = errorCodeMap{ Description: "The request signature we calculated does not match the signature you provided. Check your key and signing method.", HTTPStatusCode: http.StatusForbidden, }, + errExpiredRequest: { + Code: "AccessDenied", + Description: "The difference between the request time and the server's time exceeds the maximum allowed.", + HTTPStatusCode: http.StatusBadRequest, + }, } // EncodeResponse encodes the response headers into XML format. diff --git a/signature/signature-v4.go b/signature/signature-v4.go index f26ea40..93da2ab 100644 --- a/signature/signature-v4.go +++ b/signature/signature-v4.go @@ -8,10 +8,16 @@ import ( "fmt" "net/http" "sort" + "strconv" "strings" "time" ) +var ( + // TimeNow is a variable that holds the function to get the current time + TimeNow = time.Now +) + // ref: https://github.com/minio/minio/cmd/auth-handler.go const ( @@ -30,7 +36,7 @@ const ( amzCredential = "X-Amz-Credential" amzSignedHeaders = "X-Amz-SignedHeaders" amzSignature = "X-Amz-Signature" - amzexpires = "X-Amz-Expires" + amzExpires = "X-Amz-Expires" ) // getCanonicalHeaders generate a list of request headers with their values @@ -192,6 +198,23 @@ func V4SignVerify(r *http.Request) ErrorCode { return errMalformedDate } + // Check expiration + expiresStr := queryf.Get(amzExpires) + var expires time.Duration + if expiresStr == "" { + // If expires is not set, use the default of 15 minutes + expires = 15 * time.Minute + } else { + expiresInt, err := strconv.ParseInt(expiresStr, 10, 64) + if err != nil { + return errMalformedExpires + } + expires = time.Duration(expiresInt) * time.Second + } + if TimeNow().After(t.Add(expires)) { + return errExpiredRequest + } + // Query string. queryf.Del(amzSignature) rawquery := queryf.Encode() diff --git a/signature/signature-v4_test.go b/signature/signature-v4_test.go index 2e9783e..7c3bf5a 100644 --- a/signature/signature-v4_test.go +++ b/signature/signature-v4_test.go @@ -121,3 +121,117 @@ func TestUnsignedPayload(t *testing.T) { t.Errorf("invalid result for unsigned payload: expect none but got %+v", signature.GetAPIError(result)) } } + +func TestCheckExpiration(t *testing.T) { + originalTimeNow := signature.TimeNow + defer func() { signature.TimeNow = originalTimeNow }() + + testCases := []struct { + name string + useQueryString bool + expiresIn string + timeDelta time.Duration + expectedError bool + }{ + { + name: "Valid Header-based Authentication (Default 15min)", + useQueryString: false, + expiresIn: "", + timeDelta: 14 * time.Minute, + expectedError: false, + }, + { + name: "Expired Header-based Authentication (Default 15min)", + useQueryString: false, + expiresIn: "", + timeDelta: 16 * time.Minute, + expectedError: true, + }, + { + name: "Valid Query-based Authentication (30min)", + useQueryString: true, + expiresIn: "1800", // 30 minutes + timeDelta: 29 * time.Minute, + expectedError: false, + }, + { + name: "Expired Query-based Authentication (30min)", + useQueryString: true, + expiresIn: "1800", // 30 minutes + timeDelta: 31 * time.Minute, + expectedError: true, + }, + { + name: "Valid Query-based Authentication (5min)", + useQueryString: true, + expiresIn: "300", // 5 minutes + timeDelta: 4 * time.Minute, + expectedError: false, + }, + { + name: "Expired Query-based Authentication (5min)", + useQueryString: true, + expiresIn: "300", // 5 minutes + timeDelta: 6 * time.Minute, + expectedError: true, + }, + { + name: "Malformed Expires", + useQueryString: true, + expiresIn: "not-a-number", + timeDelta: 0, + expectedError: true, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + Body := bytes.NewReader(nil) + ak := RandString(32) + sk := RandString(64) + region := RandString(16) + + creds := credentials.NewStaticCredentials(ak, sk, "") + signature.ReloadKeys(map[string]string{ak: sk}) + signer := v4.NewSigner(creds) + + req, err := http.NewRequest(http.MethodPost, "https://s3-endpoint.example.com/bin", Body) + if err != nil { + t.Fatal(err) + } + + now := time.Now() + signature.TimeNow = func() time.Time { return now } + + if tc.useQueryString { + // For query-based authentication + req.URL.RawQuery = url.Values{ + "X-Amz-Algorithm": []string{signV4Algorithm}, + "X-Amz-Credential": []string{fmt.Sprintf("%s/%s/%s/%s/aws4_request", ak, now.Format(yyyymmdd), region, serviceS3)}, + "X-Amz-Date": []string{now.Format(iso8601Format)}, + "X-Amz-Expires": []string{tc.expiresIn}, + "X-Amz-SignedHeaders": []string{"host"}, + }.Encode() + } else { + // For header-based authentication + req.Header.Set("X-Amz-Date", now.Format(iso8601Format)) + } + + _, err = signer.Sign(req, Body, serviceS3, region, now) + if err != nil { + t.Fatal(err) + } + + // Mock time passing + signature.TimeNow = func() time.Time { return now.Add(tc.timeDelta) } + + result := signature.V4SignVerify(req) + if result == signature.ErrNone && tc.expectedError { + t.Errorf("invalid result: expected error but got no error") + } + if result != signature.ErrNone && !tc.expectedError { + t.Errorf("invalid result: didn't expect error but got error") + } + }) + } +}