Skip to content

Commit

Permalink
[Go 1.20] Restrict 1024 keygen to tests in strict fips mode (#113)
Browse files Browse the repository at this point in the history 
* 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.
  • Loading branch information
@dbenoit17
dbenoit17 authored Aug 15, 2023
1 parent a4cac33 commit 6fee13f
Show file tree
Hide file tree
Showing 2 changed files with 398 additions and 12 deletions.
310 changes: 298 additions & 12 deletions patches/002-strict-fips-runtime-detection.patch
View file
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -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"
Expand All @@ -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
+
Expand All @@ -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")
Expand All @@ -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
Expand Down
Loading

0 comments on commit 6fee13f

Please sign in to comment.