From d36353499f673c89a267a489beb80133a14a75f9 Mon Sep 17 00:00:00 2001 From: Filippo Valsorda Date: Thu, 14 Dec 2023 22:13:29 +0100 Subject: [PATCH] crypto/tls: align FIPS-only mode with BoringSSL policy This enables TLS 1.3, disables P-521, and disables non-ECDHE suites. Reapplies CL 549975. Updates #64717 Updates #62372 Change-Id: I6c608704638d59a063a657fbd4eb1126027112dd Reviewed-on: https://go-review.googlesource.com/c/go/+/603376 Reviewed-by: Roland Shoemaker LUCI-TryBot-Result: Go LUCI Reviewed-by: David Chase --- src/crypto/internal/boring/aes.go | 29 +++++++--- src/crypto/internal/boring/notboring.go | 1 + src/crypto/tls/boring_test.go | 70 ++++++++++++++++-------- src/crypto/tls/cipher_suites.go | 8 ++- src/crypto/tls/defaults.go | 8 +-- src/crypto/tls/handshake_client.go | 9 ++- src/crypto/tls/handshake_client_tls13.go | 4 -- src/crypto/tls/handshake_server_test.go | 1 + src/crypto/tls/handshake_server_tls13.go | 7 +-- 9 files changed, 93 insertions(+), 44 deletions(-) diff --git a/src/crypto/internal/boring/aes.go b/src/crypto/internal/boring/aes.go index 8819f576f4f..d18ed5cdc5c 100644 --- a/src/crypto/internal/boring/aes.go +++ b/src/crypto/internal/boring/aes.go @@ -228,26 +228,41 @@ func (c *aesCipher) NewGCM(nonceSize, tagSize int) (cipher.AEAD, error) { if tagSize != gcmTagSize { return cipher.NewGCMWithTagSize(&noGCM{c}, tagSize) } - return c.newGCM(false) + return c.newGCM(0) } +const ( + VersionTLS12 = 0x0303 + VersionTLS13 = 0x0304 +) + func NewGCMTLS(c cipher.Block) (cipher.AEAD, error) { - return c.(*aesCipher).newGCM(true) + return c.(*aesCipher).newGCM(VersionTLS12) } -func (c *aesCipher) newGCM(tls bool) (cipher.AEAD, error) { +func NewGCMTLS13(c cipher.Block) (cipher.AEAD, error) { + return c.(*aesCipher).newGCM(VersionTLS13) +} + +func (c *aesCipher) newGCM(tlsVersion uint16) (cipher.AEAD, error) { var aead *C.GO_EVP_AEAD switch len(c.key) * 8 { case 128: - if tls { + switch tlsVersion { + case VersionTLS12: aead = C._goboringcrypto_EVP_aead_aes_128_gcm_tls12() - } else { + case VersionTLS13: + aead = C._goboringcrypto_EVP_aead_aes_128_gcm_tls13() + default: aead = C._goboringcrypto_EVP_aead_aes_128_gcm() } case 256: - if tls { + switch tlsVersion { + case VersionTLS12: aead = C._goboringcrypto_EVP_aead_aes_256_gcm_tls12() - } else { + case VersionTLS13: + aead = C._goboringcrypto_EVP_aead_aes_256_gcm_tls13() + default: aead = C._goboringcrypto_EVP_aead_aes_256_gcm() } default: diff --git a/src/crypto/internal/boring/notboring.go b/src/crypto/internal/boring/notboring.go index 361dec9672f..02bc468a0de 100644 --- a/src/crypto/internal/boring/notboring.go +++ b/src/crypto/internal/boring/notboring.go @@ -50,6 +50,7 @@ func NewHMAC(h func() hash.Hash, key []byte) hash.Hash { panic("boringcrypto: no func NewAESCipher(key []byte) (cipher.Block, error) { panic("boringcrypto: not available") } func NewGCMTLS(cipher.Block) (cipher.AEAD, error) { panic("boringcrypto: not available") } +func NewGCMTLS13(cipher.Block) (cipher.AEAD, error) { panic("boringcrypto: not available") } type PublicKeyECDSA struct{ _ int } type PrivateKeyECDSA struct{ _ int } diff --git a/src/crypto/tls/boring_test.go b/src/crypto/tls/boring_test.go index be10b71bd22..56050421985 100644 --- a/src/crypto/tls/boring_test.go +++ b/src/crypto/tls/boring_test.go @@ -25,6 +25,31 @@ import ( "time" ) +func allCipherSuitesIncludingTLS13() []uint16 { + s := allCipherSuites() + for _, suite := range cipherSuitesTLS13 { + s = append(s, suite.id) + } + return s +} + +func isTLS13CipherSuite(id uint16) bool { + for _, suite := range cipherSuitesTLS13 { + if id == suite.id { + return true + } + } + return false +} + +func generateKeyShare(group CurveID) keyShare { + key, err := generateECDHEKey(rand.Reader, group) + if err != nil { + panic(err) + } + return keyShare{group: group, data: key.PublicKey().Bytes()} +} + func TestBoringServerProtocolVersion(t *testing.T) { test := func(t *testing.T, name string, v uint16, msg string) { t.Run(name, func(t *testing.T) { @@ -60,22 +85,22 @@ func TestBoringServerProtocolVersion(t *testing.T) { test(t, "VersionTLS10", VersionTLS10, "supported versions") test(t, "VersionTLS11", VersionTLS11, "supported versions") test(t, "VersionTLS12", VersionTLS12, "") - test(t, "VersionTLS13", VersionTLS13, "supported versions") + test(t, "VersionTLS13", VersionTLS13, "") }) } func isBoringVersion(v uint16) bool { - return v == VersionTLS12 + return v == VersionTLS12 || v == VersionTLS13 } func isBoringCipherSuite(id uint16) bool { switch id { - case TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256, + case TLS_AES_128_GCM_SHA256, + TLS_AES_256_GCM_SHA384, + TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256, TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384, TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256, - TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384, - TLS_RSA_WITH_AES_128_GCM_SHA256, - TLS_RSA_WITH_AES_256_GCM_SHA384: + TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384: return true } return false @@ -83,7 +108,7 @@ func isBoringCipherSuite(id uint16) bool { func isBoringCurve(id CurveID) bool { switch id { - case CurveP256, CurveP384, CurveP521: + case CurveP256, CurveP384: return true } return false @@ -95,7 +120,7 @@ func isECDSA(id uint16) bool { return suite.flags&suiteECSign == suiteECSign } } - panic(fmt.Sprintf("unknown cipher suite %#x", id)) + return false // TLS 1.3 cipher suites are not tied to the signature algorithm. } func isBoringSignatureScheme(alg SignatureScheme) bool { @@ -107,7 +132,6 @@ func isBoringSignatureScheme(alg SignatureScheme) bool { PKCS1WithSHA384, ECDSAWithP384AndSHA384, PKCS1WithSHA512, - ECDSAWithP521AndSHA512, PSSWithSHA256, PSSWithSHA384, PSSWithSHA512: @@ -118,10 +142,9 @@ func isBoringSignatureScheme(alg SignatureScheme) bool { func TestBoringServerCipherSuites(t *testing.T) { serverConfig := testConfig.Clone() - serverConfig.CipherSuites = allCipherSuites() serverConfig.Certificates = make([]Certificate, 1) - for _, id := range allCipherSuites() { + for _, id := range allCipherSuitesIncludingTLS13() { if isECDSA(id) { serverConfig.Certificates[0].Certificate = [][]byte{testECDSACertificate} serverConfig.Certificates[0].PrivateKey = testECDSAPrivateKey @@ -130,14 +153,20 @@ func TestBoringServerCipherSuites(t *testing.T) { serverConfig.Certificates[0].PrivateKey = testRSAPrivateKey } serverConfig.BuildNameToCertificate() - t.Run(fmt.Sprintf("suite=%#x", id), func(t *testing.T) { + t.Run(fmt.Sprintf("suite=%s", CipherSuiteName(id)), func(t *testing.T) { clientHello := &clientHelloMsg{ - vers: VersionTLS12, - random: make([]byte, 32), - cipherSuites: []uint16{id}, - compressionMethods: []uint8{compressionNone}, - supportedCurves: defaultCurvePreferences(), - supportedPoints: []uint8{pointFormatUncompressed}, + vers: VersionTLS12, + random: make([]byte, 32), + cipherSuites: []uint16{id}, + compressionMethods: []uint8{compressionNone}, + supportedCurves: defaultCurvePreferences(), + keyShares: []keyShare{generateKeyShare(CurveP256)}, + supportedPoints: []uint8{pointFormatUncompressed}, + supportedVersions: []uint16{VersionTLS12}, + supportedSignatureAlgorithms: defaultSupportedSignatureAlgorithmsFIPS, + } + if isTLS13CipherSuite(id) { + clientHello.supportedVersions = []uint16{VersionTLS13} } testClientHello(t, serverConfig, clientHello) @@ -156,9 +185,6 @@ func TestBoringServerCipherSuites(t *testing.T) { func TestBoringServerCurves(t *testing.T) { serverConfig := testConfig.Clone() - serverConfig.Certificates = make([]Certificate, 1) - serverConfig.Certificates[0].Certificate = [][]byte{testECDSACertificate} - serverConfig.Certificates[0].PrivateKey = testECDSAPrivateKey serverConfig.BuildNameToCertificate() for _, curveid := range defaultCurvePreferences() { @@ -288,7 +314,7 @@ func TestBoringClientHello(t *testing.T) { } if !isBoringVersion(hello.vers) { - t.Errorf("client vers=%#x, want %#x (TLS 1.2)", hello.vers, VersionTLS12) + t.Errorf("client vers=%#x", hello.vers) } for _, v := range hello.supportedVersions { if !isBoringVersion(v) { diff --git a/src/crypto/tls/cipher_suites.go b/src/crypto/tls/cipher_suites.go index eebc66880d6..917a1eff42d 100644 --- a/src/crypto/tls/cipher_suites.go +++ b/src/crypto/tls/cipher_suites.go @@ -552,7 +552,13 @@ func aeadAESGCMTLS13(key, nonceMask []byte) aead { if err != nil { panic(err) } - aead, err := cipher.NewGCM(aes) + var aead cipher.AEAD + if boring.Enabled { + aead, err = boring.NewGCMTLS13(aes) + } else { + boring.Unreachable() + aead, err = cipher.NewGCM(aes) + } if err != nil { panic(err) } diff --git a/src/crypto/tls/defaults.go b/src/crypto/tls/defaults.go index 9b28acdc2d8..ad4070df4a8 100644 --- a/src/crypto/tls/defaults.go +++ b/src/crypto/tls/defaults.go @@ -90,13 +90,16 @@ var defaultCipherSuitesTLS13NoAES = []uint16{ TLS_AES_256_GCM_SHA384, } +// The FIPS-only policies below match BoringSSL's ssl_policy_fips_202205. + var defaultSupportedVersionsFIPS = []uint16{ VersionTLS12, + VersionTLS13, } // defaultCurvePreferencesFIPS are the FIPS-allowed curves, // in preference order (most preferable first). -var defaultCurvePreferencesFIPS = []CurveID{CurveP256, CurveP384, CurveP521} +var defaultCurvePreferencesFIPS = []CurveID{CurveP256, CurveP384} // defaultSupportedSignatureAlgorithmsFIPS currently are a subset of // defaultSupportedSignatureAlgorithms without Ed25519 and SHA-1. @@ -109,7 +112,6 @@ var defaultSupportedSignatureAlgorithmsFIPS = []SignatureScheme{ PKCS1WithSHA384, ECDSAWithP384AndSHA384, PKCS1WithSHA512, - ECDSAWithP521AndSHA512, } // defaultCipherSuitesFIPS are the FIPS-allowed cipher suites. @@ -118,8 +120,6 @@ var defaultCipherSuitesFIPS = []uint16{ TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384, TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256, TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384, - TLS_RSA_WITH_AES_128_GCM_SHA256, - TLS_RSA_WITH_AES_256_GCM_SHA384, } // defaultCipherSuitesTLS13FIPS are the FIPS-allowed cipher suites for TLS 1.3. diff --git a/src/crypto/tls/handshake_client.go b/src/crypto/tls/handshake_client.go index 5025657590d..760e827f467 100644 --- a/src/crypto/tls/handshake_client.go +++ b/src/crypto/tls/handshake_client.go @@ -141,13 +141,18 @@ func (c *Conn) makeClientHello() (*clientHelloMsg, *keySharePrivateKeys, *echCon if len(hello.supportedVersions) == 1 { hello.cipherSuites = nil } - if hasAESGCMHardwareSupport { + if needFIPS() { + hello.cipherSuites = append(hello.cipherSuites, defaultCipherSuitesTLS13FIPS...) + } else if hasAESGCMHardwareSupport { hello.cipherSuites = append(hello.cipherSuites, defaultCipherSuitesTLS13...) } else { hello.cipherSuites = append(hello.cipherSuites, defaultCipherSuitesTLS13NoAES...) } - curveID := config.curvePreferences(maxVersion)[0] + if len(hello.supportedCurves) == 0 { + return nil, nil, nil, errors.New("tls: no supported elliptic curves for ECDHE") + } + curveID := hello.supportedCurves[0] keyShareKeys = &keySharePrivateKeys{curveID: curveID} if curveID == x25519Kyber768Draft00 { keyShareKeys.ecdhe, err = generateECDHEKey(config.rand(), X25519) diff --git a/src/crypto/tls/handshake_client_tls13.go b/src/crypto/tls/handshake_client_tls13.go index db5e35d9a46..21a501fbfd5 100644 --- a/src/crypto/tls/handshake_client_tls13.go +++ b/src/crypto/tls/handshake_client_tls13.go @@ -45,10 +45,6 @@ type clientHandshakeStateTLS13 struct { func (hs *clientHandshakeStateTLS13) handshake() error { c := hs.c - if needFIPS() { - return errors.New("tls: internal error: TLS 1.3 reached in FIPS mode") - } - // The server must not select TLS 1.3 in a renegotiation. See RFC 8446, // sections 4.1.2 and 4.1.3. if c.handshakes > 0 { diff --git a/src/crypto/tls/handshake_server_test.go b/src/crypto/tls/handshake_server_test.go index 788a26af755..01eae15a6b9 100644 --- a/src/crypto/tls/handshake_server_test.go +++ b/src/crypto/tls/handshake_server_test.go @@ -29,6 +29,7 @@ import ( ) func testClientHello(t *testing.T, serverConfig *Config, m handshakeMessage) { + t.Helper() testClientHelloFailure(t, serverConfig, m, "") } diff --git a/src/crypto/tls/handshake_server_tls13.go b/src/crypto/tls/handshake_server_tls13.go index 503a732e057..b8cf4c3fa50 100644 --- a/src/crypto/tls/handshake_server_tls13.go +++ b/src/crypto/tls/handshake_server_tls13.go @@ -47,10 +47,6 @@ type serverHandshakeStateTLS13 struct { func (hs *serverHandshakeStateTLS13) handshake() error { c := hs.c - if needFIPS() { - return errors.New("tls: internal error: TLS 1.3 reached in FIPS mode") - } - // For an overview of the TLS 1.3 handshake, see RFC 8446, Section 2. if err := hs.processClientHello(); err != nil { return err @@ -165,6 +161,9 @@ func (hs *serverHandshakeStateTLS13) processClientHello() error { if !hasAESGCMHardwareSupport || !aesgcmPreferred(hs.clientHello.cipherSuites) { preferenceList = defaultCipherSuitesTLS13NoAES } + if needFIPS() { + preferenceList = defaultCipherSuitesTLS13FIPS + } for _, suiteID := range preferenceList { hs.suite = mutualCipherSuiteTLS13(hs.clientHello.cipherSuites, suiteID) if hs.suite != nil {