]> Cypherpunks repositories - gostls13.git/commitdiff
[release-branch.go1.24] crypto/internal/fips140: remove key import PCTs, make keygen...
authorFilippo Valsorda <filippo@golang.org>
Fri, 5 Sep 2025 02:19:18 +0000 (22:19 -0400)
committerJunyang Shao <shaojunyang@google.com>
Thu, 25 Sep 2025 19:14:05 +0000 (12:14 -0700)
CMVP clarified with the September 2nd changes to IG 10.3.A that PCTs
don't need to run on imported keys.

However, PCT failure must enter the error state (which for us is fatal).

Thankfully, now that PCTs only run on key generation, we can be assured
they will never fail.

This change should only affect FIPS 140-3 mode.

While at it, make the CAST/PCT testing more robust, checking
TestConditional is terminated by a fatal error (and not by t.Fatal).

Fixes #74947
Updates #75523
Updates #69536

Change-Id: I6a6a696439e1560c10f3cce2cb208fd40c5bc641
Reviewed-on: https://go-review.googlesource.com/c/go/+/701438
Reviewed-by: Junyang Shao <shaojunyang@google.com>
TryBot-Bypass: Filippo Valsorda <filippo@golang.org>
Commit-Queue: Junyang Shao <shaojunyang@google.com>
Reviewed-by: Roland Shoemaker <roland@golang.org>
src/crypto/internal/fips140/cast.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/cast.go
src/crypto/internal/fips140/ed25519/ed25519.go
src/crypto/internal/fips140/mlkem/mlkem1024.go
src/crypto/internal/fips140/mlkem/mlkem768.go
src/crypto/internal/fips140/rsa/keygen.go
src/crypto/internal/fips140/rsa/rsa.go
src/crypto/internal/fips140test/cast_test.go

index 66e21d8a90dbc91246df067d6e974a218c3006df..3968dcadd4da8f49fd3e6e1331886ed2c17d3173 100644 (file)
@@ -56,9 +56,10 @@ func CAST(name string, f func() error) {
 }
 
 // PCT runs the named Pairwise Consistency Test (if operated in FIPS mode) and
-// returns any errors. If an error is returned, the key must not be used.
+// aborts the program (stopping the module input/output and entering the "error
+// state") if the test fails.
 //
-// PCTs are mandatory for every key pair that is generated/imported, including
+// PCTs are mandatory for every generated (but not imported) key pair, including
 // ephemeral keys (which effectively doubles the cost of key establishment). See
 // Implementation Guidance 10.3.A Additional Comment 1.
 //
