Bug 1532312, recognize certificate_required alert, r=mt
authorDaiki Ueno <dueno@redhat.com>
Mon, 08 Apr 2019 17:31:29 +0200
changeset 15080 15905cd1cab9c8460b245f19134043b5217e0e8b
parent 15079 bb58098d38a521c6c8b42bddb2e78f45d16d70d7
child 15081 ef0974cfd1defe7512c8978095edd81e86e8b1d8
push id3321
push userdueno@redhat.com
push dateMon, 08 Apr 2019 15:32:54 +0000
reviewersmt
bugs1532312
Bug 1532312, recognize certificate_required alert, r=mt Summary: Some servers send a certificate_required alert when the client returns no certificate while it is required. For server, it is not mandatory to send this alert, but it could make it easier for the client to distinguish bad_certificate and the declined cases. Reviewers: mt Reviewed By: mt Bug #: 1532312 Differential Revision: https://phabricator.services.mozilla.com/D22083
cpputil/tls_parser.h
gtests/ssl_gtest/ssl_auth_unittest.cc
lib/ssl/SSLerrs.h
lib/ssl/ssl3con.c
lib/ssl/ssl3prot.h
lib/ssl/sslerr.h
--- a/cpputil/tls_parser.h
+++ b/cpputil/tls_parser.h
@@ -47,16 +47,17 @@ const uint8_t kTlsAlertIllegalParameter 
 const uint8_t kTlsAlertDecodeError = 50;
 const uint8_t kTlsAlertDecryptError = 51;
 const uint8_t kTlsAlertProtocolVersion = 70;
 const uint8_t kTlsAlertInternalError = 80;
 const uint8_t kTlsAlertInappropriateFallback = 86;
 const uint8_t kTlsAlertMissingExtension = 109;
 const uint8_t kTlsAlertUnsupportedExtension = 110;
 const uint8_t kTlsAlertUnrecognizedName = 112;
