crypto/tls: allow client to pick TLS 1.3, do not enable it by default.

mutualVersion takes a maximum version, but skips TLS 1.3 because this
version is not negotiated via ClientHello.legacy_version. The server
however still uses its ServerHello.version field to select a version
from the supported_versions extension and the client must do the same.

A new getSupportedVersions method is introduced to have a single place
to handle the mapping of VersionTLS13 to the draft values.

Remove the MaxVersion override to TLS 1.3, users must set MaxVersion if
they intent to use the experimental TLS 1.3 functionality.

Fixes: ("crypto/tls: make 1.3 version negotiation more robust")

common.go

@@ -824,29 +824,42 @@ func (c *Config) mutualVersion(vers uint16) (uint16, bool) {
 
 // pickVersion returns the protocol version to use given the advertised
 // versions of the peer using the Supported Versions extension.
-func (c *Config) pickVersion(supportedVersions []uint16) (uint16, bool) {
+func (c *Config) pickVersion(peerSupportedVersions []uint16) (uint16, bool) {
+	supportedVersions := c.getSupportedVersions()
+	for _, supportedVersion := range supportedVersions {
+		for _, version := range peerSupportedVersions {
+			if version == supportedVersion {
+				return version, true
+			}
+		}
+	}
+	return 0, false
+}
+
+// getSupportedVersions returns the protocol versions that are supported by the
+// current configuration.
+func (c *Config) getSupportedVersions() []uint16 {
 	minVersion := c.minVersion()
 	maxVersion := c.maxVersion()
-	if c == nil || c.MaxVersion == 0 {
-		maxVersion = VersionTLS13 // override the default if pickVersion is used
+	// Sanity check to avoid advertising unsupported versions.
+	if minVersion < VersionSSL30 {
+		minVersion = VersionSSL30
+	}
+	if maxVersion > VersionTLS13 {
+		maxVersion = VersionTLS13
 	}
 
-	tls13Enabled := maxVersion >= VersionTLS13
-	if maxVersion > VersionTLS12 {
-		maxVersion = VersionTLS12
+	supportedVersions := []uint16{}
+	// Prefer newer versions over older versions.
+	for v := maxVersion; v >= minVersion; v-- {
+		if v == VersionTLS13 {
+			// Advertise all supported draft versions.
+			supportedVersions = append(supportedVersions, VersionTLS13Draft18)
+			continue // final TLS 1.3 version is not supported yet.
 		}
-
-	var vers uint16
-	for _, v := range supportedVersions {
-		if v >= minVersion && v <= maxVersion ||
-			(tls13Enabled && v == VersionTLS13Draft18) {
-			if v > vers {
-				vers = v
+		supportedVersions = append(supportedVersions, v)
 	}
-		}
-	}
-
-	return vers, vers != 0
+	return supportedVersions
 }
 
 // getCertificate returns the best certificate for the given ClientHelloInfo,

handshake_client.go

@@ -172,7 +172,7 @@ NextCipherSuite:
 		return unexpectedMessageError(serverHello, msg)
 	}
 
-	vers, ok := c.config.mutualVersion(serverHello.vers)
+	vers, ok := c.config.pickVersion([]uint16{serverHello.vers})
 	if !ok || vers < VersionTLS10 {
 		// TLS 1.0 is the minimum version supported as a client.
 		c.sendAlert(alertProtocolVersion)