From 267a0a3dfa3fc36518caf1034e84e6d964038d52 Mon Sep 17 00:00:00 2001 From: Andreas Auernhammer Date: Tue, 11 Sep 2018 20:17:23 +0200 Subject: [PATCH] fix `X-Amz-Credential` parsing for V4 policy signature (#6451) This commit fixes an AWS S3 incompatibility issue. The AccessKeyID may contain one or more `/` which caused the server to interpret parts of the AccessKeyID as other `X-Amz-Credential` parameters (like date, region, ...) This commit fixes this by allowing 5 or more `X-Amz-Credential` parameter strings and only interpreting the last 5. Fixes #6443 --- cmd/signature-v4-parser.go | 20 +++++++++++--------- cmd/signature-v4-parser_test.go | 19 +++++++++++++++++++ 2 files changed, 30 insertions(+), 9 deletions(-) diff --git a/cmd/signature-v4-parser.go b/cmd/signature-v4-parser.go index 499f742f4..13b343f52 100644 --- a/cmd/signature-v4-parser.go +++ b/cmd/signature-v4-parser.go @@ -56,23 +56,25 @@ func parseCredentialHeader(credElement string, region string) (ch credentialHead return ch, ErrMissingCredTag } credElements := strings.Split(strings.TrimSpace(creds[1]), "/") - if len(credElements) != 5 { + if len(credElements) < 5 { return ch, ErrCredMalformed } - if !auth.IsAccessKeyValid(credElements[0]) { + accessKey := strings.Join(credElements[:len(credElements)-4], "/") // The access key may contain one or more `/` + if !auth.IsAccessKeyValid(accessKey) { return ch, ErrInvalidAccessKeyID } // Save access key id. cred := credentialHeader{ - accessKey: credElements[0], + accessKey: accessKey, } + credElements = credElements[len(credElements)-4:] var e error - cred.scope.date, e = time.Parse(yyyymmdd, credElements[1]) + cred.scope.date, e = time.Parse(yyyymmdd, credElements[0]) if e != nil { return ch, ErrMalformedCredentialDate } - cred.scope.region = credElements[2] + cred.scope.region = credElements[1] // Verify if region is valid. sRegion := cred.scope.region // Region is set to be empty, we use whatever was sent by the @@ -87,14 +89,14 @@ func parseCredentialHeader(credElement string, region string) (ch credentialHead return ch, ErrAuthorizationHeaderMalformed } - if credElements[3] != "s3" { + if credElements[2] != "s3" { return ch, ErrInvalidService } - cred.scope.service = credElements[3] - if credElements[4] != "aws4_request" { + cred.scope.service = credElements[2] + if credElements[3] != "aws4_request" { return ch, ErrInvalidRequestVersion } - cred.scope.request = credElements[4] + cred.scope.request = credElements[3] return cred, ErrNone } diff --git a/cmd/signature-v4-parser_test.go b/cmd/signature-v4-parser_test.go index b340fadab..712864874 100644 --- a/cmd/signature-v4-parser_test.go +++ b/cmd/signature-v4-parser_test.go @@ -197,6 +197,25 @@ func TestParseCredentialHeader(t *testing.T) { "aws4_request"), expectedErrCode: ErrNone, }, + // Test Case - 10. + // Test case with right inputs -> AccessKey contains `/`. See minio/#6443 + // "aws4_request" is the valid request version. + { + inputCredentialStr: generateCredentialStr( + "LOCALKEY/DEV/1", + sampleTimeStr, + "us-west-1", + "s3", + "aws4_request"), + expectedCredentials: generateCredentials( + t, + "LOCALKEY/DEV/1", + sampleTimeStr, + "us-west-1", + "s3", + "aws4_request"), + expectedErrCode: ErrNone, + }, } for i, testCase := range testCases {