crypto/tls: disable RSA-PSS in TLS 1.2

Most of the issues that led to the decision on #30055 were related to
incompatibility with or faulty support for RSA-PSS (#29831, #29779,
v1.5 signatures). RSA-PSS is required by TLS 1.3, but is also available
to be negotiated in TLS 1.2.

Altering TLS 1.2 behavior based on GODEBUG=tls13=1 feels surprising, so
just disable RSA-PSS entirely in TLS 1.2 until TLS 1.3 is on by default,
so breakage happens all at once.

Updates #30055

Change-Id: Iee90454a20ded8895e5302e8bcbcd32e4e3031c2
Reviewed-on: https://go-review.googlesource.com/c/160998
Run-TryBot: Filippo Valsorda <filippo@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Adam Langley <agl@golang.org>

1 files changed, 109 insertions, 37 deletions

diff --git a/src/crypto/tls/handshake_server_test.go b/src/crypto/tls/handshake_server_test.go
index 411648ef68..c23f98f6bc 100644
--- a/src/crypto/tls/handshake_server_test.go
+++ b/src/crypto/tls/handshake_server_test.go
@@ -1211,6 +1211,33 @@ func TestHandshakeServerRSAPSS(t *testing.T) {
 	runServerTestTLS13(t, test)
 }
 
