diff --git a/src/crypto/internal/boring/rsa.go b/src/crypto/internal/boring/rsa.go index f4c4193c00..a1f85591a7 100644 --- a/src/crypto/internal/boring/rsa.go +++ b/src/crypto/internal/boring/rsa.go @@ -245,14 +245,28 @@ func encrypt(ctx *C.GO_EVP_PKEY_CTX, out *C.uint8_t, outLen *C.size_t, in *C.uin return C._goboringcrypto_EVP_PKEY_encrypt(ctx, out, outLen, in, inLen) } +var invalidSaltLenErr = errors.New("crypto/rsa: PSSOptions.SaltLength cannot be negative") + func SignRSAPSS(priv *PrivateKeyRSA, h crypto.Hash, hashed []byte, saltLen int) ([]byte, error) { md := cryptoHashToMD(h) if md == nil { return nil, errors.New("crypto/rsa: unsupported hash function") } - if saltLen == 0 { - saltLen = -1 + + // A salt length of -2 is valid in BoringSSL, but not in crypto/rsa, so reject + // it, and lengths < -2, before we convert to the BoringSSL sentinel values. + if saltLen <= -2 { + return nil, invalidSaltLenErr } + + // BoringSSL uses sentinel salt length values like we do, but the values don't + // fully match what we use. We both use -1 for salt length equal to hash length, + // but BoringSSL uses -2 to mean maximal size where we use 0. In the latter + // case convert to the BoringSSL version. + if saltLen == 0 { + saltLen = -2 + } + var out []byte var outLen C.size_t if priv.withKey(func(key *C.GO_RSA) C.int { @@ -271,9 +285,21 @@ func VerifyRSAPSS(pub *PublicKeyRSA, h crypto.Hash, hashed, sig []byte, saltLen if md == nil { return errors.New("crypto/rsa: unsupported hash function") } - if saltLen == 0 { - saltLen = -2 // auto-recover + + // A salt length of -2 is valid in BoringSSL, but not in crypto/rsa, so reject + // it, and lengths < -2, before we convert to the BoringSSL sentinel values. + if saltLen <= -2 { + return invalidSaltLenErr } + + // BoringSSL uses sentinel salt length values like we do, but the values don't + // fully match what we use. We both use -1 for salt length equal to hash length, + // but BoringSSL uses -2 to mean maximal size where we use 0. In the latter + // case convert to the BoringSSL version. + if saltLen == 0 { + saltLen = -2 + } + if pub.withKey(func(key *C.GO_RSA) C.int { return C._goboringcrypto_RSA_verify_pss_mgf1(key, base(hashed), C.size_t(len(hashed)), md, nil, C.int(saltLen), base(sig), C.size_t(len(sig))) diff --git a/src/crypto/rsa/pss.go b/src/crypto/rsa/pss.go index 29e79bd342..a3e9bfc83b 100644 --- a/src/crypto/rsa/pss.go +++ b/src/crypto/rsa/pss.go @@ -249,8 +249,8 @@ const ( // PSSOptions contains options for creating and verifying PSS signatures. type PSSOptions struct { - // SaltLength controls the length of the salt used in the PSS - // signature. It can either be a number of bytes, or one of the special + // SaltLength controls the length of the salt used in the PSS signature. It + // can either be a positive number of bytes, or one of the special // PSSSaltLength constants. SaltLength int @@ -272,12 +272,23 @@ func (opts *PSSOptions) saltLength() int { return opts.SaltLength } +var invalidSaltLenErr = errors.New("crypto/rsa: PSSOptions.SaltLength cannot be negative") + // SignPSS calculates the signature of digest using PSS. // // digest must be the result of hashing the input message using the given hash // function. The opts argument may be nil, in which case sensible defaults are // used. If opts.Hash is set, it overrides hash. func SignPSS(rand io.Reader, priv *PrivateKey, hash crypto.Hash, digest []byte, opts *PSSOptions) ([]byte, error) { + if boring.Enabled && rand == boring.RandReader { + bkey, err := boringPrivateKey(priv) + if err != nil { + return nil, err + } + return boring.SignRSAPSS(bkey, hash, digest, opts.saltLength()) + } + boring.UnreachableExceptTests() + if opts != nil && opts.Hash != 0 { hash = opts.Hash } @@ -288,17 +299,13 @@ func SignPSS(rand io.Reader, priv *PrivateKey, hash crypto.Hash, digest []byte, saltLength = (priv.N.BitLen()-1+7)/8 - 2 - hash.Size() case PSSSaltLengthEqualsHash: saltLength = hash.Size() - } - - if boring.Enabled && rand == boring.RandReader { - bkey, err := boringPrivateKey(priv) - if err != nil { - return nil, err + default: + // If we get here saltLength is either > 0 or < -1, in the + // latter case we fail out. + if saltLength <= 0 { + return nil, invalidSaltLenErr } - return boring.SignRSAPSS(bkey, hash, digest, saltLength) } - boring.UnreachableExceptTests() - salt := make([]byte, saltLength) if _, err := io.ReadFull(rand, salt); err != nil { return nil, err @@ -326,6 +333,12 @@ func VerifyPSS(pub *PublicKey, hash crypto.Hash, digest []byte, sig []byte, opts if len(sig) != pub.Size() { return ErrVerification } + // Salt length must be either one of the special constants (-1 or 0) + // or otherwise positive. If it is < PSSSaltLengthEqualsHash (-1) + // we return an error. + if opts.saltLength() < PSSSaltLengthEqualsHash { + return invalidSaltLenErr + } s := new(big.Int).SetBytes(sig) m := encrypt(new(big.Int), pub, s) emBits := pub.N.BitLen() - 1 diff --git a/src/crypto/rsa/pss_test.go b/src/crypto/rsa/pss_test.go index 51f9760187..ecc02e47d6 100644 --- a/src/crypto/rsa/pss_test.go +++ b/src/crypto/rsa/pss_test.go @@ -208,6 +208,9 @@ func TestPSSSigning(t *testing.T) { {PSSSaltLengthEqualsHash, 8, false}, {PSSSaltLengthAuto, PSSSaltLengthEqualsHash, false}, {8, 8, true}, + {PSSSaltLengthAuto, 42, true}, + {PSSSaltLengthAuto, 20, false}, + {PSSSaltLengthAuto, -2, false}, } hash := crypto.SHA1 @@ -273,3 +276,28 @@ func fromHex(hexStr string) []byte { } return s } + +func TestInvalidPSSSaltLength(t *testing.T) { + key, err := GenerateKey(rand.Reader, 245) + if err != nil { + t.Fatal(err) + } + + digest := sha256.Sum256([]byte("message")) + // We don't check the exact error matches, because crypto/rsa and crypto/internal/boring + // return two different error variables, which have the same content but are not equal. + if _, err := SignPSS(rand.Reader, key, crypto.SHA256, digest[:], &PSSOptions{ + SaltLength: -2, + Hash: crypto.SHA256, + }); err.Error() != invalidSaltLenErr.Error() { + t.Fatalf("SignPSS unexpected error: got %v, want %v", err, invalidSaltLenErr) + } + + // We don't check the specific error here, because crypto/rsa and crypto/internal/boring + // return different errors, so we just check that _an error_ was returned. + if err := VerifyPSS(&key.PublicKey, crypto.SHA256, []byte{1, 2, 3}, make([]byte, 31), &PSSOptions{ + SaltLength: -2, + }); err == nil { + t.Fatal("VerifyPSS unexpected success") + } +}