]> Cypherpunks repositories - gostls13.git/commitdiff
crypto/internal/fips: handle the one possible PCT failure
authorFilippo Valsorda <filippo@golang.org>
Sun, 17 Nov 2024 17:38:29 +0000 (18:38 +0100)
committerGopher Robot <gobot@golang.org>
Tue, 19 Nov 2024 23:02:31 +0000 (23:02 +0000)
Since ECDSA private keys are irredeemably malleable, an application
could construct one where the public key doesn't match the private key.
They'd be very much on their own, but crashing the program feels a bit
harsh.

Add this one to the list of issues caused by exposing the ECDSA (and
RSA) key values as big.Ints.

For #69536

Change-Id: Iaa65c73d7145e74f860ca097fa9641448442fbf9
Reviewed-on: https://go-review.googlesource.com/c/go/+/628855
Auto-Submit: Filippo Valsorda <filippo@golang.org>
Reviewed-by: Russ Cox <rsc@golang.org>
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>

src/crypto/internal/fips/cast.go
src/crypto/internal/fips/ecdh/ecdh.go
src/crypto/internal/fips/ecdsa/cast.go
src/crypto/internal/fips/ecdsa/ecdsa.go
src/crypto/internal/fips/mlkem/mlkem1024.go
src/crypto/internal/fips/mlkem/mlkem768.go
src/crypto/internal/fipstest/cast_test.go

