Skip to content

Commit

Permalink
feat(auth): add authentication expiration check in signature-v4
Browse files Browse the repository at this point in the history
  • Loading branch information
KevinWang15 authored and ncw committed Jul 15, 2024
1 parent d61b9c9 commit 0c656d1
Show file tree
Hide file tree
Showing 3 changed files with 150 additions and 1 deletion.
12 changes: 12 additions & 0 deletions signature/signature-errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,8 +39,10 @@ const (
errUnsignedHeaders
errMissingDateHeader
errMalformedDate
errMalformedExpires
errUnsupportAlgorithm
errSignatureDoesNotMatch
errExpiredRequest

// ErrNone is None(err=nil)
ErrNone
Expand Down Expand Up @@ -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.",
Expand All @@ -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.
Expand Down
25 changes: 24 additions & 1 deletion signature/signature-v4.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 (
Expand All @@ -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
Expand Down Expand Up @@ -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()
Expand Down
114 changes: 114 additions & 0 deletions signature/signature-v4_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
}
})
}
}

0 comments on commit 0c656d1

Please sign in to comment.