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

@@ -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
 	}
 
-	expectedAuth := signatureV2(r.Method, encodedResource, strings.Join(unescapedQueries, "&"), r.Header)
-	if v2Auth != expectedAuth {
+	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 !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.

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.

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
 	}
 

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