index 6be104a99ebfc00d1ad16fdc8d3e050cbb6d69d6..4d056de7b582a3bc854d74ae75342ec9a10b55a0 100644 (file)
@@ -16,26 +16,24 @@ import (
 //go:linkname fatal crypto/internal/fips.fatal
 func fatal(string)
 
-// failfipscast is a GODEBUG key allowing simulation of a Cryptographic Algorithm
-// Self-Test (CAST) failure, as required during FIPS 140-3 functional testing.
-// The value is a substring of the target CAST name.
+// failfipscast is a GODEBUG key allowing simulation of a CAST or PCT failure,
+// as required during FIPS 140-3 functional testing. The value is the whole name
+// of the target CAST or PCT.
 var failfipscast = godebug.Value("#failfipscast")
 
-// CAST runs the named Cryptographic Algorithm Self-Test or Pairwise Consistency
-// Test (if operated in FIPS mode) and aborts the program (stopping the module
-// input/output and entering the "error state") if the self-test fails.
+// CAST runs the named Cryptographic Algorithm Self-Test (if operated in FIPS
+// mode) and aborts the program (stopping the module input/output and entering
+// the "error state") if the self-test fails.
 //
 // CASTs are mandatory self-checks that must be performed by FIPS 140-3 modules
-// before the algorithm is used. See Implementation Guidance 10.3.A. PCTs are
-// mandatory for every key pair that is generated/imported, including ephemeral
-// keys (which effectively doubles the cost of key establishment). See
-// Implementation Guidance 10.3.A Additional Comment 1.
+// before the algorithm is used. See Implementation Guidance 10.3.A.
 //
 // The name must not contain commas, colons, hashes, or equal signs.
 //
-// When calling this function from init(), also import the calling package from
-// crypto/internal/fipstest, while if calling it from key generation/importing, add
-// an invocation to fipstest.TestPCTs.
+// If a package p calls CAST from its init function, an import of p should also
+// be added to crypto/internal/fipstest. If a package p calls CAST on the first
+// use of the algorithm, an invocation of that algorithm should be added to
+// fipstest.TestConditionals.
 func CAST(name string, f func() error) {
        if strings.ContainsAny(name, ",#=:") {
                panic("fips: invalid self-test name: " + name)
@@ -45,8 +43,8 @@ func CAST(name string, f func() error) {
        }
 
        err := f()
-       if failfipscast != "" && strings.Contains(name, failfipscast) {
-               err = errors.New("simulated CAST/PCT failure")
+       if name == failfipscast {
+               err = errors.New("simulated CAST failure")
        }
        if err != nil {
                fatal("FIPS 140-3 self-test failed: " + name + ": " + err.Error())
@@ -56,3 +54,29 @@ func CAST(name string, f func() error) {
                println("FIPS 140-3 self-test passed:", name)
        }
 }
+
+// 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.
+//
+// PCTs are mandatory for every key pair that is generated/imported, including
+// ephemeral keys (which effectively doubles the cost of key establishment). See
+// Implementation Guidance 10.3.A Additional Comment 1.
+//
+// The name must not contain commas, colons, hashes, or equal signs.
+//
+// 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 {
+       if strings.ContainsAny(name, ",#=:") {
+               panic("fips: invalid self-test name: " + name)
+       }
+       if !Enabled {
+               return nil
+       }
+
+       err := f()
+       if name == failfipscast {
+               err = errors.New("simulated PCT failure")
+       }
+       return err
+}
index 66edc8d1f8e96fefeddbf412eb5fb55e905a8f65..032f033dea344449744670e4a7363b93612b53f1 100644 (file)
@@ -156,7 +156,7 @@ func checkKeyAndComputePublicKey[P point[P]](key []byte, newPoint func() P, scal
        // 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.
-       fips.CAST("ECDH PCT", func() error {
+       if err := fips.PCT("ECDH PCT", func() error {
                p1, err := newPoint().ScalarBaseMult(key)
                if err != nil {
                        return err
@@ -165,7 +165,9 @@ func checkKeyAndComputePublicKey[P point[P]](key []byte, newPoint func() P, scal
                        return errors.New("crypto/ecdh: public key does not match private key")
                }
                return nil
-       })
+       }); err != nil {
+               panic(err)
+       }
 
        return publicKey, nil
 }
index 0fcc626f9c00ddf7cafd39cab7b60c6503f9409e..280516aea7276e92bdfef8dc87fe8a28f9229b55 100644 (file)
@@ -51,12 +51,14 @@ func testHash() []byte {
 }
 
 func fipsPCT[P Point[P]](c *Curve[P], k *PrivateKey) error {
-       hash := testHash()
-       sig, err := Sign(c, sha512.New, k, nil, hash)
-       if err != nil {
-               return err
-       }
-       return Verify(c, &k.pub, hash, sig)
+       return fips.PCT("ECDSA PCT", func() error {
+               hash := testHash()
+               sig, err := Sign(c, sha512.New, k, nil, hash)
+               if err != nil {
+                       return err
+               }
+               return Verify(c, &k.pub, hash, sig)
+       })
 }
 
 var fipsSelfTest = sync.OnceFunc(func() {
index 36f8cb3bb77229c97c8a73f5cc7b78f19a66fc49..a4834307d4b01f15a54cef05c90a1a82196a8247 100644 (file)
@@ -167,7 +167,11 @@ 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)}
-       fips.CAST("ECDSA PCT", func() error { return fipsPCT(c, priv) })
+       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
 }
 
@@ -209,7 +213,10 @@ func GenerateKey[P Point[P]](c *Curve[P], rand io.Reader) (*PrivateKey, error) {
                },
                d: k.Bytes(c.N),
        }
-       fips.CAST("ECDSA PCT", func() error { return fipsPCT(c, priv) })
+       if err := fipsPCT(c, priv); err != nil {
+               // This clearly can't happen, but FIPS 140-3 mandates that we check it.
+               panic(err)
+       }
        return priv, nil
 }
 
index 5ab94f99e51272b055d7dfd2592250f443d46354..30c9f3f0fb718a5b5662520fb6ea811ec9007dfa 100644 (file)
@@ -91,7 +91,10 @@ func generateKey1024(dk *DecapsulationKey1024) (*DecapsulationKey1024, error) {
        var z [32]byte
        drbg.Read(z[:])
        kemKeyGen1024(dk, &d, &z)
-       fips.CAST("ML-KEM PCT", func() error { return kemPCT1024(dk) })
+       if err := fips.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)
+       }
        fips.RecordApproved()
        return dk, nil
 }
