crypto/tls: check and record godebugs more granularly

We should call Value as late as possible to allow programs to set
GODEBUG with os.Setenv, and IncNonDefault only when (and every time) the
GODEBUG has an effect on a connection (that we'd have regularly
rejected).

Change-Id: If7a1446de407db7ca2d904d41dda13558b684dda
Reviewed-on: https://go-review.googlesource.com/c/go/+/544335
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Roland Shoemaker <roland@golang.org>
Reviewed-by: David Chase <drchase@google.com>
Auto-Submit: Filippo Valsorda <filippo@golang.org>

cipher_suites.go

@@ -17,7 +17,6 @@ import (
 	"fmt"
 	"hash"
 	"internal/cpu"
-	"internal/godebug"
 	"runtime"
 
 	"golang.org/x/crypto/chacha20poly1305"
@@ -323,25 +322,21 @@ var cipherSuitesPreferenceOrderNoAES = []uint16{
 	TLS_RSA_WITH_RC4_128_SHA,
 }
 
-// disabledCipherSuites are not used unless explicitly listed in
+// disabledCipherSuites are not used unless explicitly listed in Config.CipherSuites.
-// Config.CipherSuites. They MUST be at the end of cipherSuitesPreferenceOrder.
+var disabledCipherSuites = map[uint16]bool{
-var disabledCipherSuites = []uint16{
 	// CBC_SHA256
-	TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA256, TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256,
+	TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA256: true,
-	TLS_RSA_WITH_AES_128_CBC_SHA256,
+	TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256:   true,
+	TLS_RSA_WITH_AES_128_CBC_SHA256:         true,
 
 	// RC4
-	TLS_ECDHE_ECDSA_WITH_RC4_128_SHA, TLS_ECDHE_RSA_WITH_RC4_128_SHA,
+	TLS_ECDHE_ECDSA_WITH_RC4_128_SHA: true,
-	TLS_RSA_WITH_RC4_128_SHA,
+	TLS_ECDHE_RSA_WITH_RC4_128_SHA:   true,
+	TLS_RSA_WITH_RC4_128_SHA:         true,
 }
 
-var (
-	defaultCipherSuitesLen int
-	defaultCipherSuites    []uint16
-)
-
 // rsaKexCiphers contains the ciphers which use RSA based key exchange,
-// which we disable by default.
+// which we also disable by default unless a GODEBUG is set.
 var rsaKexCiphers = map[uint16]bool{
 	TLS_RSA_WITH_RC4_128_SHA:        true,
 	TLS_RSA_WITH_3DES_EDE_CBC_SHA:   true,
@@ -352,17 +347,21 @@ var rsaKexCiphers = map[uint16]bool{
 	TLS_RSA_WITH_AES_256_GCM_SHA384: true,
 }
 
-var rsaKEXgodebug = godebug.New("tlsrsakex")
+var defaultCipherSuites []uint16
+var defaultCipherSuitesWithRSAKex []uint16
 
 func init() {
-	rsaKexEnabled := rsaKEXgodebug.Value() == "1"
+	defaultCipherSuites = make([]uint16, 0, len(cipherSuitesPreferenceOrder))
-	for _, c := range cipherSuitesPreferenceOrder[:len(cipherSuitesPreferenceOrder)-len(disabledCipherSuites)] {
+	defaultCipherSuitesWithRSAKex = make([]uint16, 0, len(cipherSuitesPreferenceOrder))
-		if !rsaKexEnabled && rsaKexCiphers[c] {
+	for _, c := range cipherSuitesPreferenceOrder {
+		if disabledCipherSuites[c] {
 			continue
 		}
-		defaultCipherSuites = append(defaultCipherSuites, c)
+		if !rsaKexCiphers[c] {
+			defaultCipherSuites = append(defaultCipherSuites, c)
+		}
+		defaultCipherSuitesWithRSAKex = append(defaultCipherSuitesWithRSAKex, c)
 	}
-	defaultCipherSuitesLen = len(defaultCipherSuites)
 }
 
 // defaultCipherSuitesTLS13 is also the preference order, since there are no

common.go

@@ -1008,6 +1008,8 @@ func (c *Config) time() time.Time {
 	return t()
 }
 
+var tlsrsakex = godebug.New("tlsrsakex")
+
 func (c *Config) cipherSuites() []uint16 {
 	if needFIPS() {
 		return fipsCipherSuites(c)
@@ -1015,6 +1017,9 @@ func (c *Config) cipherSuites() []uint16 {
 	if c.CipherSuites != nil {
 		return c.CipherSuites
 	}
+	if tlsrsakex.Value() == "1" {
+		return defaultCipherSuitesWithRSAKex
+	}
 	return defaultCipherSuites
 }
 
@@ -1030,7 +1035,7 @@ var supportedVersions = []uint16{
 const roleClient = true
 const roleServer = false
 
-var tls10godebug = godebug.New("tls10server")
+var tls10server = godebug.New("tls10server")
 
 func (c *Config) supportedVersions(isClient bool) []uint16 {
 	versions := make([]uint16, 0, len(supportedVersions))
@@ -1039,9 +1044,7 @@ func (c *Config) supportedVersions(isClient bool) []uint16 {
 			continue
 		}
 		if (c == nil || c.MinVersion == 0) && v < VersionTLS12 {
-			if !isClient && tls10godebug.Value() == "1" {
+			if isClient || tls10server.Value() != "1" {
-				tls10godebug.IncNonDefault()
-			} else {
 				continue
 			}
 		}

conn.go

@@ -1600,7 +1600,7 @@ func (c *Conn) ConnectionState() ConnectionState {
 	return c.connectionStateLocked()
 }
 
-var ekmgodebug = godebug.New("tlsunsafeekm")
+var tlsunsafeekm = godebug.New("tlsunsafeekm")
 
 func (c *Conn) connectionStateLocked() ConnectionState {
 	var state ConnectionState
@@ -1626,8 +1626,8 @@ func (c *Conn) connectionStateLocked() ConnectionState {
 		state.ekm = noEKMBecauseRenegotiation
 	} else if c.vers != VersionTLS13 && !c.extMasterSecret {
 		state.ekm = func(label string, context []byte, length int) ([]byte, error) {
-			if ekmgodebug.Value() == "1" {
+			if tlsunsafeekm.Value() == "1" {
-				ekmgodebug.IncNonDefault()
+				tlsunsafeekm.IncNonDefault()
 				return c.ekm(label, context, length)
 			}
 			return noEKMBecauseNoEMS(label, context, length)

handshake_client.go

@@ -526,6 +526,10 @@ func (hs *clientHandshakeState) pickCipherSuite() error {
 		return errors.New("tls: server chose an unconfigured cipher suite")
 	}
 
+	if hs.c.config.CipherSuites == nil && rsaKexCiphers[hs.suite.id] {
+		tlsrsakex.IncNonDefault()
+	}
+
 	hs.c.cipherSuite = hs.suite.id
 	return nil
 }

handshake_server.go

@@ -168,6 +168,10 @@ func (c *Conn) readClientHello(ctx context.Context) (*clientHelloMsg, error) {
 	c.in.version = c.vers
 	c.out.version = c.vers
 
+	if c.config.MinVersion == 0 && c.vers < VersionTLS12 {
+		tls10server.IncNonDefault()
+	}
+
 	return clientHello, nil
 }
 
@@ -366,6 +370,10 @@ func (hs *serverHandshakeState) pickCipherSuite() error {
 	}
 	c.cipherSuite = hs.suite.id
 
+	if c.config.CipherSuites == nil && rsaKexCiphers[hs.suite.id] {
+		tlsrsakex.IncNonDefault()
+	}
+
 	for _, id := range hs.clientHello.cipherSuites {
 		if id == TLS_FALLBACK_SCSV {
 			// The client is doing a fallback connection. See RFC 7507.

tls_test.go

@@ -1491,16 +1491,8 @@ func TestCipherSuites(t *testing.T) {
 		t.Errorf("cipherSuitesPreferenceOrderNoAES is not the same size as cipherSuitesPreferenceOrder")
 	}
 
-	// Check that disabled suites are at the end of the preference lists, and
+	// Check that disabled suites are marked insecure.
-	// that they are marked insecure.
+	for id := range disabledCipherSuites {
-	for i, id := range disabledCipherSuites {
-		offset := len(cipherSuitesPreferenceOrder) - len(disabledCipherSuites)
-		if cipherSuitesPreferenceOrder[offset+i] != id {
-			t.Errorf("disabledCipherSuites[%d]: not at the end of cipherSuitesPreferenceOrder", i)
-		}
-		if cipherSuitesPreferenceOrderNoAES[offset+i] != id {
-			t.Errorf("disabledCipherSuites[%d]: not at the end of cipherSuitesPreferenceOrderNoAES", i)
-		}
 		c := CipherSuiteByID(id)
 		if c == nil {
 			t.Errorf("%#04x: no CipherSuite entry", id)