crypto/tls: align FIPS-only mode with BoringSSL policy

This enables TLS 1.3, disables P-521, and disables non-ECDHE suites.

Fixes #64717
Updates #62372

Change-Id: I3a65b239ef0198bbdbe5e55e0810e7128f90a091
Reviewed-on: https://go-review.googlesource.com/c/go/+/549975
Reviewed-by: Roland Shoemaker <roland@golang.org>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Than McIntosh <thanm@google.com>

diff --git a/src/crypto/internal/boring/aes.go b/src/crypto/internal/boring/aes.go
index 8819f576f4f4c5bdd3f26079fe54f9b6a8a62f1f..d18ed5cdc5c2593851da677e5608b3d73ec13412 100644 (file)
--- 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 NewGCMTLS13(c cipher.Block) (cipher.AEAD, error) {
+       return c.(*aesCipher).newGCM(VersionTLS13)
 }
 
-func (c *aesCipher) newGCM(tls bool) (cipher.AEAD, error) {
+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 361dec9672ff553db70c6aa6e925a0bd5da5d67e..02bc468a0de8e7f7dcc39f56de9c3e9faccb6fd5 100644 (file)
--- 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.go b/src/crypto/tls/boring.go
index 1827f764589b583fc72382ba3e9acc218a7ce4e1..aad96b1c74778487daae99e7a9094551a1053415 100644 (file)
--- a/src/crypto/tls/boring.go
+++ b/src/crypto/tls/boring.go
@@ -6,9 +6,10 @@
 
 package tls
 
-import (
-       "crypto/internal/boring/fipstls"
-)
+import "crypto/internal/boring/fipstls"
+
+// The FIPS-only policies enforced here currently match BoringSSL's
+// ssl_policy_fips_202205.
 
 // needFIPS returns fipstls.Required(); it avoids a new import in common.go.
 func needFIPS() bool {
@@ -17,19 +18,19 @@ func needFIPS() bool {
 
 // fipsMinVersion replaces c.minVersion in FIPS-only mode.
 func fipsMinVersion(c *Config) uint16 {
-       // FIPS requires TLS 1.2.
+       // FIPS requires TLS 1.2 or TLS 1.3.
        return VersionTLS12
 }
 
 // fipsMaxVersion replaces c.maxVersion in FIPS-only mode.
 func fipsMaxVersion(c *Config) uint16 {
-       // FIPS requires TLS 1.2.
-       return VersionTLS12
+       // FIPS requires TLS 1.2 or TLS 1.3.
+       return VersionTLS13
 }
 
 // default defaultFIPSCurvePreferences is the FIPS-allowed curves,
 // in preference order (most preferable first).
-var defaultFIPSCurvePreferences = []CurveID{CurveP256, CurveP384, CurveP521}
+var defaultFIPSCurvePreferences = []CurveID{CurveP256, CurveP384}
 
 // fipsCurvePreferences replaces c.curvePreferences in FIPS-only mode.
 func fipsCurvePreferences(c *Config) []CurveID {
@@ -54,8 +55,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,
 }
 
 // fipsCipherSuites replaces c.cipherSuites in FIPS-only mode.
@@ -75,8 +74,14 @@ func fipsCipherSuites(c *Config) []uint16 {
        return list
 }
 
+// defaultCipherSuitesTLS13FIPS are the FIPS-allowed cipher suites for TLS 1.3.
+var defaultCipherSuitesTLS13FIPS = []uint16{
+       TLS_AES_128_GCM_SHA256,
+       TLS_AES_256_GCM_SHA384,
+}
+
 // fipsSupportedSignatureAlgorithms currently are a subset of
-// defaultSupportedSignatureAlgorithms without Ed25519 and SHA-1.
+// defaultSupportedSignatureAlgorithms without Ed25519, SHA-1, and P-521.
 var fipsSupportedSignatureAlgorithms = []SignatureScheme{
        PSSWithSHA256,
        PSSWithSHA384,
@@ -86,7 +91,6 @@ var fipsSupportedSignatureAlgorithms = []SignatureScheme{
        PKCS1WithSHA384,
        ECDSAWithP384AndSHA384,
        PKCS1WithSHA512,
-       ECDSAWithP521AndSHA512,
 }
 
 // supportedSignatureAlgorithms returns the supported signature algorithms.

diff --git a/src/crypto/tls/boring_test.go b/src/crypto/tls/boring_test.go
index 085ff5713ec52f8907a3b761ab64f748ce915c55..a192a657b4d79c9888b69fa8ed4e3c5298c87942 100644 (file)
--- 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(name string, v uint16, msg string) {
                t.Run(name, func(t *testing.T) {
@@ -33,8 +58,11 @@ func TestBoringServerProtocolVersion(t *testing.T) {
                        clientHello := &clientHelloMsg{
                                vers:               v,
                                random:             make([]byte, 32),
-                               cipherSuites:       allCipherSuites(),
+                               cipherSuites:       allCipherSuitesIncludingTLS13(),
                                compressionMethods: []uint8{compressionNone},
+                               supportedCurves:    defaultCurvePreferences,
+                               keyShares:          []keyShare{generateKeyShare(CurveP256)},
+                               supportedPoints:    []uint8{pointFormatUncompressed},
                                supportedVersions:  []uint16{v},
                        }
                        testClientHelloFailure(t, serverConfig, clientHello, msg)
@@ -48,25 +76,25 @@ func TestBoringServerProtocolVersion(t *testing.T) {
 
        fipstls.Force()
        defer fipstls.Abandon()
-       test("VersionSSL30", VersionSSL30, "client offered only unsupported versions")
-       test("VersionTLS10", VersionTLS10, "client offered only unsupported versions")
-       test("VersionTLS11", VersionTLS11, "client offered only unsupported versions")
-       test("VersionTLS12", VersionTLS12, "")
-       test("VersionTLS13", VersionTLS13, "client offered only unsupported versions")
+       test("VersionSSL30/fipstls", VersionSSL30, "client offered only unsupported versions")
+       test("VersionTLS10/fipstls", VersionTLS10, "client offered only unsupported versions")
+       test("VersionTLS11/fipstls", VersionTLS11, "client offered only unsupported versions")
+       test("VersionTLS12/fipstls", VersionTLS12, "")
+       test("VersionTLS13/fipstls", 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
@@ -74,7 +102,7 @@ func isBoringCipherSuite(id uint16) bool {
 
 func isBoringCurve(id CurveID) bool {
        switch id {
-       case CurveP256, CurveP384, CurveP521:
+       case CurveP256, CurveP384:
                return true
        }
        return false
@@ -86,7 +114,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 {
@@ -98,7 +126,6 @@ func isBoringSignatureScheme(alg SignatureScheme) bool {
                PKCS1WithSHA384,
                ECDSAWithP384AndSHA384,
                PKCS1WithSHA512,
-               ECDSAWithP521AndSHA512,
                PSSWithSHA256,
                PSSWithSHA384,
                PSSWithSHA512:
@@ -109,10 +136,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
@@ -121,14 +147,19 @@ 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,
+                               keyShares:          []keyShare{generateKeyShare(CurveP256)},
                                supportedPoints:    []uint8{pointFormatUncompressed},
+                               supportedVersions:  []uint16{VersionTLS12},
+                       }
+                       if isTLS13CipherSuite(id) {
+                               clientHello.supportedVersions = []uint16{VersionTLS13}
                        }
 
                        testClientHello(t, serverConfig, clientHello)
@@ -160,7 +191,9 @@ func TestBoringServerCurves(t *testing.T) {
                                cipherSuites:       []uint16{TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256},
                                compressionMethods: []uint8{compressionNone},
                                supportedCurves:    []CurveID{curveid},
+                               keyShares:          []keyShare{generateKeyShare(curveid)},
                                supportedPoints:    []uint8{pointFormatUncompressed},
+                               supportedVersions:  []uint16{VersionTLS12},
                        }
 
                        testClientHello(t, serverConfig, clientHello)
@@ -279,7 +312,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 6f5bc37197a4f4a1f23f816f829ac1368a80b5a7..636689beb4dcef36f4a5af787eafb52513793e20 100644 (file)
--- a/src/crypto/tls/cipher_suites.go
+++ b/src/crypto/tls/cipher_suites.go
@@ -556,7 +556,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/handshake_client.go b/src/crypto/tls/handshake_client.go
index f016e01b4b5182dccff5944b70ef431983c63f25..89004c28989627f30c17b0bec4b127142855a9b2 100644 (file)
--- a/src/crypto/tls/handshake_client.go
+++ b/src/crypto/tls/handshake_client.go
@@ -139,7 +139,9 @@ func (c *Conn) makeClientHello() (*clientHelloMsg, *ecdh.PrivateKey, error) {
                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...)

diff --git a/src/crypto/tls/handshake_client_tls13.go b/src/crypto/tls/handshake_client_tls13.go
index 2f59f6888c5d81949040946958f88c37ab942c1a..a84cede1b0b5185a5a53b3b4568ee0fa4e4f99f0 100644 (file)
--- a/src/crypto/tls/handshake_client_tls13.go
+++ b/src/crypto/tls/handshake_client_tls13.go
@@ -41,10 +41,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 15db760716c3dfe7f7024264e99bda7e5d1903b0..c0a86a49841d613c978561c30c83c832c39c6fcb 100644 (file)
--- a/src/crypto/tls/handshake_server_test.go
+++ b/src/crypto/tls/handshake_server_test.go
@@ -27,6 +27,7 @@ import (
 )
 
 func testClientHello(t *testing.T, serverConfig *Config, m handshakeMessage) {
+       t.Helper()
        testClientHelloFailure(t, serverConfig, m, "")
 }
 
@@ -52,23 +53,32 @@ func testClientHelloFailure(t *testing.T, serverConfig *Config, m handshakeMessa
        ctx := context.Background()
        conn := Server(s, serverConfig)
        ch, err := conn.readClientHello(ctx)
-       hs := serverHandshakeState{
-               c:           conn,
-               ctx:         ctx,
-               clientHello: ch,
-       }
-       if err == nil {
+       if err == nil && conn.vers == VersionTLS13 {
+               hs := serverHandshakeStateTLS13{
+                       c:           conn,
+                       ctx:         ctx,
+                       clientHello: ch,
+               }
                err = hs.processClientHello()
-       }
-       if err == nil {
-               err = hs.pickCipherSuite()
+       } else if err == nil {
+               hs := serverHandshakeState{
+                       c:           conn,
+                       ctx:         ctx,
+                       clientHello: ch,
+               }
+               err = hs.processClientHello()
+               if err == nil {
+                       err = hs.pickCipherSuite()
+               }
        }
        s.Close()
        if len(expectedSubStr) == 0 {
                if err != nil && err != io.EOF {
+                       t.Helper()
                        t.Errorf("Got error: %s; expected to succeed", err)
                }
        } else if err == nil || !strings.Contains(err.Error(), expectedSubStr) {
+               t.Helper()
                t.Errorf("Got error: %v; expected to match substring '%s'", err, expectedSubStr)
        }
 }

diff --git a/src/crypto/tls/handshake_server_tls13.go b/src/crypto/tls/handshake_server_tls13.go
index 21d798de37db0ab5a351968efa427d5e565a523c..b68ff9db4c6d4a41765968eadc7f961ba708488d 100644 (file)
--- a/src/crypto/tls/handshake_server_tls13.go
+++ b/src/crypto/tls/handshake_server_tls13.go
@@ -45,10 +45,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
@@ -163,6 +159,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 {

diff --git a/src/crypto/tls/notboring.go b/src/crypto/tls/notboring.go
index 7d85b39c59319ed19c76193544f5476a79cdfd5e..edccb44d87a5535fd64fe7e556187bc28b9e3faf 100644 (file)
--- a/src/crypto/tls/notboring.go
+++ b/src/crypto/tls/notboring.go
@@ -18,3 +18,5 @@ func fipsCurvePreferences(c *Config) []CurveID { panic("fipsCurvePreferences") }
 func fipsCipherSuites(c *Config) []uint16      { panic("fipsCipherSuites") }
 
 var fipsSupportedSignatureAlgorithms []SignatureScheme
+
+var defaultCipherSuitesTLS13FIPS []uint16

diff --git a/src/crypto/x509/boring.go b/src/crypto/x509/boring.go
index 095b58c31590d46c90d8cfede0c51988a55f725f..e6237e96bb3b1720ff19c22252bb60d704f67f70 100644 (file)
--- a/src/crypto/x509/boring.go
+++ b/src/crypto/x509/boring.go
@@ -22,7 +22,7 @@ func boringAllowCert(c *Certificate) bool {
        }
 
        // The key must be RSA 2048, RSA 3072, RSA 4096,
-       // or ECDSA P-256, P-384, P-521.
+       // or ECDSA P-256 or P-384.
        switch k := c.PublicKey.(type) {
        default:
                return false
@@ -31,7 +31,7 @@ func boringAllowCert(c *Certificate) bool {
                        return false
                }
        case *ecdsa.PublicKey:
-               if k.Curve != elliptic.P256() && k.Curve != elliptic.P384() && k.Curve != elliptic.P521() {
+               if k.Curve != elliptic.P256() && k.Curve != elliptic.P384() {
                        return false
                }
        }