Skip to content

Commit 7ccd358

Browse files
FiloSottileFiloSottile
committed
crypto/tls: disable RSA-PSS in TLS 1.2
Most of the issues that led to the decision on golang#30055 were related to incompatibility with or faulty support for RSA-PSS (golang#29831, golang#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 golang#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 parent 5d9bc60 commit 7ccd358

16 files changed

+1047
-51
lines changed

‎doc/go1.12.html‎

Lines changed: 2 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -406,7 +406,8 @@ <h3 id="tls_1_3">TLS 1.3</h3>
406406
and renegotiation are available in TLS 1.3 and provide equivalent or
407407
better security and performance. Note that even though TLS 1.3 is backwards
408408
compatible with previous versions, certain legacy systems might not work
409-
correctly when attempting to negotiate it.
409+
correctly when attempting to negotiate it. RSA certificate keys too small
410+
to be secure (including 512-bit keys) will not work with TLS 1.3.
410411
</p>
411412

412413
<p>
@@ -500,13 +501,6 @@ <h3 id="minor_library_changes">Minor changes to the library</h3>
500501

501502
<dl id="crypto/tls"><dt><a href="/pkg/crypto/tls/">crypto/tls</a></dt>
502503
<dd>
503-
<p><!-- CL 146258 -->
504-
TLS 1.2 clients and servers will now advertise and accept RSA-PSS
505-
signature algorithms for use with regular RSA public keys. Certain
506-
insecure certificate keys (including 512-bit RSA keys) will
507-
now cause a handshake failure if RSA-PSS is selected.
508-
</p>
509-
510504
<p><!-- CL 143177 -->
511505
If a client sends an initial message that does not look like TLS, the server
512506
will no longer reply with an alert, and it will expose the underlying

‎src/crypto/tls/common.go‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -161,7 +161,7 @@ const (
161161
)
162162

