From 2350afd2e8ab054390e284c95d5b089c142db017 Mon Sep 17 00:00:00 2001 From: Roland Shoemaker Date: Wed, 7 Jun 2023 15:27:13 -0700 Subject: [PATCH] crypto/tls: restrict RSA keys in certificates to <= 8192 bits Extremely large RSA keys in certificate chains can cause a client/server to expend significant CPU time verifying signatures. Limit this by restricting the size of RSA keys transmitted during handshakes to <= 8192 bits. Based on a survey of publicly trusted RSA keys, there are currently only three certificates in circulation with keys larger than this, and all three appear to be test certificates that are not actively deployed. It is possible there are larger keys in use in private PKIs, but we target the web PKI, so causing breakage here in the interests of increasing the default safety of users of crypto/tls seems reasonable. Thanks to Mateusz Poliwczak for reporting this issue. Fixes #61460 Fixes CVE-2023-29409 Change-Id: Ie35038515a649199a36a12fc2c5df3af855dca6c Reviewed-on: https://team-review.git.corp.google.com/c/golang/go-private/+/1912161 Reviewed-by: Damien Neil Reviewed-by: Tatiana Bradley Run-TryBot: Roland Shoemaker Reviewed-on: https://go-review.googlesource.com/c/go/+/515257 TryBot-Result: Gopher Robot Auto-Submit: David Chase Run-TryBot: David Chase --- src/crypto/tls/handshake_client.go | 8 +++ src/crypto/tls/handshake_client_test.go | 78 +++++++++++++++++++++++++ src/crypto/tls/handshake_server.go | 4 ++ 3 files changed, 90 insertions(+) diff --git a/src/crypto/tls/handshake_client.go b/src/crypto/tls/handshake_client.go index 2ea74c5494500..f6911d458f9cb 100644 --- a/src/crypto/tls/handshake_client.go +++ b/src/crypto/tls/handshake_client.go @@ -936,6 +936,10 @@ func (hs *clientHandshakeState) sendFinished(out []byte) error { return nil } +// maxRSAKeySize is the maximum RSA key size in bits that we are willing +// to verify the signatures of during a TLS handshake. +const maxRSAKeySize = 8192 + // verifyServerCertificate parses and verifies the provided chain, setting // c.verifiedChains and c.peerCertificates or sending the appropriate alert. func (c *Conn) verifyServerCertificate(certificates [][]byte) error { @@ -947,6 +951,10 @@ func (c *Conn) verifyServerCertificate(certificates [][]byte) error { c.sendAlert(alertBadCertificate) return errors.New("tls: failed to parse certificate from server: " + err.Error()) } + if cert.cert.PublicKeyAlgorithm == x509.RSA && cert.cert.PublicKey.(*rsa.PublicKey).N.BitLen() > maxRSAKeySize { + c.sendAlert(alertBadCertificate) + return fmt.Errorf("tls: server sent certificate containing RSA key larger than %d bits", maxRSAKeySize) + } activeHandles[i] = cert certs[i] = cert.cert } diff --git a/src/crypto/tls/handshake_client_test.go b/src/crypto/tls/handshake_client_test.go index 08c0af62bd69c..ee9e79afabf3d 100644 --- a/src/crypto/tls/handshake_client_test.go +++ b/src/crypto/tls/handshake_client_test.go @@ -2721,3 +2721,81 @@ func testTLS13OnlyClientHelloCipherSuite(t *testing.T, ciphers []uint16) { t.Fatalf("handshake failed: %s", err) } } + +// discardConn wraps a net.Conn but discards all writes, but reports that they happened. +type discardConn struct { + net.Conn +} + +func (dc *discardConn) Write(data []byte) (int, error) { + return len(data), nil +} + +// largeRSAKeyCertPEM contains a 8193 bit RSA key +const largeRSAKeyCertPEM = `-----BEGIN CERTIFICATE----- +MIIInjCCBIWgAwIBAgIBAjANBgkqhkiG9w0BAQsFADASMRAwDgYDVQQDEwd0ZXN0 +aW5nMB4XDTIzMDYwNzIxMjMzNloXDTIzMDYwNzIzMjMzNlowEjEQMA4GA1UEAxMH +dGVzdGluZzCCBCIwDQYJKoZIhvcNAQEBBQADggQPADCCBAoCggQBAWdHsf6Rh2Ca +n2SQwn4t4OQrOjbLLdGE1pM6TBKKrHUFy62uEL8atNjlcfXIsa4aEu3xNGiqxqur +ZectlkZbm0FkaaQ1Wr9oikDY3KfjuaXdPdO/XC/h8AKNxlDOylyXwUSK/CuYb+1j +gy8yF5QFvVfwW/xwTlHmhUeSkVSQPosfQ6yXNNsmMzkd+ZPWLrfq4R+wiNtwYGu0 +WSBcI/M9o8/vrNLnIppoiBJJ13j9CR1ToEAzOFh9wwRWLY10oZhoh1ONN1KQURx4 +qedzvvP2DSjZbUccdvl2rBGvZpzfOiFdm1FCnxB0c72Cqx+GTHXBFf8bsa7KHky9 +sNO1GUanbq17WoDNgwbY6H51bfShqv0CErxatwWox3we4EcAmFHPVTCYL1oWVMGo +a3Eth91NZj+b/nGhF9lhHKGzXSv9brmLLkfvM1jA6XhNhA7BQ5Vz67lj2j3XfXdh +t/BU5pBXbL4Ut4mIhT1YnKXAjX2/LF5RHQTE8Vwkx5JAEKZyUEGOReD/B+7GOrLp +HduMT9vZAc5aR2k9I8qq1zBAzsL69lyQNAPaDYd1BIAjUety9gAYaSQffCgAgpRO +Gt+DYvxS+7AT/yEd5h74MU2AH7KrAkbXOtlwupiGwhMVTstncDJWXMJqbBhyHPF8 +3UmZH0hbL4PYmzSj9LDWQQXI2tv6vrCpfts3Cqhqxz9vRpgY7t1Wu6l/r+KxYYz3 +1pcGpPvRmPh0DJm7cPTiXqPnZcPt+ulSaSdlxmd19OnvG5awp0fXhxryZVwuiT8G +VDkhyARrxYrdjlINsZJZbQjO0t8ketXAELJOnbFXXzeCOosyOHkLwsqOO96AVJA8 +45ZVL5m95ClGy0RSrjVIkXsxTAMVG6SPAqKwk6vmTdRGuSPS4rhgckPVDHmccmuq +dfnT2YkX+wB2/M3oCgU+s30fAHGkbGZ0pCdNbFYFZLiH0iiMbTDl/0L/z7IdK0nH +GLHVE7apPraKC6xl6rPWsD2iSfrmtIPQa0+rqbIVvKP5JdfJ8J4alI+OxFw/znQe +V0/Rez0j22Fe119LZFFSXhRv+ZSvcq20xDwh00mzcumPWpYuCVPozA18yIhC9tNn +ALHndz0tDseIdy9vC71jQWy9iwri3ueN0DekMMF8JGzI1Z6BAFzgyAx3DkHtwHg7 +B7qD0jPG5hJ5+yt323fYgJsuEAYoZ8/jzZ01pkX8bt+UsVN0DGnSGsI2ktnIIk3J +l+8krjmUy6EaW79nITwoOqaeHOIp8m3UkjEcoKOYrzHRKqRy+A09rY+m/cAQaafW +4xp0Zv7qZPLwnu0jsqB4jD8Ll9yPB02ndsoV6U5PeHzTkVhPml19jKUAwFfs7TJg +kXy+/xFhYVUCAwEAATANBgkqhkiG9w0BAQsFAAOCBAIAAQnZY77pMNeypfpba2WK +aDasT7dk2JqP0eukJCVPTN24Zca+xJNPdzuBATm/8SdZK9lddIbjSnWRsKvTnO2r +/rYdlPf3jM5uuJtb8+Uwwe1s+gszelGS9G/lzzq+ehWicRIq2PFcs8o3iQMfENiv +qILJ+xjcrvms5ZPDNahWkfRx3KCg8Q+/at2n5p7XYjMPYiLKHnDC+RE2b1qT20IZ +FhuK/fTWLmKbfYFNNga6GC4qcaZJ7x0pbm4SDTYp0tkhzcHzwKhidfNB5J2vNz6l +Ur6wiYwamFTLqcOwWo7rdvI+sSn05WQBv0QZlzFX+OAu0l7WQ7yU+noOxBhjvHds +14+r9qcQZg2q9kG+evopYZqYXRUNNlZKo9MRBXhfrISulFAc5lRFQIXMXnglvAu+ +Ipz2gomEAOcOPNNVldhKAU94GAMJd/KfN0ZP7gX3YvPzuYU6XDhag5RTohXLm18w +5AF+ES3DOQ6ixu3DTf0D+6qrDuK+prdX8ivcdTQVNOQ+MIZeGSc6NWWOTaMGJ3lg +aZIxJUGdo6E7GBGiC1YTjgFKFbHzek1LRTh/LX3vbSudxwaG0HQxwsU9T4DWiMqa +Fkf2KteLEUA6HrR+0XlAZrhwoqAmrJ+8lCFX3V0gE9lpENfVHlFXDGyx10DpTB28 +DdjnY3F7EPWNzwf9P3oNT69CKW3Bk6VVr3ROOJtDxVu1ioWo3TaXltQ0VOnap2Pu +sa5wfrpfwBDuAS9JCDg4ttNp2nW3F7tgXC6xPqw5pvGwUppEw9XNrqV8TZrxduuv +rQ3NyZ7KSzIpmFlD3UwV/fGfz3UQmHS6Ng1evrUID9DjfYNfRqSGIGjDfxGtYD+j +Z1gLJZuhjJpNtwBkKRtlNtrCWCJK2hidK/foxwD7kwAPo2I9FjpltxCRywZUs07X +KwXTfBR9v6ij1LV6K58hFS+8ezZyZ05CeVBFkMQdclTOSfuPxlMkQOtjp8QWDj+F +j/MYziT5KBkHvcbrjdRtUJIAi4N7zCsPZtjik918AK1WBNRVqPbrgq/XSEXMfuvs +6JbfK0B76vdBDRtJFC1JsvnIrGbUztxXzyQwFLaR/AjVJqpVlysLWzPKWVX6/+SJ +u1NQOl2E8P6ycyBsuGnO89p0S4F8cMRcI2X1XQsZ7/q0NBrOMaEp5T3SrWo9GiQ3 +o2SBdbs3Y6MBPBtTu977Z/0RO63J3M5i2tjUiDfrFy7+VRLKr7qQ7JibohyB8QaR +9tedgjn2f+of7PnP/PEl1cCphUZeHM7QKUMPT8dbqwmKtlYY43EHXcvNOT5IBk3X +9lwJoZk/B2i+ZMRNSP34ztAwtxmasPt6RAWGQpWCn9qmttAHAnMfDqe7F7jVR6rS +u58= +-----END CERTIFICATE-----` + +func TestHandshakeRSATooBig(t *testing.T) { + testCert, _ := pem.Decode([]byte(largeRSAKeyCertPEM)) + + c := &Conn{conn: &discardConn{}, config: testConfig.Clone()} + + expectedErr := "tls: server sent certificate containing RSA key larger than 8192 bits" + err := c.verifyServerCertificate([][]byte{testCert.Bytes}) + if err == nil || err.Error() != expectedErr { + t.Errorf("Conn.verifyServerCertificate unexpected error: want %q, got %q", expectedErr, err) + } + + expectedErr = "tls: client sent certificate containing RSA key larger than 8192 bits" + err = c.processCertsFromClient(Certificate{Certificate: [][]byte{testCert.Bytes}}) + if err == nil || err.Error() != expectedErr { + t.Errorf("Conn.processCertsFromClient unexpected error: want %q, got %q", expectedErr, err) + } +} diff --git a/src/crypto/tls/handshake_server.go b/src/crypto/tls/handshake_server.go index 0f2571a7f3db9..89a16a8967dcb 100644 --- a/src/crypto/tls/handshake_server.go +++ b/src/crypto/tls/handshake_server.go @@ -864,6 +864,10 @@ func (c *Conn) processCertsFromClient(certificate Certificate) error { c.sendAlert(alertBadCertificate) return errors.New("tls: failed to parse client certificate: " + err.Error()) } + if certs[i].PublicKeyAlgorithm == x509.RSA && certs[i].PublicKey.(*rsa.PublicKey).N.BitLen() > maxRSAKeySize { + c.sendAlert(alertBadCertificate) + return fmt.Errorf("tls: client sent certificate containing RSA key larger than %d bits", maxRSAKeySize) + } } if len(certs) == 0 && requiresClientCert(c.config.ClientAuth) {