Skip to content

Commit a51957f

Browse files
rolandshoemakerdr2chase
authored andcommitted
[release-branch.go1.21] 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. Updates #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 <[email protected]> Reviewed-by: Tatiana Bradley <[email protected]> Run-TryBot: Roland Shoemaker <[email protected]> (cherry picked from commit d865c715d92887361e4bd5596e19e513f27781b7) Reviewed-on: https://go-review.googlesource.com/c/go/+/515056 Run-TryBot: David Chase <[email protected]> TryBot-Result: Gopher Robot <[email protected]>
1 parent 363f259 commit a51957f

File tree

3 files changed

+90
-0
lines changed

3 files changed

+90
-0
lines changed

src/crypto/tls/handshake_client.go

+8
Original file line numberDiff line numberDiff line change
@@ -936,6 +936,10 @@ func (hs *clientHandshakeState) sendFinished(out []byte) error {
936936
return nil
937937
}
938938

939+
// maxRSAKeySize is the maximum RSA key size in bits that we are willing
940+
// to verify the signatures of during a TLS handshake.
941+
const maxRSAKeySize = 8192
942+
939943
// verifyServerCertificate parses and verifies the provided chain, setting
940944
// c.verifiedChains and c.peerCertificates or sending the appropriate alert.
941945
func (c *Conn) verifyServerCertificate(certificates [][]byte) error {
@@ -947,6 +951,10 @@ func (c *Conn) verifyServerCertificate(certificates [][]byte) error {
947951
c.sendAlert(alertBadCertificate)
948952
return errors.New("tls: failed to parse certificate from server: " + err.Error())
949953
}
954+
if cert.cert.PublicKeyAlgorithm == x509.RSA && cert.cert.PublicKey.(*rsa.PublicKey).N.BitLen() > maxRSAKeySize {
955+
c.sendAlert(alertBadCertificate)
956+
return fmt.Errorf("tls: server sent certificate containing RSA key larger than %d bits", maxRSAKeySize)
957+
}
950958
activeHandles[i] = cert
951959
certs[i] = cert.cert
952960
}

src/crypto/tls/handshake_client_test.go

