From 2f07d4455636ece45ff843fe4d9298ea65f933c1 Mon Sep 17 00:00:00 2001 From: Filippo Valsorda Date: Wed, 22 May 2024 13:38:15 +0200 Subject: [PATCH] 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 Reviewed-by: Roland Shoemaker Reviewed-by: Carlos Amedee Auto-Submit: Filippo Valsorda --- src/crypto/rsa/pkcs1v15.go | 88 +++++++++++++++----------------------- 1 file 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 -} -- 2.48.1