From 20e08fe68cc13102046d8c4500a7f5257a9d2881 Mon Sep 17 00:00:00 2001 From: Monis Khan Date: Fri, 13 Jan 2023 00:37:56 +0000 Subject: [PATCH] crypto/tls: advertise correct ciphers in TLS 1.3 only mode This change updates the makeClientHello logic to only advertise TLS 1.3 ciphers when tls.Config.MinVersion is set to tls.VersionTLS13 (i.e the client only supports TLS 1.3). Previously, TLS 1.2 ciphers would be included in the client hello message. Fixes #57771 Change-Id: Ife4123037b0a4609578ffffb1cdf1e1d4e0a8df6 GitHub-Last-Rev: 45f4275aa9b9550e519e1be5c337b53ab8882007 GitHub-Pull-Request: golang/go#49293 Reviewed-on: https://go-review.googlesource.com/c/go/+/360794 Reviewed-by: Filippo Valsorda Run-TryBot: Filippo Valsorda Reviewed-by: Roland Shoemaker Reviewed-by: Damien Neil Reviewed-by: Marten Seemann Auto-Submit: Filippo Valsorda TryBot-Result: Gopher Robot --- src/crypto/tls/handshake_client.go | 4 ++ src/crypto/tls/handshake_client_test.go | 63 +++++++++++++++++++++++++ 2 files changed, 67 insertions(+) diff --git a/src/crypto/tls/handshake_client.go b/src/crypto/tls/handshake_client.go index c2ac75fdbf233..44949c8a22552 100644 --- a/src/crypto/tls/handshake_client.go +++ b/src/crypto/tls/handshake_client.go @@ -133,6 +133,10 @@ func (c *Conn) makeClientHello() (*clientHelloMsg, *ecdh.PrivateKey, error) { var key *ecdh.PrivateKey if hello.supportedVersions[0] == VersionTLS13 { + // Reset the list of ciphers when the client only supports TLS 1.3. + if len(hello.supportedVersions) == 1 { + hello.cipherSuites = nil + } if hasAESGCMHardwareSupport { hello.cipherSuites = append(hello.cipherSuites, defaultCipherSuitesTLS13...) } else { diff --git a/src/crypto/tls/handshake_client_test.go b/src/crypto/tls/handshake_client_test.go index 7be6f94c36adb..08c0af62bd69c 100644 --- a/src/crypto/tls/handshake_client_test.go +++ b/src/crypto/tls/handshake_client_test.go @@ -2658,3 +2658,66 @@ func TestClientHandshakeContextCancellation(t *testing.T) { t.Error("Client connection was not closed when the context was canceled") } } + +// TestTLS13OnlyClientHelloCipherSuite tests that when a client states that +// it only supports TLS 1.3, it correctly advertises only TLS 1.3 ciphers. +func TestTLS13OnlyClientHelloCipherSuite(t *testing.T) { + tls13Tests := []struct { + name string + ciphers []uint16 + }{ + { + name: "nil", + ciphers: nil, + }, + { + name: "empty", + ciphers: []uint16{}, + }, + { + name: "some TLS 1.2 cipher", + ciphers: []uint16{TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256}, + }, + { + name: "some TLS 1.3 cipher", + ciphers: []uint16{TLS_AES_128_GCM_SHA256}, + }, + { + name: "some TLS 1.2 and 1.3 ciphers", + ciphers: []uint16{TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384, TLS_AES_256_GCM_SHA384}, + }, + } + for _, tt := range tls13Tests { + tt := tt + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + testTLS13OnlyClientHelloCipherSuite(t, tt.ciphers) + }) + } +} + +func testTLS13OnlyClientHelloCipherSuite(t *testing.T, ciphers []uint16) { + serverConfig := &Config{ + Certificates: testConfig.Certificates, + GetConfigForClient: func(chi *ClientHelloInfo) (*Config, error) { + if len(chi.CipherSuites) != len(defaultCipherSuitesTLS13NoAES) { + t.Errorf("only TLS 1.3 suites should be advertised, got=%x", chi.CipherSuites) + } else { + for i := range defaultCipherSuitesTLS13NoAES { + if want, got := defaultCipherSuitesTLS13NoAES[i], chi.CipherSuites[i]; want != got { + t.Errorf("cipher at index %d does not match, want=%x, got=%x", i, want, got) + } + } + } + return nil, nil + }, + } + clientConfig := &Config{ + MinVersion: VersionTLS13, // client only supports TLS 1.3 + CipherSuites: ciphers, + InsecureSkipVerify: true, + } + if _, _, err := testHandshake(t, clientConfig, serverConfig); err != nil { + t.Fatalf("handshake failed: %s", err) + } +}