From 6fee13fbccd83d1106dc3fd8b445079671e444c5 Mon Sep 17 00:00:00 2001 From: David Benoit Date: Tue, 15 Aug 2023 13:31:53 -0400 Subject: [PATCH] [Go 1.20] Restrict 1024 keygen to tests in strict fips mode (#113) * Restrict 1024 bit keygen in strict fips mode * Add crypto test script This commit adds crypto-test.sh, which runs the Go crypto and tls tests in both default fips and strict fips modes. --- .../002-strict-fips-runtime-detection.patch | 310 +++++++++++++++++- scripts/crypto-test.sh | 100 ++++++ 2 files changed, 398 insertions(+), 12 deletions(-) create mode 100755 scripts/crypto-test.sh diff --git a/patches/002-strict-fips-runtime-detection.patch b/patches/002-strict-fips-runtime-detection.patch index ff2765e4f0..905d3ed00f 100644 --- a/patches/002-strict-fips-runtime-detection.patch +++ b/patches/002-strict-fips-runtime-detection.patch @@ -1,3 +1,13 @@ +diff --git a/api/go1.16.txt b/api/go1.16.txt +index e12a050939..e555bfdf5c 100644 +--- a/api/go1.16.txt ++++ b/api/go1.16.txt +@@ -8525,3 +8525,5 @@ pkg syscall (darwin-arm64-cgo), type WaitStatus uint32 + pkg syscall (darwin-arm64-cgo), var Stderr int + pkg syscall (darwin-arm64-cgo), var Stdin int + pkg syscall (darwin-arm64-cgo), var Stdout int ++pkg crypto/rsa, func GenerateKeyNotBoring(io.Reader, int) (*PrivateKey, error) ++pkg crypto/rsa, func GenerateMultiPrimeKeyNotBoring(io.Reader, int, int) (*PrivateKey, error) diff --git a/src/crypto/internal/backend/hostfips.go b/src/crypto/internal/backend/hostfips.go new file mode 100644 index 0000000000..6fcd7139c6 @@ -26,10 +36,10 @@ index 0000000000..6fcd7139c6 + return len(data) > 0 && data[0] == '1' +} diff --git a/src/crypto/internal/backend/nobackend.go b/src/crypto/internal/backend/nobackend.go -index 15c1ee8cbe..efb7555948 100644 +index 15c1ee8cbe..5fca37e5de 100644 --- a/src/crypto/internal/backend/nobackend.go +++ b/src/crypto/internal/backend/nobackend.go -@@ -11,12 +11,17 @@ import ( +@@ -11,14 +11,23 @@ import ( "crypto" "crypto/cipher" "crypto/internal/boring/sig" @@ -41,51 +51,66 @@ index 15c1ee8cbe..efb7555948 100644 + + "github.com/golang-fips/openssl-fips/openssl" ) - + +func init() { + strictFIPSNonCompliantBinaryCheck() +} + var enabled = false - + ++func IsStrictFIPSMode() bool { ++ return false ++} ++ // Unreachable marks code that should be unreachable + // when BoringCrypto is in use. It is a no-op without BoringCrypto. + func Unreachable() { diff --git a/src/crypto/internal/backend/not_strict_fips.go b/src/crypto/internal/backend/not_strict_fips.go new file mode 100644 -index 0000000000..f8e8fd6869 +index 0000000000..806b035aa8 --- /dev/null +++ b/src/crypto/internal/backend/not_strict_fips.go -@@ -0,0 +1,10 @@ +@@ -0,0 +1,12 @@ +//go:build !goexperiment.strictfipsruntime +// +build !goexperiment.strictfipsruntime + +package backend + ++var isStrictFIPS bool = false ++ +func strictFIPSOpenSSLRuntimeCheck() { +} + +func strictFIPSNonCompliantBinaryCheck() { +} diff --git a/src/crypto/internal/backend/openssl.go b/src/crypto/internal/backend/openssl.go -index 2087c555a4..3e5ee01efc 100644 +index 2087c555a4..be67318740 100644 --- a/src/crypto/internal/backend/openssl.go +++ b/src/crypto/internal/backend/openssl.go -@@ -14,6 +14,10 @@ import ( +@@ -14,9 +14,17 @@ import ( "github.com/golang-fips/openssl-fips/openssl" ) - + +func init() { + strictFIPSOpenSSLRuntimeCheck() +} + // Enabled controls whether FIPS crypto is enabled. var Enabled = openssl.Enabled - + ++func IsStrictFIPSMode() bool { ++ return isStrictFIPS ++} ++ + // Unreachable marks code that should be unreachable + // when OpenSSLCrypto is in use. It panics only when + // the system is in FIPS mode. diff --git a/src/crypto/internal/backend/strict_fips.go b/src/crypto/internal/backend/strict_fips.go new file mode 100644 -index 0000000000..894eeca942 +index 0000000000..c1bda67f12 --- /dev/null +++ b/src/crypto/internal/backend/strict_fips.go -@@ -0,0 +1,23 @@ +@@ -0,0 +1,25 @@ +//go:build goexperiment.strictfipsruntime +// +build goexperiment.strictfipsruntime + @@ -96,6 +121,8 @@ index 0000000000..894eeca942 + "os" +) + ++var isStrictFIPS bool = true ++ +func strictFIPSOpenSSLRuntimeCheck() { + if hostFIPSModeEnabled() && !Enabled() { + fmt.Fprintln(os.Stderr, "FIPS mode is enabled, but the required OpenSSL backend is unavailable") @@ -109,6 +136,265 @@ index 0000000000..894eeca942 + os.Exit(1) + } +} +diff --git a/src/crypto/rsa/boring_test.go b/src/crypto/rsa/boring_test.go +index 4e7fd9de4a..bd060c6a9d 100644 +--- a/src/crypto/rsa/boring_test.go ++++ b/src/crypto/rsa/boring_test.go +@@ -22,7 +22,7 @@ import ( + ) + + func TestBoringASN1Marshal(t *testing.T) { +- k, err := GenerateKey(rand.Reader, 128) ++ k, err := GenerateKey(rand.Reader, 3072) + if err != nil { + t.Fatal(err) + } +diff --git a/src/crypto/rsa/equal_test.go b/src/crypto/rsa/equal_test.go +index 90f4bf9475..688df68545 100644 +--- a/src/crypto/rsa/equal_test.go ++++ b/src/crypto/rsa/equal_test.go +@@ -13,7 +13,7 @@ import ( + ) + + func TestEqual(t *testing.T) { +- private, _ := rsa.GenerateKey(rand.Reader, 512) ++ private, _ := rsa.GenerateKey(rand.Reader, 2048) + public := &private.PublicKey + + if !public.Equal(public) { +@@ -41,7 +41,7 @@ func TestEqual(t *testing.T) { + t.Errorf("private key is not equal to itself after decoding: %v", private) + } + +- other, _ := rsa.GenerateKey(rand.Reader, 512) ++ other, _ := rsa.GenerateKey(rand.Reader, 2048) + if public.Equal(other.Public()) { + t.Errorf("different public keys are Equal") + } +diff --git a/src/crypto/rsa/pss_test.go b/src/crypto/rsa/pss_test.go +index befd1612b5..afa25a737c 100644 +--- a/src/crypto/rsa/pss_test.go ++++ b/src/crypto/rsa/pss_test.go +@@ -301,7 +301,7 @@ func fromHex(hexStr string) []byte { + } + + func TestInvalidPSSSaltLength(t *testing.T) { +- key, err := GenerateKey(rand.Reader, 245) ++ key, err := GenerateKey(rand.Reader, 2048) + if err != nil { + t.Fatal(err) + } +diff --git a/src/crypto/rsa/rsa.go b/src/crypto/rsa/rsa.go +index dd79dc5439..52f2ac347c 100644 +--- a/src/crypto/rsa/rsa.go ++++ b/src/crypto/rsa/rsa.go +@@ -264,6 +264,7 @@ func GenerateKey(random io.Reader, bits int) (*PrivateKey, error) { + return GenerateMultiPrimeKey(random, 2, bits) + } + ++ + // GenerateMultiPrimeKey generates a multi-prime RSA keypair of the given bit + // size and the given random source. + // +@@ -284,6 +285,24 @@ func GenerateKey(random io.Reader, bits int) (*PrivateKey, error) { + // + // [On the Security of Multi-prime RSA]: http://www.cacr.math.uwaterloo.ca/techreports/2006/cacr2006-16.pdf + func GenerateMultiPrimeKey(random io.Reader, nprimes int, bits int) (*PrivateKey, error) { ++ if boring.Enabled() && boring.IsStrictFIPSMode() && !(random == boring.RandReader && nprimes == 2 && ++ (bits == 2048 || bits == 3072 || bits == 4096)) { ++ return nil, errors.New("crypto/rsa: invalid primes or bits for boring") ++ } ++ return generateMultiPrimeKeyInternal(random, nprimes, bits) ++} ++ ++func GenerateKeyNotBoring(random io.Reader, bits int) (*PrivateKey, error) { ++ boring.UnreachableExceptTests() ++ return generateMultiPrimeKeyInternal(random, 2, bits) ++} ++ ++func GenerateMultiPrimeKeyNotBoring(random io.Reader, nprimes int, bits int) (*PrivateKey, error) { ++ boring.UnreachableExceptTests() ++ return generateMultiPrimeKeyInternal(random, nprimes, bits) ++} ++ ++func generateMultiPrimeKeyInternal(random io.Reader, nprimes int, bits int) (*PrivateKey, error) { + randutil.MaybeReadByte(random) + + if boring.Enabled() && random == boring.RandReader && nprimes == 2 && +@@ -324,6 +343,7 @@ func GenerateMultiPrimeKey(random io.Reader, nprimes int, bits int) (*PrivateKey + return key, nil + } + ++ + priv := new(PrivateKey) + priv.E = 65537 + +diff --git a/src/crypto/rsa/rsa_test.go b/src/crypto/rsa/rsa_test.go +index 4b7427e1ae..da8c104044 100644 +--- a/src/crypto/rsa/rsa_test.go ++++ b/src/crypto/rsa/rsa_test.go +@@ -26,7 +26,17 @@ import ( + import "crypto/internal/backend/boringtest" + + func TestKeyGeneration(t *testing.T) { +- for _, size := range []int{128, 1024, 2048, 3072} { ++ testKeys := []int{128, 1024} ++ if boring.Enabled() { ++ for _, size := range testKeys { ++ _, err := GenerateKey(rand.Reader, size) ++ if err == nil && boring.IsStrictFIPSMode() { ++ t.Errorf("Gener(%d): boring: bad accept", size) ++ } ++ } ++ testKeys = []int{2048, 3072} ++ } ++ for _, size := range testKeys { + priv, err := GenerateKey(rand.Reader, size) + if err != nil { + t.Errorf("GenerateKey(%d): %v", size, err) +@@ -53,7 +63,12 @@ func Test3PrimeKeyGeneration(t *testing.T) { + + priv, err := GenerateMultiPrimeKey(rand.Reader, 3, size) + if err != nil { ++ if boring.IsStrictFIPSMode() { ++ return ++ } + t.Errorf("failed to generate key") ++ } else if boring.IsStrictFIPSMode() { ++ t.Errorf("bad accept in strictfipsmode") + } + testKeyBasics(t, priv) + } +@@ -66,12 +81,20 @@ func Test4PrimeKeyGeneration(t *testing.T) { + + priv, err := GenerateMultiPrimeKey(rand.Reader, 4, size) + if err != nil { ++ if boring.IsStrictFIPSMode() { ++ return ++ } + t.Errorf("failed to generate key") ++ } else if boring.IsStrictFIPSMode() { ++ t.Errorf("bad accept in strictfipsmode") + } + testKeyBasics(t, priv) + } + + func TestNPrimeKeyGeneration(t *testing.T) { ++ if boring.Enabled() { ++ t.Skip("Not supported in boring mode") ++ } + primeSize := 64 + maxN := 24 + if testing.Short() { +@@ -206,7 +229,7 @@ func TestEverything(t *testing.T) { + size := size + t.Run(fmt.Sprintf("%d", size), func(t *testing.T) { + t.Parallel() +- priv, err := GenerateKey(rand.Reader, size) ++ priv, err := GenerateKeyNotBoring(rand.Reader, size) + if err != nil { + t.Errorf("GenerateKey(%d): %v", size, err) + } +diff --git a/src/crypto/tls/boring_test.go b/src/crypto/tls/boring_test.go +index 49702f59ba..e7ae7fc5ca 100644 +--- a/src/crypto/tls/boring_test.go ++++ b/src/crypto/tls/boring_test.go +@@ -329,7 +329,7 @@ func TestBoringCertAlgs(t *testing.T) { + // Set up some roots, intermediate CAs, and leaf certs with various algorithms. + // X_Y is X signed by Y. + R1 := boringCert(t, "R1", boringRSAKey(t, 2048), nil, boringCertCA|boringCertFIPSOK) +- R2 := boringCert(t, "R2", boringRSAKey(t, 512), nil, boringCertCA) ++ R2 := boringCert(t, "R2", NotBoringRSAKey(t, 512), nil, boringCertCA) + + M1_R1 := boringCert(t, "M1_R1", boringECDSAKey(t, elliptic.P256()), R1, boringCertCA|boringCertFIPSOK) + +@@ -353,9 +353,9 @@ func TestBoringCertAlgs(t *testing.T) { + // Older versions of OpenSSL allow 1024 bit leaf certs + var L2_I *boringCertificate + if boringtest.Supports(t, "RSA1024LeafCert") { +- L2_I = boringCert(t, "L2_I", boringRSAKey(t, 1024), I_R1, boringCertLeaf) ++ L2_I = boringCert(t, "L2_I", NotBoringRSAKey(t, 1024), I_R1, boringCertLeaf) + } else { +- L2_I = boringCert(t, "L2_I", boringRSAKey(t, 1024), I_R1, boringCertLeaf|boringCertNotBoring) ++ L2_I = boringCert(t, "L2_I", NotBoringRSAKey(t, 1024), I_R1, boringCertLeaf|boringCertNotBoring) + } + + // client verifying server cert +@@ -515,6 +515,15 @@ const ( + boringCertNotBoring = 0x100 + ) + ++func NotBoringRSAKey(t *testing.T, size int) *rsa.PrivateKey { ++ k, err := rsa.GenerateKeyNotBoring(rand.Reader, size) ++ if err != nil { ++ t.Fatal(err) ++ } ++ return k ++} ++ ++ + func boringRSAKey(t *testing.T, size int) *rsa.PrivateKey { + k, err := rsa.GenerateKey(rand.Reader, size) + if err != nil { +diff --git a/src/crypto/x509/boring_test.go b/src/crypto/x509/boring_test.go +index 07b3c7095e..88b69937be 100644 +--- a/src/crypto/x509/boring_test.go ++++ b/src/crypto/x509/boring_test.go +@@ -27,6 +27,14 @@ const ( + boringCertFIPSOK = 0x80 + ) + ++func notBoringRSAKey(t *testing.T, size int) *rsa.PrivateKey { ++ k, err := rsa.GenerateKeyNotBoring(rand.Reader, size) ++ if err != nil { ++ t.Fatal(err) ++ } ++ return k ++} ++ + func boringRSAKey(t *testing.T, size int) *rsa.PrivateKey { + k, err := rsa.GenerateKey(rand.Reader, size) + if err != nil { +@@ -55,7 +63,7 @@ type boringCertificate struct { + + func TestBoringAllowCert(t *testing.T) { + R1 := testBoringCert(t, "R1", boringRSAKey(t, 2048), nil, boringCertCA|boringCertFIPSOK) +- R2 := testBoringCert(t, "R2", boringRSAKey(t, 512), nil, boringCertCA) ++ R2 := testBoringCert(t, "R2", notBoringRSAKey(t, 512), nil, boringCertCA) + R3 := testBoringCert(t, "R3", boringRSAKey(t, 4096), nil, boringCertCA|boringCertFIPSOK) + + M1_R1 := testBoringCert(t, "M1_R1", boringECDSAKey(t, elliptic.P256()), R1, boringCertCA|boringCertFIPSOK) +@@ -78,7 +86,7 @@ func TestBoringAllowCert(t *testing.T) { + testBoringCert(t, "I_R3", I_R3.key, R3, boringCertCA|boringCertFIPSOK) + + testBoringCert(t, "L1_I", boringECDSAKey(t, elliptic.P384()), I_R1, boringCertLeaf|boringCertFIPSOK) +- testBoringCert(t, "L2_I", boringRSAKey(t, 1024), I_R1, boringCertLeaf) ++ testBoringCert(t, "L2_I", notBoringRSAKey(t, 1024), I_R1, boringCertLeaf) + } + + func testBoringCert(t *testing.T, name string, key interface{}, parent *boringCertificate, mode int) *boringCertificate { +diff --git a/src/crypto/x509/x509_test.go b/src/crypto/x509/x509_test.go +index 22a104f338..8f77ffda62 100644 +--- a/src/crypto/x509/x509_test.go ++++ b/src/crypto/x509/x509_test.go +@@ -2927,7 +2927,7 @@ func TestUnknownExtKey(t *testing.T) { + DNSNames: []string{"foo"}, + ExtKeyUsage: []ExtKeyUsage{ExtKeyUsage(-1)}, + } +- signer, err := rsa.GenerateKey(rand.Reader, 1024) ++ signer, err := rsa.GenerateKey(rand.Reader, 2048) + if err != nil { + t.Errorf("failed to generate key for TestUnknownExtKey") + } +@@ -3090,7 +3090,7 @@ func TestCreateCertificateBrokenSigner(t *testing.T) { + SerialNumber: big.NewInt(10), + DNSNames: []string{"example.com"}, + } +- k, err := rsa.GenerateKey(rand.Reader, 1024) ++ k, err := rsa.GenerateKey(rand.Reader, 2048) + if err != nil { + t.Fatalf("failed to generate test key: %s", err) + } diff --git a/src/internal/goexperiment/exp_strictfipsruntime_off.go b/src/internal/goexperiment/exp_strictfipsruntime_off.go new file mode 100644 index 0000000000..73a676a18b diff --git a/scripts/crypto-test.sh b/scripts/crypto-test.sh new file mode 100755 index 0000000000..aed7758f26 --- /dev/null +++ b/scripts/crypto-test.sh @@ -0,0 +1,100 @@ +#!/bin/bash + +set -eE + +quiet () { + 2>&1>/dev/null $@ +} + +# Find the GOROOT. +# If using a release branch, expect the GOROOT +# in the go submodule directory. +GOROOT=$(readlink -f $(dirname $0)/..) +quiet pushd $GOROOT +if 2>/dev/null cat .gitmodules | grep -q "url = https://github.com/golang/go.git"; then + GOROOT=${GOROOT}/go +fi +quiet popd + +export GOCACHE=/tmp/go-cache +export GO=${GOROOT}/bin/go + +# Test suites to run +SUITES="crypto,tls" +# Verbosity flags to pass to Go +VERBOSE="" + +# Parse command line arguments +while [[ $# -gt 0 ]]; do + case $1 in + --suites) + SUITES=$2 + shift;shift + ;; + -v) + VERBOSE="$VERBOSE -v" + set -x + shift + ;; + *) + >&2 echo "unsupported option $1" + exit 1 + ;; + esac +done + +notify_running() { + local mode=$1 + local suite=$2 + echo -e "\n##### ${suite} (${mode})" +} + +run_crypto_test_suite () { + local mode=$1 + local tags=$2 + local suite="crypto-fips" + notify_running ${mode} ${suite} + quiet pushd ${GOROOT}/src/crypto + GOLANG_FIPS=1 OPENSSL_FORCE_FIPS_MODE=1 \ + $GO test $tags -count=1 $($GO list ./... | grep -v tls) $VERBOSE + + local suite="crypto-fips-parity-nocgo" + notify_running ${mode} ${suite} + GOLANG_FIPS=1 OPENSSL_FORCE_FIPS_MODE=1 \ + CGO_ENABLED=0 $GO test $tags -count=1 $($GO list ./... | grep -v tls) $VERBOSE + quiet popd +} + +run_tls_test_suite () { + local mode=$1 + local tags=$2 + local suite="tls-fips" + notify_running ${mode} ${suite} + quiet pushd ${GOROOT}/src + GOLANG_FIPS=1 OPENSSL_FORCE_FIPS_MODE=1 \ + $GO test $tags -count=1 crypto/tls -run "^TestBoring" $VERBOSE + quiet popd +} + + +run_full_test_suite () { + local mode=$1 + local tags=$2 + for suite in ${SUITES//,/ }; do + if [[ "$suite" == "crypto" ]]; then + run_crypto_test_suite ${mode} ${tags} + elif [[ "$suite" == "tls" ]]; then + run_tls_test_suite ${mode} ${tags} + fi + done +} + +# Run in default mode +run_full_test_suite default "" + +# Run in strict fips mode +export GOEXPERIMENT=strictfipsruntime +run_full_test_suite strictfips "-tags=strictfipsruntime" + +echo ALL TESTS PASSED +