+func TestHandshakeServerPSSDisabled(t *testing.T) {
+	test := &serverTest{
+		name:    "RSA-PSS-Disabled",
+		command: []string{"openssl", "s_client", "-no_ticket"},
+		wait:    true,
+	}
+
+	// Restore the default signature algorithms, disabling RSA-PSS in TLS 1.2,
+	// and check that handshakes still work.
+	testSupportedSignatureAlgorithmsTLS12 := supportedSignatureAlgorithmsTLS12
+	defer func() { supportedSignatureAlgorithmsTLS12 = testSupportedSignatureAlgorithmsTLS12 }()
+	supportedSignatureAlgorithmsTLS12 = savedSupportedSignatureAlgorithmsTLS12
+
+	runServerTestTLS12(t, test)
+	runServerTestTLS13(t, test)
+
+	test = &serverTest{
+		name:    "RSA-PSS-Disabled-Required",
+		command: []string{"openssl", "s_client", "-no_ticket", "-sigalgs", "rsa_pss_rsae_sha256"},
+		wait:    true,
+
+		expectHandshakeErrorIncluding: "peer doesn't support any common signature algorithms",
+	}
+
+	runServerTestTLS12(t, test)
+}
+
 func benchmarkHandshakeServer(b *testing.B, version uint16, cipherSuite uint16, curve CurveID, cert []byte, key crypto.PrivateKey) {
 	config := testConfig.Clone()
 	config.CipherSuites = []uint16{cipherSuite}
@@ -1390,49 +1417,82 @@ func TestClientAuth(t *testing.T) {
 		defer os.Remove(ecdsaCertPath)
 		ecdsaKeyPath = tempFile(clientECDSAKeyPEM)
 		defer os.Remove(ecdsaKeyPath)
-	} else {
-		t.Parallel()
 	}
 
-	config := testConfig.Clone()
-	config.ClientAuth = RequestClientCert
+	t.Run("Normal", func(t *testing.T) {
+		config := testConfig.Clone()
+		config.ClientAuth = RequestClientCert
 
-	test := &serverTest{
-		name:    "ClientAuthRequestedNotGiven",
-		command: []string{"openssl", "s_client", "-no_ticket", "-cipher", "AES128-SHA"},
-		config:  config,
-	}
-	runServerTestTLS12(t, test)
-	runServerTestTLS13(t, test)
+		test := &serverTest{
+			name:    "ClientAuthRequestedNotGiven",
+			command: []string{"openssl", "s_client", "-no_ticket", "-cipher", "AES128-SHA"},
+			config:  config,
+		}
+		runServerTestTLS12(t, test)
+		runServerTestTLS13(t, test)
 
-	test = &serverTest{
-		name: "ClientAuthRequestedAndGiven",
-		command: []string{"openssl", "s_client", "-no_ticket", "-cipher", "AES128-SHA",
-			"-cert", certPath, "-key", keyPath, "-sigalgs", "rsa_pss_rsae_sha256"},
-		config:            config,
-		expectedPeerCerts: []string{clientCertificatePEM},
-	}
-	runServerTestTLS12(t, test)
-	runServerTestTLS13(t, test)
+		config.ClientAuth = RequireAnyClientCert
 
-	test = &serverTest{
-		name: "ClientAuthRequestedAndECDSAGiven",
-		command: []string{"openssl", "s_client", "-no_ticket", "-cipher", "AES128-SHA",
-			"-cert", ecdsaCertPath, "-key", ecdsaKeyPath},
-		config:            config,
-		expectedPeerCerts: []string{clientECDSACertificatePEM},
-	}
-	runServerTestTLS12(t, test)
-	runServerTestTLS13(t, test)
+		test = &serverTest{
+			name: "ClientAuthRequestedAndGiven",
+			command: []string{"openssl", "s_client", "-no_ticket", "-cipher", "AES128-SHA",
+				"-cert", certPath, "-key", keyPath, "-sigalgs", "rsa_pss_rsae_sha256"},
+			config:            config,
+			expectedPeerCerts: []string{clientCertificatePEM},
+		}
+		runServerTestTLS12(t, test)
+		runServerTestTLS13(t, test)
 
-	test = &serverTest{
-		name: "ClientAuthRequestedAndPKCS1v15Given",
-		command: []string{"openssl", "s_client", "-no_ticket", "-cipher", "AES128-SHA",
-			"-cert", certPath, "-key", keyPath, "-sigalgs", "rsa_pkcs1_sha256"},
-		config:            config,
-		expectedPeerCerts: []string{clientCertificatePEM},
-	}
-	runServerTestTLS12(t, test)
+		test = &serverTest{
+			name: "ClientAuthRequestedAndECDSAGiven",
+			command: []string{"openssl", "s_client", "-no_ticket", "-cipher", "AES128-SHA",
+				"-cert", ecdsaCertPath, "-key", ecdsaKeyPath},
+			config:            config,
+			expectedPeerCerts: []string{clientECDSACertificatePEM},
+		}
+		runServerTestTLS12(t, test)
+		runServerTestTLS13(t, test)
+
+		test = &serverTest{
+			name: "ClientAuthRequestedAndPKCS1v15Given",
+			command: []string{"openssl", "s_client", "-no_ticket", "-cipher", "AES128-SHA",
+				"-cert", certPath, "-key", keyPath, "-sigalgs", "rsa_pkcs1_sha256"},
+			config:            config,
+			expectedPeerCerts: []string{clientCertificatePEM},
+		}
+		runServerTestTLS12(t, test)
+	})
+
+	// Restore the default signature algorithms, disabling RSA-PSS in TLS 1.2,
+	// and check that handshakes still work.
+	testSupportedSignatureAlgorithmsTLS12 := supportedSignatureAlgorithmsTLS12
+	defer func() { supportedSignatureAlgorithmsTLS12 = testSupportedSignatureAlgorithmsTLS12 }()
+	supportedSignatureAlgorithmsTLS12 = savedSupportedSignatureAlgorithmsTLS12
+
+	t.Run("PSSDisabled", func(t *testing.T) {
+		config := testConfig.Clone()
+		config.ClientAuth = RequireAnyClientCert
+
+		test := &serverTest{
+			name: "ClientAuthRequestedAndGiven-PSS-Disabled",
+			command: []string{"openssl", "s_client", "-no_ticket", "-cipher", "AES128-SHA",
+				"-cert", certPath, "-key", keyPath},
+			config:            config,
+			expectedPeerCerts: []string{clientCertificatePEM},
+		}
+		runServerTestTLS12(t, test)
+		runServerTestTLS13(t, test)
+
+		test = &serverTest{
+			name: "ClientAuthRequestedAndGiven-PSS-Disabled-Required",
+			command: []string{"openssl", "s_client", "-no_ticket", "-cipher", "AES128-SHA",
+				"-cert", certPath, "-key", keyPath, "-client_sigalgs", "rsa_pss_rsae_sha256"},
+			config: config,
+
+			expectHandshakeErrorIncluding: "client didn't provide a certificate",
+		}
+		runServerTestTLS12(t, test)
+	})
 }
 
 func TestSNIGivenOnFailure(t *testing.T) {
@@ -1722,6 +1782,7 @@ T+E0J8wlH24pgwQHzy7Ko2qLwn1b5PW8ecrlvP1g
 	if err != nil {
 		t.Fatal(err)
 	}
+
 	done := make(chan struct{})
 	go func() {
 		config := testConfig.Clone()
@@ -1739,4 +1800,15 @@ T+E0J8wlH24pgwQHzy7Ko2qLwn1b5PW8ecrlvP1g
 		t.Errorf(`expected "handshake failure", got %q`, err)
 	}
 	<-done
+
+	// With RSA-PSS disabled and TLS 1.2, this should work.
+
+	testSupportedSignatureAlgorithmsTLS12 := supportedSignatureAlgorithmsTLS12
+	defer func() { supportedSignatureAlgorithmsTLS12 = testSupportedSignatureAlgorithmsTLS12 }()
+	supportedSignatureAlgorithmsTLS12 = savedSupportedSignatureAlgorithmsTLS12
+
+	serverConfig := testConfig.Clone()
+	serverConfig.Certificates = []Certificate{cert}
+	serverConfig.MaxVersion = VersionTLS12
+	testHandshake(t, testConfig, serverConfig)
 }