crypto/tls: align cert decode alert w/ BSSL

For malformed client/server certificates in a TLS handshake send
a decode_error alert, matching BoringSSL behaviour.

Previously crypto/tls used a bad_certificate alert for this purpose.
The TLS specification is imprecise enough to allow this to be considered
a spec. justified choice, but since all other places in the protocol
encourage using decode_error for structurally malformed messages we may
as well do the same here and get some extra cross-impl consistency for
free.

This also allows un-ignoring the BoGo
GarbageCertificate-[Client|Server]-[TLS12|TLS13] tests.

Updates #72006

Change-Id: Ide45ba1602816e71c3289a60e77587266c3b9036
Reviewed-on: https://go-review.googlesource.com/c/go/+/652995
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Junyang Shao <shaojunyang@google.com>
Reviewed-by: Roland Shoemaker <roland@golang.org>

diff --git a/src/crypto/tls/bogo_config.json b/src/crypto/tls/bogo_config.json
index 1521594034f3467b864d2b178b89dd501b4cac1a..6a9a6dfcc5f8b0f68b9d63d60247452ba4989679 100644 (file)
--- a/src/crypto/tls/bogo_config.json
+++ b/src/crypto/tls/bogo_config.json
@@ -54,7 +54,6 @@
         "KyberKeyShareIncludedSecond": "we always send the Kyber key share first",
         "KyberKeyShareIncludedThird": "we always send the Kyber key share first",
         "GREASE-Server-TLS13": "We don't send GREASE extensions",
-        "GarbageCertificate*": "TODO ask davidben, alertDecode vs alertBadCertificate",
         "SendBogusAlertType": "sending wrong alert type",
         "EchoTLS13CompatibilitySessionID": "TODO reject compat session ID",
         "*Client-P-224*": "no P-224 support",

diff --git a/src/crypto/tls/handshake_client.go b/src/crypto/tls/handshake_client.go
index 1be0c82c4bce1455fdca5c3719be456905d60279..f6930c5d1b7381b65f1d67b5805db706f8e14e09 100644 (file)
--- a/src/crypto/tls/handshake_client.go
+++ b/src/crypto/tls/handshake_client.go
@@ -1096,7 +1096,7 @@ func (c *Conn) verifyServerCertificate(certificates [][]byte) error {
        for i, asn1Data := range certificates {
                cert, err := globalCertCache.newCert(asn1Data)
                if err != nil {
-                       c.sendAlert(alertBadCertificate)
+                       c.sendAlert(alertDecodeError)
                        return errors.New("tls: failed to parse certificate from server: " + err.Error())
                }
                if cert.cert.PublicKeyAlgorithm == x509.RSA {

diff --git a/src/crypto/tls/handshake_server.go b/src/crypto/tls/handshake_server.go
index 641bbec0c96fff5087ec644006d3bcbf1e626154..bb3d3065e2c3fe4b59f6ff5ef5c10791c61cc31b 100644 (file)
--- a/src/crypto/tls/handshake_server.go
+++ b/src/crypto/tls/handshake_server.go
@@ -909,7 +909,7 @@ func (c *Conn) processCertsFromClient(certificate Certificate) error {
        var err error
        for i, asn1Data := range certificates {
                if certs[i], err = x509.ParseCertificate(asn1Data); err != nil {
-                       c.sendAlert(alertBadCertificate)
+                       c.sendAlert(alertDecodeError)
                        return errors.New("tls: failed to parse client certificate: " + err.Error())
                }
                if certs[i].PublicKeyAlgorithm == x509.RSA {