-
Notifications
You must be signed in to change notification settings - Fork 28
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
- Loading branch information
Showing
3 changed files
with
341 additions
and
12 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,325 @@ | ||
| From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 | ||
| From: qmuntal <qmuntaldiaz@microsoft.com> | ||
| Date: Fri, 2 Feb 2024 09:25:38 +0100 | ||
| Subject: [PATCH] Support TLS 1.3 in fipstls mode | ||
|
|
||
| --- | ||
| src/crypto/tls/boring.go | 12 +++-- | ||
| src/crypto/tls/boring_test.go | 56 +++++++++++++++++++----- | ||
| src/crypto/tls/cipher_suites.go | 13 +++++- | ||
| src/crypto/tls/handshake_client.go | 4 +- | ||
| src/crypto/tls/handshake_client_tls13.go | 4 -- | ||
| src/crypto/tls/handshake_server_test.go | 28 ++++++++---- | ||
| src/crypto/tls/handshake_server_tls13.go | 7 ++- | ||
| src/crypto/tls/notboring.go | 2 + | ||
| 8 files changed, 93 insertions(+), 33 deletions(-) | ||
|
|
||
| diff --git a/src/crypto/tls/boring.go b/src/crypto/tls/boring.go | ||
| index ecd0f5a7b3e9ed..35808a3acd5403 100644 | ||
| --- a/src/crypto/tls/boring.go | ||
| +++ b/src/crypto/tls/boring.go | ||
| @@ -17,14 +17,14 @@ 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, | ||
| @@ -75,6 +75,12 @@ 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. | ||
| var fipsSupportedSignatureAlgorithms = []SignatureScheme{ | ||
| diff --git a/src/crypto/tls/boring_test.go b/src/crypto/tls/boring_test.go | ||
| index 3e63ba6a053c42..e278f8ad1a8118 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(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,15 +76,15 @@ 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 { | ||
| @@ -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 { | ||
| @@ -109,10 +137,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 | ||
| @@ -128,7 +155,12 @@ func TestBoringServerCipherSuites(t *testing.T) { | ||
| 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 +192,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 +313,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 68598961fda6e4..8c86b8ad998559 100644 | ||
| --- a/src/crypto/tls/cipher_suites.go | ||
| +++ b/src/crypto/tls/cipher_suites.go | ||
| @@ -17,6 +17,7 @@ import ( | ||
| "fmt" | ||
| "hash" | ||
| "internal/cpu" | ||
| + "internal/goexperiment" | ||
| "runtime" | ||
|
|
||
| "golang.org/x/crypto/chacha20poly1305" | ||
| @@ -540,7 +541,17 @@ func aeadAESGCMTLS13(key, nonceMask []byte) aead { | ||
| if err != nil { | ||
| panic(err) | ||
| } | ||
| - aead, err := cipher.NewGCM(aes) | ||
| + var aead cipher.AEAD | ||
| + if boring.Enabled { | ||
| + if goexperiment.BoringCrypto { | ||
| + aead, err = cipher.NewGCM(aes) | ||
| + } else { | ||
| + 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 0bb6a0aaef2fff..aee451ad7aa10f 100644 | ||
| --- a/src/crypto/tls/handshake_client.go | ||
| +++ b/src/crypto/tls/handshake_client.go | ||
| @@ -129,7 +129,9 @@ func (c *Conn) makeClientHello() (*clientHelloMsg, *ecdh.PrivateKey, error) { | ||
|
|
||
| var key *ecdh.PrivateKey | ||
| if hello.supportedVersions[0] == VersionTLS13 { | ||
| - 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 4a8661085ebf57..87fe11de5c5be4 100644 | ||
| --- 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 04abdcca89040d..778972d38dbcf1 100644 | ||
| --- 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 d1c5780533bf6a..2980ff58e65d31 100644 | ||
| --- 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 | ||
| @@ -158,6 +154,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 5a133c9b2f94c7..7625ccb867dd92 100644 | ||
| --- 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 |