+78
Original file line numberDiff line numberDiff line change
@@ -2721,3 +2721,81 @@ func testTLS13OnlyClientHelloCipherSuite(t *testing.T, ciphers []uint16) {
27212721
t.Fatalf("handshake failed: %s", err)
27222722
}
27232723
}
2724+
2725+
// discardConn wraps a net.Conn but discards all writes, but reports that they happened.
2726+
type discardConn struct {
2727+
net.Conn
2728+
}
2729+
2730+
func (dc *discardConn) Write(data []byte) (int, error) {
2731+
return len(data), nil
2732+
}
2733+
2734+
// largeRSAKeyCertPEM contains a 8193 bit RSA key
2735+
const largeRSAKeyCertPEM = `-----BEGIN CERTIFICATE-----
2736+
MIIInjCCBIWgAwIBAgIBAjANBgkqhkiG9w0BAQsFADASMRAwDgYDVQQDEwd0ZXN0
2737+
aW5nMB4XDTIzMDYwNzIxMjMzNloXDTIzMDYwNzIzMjMzNlowEjEQMA4GA1UEAxMH
2738+
dGVzdGluZzCCBCIwDQYJKoZIhvcNAQEBBQADggQPADCCBAoCggQBAWdHsf6Rh2Ca
2739+
n2SQwn4t4OQrOjbLLdGE1pM6TBKKrHUFy62uEL8atNjlcfXIsa4aEu3xNGiqxqur
2740+
ZectlkZbm0FkaaQ1Wr9oikDY3KfjuaXdPdO/XC/h8AKNxlDOylyXwUSK/CuYb+1j
2741+
gy8yF5QFvVfwW/xwTlHmhUeSkVSQPosfQ6yXNNsmMzkd+ZPWLrfq4R+wiNtwYGu0
2742+
WSBcI/M9o8/vrNLnIppoiBJJ13j9CR1ToEAzOFh9wwRWLY10oZhoh1ONN1KQURx4
2743+
qedzvvP2DSjZbUccdvl2rBGvZpzfOiFdm1FCnxB0c72Cqx+GTHXBFf8bsa7KHky9
2744+
sNO1GUanbq17WoDNgwbY6H51bfShqv0CErxatwWox3we4EcAmFHPVTCYL1oWVMGo
2745+
a3Eth91NZj+b/nGhF9lhHKGzXSv9brmLLkfvM1jA6XhNhA7BQ5Vz67lj2j3XfXdh
2746+
t/BU5pBXbL4Ut4mIhT1YnKXAjX2/LF5RHQTE8Vwkx5JAEKZyUEGOReD/B+7GOrLp
2747+
HduMT9vZAc5aR2k9I8qq1zBAzsL69lyQNAPaDYd1BIAjUety9gAYaSQffCgAgpRO
2748+
Gt+DYvxS+7AT/yEd5h74MU2AH7KrAkbXOtlwupiGwhMVTstncDJWXMJqbBhyHPF8
2749+
3UmZH0hbL4PYmzSj9LDWQQXI2tv6vrCpfts3Cqhqxz9vRpgY7t1Wu6l/r+KxYYz3
2750+
1pcGpPvRmPh0DJm7cPTiXqPnZcPt+ulSaSdlxmd19OnvG5awp0fXhxryZVwuiT8G
2751+
VDkhyARrxYrdjlINsZJZbQjO0t8ketXAELJOnbFXXzeCOosyOHkLwsqOO96AVJA8
2752+
45ZVL5m95ClGy0RSrjVIkXsxTAMVG6SPAqKwk6vmTdRGuSPS4rhgckPVDHmccmuq
2753+
dfnT2YkX+wB2/M3oCgU+s30fAHGkbGZ0pCdNbFYFZLiH0iiMbTDl/0L/z7IdK0nH
2754+
GLHVE7apPraKC6xl6rPWsD2iSfrmtIPQa0+rqbIVvKP5JdfJ8J4alI+OxFw/znQe
2755+
V0/Rez0j22Fe119LZFFSXhRv+ZSvcq20xDwh00mzcumPWpYuCVPozA18yIhC9tNn
2756+
ALHndz0tDseIdy9vC71jQWy9iwri3ueN0DekMMF8JGzI1Z6BAFzgyAx3DkHtwHg7
2757+
B7qD0jPG5hJ5+yt323fYgJsuEAYoZ8/jzZ01pkX8bt+UsVN0DGnSGsI2ktnIIk3J
2758+
l+8krjmUy6EaW79nITwoOqaeHOIp8m3UkjEcoKOYrzHRKqRy+A09rY+m/cAQaafW
2759+
4xp0Zv7qZPLwnu0jsqB4jD8Ll9yPB02ndsoV6U5PeHzTkVhPml19jKUAwFfs7TJg
2760+
kXy+/xFhYVUCAwEAATANBgkqhkiG9w0BAQsFAAOCBAIAAQnZY77pMNeypfpba2WK
2761+
aDasT7dk2JqP0eukJCVPTN24Zca+xJNPdzuBATm/8SdZK9lddIbjSnWRsKvTnO2r
2762+
/rYdlPf3jM5uuJtb8+Uwwe1s+gszelGS9G/lzzq+ehWicRIq2PFcs8o3iQMfENiv
2763+
qILJ+xjcrvms5ZPDNahWkfRx3KCg8Q+/at2n5p7XYjMPYiLKHnDC+RE2b1qT20IZ
2764+
FhuK/fTWLmKbfYFNNga6GC4qcaZJ7x0pbm4SDTYp0tkhzcHzwKhidfNB5J2vNz6l
2765+
Ur6wiYwamFTLqcOwWo7rdvI+sSn05WQBv0QZlzFX+OAu0l7WQ7yU+noOxBhjvHds
2766+
14+r9qcQZg2q9kG+evopYZqYXRUNNlZKo9MRBXhfrISulFAc5lRFQIXMXnglvAu+
2767+
Ipz2gomEAOcOPNNVldhKAU94GAMJd/KfN0ZP7gX3YvPzuYU6XDhag5RTohXLm18w
2768+
5AF+ES3DOQ6ixu3DTf0D+6qrDuK+prdX8ivcdTQVNOQ+MIZeGSc6NWWOTaMGJ3lg
2769+
aZIxJUGdo6E7GBGiC1YTjgFKFbHzek1LRTh/LX3vbSudxwaG0HQxwsU9T4DWiMqa
2770+
Fkf2KteLEUA6HrR+0XlAZrhwoqAmrJ+8lCFX3V0gE9lpENfVHlFXDGyx10DpTB28
2771+
DdjnY3F7EPWNzwf9P3oNT69CKW3Bk6VVr3ROOJtDxVu1ioWo3TaXltQ0VOnap2Pu
2772+
sa5wfrpfwBDuAS9JCDg4ttNp2nW3F7tgXC6xPqw5pvGwUppEw9XNrqV8TZrxduuv
2773+
rQ3NyZ7KSzIpmFlD3UwV/fGfz3UQmHS6Ng1evrUID9DjfYNfRqSGIGjDfxGtYD+j
2774+
Z1gLJZuhjJpNtwBkKRtlNtrCWCJK2hidK/foxwD7kwAPo2I9FjpltxCRywZUs07X
2775+
KwXTfBR9v6ij1LV6K58hFS+8ezZyZ05CeVBFkMQdclTOSfuPxlMkQOtjp8QWDj+F
2776+
j/MYziT5KBkHvcbrjdRtUJIAi4N7zCsPZtjik918AK1WBNRVqPbrgq/XSEXMfuvs
2777+
6JbfK0B76vdBDRtJFC1JsvnIrGbUztxXzyQwFLaR/AjVJqpVlysLWzPKWVX6/+SJ
2778+
u1NQOl2E8P6ycyBsuGnO89p0S4F8cMRcI2X1XQsZ7/q0NBrOMaEp5T3SrWo9GiQ3
2779+
o2SBdbs3Y6MBPBtTu977Z/0RO63J3M5i2tjUiDfrFy7+VRLKr7qQ7JibohyB8QaR
2780+
9tedgjn2f+of7PnP/PEl1cCphUZeHM7QKUMPT8dbqwmKtlYY43EHXcvNOT5IBk3X
2781+
9lwJoZk/B2i+ZMRNSP34ztAwtxmasPt6RAWGQpWCn9qmttAHAnMfDqe7F7jVR6rS
2782+
u58=
2783+
-----END CERTIFICATE-----`
2784+
2785+
func TestHandshakeRSATooBig(t *testing.T) {
2786+
testCert, _ := pem.Decode([]byte(largeRSAKeyCertPEM))
2787+
2788+
c := &Conn{conn: &discardConn{}, config: testConfig.Clone()}
2789+
2790+
expectedErr := "tls: server sent certificate containing RSA key larger than 8192 bits"
2791+
err := c.verifyServerCertificate([][]byte{testCert.Bytes})
2792+
if err == nil || err.Error() != expectedErr {
2793+
t.Errorf("Conn.verifyServerCertificate unexpected error: want %q, got %q", expectedErr, err)
2794+
}
2795+
2796+
expectedErr = "tls: client sent certificate containing RSA key larger than 8192 bits"
2797+
err = c.processCertsFromClient(Certificate{Certificate: [][]byte{testCert.Bytes}})
2798+
if err == nil || err.Error() != expectedErr {
2799+
t.Errorf("Conn.processCertsFromClient unexpected error: want %q, got %q", expectedErr, err)
2800+
}
2801+
}

src/crypto/tls/handshake_server.go

+4
Original file line numberDiff line numberDiff line change
@@ -864,6 +864,10 @@ func (c *Conn) processCertsFromClient(certificate Certificate) error {
864864
c.sendAlert(alertBadCertificate)
865865
return errors.New("tls: failed to parse client certificate: " + err.Error())
866866
}
867+
if certs[i].PublicKeyAlgorithm == x509.RSA && certs[i].PublicKey.(*rsa.PublicKey).N.BitLen() > maxRSAKeySize {
868+
c.sendAlert(alertBadCertificate)
869+
return fmt.Errorf("tls: client sent certificate containing RSA key larger than %d bits", maxRSAKeySize)
870+
}
867871
}
868872

869873
if len(certs) == 0 && requiresClientCert(c.config.ClientAuth) {

0 commit comments

Comments
 (0)