]> Cypherpunks repositories - gostls13.git/commitdiff
crypto: use provided random Reader in FIPS mode
authorFilippo Valsorda <filippo@golang.org>
Wed, 11 Dec 2024 13:50:00 +0000 (14:50 +0100)
committerGopher Robot <gobot@golang.org>
Wed, 11 Dec 2024 21:26:50 +0000 (13:26 -0800)
This removes the difference in behavior between FIPS mode on and off.

Instead of the sentinel type we could have moved the Reader to the
drbg package and checked for equality, but then we would have locked the
crypto/rand.Reader implementation to the one in the FIPS module (which
we might have to support for years).

In internal/ed25519.GenerateKey we remove the random parameter entirely,
since that function is not actually used by crypto/ed25519.GenerateKey,
which instead commits to being deterministic.

Fixes #70772

Change-Id: Ic1c7ca2c1cd59eb9cd090a8b235c0ce218921ac5
Reviewed-on: https://go-review.googlesource.com/c/go/+/635195
Reviewed-by: Roland Shoemaker <roland@golang.org>
Auto-Submit: Filippo Valsorda <filippo@golang.org>
Reviewed-by: Russ Cox <rsc@golang.org>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>

15 files changed:
src/crypto/ecdh/nist.go
src/crypto/ecdsa/ecdsa.go
src/crypto/internal/fips140/drbg/rand.go
src/crypto/internal/fips140/ecdh/ecdh.go
src/crypto/internal/fips140/ecdsa/cast.go
src/crypto/internal/fips140/ecdsa/ecdsa.go
src/crypto/internal/fips140/ed25519/ed25519.go
src/crypto/internal/fips140/rsa/keygen.go
src/crypto/internal/fips140/rsa/pkcs1v22.go
src/crypto/internal/fips140only/fips140only.go
src/crypto/internal/fips140test/cast_test.go
src/crypto/rand/rand.go
src/crypto/rsa/fips.go
src/crypto/rsa/rsa.go
src/crypto/rsa/rsa_test.go

index 0f4a65e5affb800c2c9f17cb4bbc160472ef62aa..acef8298943c2bb07ed395367e39005efd343d2b 100644 (file)
@@ -8,6 +8,7 @@ import (
        "bytes"
        "crypto/internal/boring"
        "crypto/internal/fips140/ecdh"
+       "crypto/internal/fips140only"
        "errors"
        "io"
 )
@@ -43,6 +44,10 @@ func (c *nistCurve) GenerateKey(rand io.Reader) (*PrivateKey, error) {
                return k, nil
        }
 
