From 095cfba86be8db249ada8605fa31764fd4ce9241 Mon Sep 17 00:00:00 2001 From: Roland Shoemaker Date: Tue, 8 Aug 2023 18:25:59 -0700 Subject: [release-branch.go1.20] crypto/tls: add GODEBUG to control max RSA key size Add a new GODEBUG setting, tlsmaxrsasize, which allows controlling the maximum RSA key size we will accept during TLS handshakes. Fixes #61966 Change-Id: I52f060be132014d219f4cd438f59990011a35c96 Reviewed-on: https://go-review.googlesource.com/c/go/+/517495 Auto-Submit: Roland Shoemaker Reviewed-by: Russ Cox Run-TryBot: Roland Shoemaker TryBot-Result: Gopher Robot Reviewed-on: https://go-review.googlesource.com/c/go/+/518555 Auto-Submit: Dmitri Shuralyov --- src/crypto/tls/conn.go | 5 ++++ src/crypto/tls/handshake_client.go | 26 ++++++++++++++---- src/crypto/tls/handshake_client_test.go | 47 +++++++++++++++++++++++++-------- src/crypto/tls/handshake_server.go | 9 ++++--- 4 files changed, 68 insertions(+), 19 deletions(-) diff --git a/src/crypto/tls/conn.go b/src/crypto/tls/conn.go index 1eefb17206..f50b511f68 100644 --- a/src/crypto/tls/conn.go +++ b/src/crypto/tls/conn.go @@ -1414,6 +1414,11 @@ func (c *Conn) closeNotify() error { // // For control over canceling or setting a timeout on a handshake, use // HandshakeContext or the Dialer's DialContext method instead. +// +// In order to avoid denial of service attacks, the maximum RSA key size allowed +// in certificates sent by either the TLS server or client is limited to 8192 +// bits. This limit can be overridden by setting tlsmaxrsasize in the GODEBUG +// environment variable (e.g. GODEBUG=tlsmaxrsasize=4096). func (c *Conn) Handshake() error { return c.HandshakeContext(context.Background()) } diff --git a/src/crypto/tls/handshake_client.go b/src/crypto/tls/handshake_client.go index 0ef87d4d77..0bb6a0aaef 100644 --- a/src/crypto/tls/handshake_client.go +++ b/src/crypto/tls/handshake_client.go @@ -17,8 +17,10 @@ import ( "errors" "fmt" "hash" + "internal/godebug" "io" "net" + "strconv" "strings" "time" ) @@ -857,9 +859,20 @@ func (hs *clientHandshakeState) sendFinished(out []byte) error { return nil } -// maxRSAKeySize is the maximum RSA key size in bits that we are willing +// defaultMaxRSAKeySize is the maximum RSA key size in bits that we are willing // to verify the signatures of during a TLS handshake. -const maxRSAKeySize = 8192 +const defaultMaxRSAKeySize = 8192 + +var tlsmaxrsasize = godebug.New("tlsmaxrsasize") + +func checkKeySize(n int) (max int, ok bool) { + if v := tlsmaxrsasize.Value(); v != "" { + if max, err := strconv.Atoi(v); err == nil { + return max, n <= max + } + } + return defaultMaxRSAKeySize, n <= defaultMaxRSAKeySize +} // verifyServerCertificate parses and verifies the provided chain, setting // c.verifiedChains and c.peerCertificates or sending the appropriate alert. @@ -872,9 +885,12 @@ 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) + if cert.cert.PublicKeyAlgorithm == x509.RSA { + n := cert.cert.PublicKey.(*rsa.PublicKey).N.BitLen() + if max, ok := checkKeySize(n); !ok { + c.sendAlert(alertBadCertificate) + return fmt.Errorf("tls: server sent certificate containing RSA key larger than %d bits", max) + } } 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 6b5b61aed5..d0afc72e10 100644 --- a/src/crypto/tls/handshake_client_test.go +++ b/src/crypto/tls/handshake_client_test.go @@ -2678,19 +2678,44 @@ u58= -----END CERTIFICATE-----` func TestHandshakeRSATooBig(t *testing.T) { - testCert, _ := pem.Decode([]byte(largeRSAKeyCertPEM)) + for _, tc := range []struct { + name string + godebug string + expectedServerErr string + expectedClientErr string + }{ + { + name: "key too large", + expectedServerErr: "tls: server sent certificate containing RSA key larger than 8192 bits", + expectedClientErr: "tls: client sent certificate containing RSA key larger than 8192 bits", + }, + { + name: "acceptable key (GODEBUG=tlsmaxrsasize=8193)", + godebug: "tlsmaxrsasize=8193", + }, + } { + t.Run(tc.name, func(t *testing.T) { + if tc.godebug != "" { + t.Setenv("GODEBUG", tc.godebug) + } - c := &Conn{conn: &discardConn{}, config: testConfig.Clone()} + testCert, _ := pem.Decode([]byte(largeRSAKeyCertPEM)) - 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) - } + c := &Conn{conn: &discardConn{}, config: testConfig.Clone()} + + err := c.verifyServerCertificate([][]byte{testCert.Bytes}) + if tc.expectedServerErr == "" && err != nil { + t.Errorf("Conn.verifyServerCertificate unexpected error: %s", err) + } else if tc.expectedServerErr != "" && (err == nil || err.Error() != tc.expectedServerErr) { + t.Errorf("Conn.verifyServerCertificate unexpected error: want %q, got %q", tc.expectedServerErr, 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) + err = c.processCertsFromClient(Certificate{Certificate: [][]byte{testCert.Bytes}}) + if tc.expectedClientErr == "" && err != nil { + t.Errorf("Conn.processCertsFromClient unexpected error: %s", err) + } else if tc.expectedClientErr != "" && (err == nil || err.Error() != tc.expectedClientErr) { + t.Errorf("Conn.processCertsFromClient unexpected error: want %q, got %q", tc.expectedClientErr, err) + } + }) } } diff --git a/src/crypto/tls/handshake_server.go b/src/crypto/tls/handshake_server.go index 9def927314..b6a3c9c207 100644 --- a/src/crypto/tls/handshake_server.go +++ b/src/crypto/tls/handshake_server.go @@ -818,9 +818,12 @@ 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 certs[i].PublicKeyAlgorithm == x509.RSA { + n := certs[i].PublicKey.(*rsa.PublicKey).N.BitLen() + if max, ok := checkKeySize(n); !ok { + c.sendAlert(alertBadCertificate) + return fmt.Errorf("tls: client sent certificate containing RSA key larger than %d bits", max) + } } } -- cgit v1.2.3-54-g00ecf