@@ -66,17 +67,23 @@ func CAST(name string, f func() error) {
 //
 // If a package p calls PCT during key generation, an invocation of that
 // function should be added to fipstest.TestConditionals.
-func PCT(name string, f func() error) error {
+func PCT(name string, f func() error) {
        if strings.ContainsAny(name, ",#=:") {
                panic("fips: invalid self-test name: " + name)
        }
        if !Enabled {
-               return nil
+               return
        }
 
        err := f()
        if name == failfipscast {
                err = errors.New("simulated PCT failure")
        }
-       return err
+       if err != nil {
+               fatal("FIPS 140-3 self-test failed: " + name + ": " + err.Error())
+               panic("unreachable")
+       }
+       if debug {
+               println("FIPS 140-3 PCT passed:", name)
+       }
 }
index bf71c75a92c6eb1662e38a38eaed742fad729d3a..967032aab28fc0002a8f552e78d6a5141c487a72 100644 (file)
@@ -161,6 +161,27 @@ func GenerateKey[P Point[P]](c *Curve[P], rand io.Reader) (*PrivateKey, error) {
                if err != nil {
                        continue
                }
+
+               // A "Pairwise Consistency Test" makes no sense if we just generated the
+               // public key from an ephemeral private key. Moreover, there is no way to
+               // check it aside from redoing the exact same computation again. SP 800-56A
+               // Rev. 3, Section 5.6.2.1.4 acknowledges that, and doesn't require it.
+               // However, ISO 19790:2012, Section 7.10.3.3 has a blanket requirement for a
+               // PCT for all generated keys (AS10.35) and FIPS 140-3 IG 10.3.A, Additional
+               // Comment 1 goes out of its way to say that "the PCT shall be performed
+               // consistent [...], even if the underlying standard does not require a
+               // PCT". So we do it. And make ECDH nearly 50% slower (only) in FIPS mode.
+               fips140.PCT("ECDH PCT", func() error {
+                       p1, err := c.newPoint().ScalarBaseMult(privateKey.d)
+                       if err != nil {
+                               return err
+                       }
+                       if !bytes.Equal(p1.Bytes(), privateKey.pub.q) {
+                               return errors.New("crypto/ecdh: public key does not match private key")
+                       }
+                       return nil
+               })
+
                return privateKey, nil
        }
 }
@@ -188,28 +209,6 @@ func NewPrivateKey[P Point[P]](c *Curve[P], key []byte) (*PrivateKey, error) {
                panic("crypto/ecdh: internal error: public key is the identity element")
        }
 
-       // A "Pairwise Consistency Test" makes no sense if we just generated the
-       // public key from an ephemeral private key. Moreover, there is no way to
-       // check it aside from redoing the exact same computation again. SP 800-56A
-       // Rev. 3, Section 5.6.2.1.4 acknowledges that, and doesn't require it.
-       // However, ISO 19790:2012, Section 7.10.3.3 has a blanket requirement for a
-       // PCT for all generated keys (AS10.35) and FIPS 140-3 IG 10.3.A, Additional
-       // Comment 1 goes out of its way to say that "the PCT shall be performed
-       // consistent [...], even if the underlying standard does not require a
-       // PCT". So we do it. And make ECDH nearly 50% slower (only) in FIPS mode.
-       if err := fips140.PCT("ECDH PCT", func() error {
-               p1, err := c.newPoint().ScalarBaseMult(key)
-               if err != nil {
-                       return err
-               }
-               if !bytes.Equal(p1.Bytes(), publicKey) {
-                       return errors.New("crypto/ecdh: public key does not match private key")
-               }
-               return nil
-       }); err != nil {
-               panic(err)
-       }
-
        k := &PrivateKey{d: bytes.Clone(key), pub: PublicKey{curve: c.curve, q: publicKey}}
        return k, nil
 }
index 219b7211e74e92c4d0b144a092c584c802e36e08..6bc9fd1f46d2d015604b2cf1ca7ffc4792a31e52 100644 (file)
@@ -51,8 +51,8 @@ func testHash() []byte {
        }
 }
 