+       if fips140only.Enabled && !fips140only.ApprovedRandomReader(rand) {
+               return nil, errors.New("crypto/ecdh: only crypto/rand.Reader is allowed in FIPS 140-only mode")
+       }
+
        privateKey, err := c.generate(rand)
        if err != nil {
                return nil, err
index 0ad669795c56b28baeb0985cf68d270f299ac9d7..77727aaf96befb1d8718ddea5124c1b85ac7de35 100644 (file)
@@ -21,6 +21,7 @@ import (
        "crypto/internal/boring"
        "crypto/internal/boring/bbig"
        "crypto/internal/fips140/ecdsa"
+       "crypto/internal/fips140only"
        "crypto/internal/randutil"
        "crypto/sha512"
        "crypto/subtle"
@@ -182,6 +183,9 @@ func GenerateKey(c elliptic.Curve, rand io.Reader) (*PrivateKey, error) {
 }
 
 func generateFIPS[P ecdsa.Point[P]](curve elliptic.Curve, c *ecdsa.Curve[P], rand io.Reader) (*PrivateKey, error) {
+       if fips140only.Enabled && fips140only.ApprovedRandomReader(rand) {
+               return nil, errors.New("crypto/ecdsa: only crypto/rand.Reader is allowed in FIPS 140-only mode")
+       }
        privateKey, err := ecdsa.GenerateKey(c, rand)
        if err != nil {
                return nil, err
@@ -228,6 +232,9 @@ func SignASN1(rand io.Reader, priv *PrivateKey, hash []byte) ([]byte, error) {
 }
 
 func signFIPS[P ecdsa.Point[P]](c *ecdsa.Curve[P], priv *PrivateKey, rand io.Reader, hash []byte) ([]byte, error) {
+       if fips140only.Enabled && !fips140only.ApprovedRandomReader(rand) {
+               return nil, errors.New("crypto/ecdsa: only crypto/rand.Reader is allowed in FIPS 140-only mode")
+       }
        // privateKeyToFIPS is very slow in FIPS mode because it performs a
        // Sign+Verify cycle per FIPS 140-3 IG 10.3.A. We should find a way to cache
        // it or attach it to the PrivateKey.
index 736a4b0cc0f4b3529cbe3dce0965805e97cd9d69..967fb0673ef1a18f158b69425c44ed3cdb300eba 100644 (file)
@@ -7,7 +7,9 @@ package drbg
 import (
        "crypto/internal/entropy"
        "crypto/internal/fips140"
+       "crypto/internal/randutil"
        "crypto/internal/sysrand"
+       "io"
        "sync"
 )
 
@@ -56,3 +58,38 @@ func Read(b []byte) {
                b = b[size:]
        }
 }
+
+// DefaultReader is a sentinel type, embedded in the default
+// [crypto/rand.Reader], used to recognize it when passed to
+// APIs that accept a rand io.Reader.
+type DefaultReader interface{ defaultReader() }
+
+// ReadWithReader uses Reader to fill b with cryptographically secure random
+// bytes. It is intended for use in APIs that expose a rand io.Reader.
+//
+// If Reader is not the default Reader from crypto/rand,
+// [randutil.MaybeReadByte] and [fips140.RecordNonApproved] are called.
+func ReadWithReader(r io.Reader, b []byte) error {
+       if _, ok := r.(DefaultReader); ok {
+               Read(b)
+               return nil
+       }
+
+       fips140.RecordNonApproved()
+       randutil.MaybeReadByte(r)
+       _, err := io.ReadFull(r, b)
+       return err
+}
+
+// ReadWithReaderDeterministic is like ReadWithReader, but it doesn't call
+// [randutil.MaybeReadByte] on non-default Readers.
+func ReadWithReaderDeterministic(r io.Reader, b []byte) error {
+       if _, ok := r.(DefaultReader); ok {
+               Read(b)
+               return nil
+       }
+
+       fips140.RecordNonApproved()
+       _, err := io.ReadFull(r, b)
+       return err
+}
index 19a45c00db9ce972dd30fd70b9e3e14aeb0569aa..bf71c75a92c6eb1662e38a38eaed742fad729d3a 100644 (file)
@@ -10,7 +10,6 @@ import (
        "crypto/internal/fips140/drbg"
        "crypto/internal/fips140/nistec"
        "crypto/internal/fips140deps/byteorder"
-       "crypto/internal/randutil"
        "errors"
        "io"
        "math/bits"
@@ -137,8 +136,6 @@ var p521Order = []byte{0x01, 0xff,
 }
 
 // GenerateKey generates a new ECDSA private key pair for the specified curve.
-//
-// In FIPS mode, rand is ignored.
 func GenerateKey[P Point[P]](c *Curve[P], rand io.Reader) (*PrivateKey, error) {
        fips140.RecordApproved()
        // This procedure is equivalent to Key Pair Generation by Testing
@@ -146,18 +143,13 @@ func GenerateKey[P Point[P]](c *Curve[P], rand io.Reader) (*PrivateKey, error) {
 
        for {
                key := make([]byte, len(c.N))
-               if fips140.Enabled {
-                       drbg.Read(key)
-               } else {
-                       randutil.MaybeReadByte(rand)
-                       if _, err := io.ReadFull(rand, key); err != nil {
-                               return nil, err
-                       }
-                       // In tests, rand will return all zeros and NewPrivateKey will reject
-                       // the zero key as it generates the identity as a public key. This also
-                       // makes this function consistent with crypto/elliptic.GenerateKey.
-                       key[1] ^= 0x42
+               if err := drbg.ReadWithReader(rand, key); err != nil {
+                       return nil, err
                }
+               // In tests, rand will return all zeros and NewPrivateKey will reject
+               // the zero key as it generates the identity as a public key. This also
+               // makes this function consistent with crypto/elliptic.GenerateKey.
+               key[1] ^= 0x42
 
                // Mask off any excess bits if the size of the underlying field is not a
                // whole number of bytes, which is only the case for P-521.
index a324cf929d8bf21f5b0a1a3a96054392a04f20f4..219b7211e74e92c4d0b144a092c584c802e36e08 100644 (file)
@@ -54,7 +54,8 @@ func testHash() []byte {
 func fipsPCT[P Point[P]](c *Curve[P], k *PrivateKey) error {
        return fips140.PCT("ECDSA PCT", func() error {
                hash := testHash()
-               sig, err := Sign(c, sha512.New, k, nil, hash)
+               drbg := newDRBG(sha512.New, k.d, bits2octets(P256(), hash), nil)
+               sig, err := sign(c, k, drbg, hash)
                if err != nil {
                        return err
                }
index 61b40122a0fab4b769d7bdd1b5bf0b37c8bdd5de..9459b03de764844328494df613705706056bb98e 100644 (file)
@@ -10,7 +10,6 @@ import (
        "crypto/internal/fips140/bigmod"
        "crypto/internal/fips140/drbg"
        "crypto/internal/fips140/nistec"
-       "crypto/internal/randutil"
        "errors"
        "io"
        "sync"
@@ -187,20 +186,11 @@ func NewPublicKey[P Point[P]](c *Curve[P], Q []byte) (*PublicKey, error) {
 }
 
 // GenerateKey generates a new ECDSA private key pair for the specified curve.
-//
-// In FIPS mode, rand is ignored.
 func GenerateKey[P Point[P]](c *Curve[P], rand io.Reader) (*PrivateKey, error) {
        fips140.RecordApproved()
 
        k, Q, err := randomPoint(c, func(b []byte) error {
-               if fips140.Enabled {
-                       drbg.Read(b)
-                       return nil
-               } else {
-                       randutil.MaybeReadByte(rand)
-                       _, err := io.ReadFull(rand, b)
-                       return err
-               }
+               return drbg.ReadWithReader(rand, b)
        })
        if err != nil {
                return nil, err
@@ -281,8 +271,6 @@ type Signature struct {
 // the hash function H) using the private key, priv. If the hash is longer than
 // the bit-length of the private key's curve order, the hash will be truncated
 // to that length.
-//
-// The signature is randomized. If FIPS mode is enabled, rand is ignored.
 func Sign[P Point[P], H fips140.Hash](c *Curve[P], h func() H, priv *PrivateKey, rand io.Reader, hash []byte) (*Signature, error) {
        if priv.pub.curve != c.curve {
                return nil, errors.New("ecdsa: private key does not match curve")
@@ -296,13 +284,8 @@ func Sign[P Point[P], H fips140.Hash](c *Curve[P], h func() H, priv *PrivateKey,
        // advantage of closely resembling Deterministic ECDSA.
 
        Z := make([]byte, len(priv.d))
-       if fips140.Enabled {
-               drbg.Read(Z)
-       } else {
-               randutil.MaybeReadByte(rand)
-               if _, err := io.ReadFull(rand, Z); err != nil {
-                       return nil, err
-               }
+       if err := drbg.ReadWithReader(rand, Z); err != nil {
+               return nil, err
        }
 
        // See https://github.com/cfrg/draft-irtf-cfrg-det-sigs-with-noise/issues/6
index 9824cbdf8149266a90e88e765c240788bef6f42c..bbdc5b4a8ba2f9016a4b12de384c77aae73569e0 100644 (file)
@@ -11,7 +11,6 @@ import (
        "crypto/internal/fips140/edwards25519"
        "crypto/internal/fips140/sha512"
        "errors"
-       "io"
        "strconv"
 )
 
@@ -61,24 +60,14 @@ func (pub *PublicKey) Bytes() []byte {
 }
 
 // GenerateKey generates a new Ed25519 private key pair.
-//
-// In FIPS mode, rand is ignored. Otherwise, the output of this function is
-// deterministic, and equivalent to reading 32 bytes from rand, and passing them
-// to [NewKeyFromSeed].
-func GenerateKey(rand io.Reader) (*PrivateKey, error) {
+func GenerateKey() (*PrivateKey, error) {
        priv := &PrivateKey{}
-       return generateKey(priv, rand)
+       return generateKey(priv)
 }
 
-func generateKey(priv *PrivateKey, rand io.Reader) (*PrivateKey, error) {
+func generateKey(priv *PrivateKey) (*PrivateKey, error) {
        fips140.RecordApproved()
-       if fips140.Enabled {
-               drbg.Read(priv.seed[:])
-       } else {
-               if _, err := io.ReadFull(rand, priv.seed[:]); err != nil {
-                       return nil, err
-               }
-       }
+       drbg.Read(priv.seed[:])
        precomputePrivateKey(priv)
        if err := fipsPCT(priv); err != nil {
                // This clearly can't happen, but FIPS 140-3 requires that we check.
index a9e12eb1e8e920a3dcdc184c5aa62dec8ebd93ac..df76772ef5878d7443f792c68c8e73e2ce742ca2 100644 (file)
@@ -8,15 +8,12 @@ import (
        "crypto/internal/fips140"
        "crypto/internal/fips140/bigmod"
        "crypto/internal/fips140/drbg"
-       "crypto/internal/randutil"
        "errors"
        "io"
 )
 
 // GenerateKey generates a new RSA key pair of the given bit size.
 // bits must be at least 128.
-//
-// When operating in FIPS mode, rand is ignored.
 func GenerateKey(rand io.Reader, bits int) (*PrivateKey, error) {
        if bits < 128 {
                return nil, errors.New("rsa: key too small")
@@ -94,7 +91,7 @@ func GenerateKey(rand io.Reader, bits int) (*PrivateKey, error) {
 }
 
 // randomPrime returns a random prime number of the given bit size following
-// the process in FIPS 186-5, Appendix A.1.3. rand is ignored in FIPS mode.
+// the process in FIPS 186-5, Appendix A.1.3.
 func randomPrime(rand io.Reader, bits int) ([]byte, error) {
        if bits < 64 {
                return nil, errors.New("rsa: prime size must be at least 32-bit")
@@ -102,13 +99,8 @@ func randomPrime(rand io.Reader, bits int) ([]byte, error) {
 
        b := make([]byte, (bits+7)/8)
        for {
-               if fips140.Enabled {
-                       drbg.Read(b)
-               } else {
-                       randutil.MaybeReadByte(rand)
-                       if _, err := io.ReadFull(rand, b); err != nil {
-                               return nil, err
-                       }
+               if err := drbg.ReadWithReader(rand, b); err != nil {
+                       return nil, err
                }
                if excess := len(b)*8 - bits; excess != 0 {
                        b[0] >>= excess
index a62d7e485f6df7736a73b4f4ea2d58daf27a0f5c..a5bc56dafcd9ff75117b0114f1fac24032ed7afa 100644 (file)
@@ -264,8 +264,6 @@ func PSSMaxSaltLength(pub *PublicKey, hash fips140.Hash) (int, error) {
 }
 
 // SignPSS calculates the signature of hashed using RSASSA-PSS.
-//
-// In FIPS mode, rand is ignored and can be nil.
 func SignPSS(rand io.Reader, priv *PrivateKey, hash fips140.Hash, hashed []byte, saltLength int) ([]byte, error) {
        fipsSelfTest()
        fips140.RecordApproved()
@@ -286,12 +284,8 @@ func SignPSS(rand io.Reader, priv *PrivateKey, hash fips140.Hash, hashed []byte,
                fips140.RecordNonApproved()
        }
        salt := make([]byte, saltLength)
-       if fips140.Enabled {
-               drbg.Read(salt)
-       } else {
-               if _, err := io.ReadFull(rand, salt); err != nil {
-                       return nil, err
-               }
+       if err := drbg.ReadWithReaderDeterministic(rand, salt); err != nil {
+               return nil, err
        }
 
        emBits := priv.pub.N.BitLen() - 1
@@ -374,8 +368,6 @@ func checkApprovedHash(hash fips140.Hash) {
 }
 
 // EncryptOAEP encrypts the given message with RSAES-OAEP.
-//
-// In FIPS mode, random is ignored and can be nil.
 func EncryptOAEP(hash, mgfHash fips140.Hash, random io.Reader, pub *PublicKey, msg []byte, label []byte) ([]byte, error) {
        // Note that while we don't commit to deterministic execution with respect
        // to the random stream, we also don't apply MaybeReadByte, so per Hyrum's
@@ -408,13 +400,8 @@ func EncryptOAEP(hash, mgfHash fips140.Hash, random io.Reader, pub *PublicKey, m
        db[len(db)-len(msg)-1] = 1
        copy(db[len(db)-len(msg):], msg)
 
-       if fips140.Enabled {
-               drbg.Read(seed)
-       } else {
-               _, err := io.ReadFull(random, seed)
-               if err != nil {
-                       return nil, err
-               }
+       if err := drbg.ReadWithReaderDeterministic(random, seed); err != nil {
+               return nil, err
        }
 
        mgf1XOR(db, mgfHash, seed)
index 6ad97befbe06daa704f8ab34be87d3fea80fbf0e..7126781af0d8bc64516464eb201bcd846b7bd683 100644 (file)
@@ -5,11 +5,13 @@
 package fips140only
 
 import (
+       "crypto/internal/fips140/drbg"
        "crypto/internal/fips140/sha256"
        "crypto/internal/fips140/sha3"
        "crypto/internal/fips140/sha512"
        "hash"
        "internal/godebug"
+       "io"
 )
 
 // Enabled reports whether FIPS 140-only mode is enabled, in which non-approved
@@ -24,3 +26,8 @@ func ApprovedHash(h hash.Hash) bool {
                return false
        }
 }
+
+func ApprovedRandomReader(r io.Reader) bool {
+       _, ok := r.(drbg.DefaultReader)
+       return ok
+}
index c6e3212f3f064f9beed9d4ef47f05af102122262..b2aee15eab70f50691e693177483cd3cf8de8b5f 100644 (file)
@@ -85,7 +85,7 @@ func TestConditionals(t *testing.T) {
                t.Fatal(err)
        }
        ecdsa.SignDeterministic(ecdsa.P256(), sha256.New, kDSA, make([]byte, 32))
-       k25519, err := ed25519.GenerateKey(rand.Reader)
+       k25519, err := ed25519.GenerateKey()
        if err != nil {
                t.Fatal(err)
        }
index 5dd875e6e725751144aabb37c467c58621c44850..1ca16caa9563e68c5684256cbd92c0ea6d16ac6b 100644 (file)
@@ -38,7 +38,9 @@ func init() {
        Reader = &reader{}
 }
 
-type reader struct{}
+type reader struct {
+       drbg.DefaultReader
+}
 
 func (r *reader) Read(b []byte) (n int, err error) {
        boring.Unreachable()
index bc23d597098f0c3f08ae54ffd1d1ea0a95b71052..24dfb38cf625bda5f87f003c140175cd7763bae4 100644 (file)
@@ -17,6 +17,9 @@ import (
 const (
        // PSSSaltLengthAuto causes the salt in a PSS signature to be as large
        // as possible when signing, and to be auto-detected when verifying.
+       //
+       // When signing in FIPS 140-3 mode, the salt length is capped at the length
+       // of the hash function used in the signature.
        PSSSaltLengthAuto = 0
        // PSSSaltLengthEqualsHash causes the salt length to equal the length
        // of the hash used in the signature.
@@ -67,6 +70,9 @@ func SignPSS(rand io.Reader, priv *PrivateKey, hash crypto.Hash, digest []byte,
        if fips140only.Enabled && !fips140only.ApprovedHash(hash.New()) {
                return nil, errors.New("crypto/rsa: use of hash functions other than SHA-2 or SHA-3 is not allowed in FIPS 140-only mode")
        }
+       if fips140only.Enabled && !fips140only.ApprovedRandomReader(rand) {
+               return nil, errors.New("crypto/rsa: only crypto/rand.Reader is allowed in FIPS 140-only mode")
+       }
 
        if opts != nil && opts.Hash != 0 {
                hash = opts.Hash
@@ -188,6 +194,9 @@ func EncryptOAEP(hash hash.Hash, random io.Reader, pub *PublicKey, msg []byte, l
        if fips140only.Enabled && !fips140only.ApprovedHash(hash) {
                return nil, errors.New("crypto/rsa: use of hash functions other than SHA-2 or SHA-3 is not allowed in FIPS 140-only mode")
        }
+       if fips140only.Enabled && !fips140only.ApprovedRandomReader(random) {
+               return nil, errors.New("crypto/rsa: only crypto/rand.Reader is allowed in FIPS 140-only mode")
+       }
 
        defer hash.Reset()
 
index 0f58f2226f848e53c42b33d20086fea2f48e361f..fb23f003a6f2177ed6a1d1c0dfc1220d80425a9c 100644 (file)
@@ -322,6 +322,9 @@ func GenerateKey(random io.Reader, bits int) (*PrivateKey, error) {
        if fips140only.Enabled && bits%2 == 1 {
                return nil, errors.New("crypto/rsa: use of keys with odd size is not allowed in FIPS 140-only mode")
        }
+       if fips140only.Enabled && !fips140only.ApprovedRandomReader(random) {
+               return nil, errors.New("crypto/rsa: only crypto/rand.Reader is allowed in FIPS 140-only mode")
+       }
 
        k, err := rsa.GenerateKey(random, bits)
        if err != nil {
index c395732c8b27b8eeaeae056b954f821606ae67df..2474ab82dfa207eca2a77fdad664c2e841d90cb2 100644 (file)
@@ -10,7 +10,6 @@ import (
        "crypto"
        "crypto/internal/boring"
        "crypto/internal/cryptotest"
-       "crypto/internal/fips140"
        "crypto/rand"
        . "crypto/rsa"
        "crypto/sha1"
@@ -782,9 +781,6 @@ type testEncryptOAEPStruct struct {
 }
 
 func TestEncryptOAEP(t *testing.T) {
-       if fips140.Enabled {
-               t.Skip("FIPS mode overrides the deterministic random source")
-       }
        sha1 := sha1.New()
        n := new(big.Int)
        for i, test := range testEncryptOAEPData {