crypto/rsa: refactor PKCS#1 v1.5 signature and verification

VerifyPKCS1v15 doesn't need to be constant time and can do the safer and
simpler construct-and-compare.

Updates #67043

Change-Id: I014cfd4485fad409c5f86be71488da63af25a584
Reviewed-on: https://go-review.googlesource.com/c/go/+/587278
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Roland Shoemaker <roland@golang.org>
Reviewed-by: Carlos Amedee <carlos@golang.org>
Auto-Submit: Filippo Valsorda <filippo@golang.org>

1 files changed, 35 insertions, 53 deletions

diff --git a/src/crypto/rsa/pkcs1v15.go b/src/crypto/rsa/pkcs1v15.go
index 84b19fbcb4..2f958022f9 100644
--- a/src/crypto/rsa/pkcs1v15.go
+++ b/src/crypto/rsa/pkcs1v15.go
@@ -5,6 +5,7 @@
 package rsa
 
 import (
+	"bytes"
 	"crypto"
 	"crypto/internal/boring"
 	"crypto/internal/randutil"
@@ -285,17 +286,13 @@ var hashPrefixes = map[crypto.Hash][]byte{
 // messages to signatures and identify the signed messages. As ever,
 // signatures provide authenticity, not confidentiality.
 func SignPKCS1v15(random io.Reader, priv *PrivateKey, hash crypto.Hash, hashed []byte) ([]byte, error) {
-	hashLen, prefix, err := pkcs1v15HashInfo(hash, len(hashed))
+	// pkcs1v15ConstructEM is called before boring.SignRSAPKCS1v15 to return
+	// consistent errors, including ErrMessageTooLong.
+	em, err := pkcs1v15ConstructEM(&priv.PublicKey, hash, hashed)
 	if err != nil {
 		return nil, err
 	}
 
-	tLen := len(prefix) + hashLen
-	k := priv.Size()
-	if k < tLen+11 {
-		return nil, ErrMessageTooLong
-	}
-
 	if boring.Enabled {
 		bkey, err := boringPrivateKey(priv)
 		if err != nil {
@@ -304,16 +301,37 @@ func SignPKCS1v15(random io.Reader, priv *PrivateKey, hash crypto.Hash, hashed [
 		return boring.SignRSAPKCS1v15(bkey, hash, hashed)
 	}
 
+	return decrypt(priv, em, withCheck)
+}
+
+func pkcs1v15ConstructEM(pub *PublicKey, hash crypto.Hash, hashed []byte) ([]byte, error) {
+	// Special case: crypto.Hash(0) is used to indicate that the data is
+	// signed directly.
+	var prefix []byte
+	if hash != 0 {
+		if len(hashed) != hash.Size() {
+			return nil, errors.New("crypto/rsa: input must be hashed message")
+		}
+		var ok bool
+		prefix, ok = hashPrefixes[hash]
+		if !ok {
+			return nil, errors.New("crypto/rsa: unsupported hash function")
+		}
+	}
+
 	// EM = 0x00 || 0x01 || PS || 0x00 || T
+	k := pub.Size()
+	if k < len(prefix)+len(hashed)+2+8+1 {
+		return nil, ErrMessageTooLong
+	}
 	em := make([]byte, k)
 	em[1] = 1
-	for i := 2; i < k-tLen-1; i++ {
+	for i := 2; i < k-len(prefix)-len(hashed)-1; i++ {
 		em[i] = 0xff
 	}
-	copy(em[k-tLen:k-hashLen], prefix)
-	copy(em[k-hashLen:k], hashed)
-
-	return decrypt(priv, em, withCheck)
+	copy(em[k-len(prefix)-len(hashed):], prefix)
+	copy(em[k-len(hashed):], hashed)
+	return em, nil
 }
 
 // VerifyPKCS1v15 verifies an RSA PKCS #1 v1.5 signature.
@@ -336,21 +354,10 @@ func VerifyPKCS1v15(pub *PublicKey, hash crypto.Hash, hashed []byte, sig []byte)
 		return nil
 	}
 
-	hashLen, prefix, err := pkcs1v15HashInfo(hash, len(hashed))
-	if err != nil {
-		return err
-	}
-
-	tLen := len(prefix) + hashLen
-	k := pub.Size()
-	if k < tLen+11 {
-		return ErrVerification
-	}
-
 	// RFC 8017 Section 8.2.2: If the length of the signature S is not k
 	// octets (where k is the length in octets of the RSA modulus n), output
 	// "invalid signature" and stop.
-	if k != len(sig) {
+	if pub.Size() != len(sig) {
 		return ErrVerification
 	}
 
@@ -358,39 +365,14 @@ func VerifyPKCS1v15(pub *PublicKey, hash crypto.Hash, hashed []byte, sig []byte)
 	if err != nil {
 		return ErrVerification
 	}
-	// EM = 0x00 || 0x01 || PS || 0x00 || T
-
-	ok := subtle.ConstantTimeByteEq(em[0], 0)
-	ok &= subtle.ConstantTimeByteEq(em[1], 1)
-	ok &= subtle.ConstantTimeCompare(em[k-hashLen:k], hashed)
-	ok &= subtle.ConstantTimeCompare(em[k-tLen:k-hashLen], prefix)
-	ok &= subtle.ConstantTimeByteEq(em[k-tLen-1], 0)
 
-	for i := 2; i < k-tLen-1; i++ {
-		ok &= subtle.ConstantTimeByteEq(em[i], 0xff)
+	expected, err := pkcs1v15ConstructEM(pub, hash, hashed)
+	if err != nil {
+		return ErrVerification
 	}
-
-	if ok != 1 {
+	if !bytes.Equal(em, expected) {
 		return ErrVerification
 	}
 
 	return nil
 }
-
-func pkcs1v15HashInfo(hash crypto.Hash, inLen int) (hashLen int, prefix []byte, err error) {
-	// Special case: crypto.Hash(0) is used to indicate that the data is
-	// signed directly.
-	if hash == 0 {
-		return inLen, nil, nil
-	}
-
-	hashLen = hash.Size()
-	if inLen != hashLen {
-		return 0, nil, errors.New("crypto/rsa: input must be hashed message")
-	}
-	prefix, ok := hashPrefixes[hash]
-	if !ok {
-		return 0, nil, errors.New("crypto/rsa: unsupported hash function")
-	}
-	return
-}