[go] crypto/tls: set default minimum client version to TLS 1.2
1,070 views
Skip to first unread message
Filippo Valsorda (Gerrit)
Nov 5, 2021, 6:03:33 PM11/5/21
to Filippo Valsorda, goph...@pubsubhelper.golang.org, golang-...@googlegroups.com, Go Bot, Roland Shoemaker, golang-co...@googlegroups.com
Filippo Valsorda submitted this change.
1 is the latest approved patch-set.
The change was submitted with unreviewed changes in the following files:
```
The name of the file: src/crypto/tls/common.go
Insertions: 12, Deletions: 12.
@@ -18,9 +18,9 @@
"crypto/x509"
"errors"
"fmt"
+ "internal/godebug"
"io"
"net"
- "os"
"strings"
"sync"
"time"
@@ -975,18 +975,18 @@
}
// debugEnableTLS10 enables TLS 1.0. See issue 45428.
-var debugEnableTLS10 = strings.Contains(os.Getenv("GODEBUG"), "tls10default=1")
+var debugEnableTLS10 = godebug.Get("tls10default") == "1"
-const (
- roleClient = true
- roleServer = false
-)
+// roleClient and roleServer are meant to call supportedVersions and parents
+// with more readability at the callsite.
+const roleClient = true
+const roleServer = false
-func (c *Config) supportedVersions(role bool) []uint16 {
+func (c *Config) supportedVersions(isClient bool) []uint16 {
versions := make([]uint16, 0, len(supportedVersions))
for _, v := range supportedVersions {
if (c == nil || c.MinVersion == 0) && !debugEnableTLS10 &&
- role == roleClient && v < VersionTLS12 {
+ isClient && v < VersionTLS12 {
continue
}
if c != nil && c.MinVersion != 0 && v < c.MinVersion {
@@ -1000,8 +1000,8 @@
return versions
}
-func (c *Config) maxSupportedVersion(role bool) uint16 {
- supportedVersions := c.supportedVersions(role)
+func (c *Config) maxSupportedVersion(isClient bool) uint16 {
+ supportedVersions := c.supportedVersions(isClient)
if len(supportedVersions) == 0 {
return 0
}
@@ -1042,8 +1042,8 @@
// mutualVersion returns the protocol version to use given the advertised
// versions of the peer. Priority is given to the peer preference order.
-func (c *Config) mutualVersion(role bool, peerVersions []uint16) (uint16, bool) {
- supportedVersions := c.supportedVersions(role)
+func (c *Config) mutualVersion(isClient bool, peerVersions []uint16) (uint16, bool) {
+ supportedVersions := c.supportedVersions(isClient)
for _, peerVersion := range peerVersions {
for _, v := range supportedVersions {
if v == peerVersion {
```
Approvals:
Roland Shoemaker: Looks good to me, approved
Filippo Valsorda: Trusted; Run TryBots
Go Bot: TryBots succeeded
crypto/tls: set default minimum client version to TLS 1.2
Updates #45428
Change-Id: I5d70066d4091196ec6f8bfc2edf3d78fdc0520c1
Reviewed-on: https://go-review.googlesource.com/c/go/+/359779
Run-TryBot: Filippo Valsorda <fil...@golang.org>
TryBot-Result: Go Bot <go...@golang.org>
Trust: Filippo Valsorda <fil...@golang.org>
Reviewed-by: Roland Shoemaker <rol...@golang.org>
---
M src/crypto/tls/common.go
M src/crypto/tls/handshake_server.go
M src/crypto/tls/handshake_client.go
M src/crypto/tls/handshake_test.go
M src/crypto/tls/handshake_server_test.go
M src/crypto/tls/handshake_server_tls13.go
6 files changed, 75 insertions(+), 17 deletions(-)
diff --git a/src/crypto/tls/common.go b/src/crypto/tls/common.go
index 610a516..bb5bec3 100644
--- a/src/crypto/tls/common.go
+++ b/src/crypto/tls/common.go
@@ -18,6 +18,7 @@
"crypto/x509"
"errors"
"fmt"
+ "internal/godebug"
"io"
"net"
"strings"
@@ -682,11 +683,20 @@
ClientSessionCache ClientSessionCache
// MinVersion contains the minimum TLS version that is acceptable.
- // If zero, TLS 1.0 is currently taken as the minimum.
+ //
+ // By default, TLS 1.2 is currently used as the minimum when acting as a
+ // client, and TLS 1.0 when acting as a server. TLS 1.0 is the minimum
+ // supported by this package, both as a client and as a server.
+ //
+ // The client-side default can temporarily be reverted to TLS 1.0 by
+ // including the value "x509sha1=1" in the GODEBUG environment variable.
+ // Note that this option will be removed in Go 1.19 (but it will still be
+ // possible to set this field to VersionTLS10 explicitly).
MinVersion uint16
// MaxVersion contains the maximum TLS version that is acceptable.
- // If zero, the maximum version supported by this package is used,
+ //
+ // By default, the maximum version supported by this package is used,
// which is currently TLS 1.3.
MaxVersion uint16
@@ -964,9 +974,21 @@
VersionTLS10,
}
-func (c *Config) supportedVersions() []uint16 {
+// debugEnableTLS10 enables TLS 1.0. See issue 45428.
+var debugEnableTLS10 = godebug.Get("tls10default") == "1"
+
+// roleClient and roleServer are meant to call supportedVersions and parents
+// with more readability at the callsite.
+const roleClient = true
+const roleServer = false
+
+func (c *Config) supportedVersions(isClient bool) []uint16 {
versions := make([]uint16, 0, len(supportedVersions))
for _, v := range supportedVersions {
+ if (c == nil || c.MinVersion == 0) && !debugEnableTLS10 &&
+ isClient && v < VersionTLS12 {
+ continue
+ }
if c != nil && c.MinVersion != 0 && v < c.MinVersion {
continue
}
@@ -978,8 +1000,8 @@
return versions
}
-func (c *Config) maxSupportedVersion() uint16 {
- supportedVersions := c.supportedVersions()
+func (c *Config) maxSupportedVersion(isClient bool) uint16 {
+ supportedVersions := c.supportedVersions(isClient)
if len(supportedVersions) == 0 {
return 0
}
@@ -1020,8 +1042,8 @@
// mutualVersion returns the protocol version to use given the advertised
// versions of the peer. Priority is given to the peer preference order.
-func (c *Config) mutualVersion(peerVersions []uint16) (uint16, bool) {
- supportedVersions := c.supportedVersions()
+func (c *Config) mutualVersion(isClient bool, peerVersions []uint16) (uint16, bool) {
+ supportedVersions := c.supportedVersions(isClient)
for _, peerVersion := range peerVersions {
for _, v := range supportedVersions {
if v == peerVersion {
@@ -1100,7 +1122,7 @@
if config == nil {
config = &Config{}
}
- vers, ok := config.mutualVersion(chi.SupportedVersions)
+ vers, ok := config.mutualVersion(roleServer, chi.SupportedVersions)
if !ok {
return errors.New("no mutually supported protocol versions")
}
diff --git a/src/crypto/tls/handshake_client.go b/src/crypto/tls/handshake_client.go
index 4af3d99..2ae6f3f 100644
--- a/src/crypto/tls/handshake_client.go
+++ b/src/crypto/tls/handshake_client.go
@@ -52,12 +52,12 @@
return nil, nil, errors.New("tls: NextProtos values too large")
}
- supportedVersions := config.supportedVersions()
+ supportedVersions := config.supportedVersions(roleClient)
if len(supportedVersions) == 0 {
return nil, nil, errors.New("tls: no supported versions satisfy MinVersion and MaxVersion")
}
- clientHelloVersion := config.maxSupportedVersion()
+ clientHelloVersion := config.maxSupportedVersion(roleClient)
// The version at the beginning of the ClientHello was capped at TLS 1.2
// for compatibility reasons. The supported_versions extension is used
// to negotiate versions now. See RFC 8446, Section 4.2.1.
@@ -194,7 +194,7 @@
// If we are negotiating a protocol version that's lower than what we
// support, check for the server downgrade canaries.
// See RFC 8446, Section 4.1.3.
- maxVers := c.config.maxSupportedVersion()
+ maxVers := c.config.maxSupportedVersion(roleClient)
tls12Downgrade := string(serverHello.random[24:]) == downgradeCanaryTLS12
tls11Downgrade := string(serverHello.random[24:]) == downgradeCanaryTLS11
if maxVers == VersionTLS13 && c.vers <= VersionTLS12 && (tls12Downgrade || tls11Downgrade) ||
@@ -362,7 +362,7 @@
peerVersion = serverHello.supportedVersion
}
- vers, ok := c.config.mutualVersion([]uint16{peerVersion})
+ vers, ok := c.config.mutualVersion(roleClient, []uint16{peerVersion})
if !ok {
c.sendAlert(alertProtocolVersion)
return fmt.Errorf("tls: server selected unsupported protocol version %x", peerVersion)
diff --git a/src/crypto/tls/handshake_server.go b/src/crypto/tls/handshake_server.go
index 43f30e2..5cb1527 100644
--- a/src/crypto/tls/handshake_server.go
+++ b/src/crypto/tls/handshake_server.go
@@ -156,7 +156,7 @@
if len(clientHello.supportedVersions) == 0 {
clientVersions = supportedVersionsFromMax(clientHello.vers)
}
- c.vers, ok = c.config.mutualVersion(clientVersions)
+ c.vers, ok = c.config.mutualVersion(roleServer, clientVersions)
if !ok {
c.sendAlert(alertProtocolVersion)
return nil, fmt.Errorf("tls: client offered only unsupported versions: %x", clientVersions)
@@ -191,7 +191,7 @@
hs.hello.random = make([]byte, 32)
serverRandom := hs.hello.random
// Downgrade protection canaries. See RFC 8446, Section 4.1.3.
- maxVers := c.config.maxSupportedVersion()
+ maxVers := c.config.maxSupportedVersion(roleServer)
if maxVers >= VersionTLS12 && c.vers < maxVers || testingOnlyForceDowngradeCanary {
if c.vers == VersionTLS12 {
copy(serverRandom[24:], downgradeCanaryTLS12)
@@ -354,7 +354,7 @@
for _, id := range hs.clientHello.cipherSuites {
if id == TLS_FALLBACK_SCSV {
// The client is doing a fallback connection. See RFC 7507.
- if hs.clientHello.vers < c.config.maxSupportedVersion() {
+ if hs.clientHello.vers < c.config.maxSupportedVersion(roleServer) {
c.sendAlert(alertInappropriateFallback)
return errors.New("tls: client using inappropriate protocol fallback")
}
diff --git a/src/crypto/tls/handshake_server_test.go b/src/crypto/tls/handshake_server_test.go
index f61b4c8..5fb2ebb 100644
--- a/src/crypto/tls/handshake_server_test.go
+++ b/src/crypto/tls/handshake_server_test.go
@@ -385,13 +385,30 @@
}
clientConfig := &Config{
InsecureSkipVerify: true,
+ MinVersion: VersionTLS10,
}
state, _, err := testHandshake(t, clientConfig, serverConfig)
if err != nil {
t.Fatalf("handshake failed: %s", err)
}
if state.Version != VersionTLS11 {
- t.Fatalf("Incorrect version %x, should be %x", state.Version, VersionTLS11)
+ t.Fatalf("incorrect version %x, should be %x", state.Version, VersionTLS11)
+ }
+
+ clientConfig.MinVersion = 0
+ _, _, err = testHandshake(t, clientConfig, serverConfig)
+ if err == nil {
+ t.Fatalf("expected failure to connect with TLS 1.0/1.1")
+ }
+
+ defer func(old bool) { debugEnableTLS10 = old }(debugEnableTLS10)
+ debugEnableTLS10 = true
+ _, _, err = testHandshake(t, clientConfig, serverConfig)
+ if err != nil {
+ t.Fatalf("handshake failed: %s", err)
+ }
+ if state.Version != VersionTLS11 {
+ t.Fatalf("incorrect version %x, should be %x", state.Version, VersionTLS11)
}
}
@@ -472,6 +489,7 @@
InsecureSkipVerify: true,
ClientSessionCache: NewLRUClientSessionCache(1),
ServerName: "servername",
+ MinVersion: VersionTLS10,
}
// Establish a session at TLS 1.1.
diff --git a/src/crypto/tls/handshake_server_tls13.go b/src/crypto/tls/handshake_server_tls13.go
index 08251b8..0b19502 100644
--- a/src/crypto/tls/handshake_server_tls13.go
+++ b/src/crypto/tls/handshake_server_tls13.go
@@ -110,7 +110,7 @@
if id == TLS_FALLBACK_SCSV {
// Use c.vers instead of max(supported_versions) because an attacker
// could defeat this by adding an arbitrary high version otherwise.
- if c.vers < c.config.maxSupportedVersion() {
+ if c.vers < c.config.maxSupportedVersion(roleServer) {
c.sendAlert(alertInappropriateFallback)
return errors.New("tls: client using inappropriate protocol fallback")
}
diff --git a/src/crypto/tls/handshake_test.go b/src/crypto/tls/handshake_test.go
index 90ac9bd..bacc8b7 100644
--- a/src/crypto/tls/handshake_test.go
+++ b/src/crypto/tls/handshake_test.go
@@ -363,6 +363,8 @@
Certificates: make([]Certificate, 2),
InsecureSkipVerify: true,
CipherSuites: allCipherSuites(),
+ MinVersion: VersionTLS10,
+ MaxVersion: VersionTLS13,
}
testConfig.Certificates[0].Certificate = [][]byte{testRSACertificate}
testConfig.Certificates[0].PrivateKey = testRSAPrivateKey
To view, visit change 359779. To unsubscribe, or for help writing mail filters, visit settings.
Reply all
Reply to author
Forward
0 new messages