Skip to content

Commit

Permalink
Merge pull request #1068 from microsoft/dev/qmuntal/tls13
Browse files Browse the repository at this point in the history
Allow TLS 1.3 when crypto/tls/fipsonly is imported
  • Loading branch information
qmuntal authored Oct 17, 2023
2 parents de2cf6c + 5bbd416 commit 0ce9216
Show file tree
Hide file tree
Showing 2 changed files with 162 additions and 2 deletions.
9 changes: 7 additions & 2 deletions eng/doc/fips/UserGuide.md
Original file line number Diff line number Diff line change
Expand Up @@ -1380,18 +1380,23 @@ Package tls will automatically use FIPS compliant primitives implemented in othe

When using TLS in FIPS-only mode the TLS handshake has the following restrictions:

- TLS versions: `tls.VersionTLS12`
- TLS versions:
- `tls.VersionTLS12`
- `tls.VersionTLS13`
- ECDSA elliptic curves:
- `tls.CurveP256`
- `tls.CurveP384`
- `tls.CurveP521`
- Cipher suites:
- Cipher suites for TLS 1.2:
- `tls.TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256`
- `tls.TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384`
- `tls.TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256`
- `tls.TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384`
- `tls.TLS_RSA_WITH_AES_128_GCM_SHA256`
- `tls.TLS_RSA_WITH_AES_256_GCM_SHA384`
- Cipher suites for TLS 1.3:
- `tls.TLS_AES_128_GCM_SHA256`
- `tls.TLS_AES_256_GCM_SHA384`
- x509 certificate public key:
- `rsa.PublicKey` with a bit length of 2048 or 3072. Bit length of 4096 is still not supported, see [this issue](https://github.com/golang/go/issues/41147) for more info.
- `ecdsa.PublicKey` with a supported elliptic curve.
Expand Down
155 changes: 155 additions & 0 deletions patches/0010-allow-TLS-1.3-when-crypto-tls-fipsonly-is-imported.patch
Original file line number Diff line number Diff line change
@@ -0,0 +1,155 @@
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: Quim Muntal <[email protected]>
Date: Mon, 16 Oct 2023 12:04:32 +0000
Subject: [PATCH] allow TLS 1.3 when crypto/tls/fipsonly is imported

---
src/crypto/tls/boring.go | 2 +-
src/crypto/tls/boring_test.go | 19 ++++++++++++++-----
src/crypto/tls/cipher_suites.go | 5 +++++
src/crypto/tls/handshake_client.go | 4 +++-
src/crypto/tls/handshake_client_tls13.go | 4 ----
src/crypto/tls/handshake_server_tls13.go | 8 +++-----
6 files changed, 26 insertions(+), 16 deletions(-)

diff --git a/src/crypto/tls/boring.go b/src/crypto/tls/boring.go
index ecd0f5a7b3e9ed..38d546a607774f 100644
--- a/src/crypto/tls/boring.go
+++ b/src/crypto/tls/boring.go
@@ -24,7 +24,7 @@ func fipsMinVersion(c *Config) uint16 {
// fipsMaxVersion replaces c.maxVersion in FIPS-only mode.
func fipsMaxVersion(c *Config) uint16 {
// FIPS requires TLS 1.2.
- return VersionTLS12
+ return VersionTLS13
}

// default defaultFIPSCurvePreferences is the FIPS-allowed curves,
diff --git a/src/crypto/tls/boring_test.go b/src/crypto/tls/boring_test.go
index 86595e588cf604..de90b1860a39be 100644
--- a/src/crypto/tls/boring_test.go
+++ b/src/crypto/tls/boring_test.go
@@ -52,14 +52,14 @@ func TestBoringServerProtocolVersion(t *testing.T) {
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("VersionTLS13", VersionTLS13, "")
}

func isBoringVersion(v uint16) bool {
- return v == VersionTLS12
+ return v == VersionTLS12 || v == VersionTLS13
}

-func isBoringCipherSuite(id uint16) bool {
+func isBoringCipherSuiteTLS12(id uint16) bool {
switch id {
case TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256,
TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384,
@@ -72,6 +72,15 @@ func isBoringCipherSuite(id uint16) bool {
return false
}

+func isBoringCipherSuiteTLS13(id uint16) bool {
+ switch id {
+ case TLS_AES_128_GCM_SHA256,
+ TLS_AES_256_GCM_SHA384:
+ return true
+ }
+ return false
+}
+
func isBoringCurve(id CurveID) bool {
switch id {
case CurveP256, CurveP384, CurveP521:
@@ -136,7 +145,7 @@ func TestBoringServerCipherSuites(t *testing.T) {
fipstls.Force()
defer fipstls.Abandon()
msg := ""
- if !isBoringCipherSuite(id) {
+ if !isBoringCipherSuiteTLS12(id) {
msg = "no cipher suite supported by both client and server"
}
testClientHelloFailure(t, serverConfig, clientHello, msg)
@@ -287,7 +296,7 @@ func TestBoringClientHello(t *testing.T) {
}
}
for _, id := range hello.cipherSuites {
- if !isBoringCipherSuite(id) {
+ if !isBoringCipherSuiteTLS12(id) && !isBoringCipherSuiteTLS13(id) {
t.Errorf("client offered disallowed suite %#x", id)
}
}
diff --git a/src/crypto/tls/cipher_suites.go b/src/crypto/tls/cipher_suites.go
index 0a6d665ee3096d..b328f06773be5b 100644
--- a/src/crypto/tls/cipher_suites.go
+++ b/src/crypto/tls/cipher_suites.go
@@ -348,6 +348,11 @@ var defaultCipherSuitesTLS13 = []uint16{
TLS_CHACHA20_POLY1305_SHA256,
}

+var defaultCipherSuitesTLS13FIPS = []uint16{
+ TLS_AES_128_GCM_SHA256,
+ TLS_AES_256_GCM_SHA384,
+}
+
var defaultCipherSuitesTLS13NoAES = []uint16{
TLS_CHACHA20_POLY1305_SHA256,
TLS_AES_128_GCM_SHA256,
diff --git a/src/crypto/tls/handshake_client.go b/src/crypto/tls/handshake_client.go
index f96b2b28862234..e07e4220cbb00e 100644
--- 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 2f59f6888c5d81..a84cede1b0b518 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_tls13.go b/src/crypto/tls/handshake_server_tls13.go
index 6fae2b4ba22540..c9d0433ddd3a1f 100644
--- a/src/crypto/tls/handshake_server_tls13.go
+++ b/src/crypto/tls/handshake_server_tls13.go
@@ -46,10 +46,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
@@ -161,7 +157,9 @@ func (hs *serverHandshakeStateTLS13) processClientHello() error {
hs.hello.compressionMethod = compressionNone

preferenceList := defaultCipherSuitesTLS13
- if !hasAESGCMHardwareSupport || !aesgcmPreferred(hs.clientHello.cipherSuites) {
+ if needFIPS() {
+ preferenceList = defaultCipherSuitesTLS13FIPS
+ } else if !hasAESGCMHardwareSupport || !aesgcmPreferred(hs.clientHello.cipherSuites) {
preferenceList = defaultCipherSuitesTLS13NoAES
}
for _, suiteID := range preferenceList {

0 comments on commit 0ce9216

Please sign in to comment.