+const uint8_t kTlsAlertCertificateRequired = 116;
 const uint8_t kTlsAlertNoApplicationProtocol = 120;
 
 const uint8_t kTlsFakeChangeCipherSpec[] = {
     ssl_ct_change_cipher_spec,  // Type
     0xfe,
     0xff,  // Version
     0x00,
     0x00,
--- a/gtests/ssl_gtest/ssl_auth_unittest.cc
+++ b/gtests/ssl_gtest/ssl_auth_unittest.cc
@@ -489,16 +489,19 @@ TEST_F(TlsConnectStreamTls13, PostHandsh
   auto capture_cert_req = MakeTlsFilter<TlsCertificateRequestContextRecorder>(
       server_, kTlsHandshakeCertificateRequest);
   auto capture_certificate =
       MakeTlsFilter<TlsCertificateRequestContextRecorder>(
           client_, kTlsHandshakeCertificate);
   client_->SetupClientAuth();
   EXPECT_EQ(SECSuccess, SSL_OptionSet(client_->ssl_fd(),
                                       SSL_ENABLE_POST_HANDSHAKE_AUTH, PR_TRUE));
+  EXPECT_EQ(SECSuccess,
+            SSL_OptionSet(server_->ssl_fd(), SSL_REQUIRE_CERTIFICATE,
+                          SSL_REQUIRE_ALWAYS));
   // Client to decline the certificate request.
   EXPECT_EQ(SECSuccess,
             SSL_GetClientAuthDataHook(
                 client_->ssl_fd(),
                 [](void*, PRFileDesc*, CERTDistNames*, CERTCertificate**,
                    SECKEYPrivateKey**) -> SECStatus { return SECFailure; },
                 nullptr));
   size_t called = 0;
@@ -507,20 +510,25 @@ TEST_F(TlsConnectStreamTls13, PostHandsh
         called++;
         return SECSuccess;
       });
   Connect();
   EXPECT_EQ(0U, called);
   // Send CertificateRequest.
   EXPECT_EQ(SECSuccess, SSL_SendCertificateRequest(server_->ssl_fd()))
       << "Unexpected error: " << PORT_ErrorToName(PORT_GetError());
-  server_->SendData(50);
-  client_->ReadBytes(50);
-  client_->SendData(50);
-  server_->ReadBytes(50);
+  server_->SendData(50);   // send Certificate Request
+  client_->ReadBytes(50);  // read Certificate Request
+  client_->SendData(50);   // send empty Certificate+Finished
+  server_->ExpectSendAlert(kTlsAlertCertificateRequired);
+  server_->ReadBytes(50);  // read empty Certificate+Finished
+  server_->ExpectReadWriteError();
+  server_->SendData(50);  // send alert
+  client_->ExpectReceiveAlert(kTlsAlertCertificateRequired);
+  client_->ReadBytes(50);  // read alert
   // AuthCertificateCallback is not called, because the client sends
   // an empty certificate_list.
   EXPECT_EQ(0U, called);
   EXPECT_TRUE(capture_cert_req->filtered());
   EXPECT_TRUE(capture_certificate->filtered());
   // Check if a non-empty request context is generated and it is
   // properly sent back.
   EXPECT_LT(0U, capture_cert_req->buffer().len());
--- a/lib/ssl/SSLerrs.h
+++ b/lib/ssl/SSLerrs.h
@@ -559,8 +559,11 @@ ER3(SSL_ERROR_RX_MALFORMED_ESNI_KEYS, (S
 ER3(SSL_ERROR_RX_MALFORMED_ESNI_EXTENSION, (SSL_ERROR_BASE + 177),
     "SSL received a malformed ESNI extension")
 
 ER3(SSL_ERROR_MISSING_ESNI_EXTENSION, (SSL_ERROR_BASE + 178),
     "SSL did not receive an ESNI extension")
 
 ER3(SSL_ERROR_RX_UNEXPECTED_RECORD_TYPE, (SSL_ERROR_BASE + 179),
     "SSL received an unexpected record type.")
+
+ER3(SSL_ERROR_RX_CERTIFICATE_REQUIRED_ALERT, (SSL_ERROR_BASE + 181),
+    "SSL received a certificate_required alert.")
--- a/lib/ssl/ssl3con.c
+++ b/lib/ssl/ssl3con.c
@@ -2678,17 +2678,22 @@ ssl3_HandleNoCertificate(sslSocket *ss)
      * know the server is paying attention to the certificate.
      */
     if ((ss->opt.requireCertificate == SSL_REQUIRE_ALWAYS) ||
         (!ss->firstHsDone &&
          (ss->opt.requireCertificate == SSL_REQUIRE_FIRST_HANDSHAKE))) {
         PRFileDesc *lower;
 
         ssl_UncacheSessionID(ss);
-        SSL3_SendAlert(ss, alert_fatal, bad_certificate);
+
+        if (ss->version >= SSL_LIBRARY_VERSION_TLS_1_3) {
+            SSL3_SendAlert(ss, alert_fatal, certificate_required);
+        } else {
+            SSL3_SendAlert(ss, alert_fatal, bad_certificate);
+        }
 
         lower = ss->fd->lower;
 #ifdef _WIN32
         lower->methods->shutdown(lower, PR_SHUTDOWN_SEND);
 #else
         lower->methods->shutdown(lower, PR_SHUTDOWN_BOTH);
 #endif
         PORT_SetError(SSL_ERROR_NO_CERTIFICATE);
@@ -2914,16 +2919,19 @@ ssl3_HandleAlert(sslSocket *ss, sslBuffe
             error = SSL_ERROR_DECOMPRESSION_FAILURE_ALERT;
             break;
         case handshake_failure:
             error = SSL_ERROR_HANDSHAKE_FAILURE_ALERT;
             break;
         case no_certificate:
             error = SSL_ERROR_NO_CERTIFICATE;
             break;
+        case certificate_required:
+            error = SSL_ERROR_RX_CERTIFICATE_REQUIRED_ALERT;
+            break;
         case bad_certificate:
             error = SSL_ERROR_BAD_CERT_ALERT;
             break;
         case unsupported_certificate:
             error = SSL_ERROR_UNSUPPORTED_CERT_ALERT;
             break;
         case certificate_revoked:
             error = SSL_ERROR_REVOKED_CERT_ALERT;
--- a/lib/ssl/ssl3prot.h
+++ b/lib/ssl/ssl3prot.h
@@ -69,16 +69,17 @@ typedef enum {
 
     /* Alerts for client hello extensions */
     missing_extension = 109,
     unsupported_extension = 110,
     certificate_unobtainable = 111,
     unrecognized_name = 112,
     bad_certificate_status_response = 113,
     bad_certificate_hash_value = 114,
+    certificate_required = 116,
     no_application_protocol = 120,
 
     /* invalid alert */
     no_alert = 256
 } SSL3AlertDescription;
 
 typedef PRUint8 SSL3Random[SSL3_RANDOM_LENGTH];
 
--- a/lib/ssl/sslerr.h
+++ b/lib/ssl/sslerr.h
@@ -264,15 +264,16 @@ typedef enum {
     SSL_ERROR_BAD_RESUMPTION_TOKEN_ERROR = (SSL_ERROR_BASE + 173),
     SSL_ERROR_RX_MALFORMED_DTLS_ACK = (SSL_ERROR_BASE + 174),
     SSL_ERROR_DH_KEY_TOO_LONG = (SSL_ERROR_BASE + 175),
     SSL_ERROR_RX_MALFORMED_ESNI_KEYS = (SSL_ERROR_BASE + 176),
     SSL_ERROR_RX_MALFORMED_ESNI_EXTENSION = (SSL_ERROR_BASE + 177),
     SSL_ERROR_MISSING_ESNI_EXTENSION = (SSL_ERROR_BASE + 178),
     SSL_ERROR_RX_UNEXPECTED_RECORD_TYPE = (SSL_ERROR_BASE + 179),
     SSL_ERROR_MISSING_POST_HANDSHAKE_AUTH_EXTENSION = (SSL_ERROR_BASE + 180),
+    SSL_ERROR_RX_CERTIFICATE_REQUIRED_ALERT = (SSL_ERROR_BASE + 181),
     SSL_ERROR_END_OF_LIST   /* let the c compiler determine the value of this. */
 } SSLErrorCodes;
 #endif /* NO_SECURITY_ERROR_ENUM */
 
 /* clang-format on */
 
 #endif /* __SSL_ERR_H_ */