Repository: randombit/botan
Pull request #3754 (2023-10-13 08:11:07)

FIX: TLS 1.3 server hit assertion if application did not provide a cert chain
### Description

Server applications are expected to provide a certificate chain via `Credentials_Manager::find_cert_chain()`. This callback defines constraints that the certificate must fulfill. Applications may respond with an empty chain, if the requested certificate constraints are not met. In this case, the TLS 1.3 server implementation did not abort the handshake but tried replying with an empty Certificate message. This, in turn, resulted in the triggering of an assertion when trying to create a CertificateVerify message accompanying the (empty) Certificate message.

### Impact

As some of the certificate constraints can be defined by the peer, this bug may potentially be triggered remotely. In the default build configuration, botan-assertions result in an exception. The TLS server would then report an `internal_error` alert, which should be okay in the described situation. However, if the build configuration enabled `--terminate-on-assert` this would be a potential DoS vector.

### Mitigation

Applications can avoid this issue by throwing an exception in their implementation of `Credentials_Manager::find_cert_chain()` instead of returning an empty certificate chain in the described situation. For instance: `Botan::TLS::TLS_Exception(Botan::TLS::Alert::HandshakeFailure, "<some message>")`.


Patch:
diff --git a/src/lib/tls/tls13/msg_certificate_13.cpp b/src/lib/tls/tls13/msg_certificate_13.cpp
index 5ab6bc246..dbdb5c58b 100644
--- a/src/lib/tls/tls13/msg_certificate_13.cpp
+++ b/src/lib/tls/tls13/msg_certificate_13.cpp
@@ -174,14 +174,20 @@ Certificate_13::Certificate_13(const Client_Hello_13& client_hello,
       m_request_context(), m_side(Connection_Side::Server) {
    BOTAN_ASSERT_NOMSG(client_hello.extensions().has<Signature_Algorithms>());
 
-   setup_entries(
+   auto cert_chain =
       credentials_manager.find_cert_chain(filter_signature_schemes(client_hello.signature_schemes()),
                                           to_algorithm_identifiers(client_hello.certificate_signature_schemes()),
                                           {},
                                           "tls-server",
-                                          client_hello.sni_hostname()),
-      client_hello.extensions().get<Certificate_Status_Request>(),
-      callbacks);
+                                          client_hello.sni_hostname());
+
+   // RFC 8446 4.4.2
+   //    The server's certificate_list MUST always be non-empty.
+   if(cert_chain.empty()) {
+      throw TLS_Exception(Alert::HandshakeFailure, "No sufficient server certificate available");
+   }
+
+   setup_entries(std::move(cert_chain), client_hello.extensions().get<Certificate_Status_Request>(), callbacks);
 }
 
 /**


Test patch:
diff --git a/src/scripts/test_cli.py b/src/scripts/test_cli.py
index a3a3ba85a..43dee6636 100755
--- a/src/scripts/test_cli.py
+++ b/src/scripts/test_cli.py
@@ -964,10 +964,17 @@ def cli_tls_socket_tests(tmp_dir):
             self.protocol_version = protocol_version
             self.policy = policy
             self.stdout_regex = kwargs.get("stdout_regex")
+            self.expect_error = kwargs.get("expect_error", False)
             self.psk = kwargs.get("psk")
             self.psk_identity = kwargs.get("psk_identity")
 
     configs = [
+        # Regression test: TLS 1.3 server hit an assertion when no certificate
+        #                  chain was found. Here, we provoke this by requiring
+        #                  an RSA-based certificate (server uses ECDSA).
+        TestConfig("No server cert", "1.3", "allow_tls12=false\nallow_tls13=true\nsignature_methods=RSA\n",
+                   stdout_regex='Alert: handshake_failure', expect_error=True),
+
         TestConfig("TLS 1.3", "1.3", "allow_tls12=false\nallow_tls13=true\n"),
         TestConfig("TLS 1.2", "1.2", "allow_tls12=true\nallow_tls13=false\n"),
 
@@ -1031,14 +1038,17 @@ def cli_tls_socket_tests(tmp_dir):
 
         (stdout, stderr) = tls_client.communicate()
 
-        if stderr:
-            logging.error("Got unexpected stderr output (%s) %s", tls_config.name, stderr)
+        if not tls_config.expect_error:
+            if stderr:
+                logging.error("Got unexpected stderr output (%s) %s", tls_config.name, stderr)
 
-        if b'Handshake complete' not in stdout:
-            logging.error('Failed to complete handshake (%s): %s', tls_config.name, stdout)
+            if b'Handshake complete' not in stdout:
+                logging.error('Failed to complete handshake (%s): %s', tls_config.name, stdout)
 
-        if client_msg not in stdout:
-            logging.error("Missing client message from stdout (%s): %s", tls_config.name, stdout)
+            if client_msg not in stdout:
+                logging.error("Missing client message from stdout (%s): %s", tls_config.name, stdout)
+        elif tls_client.returncode == 0:
+            logging.error('Expected an error, but tls_client finished with success')
 
         if tls_config.stdout_regex:
             match = re.search(tls_config.stdout_regex, stdout.decode('utf-8'))
@@ -1046,10 +1056,13 @@ def cli_tls_socket_tests(tmp_dir):
                 logging.error("Client log did not match expected regex (%s): %s", tls_config.name, tls_config.stdout_regex)
                 logging.error("Client said (stdout): %s", stdout)
 
-    (srv_stdout, srv_stderr) = tls_server.communicate(None, 5)
-    if srv_stderr:
-        logging.error("server said (stdout): %s", srv_stdout)
-        logging.error("server said (stderr): %s", srv_stderr)
+    try:
+        (srv_stdout, srv_stderr) = tls_server.communicate(None, 5)
+    except subprocess.TimeoutExpired:
+        tls_server.kill()
+        tls_server.communicate()
+    logging.debug("server said (stdout): %s", srv_stdout)
+    logging.debug("server said (stderr): %s", srv_stderr)
 
 def cli_tls_online_pqc_hybrid_tests(tmp_dir):
     if not run_socket_tests() or not run_online_tests() or not check_for_command("tls_client"):