-func fipsPCT[P Point[P]](c *Curve[P], k *PrivateKey) error {
-       return fips140.PCT("ECDSA PCT", func() error {
+func fipsPCT[P Point[P]](c *Curve[P], k *PrivateKey) {
+       fips140.PCT("ECDSA PCT", func() error {
                hash := testHash()
                drbg := newDRBG(sha512.New, k.d, bits2octets(P256(), hash), nil)
                sig, err := sign(c, k, drbg, hash)
index 11389e8210bc688ddd4ca60e6a2cd20b6a17ecb6..de2f5650484e68ca0b4fc66a4dbae45eaa68e423 100644 (file)
@@ -166,11 +166,6 @@ func NewPrivateKey[P Point[P]](c *Curve[P], D, Q []byte) (*PrivateKey, error) {
                return nil, err
        }
        priv := &PrivateKey{pub: *pub, d: d.Bytes(c.N)}
-       if err := fipsPCT(c, priv); err != nil {
-               // This can happen if the application went out of its way to make an
-               // ecdsa.PrivateKey with a mismatching PublicKey.
-               return nil, err
-       }
        return priv, nil
 }
 
@@ -203,10 +198,7 @@ func GenerateKey[P Point[P]](c *Curve[P], rand io.Reader) (*PrivateKey, error) {
                },
                d: k.Bytes(c.N),
        }
-       if err := fipsPCT(c, priv); err != nil {
-               // This clearly can't happen, but FIPS 140-3 mandates that we check it.
-               panic(err)
-       }
+       fipsPCT(c, priv)
        return priv, nil
 }
 
index a680c2514b816ea6657e646e4de3b8cbecf6c116..2a3426bd42f50ebe0ca88e12d61321da285f0867 100644 (file)
@@ -12,8 +12,8 @@ import (
        "sync"
 )
 
-func fipsPCT(k *PrivateKey) error {
-       return fips140.PCT("Ed25519 sign and verify PCT", func() error {
+func fipsPCT(k *PrivateKey) {
+       fips140.PCT("Ed25519 sign and verify PCT", func() error {
                return pairwiseTest(k)
        })
 }
index bbdc5b4a8ba2f9016a4b12de384c77aae73569e0..8beda341d941ad2ee8c847c71603405befeab097 100644 (file)
@@ -69,10 +69,7 @@ func generateKey(priv *PrivateKey) (*PrivateKey, error) {
        fips140.RecordApproved()
        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.
-               panic(err)
-       }
+       fipsPCT(priv)
        return priv, nil
 }
 
@@ -88,10 +85,6 @@ func newPrivateKeyFromSeed(priv *PrivateKey, seed []byte) (*PrivateKey, error) {
        }
        copy(priv.seed[:], seed)
        precomputePrivateKey(priv)
-       if err := fipsPCT(priv); err != nil {
-               // This clearly can't happen, but FIPS 140-3 requires that we check.
-               panic(err)
-       }
        return priv, nil
 }
 
@@ -137,12 +130,6 @@ func newPrivateKey(priv *PrivateKey, privBytes []byte) (*PrivateKey, error) {
 
        copy(priv.prefix[:], h[32:])
 
-       if err := fipsPCT(priv); err != nil {
-               // This can happen if the application messed with the private key
-               // encoding, and the public key doesn't match the seed anymore.
-               return nil, err
-       }
-
        return priv, nil
 }
 
index 034bf3b5d6682d3164d4290437529ad2eae5d590..1419cf20fa9c67b96592806d3ccd56d5bee1be0f 100644 (file)
@@ -118,10 +118,7 @@ func generateKey1024(dk *DecapsulationKey1024) (*DecapsulationKey1024, error) {
        var z [32]byte
        drbg.Read(z[:])
        kemKeyGen1024(dk, &d, &z)
-       if err := fips140.PCT("ML-KEM PCT", func() error { return kemPCT1024(dk) }); err != nil {
-               // This clearly can't happen, but FIPS 140-3 requires us to check.
-               panic(err)
-       }
+       fips140.PCT("ML-KEM PCT", func() error { return kemPCT1024(dk) })
        fips140.RecordApproved()
        return dk, nil
 }
@@ -149,10 +146,6 @@ func newKeyFromSeed1024(dk *DecapsulationKey1024, seed []byte) (*DecapsulationKe
        d := (*[32]byte)(seed[:32])
        z := (*[32]byte)(seed[32:])
        kemKeyGen1024(dk, d, z)
-       if err := fips140.PCT("ML-KEM PCT", func() error { return kemPCT1024(dk) }); err != nil {
-               // This clearly can't happen, but FIPS 140-3 requires us to check.
-               panic(err)
-       }
        fips140.RecordApproved()
        return dk, nil
 }
index 77043830d4d962e2162011e09311f482a370210e..298660e4e977dd586dc90140d149095e07484fc1 100644 (file)
@@ -177,10 +177,7 @@ func generateKey(dk *DecapsulationKey768) (*DecapsulationKey768, error) {
        var z [32]byte
        drbg.Read(z[:])
        kemKeyGen(dk, &d, &z)
-       if err := fips140.PCT("ML-KEM PCT", func() error { return kemPCT(dk) }); err != nil {
-               // This clearly can't happen, but FIPS 140-3 requires us to check.
-               panic(err)
-       }
+       fips140.PCT("ML-KEM PCT", func() error { return kemPCT(dk) })
        fips140.RecordApproved()
        return dk, nil
 }
@@ -208,10 +205,6 @@ func newKeyFromSeed(dk *DecapsulationKey768, seed []byte) (*DecapsulationKey768,
        d := (*[32]byte)(seed[:32])
        z := (*[32]byte)(seed[32:])
        kemKeyGen(dk, d, z)
-       if err := fips140.PCT("ML-KEM PCT", func() error { return kemPCT(dk) }); err != nil {
-               // This clearly can't happen, but FIPS 140-3 requires us to check.
-               panic(err)
-       }
        fips140.RecordApproved()
        return dk, nil
 }
index 658eb9ab2458aa359adbf645b7b129ab5398b1ff..997ebf81bc23b0958909e77ac1460fe20c34687e 100644 (file)
@@ -105,7 +105,28 @@ func GenerateKey(rand io.Reader, bits int) (*PrivateKey, error) {
                // negligible chance of failure we can defer the check to the end of key
                // generation and return an error if it fails. See [checkPrivateKey].
 
-               return newPrivateKey(N, 65537, d, P, Q)
+               k, err := newPrivateKey(N, 65537, d, P, Q)
+               if err != nil {
+                       return nil, err
+               }
+
+               if k.fipsApproved {
+                       fips140.PCT("RSA sign and verify PCT", func() error {
+                               hash := []byte{
+                                       0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07, 0x08,
+                                       0x09, 0x0a, 0x0b, 0x0c, 0x0d, 0x0e, 0x0f, 0x10,
+                                       0x11, 0x12, 0x13, 0x14, 0x15, 0x16, 0x17, 0x18,
+                                       0x19, 0x1a, 0x1b, 0x1c, 0x1d, 0x1e, 0x1f, 0x20,
+                               }
+                               sig, err := signPKCS1v15(k, "SHA-256", hash)
+                               if err != nil {
+                                       return err
+                               }
+                               return verifyPKCS1v15(k.PublicKey(), "SHA-256", hash, sig)
+                       })
+               }
+
+               return k, nil
        }
 }
 
index 0bbf7010506107da359f38727e1e9489ecd9c753..764338940a3282cd9588159d551cd56ce8313858 100644 (file)
@@ -310,26 +310,6 @@ func checkPrivateKey(priv *PrivateKey) error {
                return errors.New("crypto/rsa: d too small")
        }
 
-       // If the key is still in scope for FIPS mode, perform a Pairwise
-       // Consistency Test.
-       if priv.fipsApproved {
-               if err := fips140.PCT("RSA sign and verify PCT", func() error {
-                       hash := []byte{
-                               0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07, 0x08,
-                               0x09, 0x0a, 0x0b, 0x0c, 0x0d, 0x0e, 0x0f, 0x10,
-                               0x11, 0x12, 0x13, 0x14, 0x15, 0x16, 0x17, 0x18,
-                               0x19, 0x1a, 0x1b, 0x1c, 0x1d, 0x1e, 0x1f, 0x20,
-                       }
-                       sig, err := signPKCS1v15(priv, "SHA-256", hash)
-                       if err != nil {
-                               return err
-                       }
-                       return verifyPKCS1v15(priv.PublicKey(), "SHA-256", hash, sig)
-               }); err != nil {
-                       return err
-               }
-       }
-
        return nil
 }
 
index 79cff9b85747a7e2b315a309695e6fa114bd6dea..4294ff2cbfdac1a2842b3a4579bd2fb93816fd11 100644 (file)
@@ -5,9 +5,8 @@
 package fipstest
 
 import (
+       "crypto"
        "crypto/rand"
-       "crypto/x509"
-       "encoding/pem"
        "fmt"
        "internal/testenv"
        "io/fs"
@@ -17,6 +16,7 @@ import (
        "strings"
        "testing"
 
+       "crypto/internal/fips140"
        // Import packages that define CASTs to test them.
        _ "crypto/internal/fips140/aes"
        _ "crypto/internal/fips140/aes/gcm"
@@ -50,8 +50,6 @@ var allCASTs = []string{
        "KAS-ECC-SSC P-256",
        "ML-KEM PCT",
        "ML-KEM PCT",
-       "ML-KEM PCT",
-       "ML-KEM PCT",
        "ML-KEM-768",
        "PBKDF2",
        "RSA sign and verify PCT",
@@ -107,59 +105,63 @@ func TestAllCASTs(t *testing.T) {
 // TestConditionals causes the conditional CASTs and PCTs to be invoked.
 func TestConditionals(t *testing.T) {
        mlkem.GenerateKey768()
-       k, err := ecdh.GenerateKey(ecdh.P256(), rand.Reader)
+       kDH, err := ecdh.GenerateKey(ecdh.P256(), rand.Reader)
        if err != nil {
-               t.Fatal(err)
+               t.Error(err)
+       } else {
+               ecdh.ECDH(ecdh.P256(), kDH, kDH.PublicKey())
        }
-       ecdh.ECDH(ecdh.P256(), k, k.PublicKey())
        kDSA, err := ecdsa.GenerateKey(ecdsa.P256(), rand.Reader)
        if err != nil {
-               t.Fatal(err)
+               t.Error(err)
+       } else {
+               ecdsa.SignDeterministic(ecdsa.P256(), sha256.New, kDSA, make([]byte, 32))
        }
-       ecdsa.SignDeterministic(ecdsa.P256(), sha256.New, kDSA, make([]byte, 32))
        k25519, err := ed25519.GenerateKey()
        if err != nil {
-               t.Fatal(err)
+               t.Error(err)
+       } else {
+               ed25519.Sign(k25519, make([]byte, 32))
        }
-       ed25519.Sign(k25519, make([]byte, 32))
-       rsa.VerifyPKCS1v15(&rsa.PublicKey{}, "", nil, nil)
-       // Parse an RSA key to hit the PCT rather than generating one (which is slow).
-       block, _ := pem.Decode([]byte(strings.ReplaceAll(
-               `-----BEGIN RSA TESTING KEY-----
-MIIEowIBAAKCAQEAsPnoGUOnrpiSqt4XynxA+HRP7S+BSObI6qJ7fQAVSPtRkqso
-tWxQYLEYzNEx5ZSHTGypibVsJylvCfuToDTfMul8b/CZjP2Ob0LdpYrNH6l5hvFE
-89FU1nZQF15oVLOpUgA7wGiHuEVawrGfey92UE68mOyUVXGweJIVDdxqdMoPvNNU
-l86BU02vlBiESxOuox+dWmuVV7vfYZ79Toh/LUK43YvJh+rhv4nKuF7iHjVjBd9s
-B6iDjj70HFldzOQ9r8SRI+9NirupPTkF5AKNe6kUhKJ1luB7S27ZkvB3tSTT3P59
-3VVJvnzOjaA1z6Cz+4+eRvcysqhrRgFlwI9TEwIDAQABAoIBAEEYiyDP29vCzx/+
-dS3LqnI5BjUuJhXUnc6AWX/PCgVAO+8A+gZRgvct7PtZb0sM6P9ZcLrweomlGezI
-FrL0/6xQaa8bBr/ve/a8155OgcjFo6fZEw3Dz7ra5fbSiPmu4/b/kvrg+Br1l77J
-aun6uUAs1f5B9wW+vbR7tzbT/mxaUeDiBzKpe15GwcvbJtdIVMa2YErtRjc1/5B2
-BGVXyvlJv0SIlcIEMsHgnAFOp1ZgQ08aDzvilLq8XVMOahAhP1O2A3X8hKdXPyrx
-IVWE9bS9ptTo+eF6eNl+d7htpKGEZHUxinoQpWEBTv+iOoHsVunkEJ3vjLP3lyI/
-fY0NQ1ECgYEA3RBXAjgvIys2gfU3keImF8e/TprLge1I2vbWmV2j6rZCg5r/AS0u
-pii5CvJ5/T5vfJPNgPBy8B/yRDs+6PJO1GmnlhOkG9JAIPkv0RBZvR0PMBtbp6nT
-Y3yo1lwamBVBfY6rc0sLTzosZh2aGoLzrHNMQFMGaauORzBFpY5lU50CgYEAzPHl
-u5DI6Xgep1vr8QvCUuEesCOgJg8Yh1UqVoY/SmQh6MYAv1I9bLGwrb3WW/7kqIoD
-fj0aQV5buVZI2loMomtU9KY5SFIsPV+JuUpy7/+VE01ZQM5FdY8wiYCQiVZYju9X
-Wz5LxMNoz+gT7pwlLCsC4N+R8aoBk404aF1gum8CgYAJ7VTq7Zj4TFV7Soa/T1eE
-k9y8a+kdoYk3BASpCHJ29M5R2KEA7YV9wrBklHTz8VzSTFTbKHEQ5W5csAhoL5Fo
-qoHzFFi3Qx7MHESQb9qHyolHEMNx6QdsHUn7rlEnaTTyrXh3ifQtD6C0yTmFXUIS
-CW9wKApOrnyKJ9nI0HcuZQKBgQCMtoV6e9VGX4AEfpuHvAAnMYQFgeBiYTkBKltQ
-XwozhH63uMMomUmtSG87Sz1TmrXadjAhy8gsG6I0pWaN7QgBuFnzQ/HOkwTm+qKw
-AsrZt4zeXNwsH7QXHEJCFnCmqw9QzEoZTrNtHJHpNboBuVnYcoueZEJrP8OnUG3r
-UjmopwKBgAqB2KYYMUqAOvYcBnEfLDmyZv9BTVNHbR2lKkMYqv5LlvDaBxVfilE0
-2riO4p6BaAdvzXjKeRrGNEKoHNBpOSfYCOM16NjL8hIZB1CaV3WbT5oY+jp7Mzd5
-7d56RZOE+ERK2uz/7JX9VSsM/LbH9pJibd4e8mikDS9ntciqOH/3
------END RSA TESTING KEY-----`, "TESTING KEY", "PRIVATE KEY")))
-       if _, err := x509.ParsePKCS1PrivateKey(block.Bytes); err != nil {
-               t.Fatal(err)
+       kRSA, err := rsa.GenerateKey(rand.Reader, 2048)
+       if err != nil {
+               t.Error(err)
+       } else {
+               rsa.SignPKCS1v15(kRSA, crypto.SHA256.String(), make([]byte, 32))
        }
        t.Log("completed successfully")
 }
 
+func TestCASTPasses(t *testing.T) {
+       testenv.MustHaveExec(t)
+       if err := fips140.Supported(); err != nil {
+               t.Skipf("FIPS140 not supported: %v", err)
+       }
+
+       cmd := testenv.Command(t, testenv.Executable(t), "-test.run=^TestConditionals$", "-test.v")
+       cmd.Env = append(cmd.Env, "GODEBUG=fips140=debug")
+       out, err := cmd.CombinedOutput()
+       t.Logf("%s", out)
+       if err != nil || !strings.Contains(string(out), "completed successfully") {
+               t.Errorf("TestConditionals did not complete successfully")
+       }
+
+       for _, name := range allCASTs {
+               t.Run(name, func(t *testing.T) {
+                       if !strings.Contains(string(out), fmt.Sprintf("passed: %s\n", name)) {
+                               t.Errorf("CAST/PCT %s success was not logged", name)
+                       } else {
+                               t.Logf("CAST/PCT succeeded: %s", name)
+                       }
+               })
+       }
+}
+
 func TestCASTFailures(t *testing.T) {
        testenv.MustHaveExec(t)
+       if err := fips140.Supported(); err != nil {
+               t.Skipf("FIPS140 not supported: %v", err)
+       }
 
        for _, name := range allCASTs {
                t.Run(name, func(t *testing.T) {
@@ -168,7 +170,6 @@ func TestCASTFailures(t *testing.T) {
                        if !testing.Verbose() {
                                t.Parallel()
                        }
-                       t.Logf("CAST/PCT succeeded: %s", name)
                        t.Logf("Testing CAST/PCT failure...")
                        cmd := testenv.Command(t, testenv.Executable(t), "-test.run=TestConditionals", "-test.v")
                        cmd.Env = append(cmd.Env, fmt.Sprintf("GODEBUG=failfipscast=%s,fips140=on", name))
@@ -179,6 +180,8 @@ func TestCASTFailures(t *testing.T) {
                        }
                        if strings.Contains(string(out), "completed successfully") {
                                t.Errorf("CAST/PCT %s failure did not stop the program", name)
+                       } else if !strings.Contains(string(out), "self-test failed: "+name) {
+                               t.Errorf("CAST/PCT %s failure did not log the expected message", name)
                        } else {
                                t.Logf("CAST/PCT %s failed as expected and caused the program to exit", name)
                        }