@@ -119,7 +122,10 @@ func newKeyFromSeed1024(dk *DecapsulationKey1024, seed []byte) (*DecapsulationKe
        d := (*[32]byte)(seed[:32])
        z := (*[32]byte)(seed[32:])
        kemKeyGen1024(dk, d, z)
-       fips.CAST("ML-KEM PCT", func() error { return kemPCT1024(dk) })
+       if err := fips.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)
+       }
        fips.RecordApproved()
        return dk, nil
 }
index df49f51b8f237aedf25f89913eedddd3d054ba69..dcab3d8842f7b374acbdda43c35a76a37c9dfcda 100644 (file)
@@ -148,7 +148,10 @@ func generateKey(dk *DecapsulationKey768) (*DecapsulationKey768, error) {
        var z [32]byte
        drbg.Read(z[:])
        kemKeyGen(dk, &d, &z)
-       fips.CAST("ML-KEM PCT", func() error { return kemPCT(dk) })
+       if err := fips.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)
+       }
        fips.RecordApproved()
        return dk, nil
 }
@@ -176,7 +179,10 @@ func newKeyFromSeed(dk *DecapsulationKey768, seed []byte) (*DecapsulationKey768,
        d := (*[32]byte)(seed[:32])
        z := (*[32]byte)(seed[32:])
        kemKeyGen(dk, d, z)
-       fips.CAST("ML-KEM PCT", func() error { return kemPCT(dk) })
+       if err := fips.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)
+       }
        fips.RecordApproved()
        return dk, nil
 }
index bbfe1012a046019f49a8ce16dfe2af36e5816a26..2b1523b3a358aa1d68b283084222d569f49258a7 100644 (file)
@@ -23,7 +23,6 @@ import (
        _ "crypto/internal/fips/hmac"
        "crypto/internal/fips/mlkem"
        "crypto/internal/fips/sha256"
-       _ "crypto/internal/fips/sha256"
        _ "crypto/internal/fips/sha3"
        _ "crypto/internal/fips/sha512"
        _ "crypto/internal/fips/tls12"
@@ -44,9 +43,9 @@ func findAllCASTs(t *testing.T) map[string]struct{} {
        fipsDir := strings.TrimSpace(string(out))
        t.Logf("FIPS module directory: %s", fipsDir)
 
-       // Find all invocations of fips.CAST.
+       // Find all invocations of fips.CAST or fips.PCT.
        allCASTs := make(map[string]struct{})
-       castRe := regexp.MustCompile(`fips\.CAST\("([^"]+)"`)
+       castRe := regexp.MustCompile(`fips\.(CAST|PCT)\("([^"]+)"`)
        if err := fs.WalkDir(os.DirFS(fipsDir), ".", func(path string, d fs.DirEntry, err error) error {
                if err != nil {
                        return err
@@ -59,7 +58,7 @@ func findAllCASTs(t *testing.T) map[string]struct{} {
                        return err
                }
                for _, m := range castRe.FindAllSubmatch(data, -1) {
-                       allCASTs[string(m[1])] = struct{}{}
+                       allCASTs[string(m[2])] = struct{}{}
                }
                return nil
        }); err != nil {
@@ -99,11 +98,11 @@ func TestCASTFailures(t *testing.T) {
                        if err == nil {
                                t.Error(err)
                        } else {
-                               t.Logf("CAST %s failed and caused the program to exit", name)
+                               t.Logf("CAST/PCT %s failed and caused the program to exit or the test to fail", name)
                                t.Logf("%s", out)
                        }
                        if strings.Contains(string(out), "completed successfully") {
-                               t.Errorf("CAST %s failure did not stop the program", name)
+                               t.Errorf("CAST/PCT %s failure did not stop the program", name)
                        }
                })
        }