From a6318dbdaf0a33d80e52bb762e153e4d7ca396f7 Mon Sep 17 00:00:00 2001 From: Andreas Auernhammer Date: Tue, 2 Jan 2018 07:30:02 +0100 Subject: [PATCH] fix timing oracle attack against signature V2/V4 verification (#5335) This change replaces the non-constant time comparison of request signatures with a constant time implementation. This prevents a timing attack which can be used to learn a valid signature for a request without knowing the secret key. Fixes #5334 --- cmd/signature-v2.go | 38 +++++++++++++++++++++++++++------ cmd/signature-v4-parser_test.go | 4 ++-- cmd/signature-v4.go | 18 ++++++++++++---- cmd/streaming-signature-v4.go | 6 +++--- 4 files changed, 51 insertions(+), 15 deletions(-) diff --git a/cmd/signature-v2.go b/cmd/signature-v2.go index 12edd3bdd..39230ce4b 100644 --- a/cmd/signature-v2.go +++ b/cmd/signature-v2.go @@ -19,6 +19,7 @@ package cmd import ( "crypto/hmac" "crypto/sha1" + "crypto/subtle" "encoding/base64" "fmt" "net/http" @@ -71,7 +72,7 @@ func doesPolicySignatureV2Match(formValues http.Header) APIErrorCode { } policy := formValues.Get("Policy") signature := formValues.Get("Signature") - if signature != calculateSignatureV2(policy, cred.SecretKey) { + if !compareSignatureV2(signature, calculateSignatureV2(policy, cred.SecretKey)) { return ErrSignatureDoesNotMatch } return ErrNone @@ -124,6 +125,9 @@ func doesPresignV2SignatureMatch(r *http.Request) APIErrorCode { // Extract the necessary values from presigned query, construct a list of new filtered queries. for _, query := range unescapedQueries { keyval := strings.SplitN(query, "=", 2) + if len(keyval) != 2 { + return ErrInvalidQueryParams + } switch keyval[0] { case "AWSAccessKeyId": accessKey = keyval[1] @@ -163,7 +167,7 @@ func doesPresignV2SignatureMatch(r *http.Request) APIErrorCode { } expectedSignature := preSignatureV2(r.Method, encodedResource, strings.Join(filteredQueries, "&"), r.Header, expires) - if gotSignature != expectedSignature { + if !compareSignatureV2(gotSignature, expectedSignature) { return ErrSignatureDoesNotMatch } @@ -247,11 +251,15 @@ func doesSignV2Match(r *http.Request) APIErrorCode { return ErrInvalidRequest } + prefix := fmt.Sprintf("%s %s:", signV2Algorithm, globalServerConfig.GetCredential().AccessKey) + if !strings.HasPrefix(v2Auth, prefix) { + return ErrSignatureDoesNotMatch + } + v2Auth = v2Auth[len(prefix):] expectedAuth := signatureV2(r.Method, encodedResource, strings.Join(unescapedQueries, "&"), r.Header) - if v2Auth != expectedAuth { + if !compareSignatureV2(v2Auth, expectedAuth) { return ErrSignatureDoesNotMatch } - return ErrNone } @@ -268,12 +276,30 @@ func preSignatureV2(method string, encodedResource string, encodedQuery string, return calculateSignatureV2(stringToSign, cred.SecretKey) } -// Return signature-v2 authrization header. +// Return the signature v2 of a given request. func signatureV2(method string, encodedResource string, encodedQuery string, headers http.Header) string { cred := globalServerConfig.GetCredential() stringToSign := getStringToSignV2(method, encodedResource, encodedQuery, headers, "") signature := calculateSignatureV2(stringToSign, cred.SecretKey) - return fmt.Sprintf("%s %s:%s", signV2Algorithm, cred.AccessKey, signature) + return signature +} + +// compareSignatureV2 returns true if and only if both signatures +// are equal. The signatures are expected to be base64 encoded strings +// according to the AWS S3 signature V2 spec. +func compareSignatureV2(sig1, sig2 string) bool { + // Decode signature string to binary byte-sequence representation is required + // as Base64 encoding of a value is not unique: + // For example "aGVsbG8=" and "aGVsbG8=\r" will result in the same byte slice. + signature1, err := base64.StdEncoding.DecodeString(sig1) + if err != nil { + return false + } + signature2, err := base64.StdEncoding.DecodeString(sig2) + if err != nil { + return false + } + return subtle.ConstantTimeCompare(signature1, signature2) == 1 } // Return canonical headers. diff --git a/cmd/signature-v4-parser_test.go b/cmd/signature-v4-parser_test.go index d3652f999..594e7f8b7 100644 --- a/cmd/signature-v4-parser_test.go +++ b/cmd/signature-v4-parser_test.go @@ -427,7 +427,7 @@ func TestParseSignV4(t *testing.T) { validateCredentialfields(t, i+1, testCase.expectedAuthField.Credential, parsedAuthField.Credential) // validating the extraction/parsing of signature field. - if testCase.expectedAuthField.Signature != parsedAuthField.Signature { + if !compareSignatureV4(testCase.expectedAuthField.Signature, parsedAuthField.Signature) { t.Errorf("Test %d: Parsed Signature field mismatch: Expected \"%s\", got \"%s\"", i+1, testCase.expectedAuthField.Signature, parsedAuthField.Signature) } @@ -795,7 +795,7 @@ func TestParsePreSignV4(t *testing.T) { t.Errorf("Test %d: Expected the result to be \"%v\", but got \"%v\". ", i+1, testCase.expectedPreSignValues.SignedHeaders, parsedPreSign.SignedHeaders) } // validating signature field. - if testCase.expectedPreSignValues.Signature != parsedPreSign.Signature { + if !compareSignatureV4(testCase.expectedPreSignValues.Signature, parsedPreSign.Signature) { t.Errorf("Test %d: Signature field mismatch: Expected \"%s\", got \"%s\"", i+1, testCase.expectedPreSignValues.Signature, parsedPreSign.Signature) } // validating expiry duration. diff --git a/cmd/signature-v4.go b/cmd/signature-v4.go index c765235d0..e6142dcd1 100644 --- a/cmd/signature-v4.go +++ b/cmd/signature-v4.go @@ -26,6 +26,7 @@ package cmd import ( "bytes" + "crypto/subtle" "encoding/hex" "net/http" "net/url" @@ -34,7 +35,7 @@ import ( "strings" "time" - "github.com/minio/sha256-simd" + sha256 "github.com/minio/sha256-simd" ) // AWS Signature Version '4' constants. @@ -146,6 +147,15 @@ func doesPolicySignatureMatch(formValues http.Header) APIErrorCode { return doesPolicySignatureV4Match(formValues) } +// compareSignatureV4 returns true if and only if both signatures +// are equal. The signatures are expected to be HEX encoded strings +// according to the AWS S3 signature V4 spec. +func compareSignatureV4(sig1, sig2 string) bool { + // The CTC using []byte(str) works because the hex encoding + // is unique for a sequence of bytes. See also compareSignatureV2. + return subtle.ConstantTimeCompare([]byte(sig1), []byte(sig2)) == 1 +} + // doesPolicySignatureMatch - Verify query headers with post policy // - http://docs.aws.amazon.com/AmazonS3/latest/API/sigv4-HTTPPOSTConstructPolicy.html // returns ErrNone if the signature matches. @@ -180,7 +190,7 @@ func doesPolicySignatureV4Match(formValues http.Header) APIErrorCode { newSignature := getSignature(signingKey, formValues.Get("Policy")) // Verify signature. - if newSignature != formValues.Get("X-Amz-Signature") { + if !compareSignatureV4(newSignature, formValues.Get("X-Amz-Signature")) { return ErrSignatureDoesNotMatch } @@ -301,7 +311,7 @@ func doesPresignedSignatureMatch(hashedPayload string, r *http.Request, region s newSignature := getSignature(presignedSigningKey, presignedStringToSign) // Verify signature. - if req.URL.Query().Get("X-Amz-Signature") != newSignature { + if !compareSignatureV4(req.URL.Query().Get("X-Amz-Signature"), newSignature) { return ErrSignatureDoesNotMatch } return ErrNone @@ -380,7 +390,7 @@ func doesSignatureMatch(hashedPayload string, r *http.Request, region string) AP newSignature := getSignature(signingKey, stringToSign) // Verify if signature match. - if newSignature != signV4Values.Signature { + if !compareSignatureV4(newSignature, signV4Values.Signature) { return ErrSignatureDoesNotMatch } diff --git a/cmd/streaming-signature-v4.go b/cmd/streaming-signature-v4.go index 0309a62a3..6b53bea9e 100644 --- a/cmd/streaming-signature-v4.go +++ b/cmd/streaming-signature-v4.go @@ -29,7 +29,7 @@ import ( "time" humanize "github.com/dustin/go-humanize" - "github.com/minio/sha256-simd" + sha256 "github.com/minio/sha256-simd" ) // Streaming AWS Signature Version '4' constants. @@ -142,7 +142,7 @@ func calculateSeedSignature(r *http.Request) (signature string, region string, d newSignature := getSignature(signingKey, stringToSign) // Verify if signature match. - if newSignature != signV4Values.Signature { + if !compareSignatureV4(newSignature, signV4Values.Signature) { return "", "", time.Time{}, ErrSignatureDoesNotMatch } @@ -308,7 +308,7 @@ func (cr *s3ChunkedReader) Read(buf []byte) (n int, err error) { hashedChunk := hex.EncodeToString(cr.chunkSHA256Writer.Sum(nil)) // Calculate the chunk signature. newSignature := getChunkSignature(cr.seedSignature, cr.region, cr.seedDate, hashedChunk) - if cr.chunkSignature != newSignature { + if !compareSignatureV4(cr.chunkSignature, newSignature) { // Chunk signature doesn't match we return signature does not match. cr.err = errSignatureMismatch return 0, cr.err