163163
// supportedSignatureAlgorithms contains the signature and hash algorithms that
164-
// the code advertises as supported in a TLS 1.2 ClientHello and in a TLS 1.2
164+
// the code advertises as supported in a TLS 1.2+ ClientHello and in a TLS 1.2+
165165
// CertificateRequest. The two fields are merged to match with TLS 1.3.
166166
// Note that in TLS 1.2, the ECDSA algorithms are not constrained to P-256, etc.
167167
var supportedSignatureAlgorithms = []SignatureScheme{
@@ -178,6 +178,9 @@ var supportedSignatureAlgorithms = []SignatureScheme{
178178
ECDSAWithSHA1,
179179
}
180180

181+
// RSA-PSS is disabled in TLS 1.2 for Go 1.12. See Issue 30055.
182+
var supportedSignatureAlgorithmsTLS12 = supportedSignatureAlgorithms[3:]
183+
181184
// helloRetryRequestRandom is set as the Random value of a ServerHello
182185
// to signal that the message is actually a HelloRetryRequest.
183186
var helloRetryRequestRandom = []byte{ // See RFC 8446, Section 4.1.3.

‎src/crypto/tls/conn_test.go‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -142,6 +142,7 @@ func runDynamicRecordSizingTest(t *testing.T, config *Config) {
142142

143143
handshakeDone := make(chan struct{})
144144
recordSizesChan := make(chan []int, 1)
145+
defer func() { <-recordSizesChan }() // wait for the goroutine to exit
145146
go func() {
146147
// This goroutine performs a TLS handshake over clientConn and
147148
// then reads TLS records until EOF. It writes a slice that

‎src/crypto/tls/handshake_client_test.go‎

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -855,6 +855,30 @@ func TestHandshakeClientCertRSAPKCS1v15(t *testing.T) {
855855
runClientTestTLS12(t, test)
856856
}
857857

858+
func TestHandshakeClientCertPSSDisabled(t *testing.T) {
859+
config := testConfig.Clone()
860+
cert, _ := X509KeyPair([]byte(clientCertificatePEM), []byte(clientKeyPEM))
861+
config.Certificates = []Certificate{cert}
862+
863+
test := &clientTest{
864+
name: "ClientCert-RSA-PSS-Disabled",
865+
args: []string{"-cipher", "AES128", "-Verify", "1"},
866+
config: config,
867+
}
868+
869+
// Restore the default signature algorithms, disabling RSA-PSS in TLS 1.2,
870+
// and check that handshakes still work.
871+
testSupportedSignatureAlgorithmsTLS12 := supportedSignatureAlgorithmsTLS12
872+
defer func() { supportedSignatureAlgorithmsTLS12 = testSupportedSignatureAlgorithmsTLS12 }()
873+
supportedSignatureAlgorithmsTLS12 = savedSupportedSignatureAlgorithmsTLS12
874+
875+
// Use t.Run to ensure the defer runs after all parallel tests end.
876+
t.Run("", func(t *testing.T) {
877+
runClientTestTLS12(t, test)
878+
runClientTestTLS13(t, test)
879+
})
880+
}
881+
858882
func TestClientKeyUpdate(t *testing.T) {
859883
test := &clientTest{
860884
name: "KeyUpdate",

‎src/crypto/tls/handshake_server.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -463,7 +463,7 @@ func (hs *serverHandshakeState) doFullHandshake() error {
463463
}
464464
if c.vers >= VersionTLS12 {
465465
certReq.hasSignatureAlgorithm = true
466-
certReq.supportedSignatureAlgorithms = supportedSignatureAlgorithms
466+
certReq.supportedSignatureAlgorithms = supportedSignatureAlgorithmsTLS12
467467
}
468468

469469
// An empty list of certificateAuthorities signals to
@@ -559,7 +559,7 @@ func (hs *serverHandshakeState) doFullHandshake() error {
559559
}
560560

561561
// Determine the signature type.
562-
_, sigType, hashFunc, err := pickSignatureAlgorithm(pub, []SignatureScheme{certVerify.signatureAlgorithm}, supportedSignatureAlgorithms, c.vers)
562+
_, sigType, hashFunc, err := pickSignatureAlgorithm(pub, []SignatureScheme{certVerify.signatureAlgorithm}, supportedSignatureAlgorithmsTLS12, c.vers)
563563
if err != nil {
564564
c.sendAlert(alertIllegalParameter)
565565
return err

‎src/crypto/tls/handshake_server_test.go‎

Lines changed: 109 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -1211,6 +1211,33 @@ func TestHandshakeServerRSAPSS(t *testing.T) {
12111211
runServerTestTLS13(t, test)
12121212
}
12131213

1214+
func TestHandshakeServerPSSDisabled(t *testing.T) {
1215+
test := &serverTest{
1216+
name: "RSA-PSS-Disabled",
1217+
command: []string{"openssl", "s_client", "-no_ticket"},
1218+
wait: true,
1219+
}
1220+
1221+
// Restore the default signature algorithms, disabling RSA-PSS in TLS 1.2,
1222+
// and check that handshakes still work.
1223+
testSupportedSignatureAlgorithmsTLS12 := supportedSignatureAlgorithmsTLS12
1224+
defer func() { supportedSignatureAlgorithmsTLS12 = testSupportedSignatureAlgorithmsTLS12 }()
1225+
supportedSignatureAlgorithmsTLS12 = savedSupportedSignatureAlgorithmsTLS12
1226+
1227+
runServerTestTLS12(t, test)
1228+
runServerTestTLS13(t, test)
1229+
1230+
test = &serverTest{
1231+
name: "RSA-PSS-Disabled-Required",
1232+
command: []string{"openssl", "s_client", "-no_ticket", "-sigalgs", "rsa_pss_rsae_sha256"},
1233+
wait: true,
1234+
1235+
expectHandshakeErrorIncluding: "peer doesn't support any common signature algorithms",
1236+
}
1237+
1238+
runServerTestTLS12(t, test)
1239+
}
1240+
12141241
func benchmarkHandshakeServer(b *testing.B, version uint16, cipherSuite uint16, curve CurveID, cert []byte, key crypto.PrivateKey) {
12151242
config := testConfig.Clone()
12161243
config.CipherSuites = []uint16{cipherSuite}
@@ -1390,49 +1417,82 @@ func TestClientAuth(t *testing.T) {
13901417
defer os.Remove(ecdsaCertPath)
13911418
ecdsaKeyPath = tempFile(clientECDSAKeyPEM)
13921419
defer os.Remove(ecdsaKeyPath)
1393-
} else {
1394-
t.Parallel()
13951420
}
13961421

1397-
config := testConfig.Clone()
1398-
config.ClientAuth = RequestClientCert
1422+
t.Run("Normal", func(t *testing.T) {
1423+
config := testConfig.Clone()
1424+
config.ClientAuth = RequestClientCert
13991425

1400-
test := &serverTest{
1401-
name: "ClientAuthRequestedNotGiven",
1402-
command: []string{"openssl", "s_client", "-no_ticket", "-cipher", "AES128-SHA"},
1403-
config: config,
1404-
}
1405-
runServerTestTLS12(t, test)
1406-
runServerTestTLS13(t, test)
1426+
test := &serverTest{
1427+
name: "ClientAuthRequestedNotGiven",
1428+
command: []string{"openssl", "s_client", "-no_ticket", "-cipher", "AES128-SHA"},
1429+
config: config,
1430+
}
1431+
runServerTestTLS12(t, test)
1432+
runServerTestTLS13(t, test)
14071433

1408-
test = &serverTest{
1409-
name: "ClientAuthRequestedAndGiven",
1410-
command: []string{"openssl", "s_client", "-no_ticket", "-cipher", "AES128-SHA",
1411-
"-cert", certPath, "-key", keyPath, "-sigalgs", "rsa_pss_rsae_sha256"},
1412-
config: config,
1413-
expectedPeerCerts: []string{clientCertificatePEM},
1414-
}
1415-
runServerTestTLS12(t, test)
1416-
runServerTestTLS13(t, test)
1434+
config.ClientAuth = RequireAnyClientCert
14171435

1418-
test = &serverTest{
1419-
name: "ClientAuthRequestedAndECDSAGiven",
1420-
command: []string{"openssl", "s_client", "-no_ticket", "-cipher", "AES128-SHA",
1421-
"-cert", ecdsaCertPath, "-key", ecdsaKeyPath},
1422-
config: config,
1423-
expectedPeerCerts: []string{clientECDSACertificatePEM},
1424-
}
1425-
runServerTestTLS12(t, test)
1426-
runServerTestTLS13(t, test)
1436+
test = &serverTest{
1437+
name: "ClientAuthRequestedAndGiven",
1438+
command: []string{"openssl", "s_client", "-no_ticket", "-cipher", "AES128-SHA",
1439+
"-cert", certPath, "-key", keyPath, "-sigalgs", "rsa_pss_rsae_sha256"},
1440+
config: config,
1441+
expectedPeerCerts: []string{clientCertificatePEM},
1442+
}
1443+
runServerTestTLS12(t, test)
1444+
runServerTestTLS13(t, test)
1445+
1446+
test = &serverTest{
1447+
name: "ClientAuthRequestedAndECDSAGiven",
1448+
command: []string{"openssl", "s_client", "-no_ticket", "-cipher", "AES128-SHA",
1449+
"-cert", ecdsaCertPath, "-key", ecdsaKeyPath},
1450+
config: config,
1451+
expectedPeerCerts: []string{clientECDSACertificatePEM},
1452+
}
1453+
runServerTestTLS12(t, test)
1454+
runServerTestTLS13(t, test)
1455+
1456+
test = &serverTest{
1457+
name: "ClientAuthRequestedAndPKCS1v15Given",
1458+
command: []string{"openssl", "s_client", "-no_ticket", "-cipher", "AES128-SHA",
1459+
"-cert", certPath, "-key", keyPath, "-sigalgs", "rsa_pkcs1_sha256"},
1460+
config: config,
1461+
expectedPeerCerts: []string{clientCertificatePEM},
1462+
}
1463+
runServerTestTLS12(t, test)
1464+
})
14271465

1428-
test = &serverTest{
1429-
name: "ClientAuthRequestedAndPKCS1v15Given",
1430-
command: []string{"openssl", "s_client", "-no_ticket", "-cipher", "AES128-SHA",
1431-
"-cert", certPath, "-key", keyPath, "-sigalgs", "rsa_pkcs1_sha256"},
1432-
config: config,
1433-
expectedPeerCerts: []string{clientCertificatePEM},
1434-
}
1435-
runServerTestTLS12(t, test)
1466+
// Restore the default signature algorithms, disabling RSA-PSS in TLS 1.2,
1467+
// and check that handshakes still work.
1468+
testSupportedSignatureAlgorithmsTLS12 := supportedSignatureAlgorithmsTLS12
1469+
defer func() { supportedSignatureAlgorithmsTLS12 = testSupportedSignatureAlgorithmsTLS12 }()
1470+
supportedSignatureAlgorithmsTLS12 = savedSupportedSignatureAlgorithmsTLS12
1471+
1472+
t.Run("PSSDisabled", func(t *testing.T) {
1473+
config := testConfig.Clone()
1474+
config.ClientAuth = RequireAnyClientCert
1475+
1476+
test := &serverTest{
1477+
name: "ClientAuthRequestedAndGiven-PSS-Disabled",
1478+
command: []string{"openssl", "s_client", "-no_ticket", "-cipher", "AES128-SHA",
1479+
"-cert", certPath, "-key", keyPath},
1480+
config: config,
1481+
expectedPeerCerts: []string{clientCertificatePEM},
1482+
}
1483+
runServerTestTLS12(t, test)
1484+
runServerTestTLS13(t, test)
1485+
1486+
test = &serverTest{
1487+
name: "ClientAuthRequestedAndGiven-PSS-Disabled-Required",
1488+
command: []string{"openssl", "s_client", "-no_ticket", "-cipher", "AES128-SHA",
1489+
"-cert", certPath, "-key", keyPath, "-client_sigalgs", "rsa_pss_rsae_sha256"},
1490+
config: config,
1491+
1492+
expectHandshakeErrorIncluding: "client didn't provide a certificate",
1493+
}
1494+
runServerTestTLS12(t, test)
1495+
})
14361496
}
14371497

14381498
func TestSNIGivenOnFailure(t *testing.T) {
@@ -1722,6 +1782,7 @@ T+E0J8wlH24pgwQHzy7Ko2qLwn1b5PW8ecrlvP1g
17221782
if err != nil {
17231783
t.Fatal(err)
17241784
}
1785+
17251786
done := make(chan struct{})
17261787
go func() {
17271788
config := testConfig.Clone()
@@ -1739,4 +1800,15 @@ T+E0J8wlH24pgwQHzy7Ko2qLwn1b5PW8ecrlvP1g
17391800
t.Errorf(`expected "handshake failure", got %q`, err)
17401801
}
17411802
<-done
1803+
1804+
// With RSA-PSS disabled and TLS 1.2, this should work.
1805+
1806+
testSupportedSignatureAlgorithmsTLS12 := supportedSignatureAlgorithmsTLS12
1807+
defer func() { supportedSignatureAlgorithmsTLS12 = testSupportedSignatureAlgorithmsTLS12 }()
1808+
supportedSignatureAlgorithmsTLS12 = savedSupportedSignatureAlgorithmsTLS12
1809+
1810+
serverConfig := testConfig.Clone()
1811+
serverConfig.Certificates = []Certificate{cert}
1812+
serverConfig.MaxVersion = VersionTLS12
1813+
testHandshake(t, testConfig, serverConfig)
17421814
}

‎src/crypto/tls/key_agreement.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -177,7 +177,7 @@ NextCandidate:
177177
return nil, errors.New("tls: certificate private key does not implement crypto.Signer")
178178
}
179179

180-
signatureAlgorithm, sigType, hashFunc, err := pickSignatureAlgorithm(priv.Public(), clientHello.supportedSignatureAlgorithms, supportedSignatureAlgorithms, ka.version)
180+
signatureAlgorithm, sigType, hashFunc, err := pickSignatureAlgorithm(priv.Public(), clientHello.supportedSignatureAlgorithms, supportedSignatureAlgorithmsTLS12, ka.version)
181181
if err != nil {
182182
return nil, err
183183
}

0 commit comments

Comments
 (0)