net/tls: name the peer as the source of received TLS alerts (CORE-12666)

src/net/tls_openssl.cc

@@ -41,6 +41,7 @@
 #include <cassert>
 #include <chrono>
 #include <functional>
+#include <ranges>
 #include <span>
 #include <system_error>
 #include <unordered_set>
@@ -187,6 +188,21 @@ bool contains_openssl_error(const std::vector<openssl_errc> & error_codes, int l
     });
 }
 
+// OpenSSL encodes a TLS alert as an ERR_LIB_SSL reason of SSL_AD_REASON_OFFSET
+// plus the one-byte alert description. ERR_GET_REASON() keeps the ERR_RFLAG_*
+// bits, so flagged reasons exceed the offset without being alerts.
+static std::optional<uint8_t> tls_alert_of_reason(openssl_errc error) {
+    const auto code = static_cast<unsigned long>(error);
+    if (ERR_GET_LIB(code) != ERR_LIB_SSL) {
+        return std::nullopt;
+    }
+    const auto alert_description = ERR_GET_REASON(code) - SSL_AD_REASON_OFFSET;
+    if (!std::in_range<uint8_t>(alert_description)) {
+        return std::nullopt;
+    }
+    return static_cast<uint8_t>(alert_description);
+}
+
 template<typename T>
 sstring asn1_str_to_str(T* asn1) {
     const auto len = ASN1_STRING_length(asn1);
@@ -843,6 +859,21 @@ class openssl_session : public enable_shared_from_this<openssl_session>, public
         if (1 != SSL_set_ex_data(_ssl.get(), SSL_EX_DATA_SESSION, this)) {
             throw make_openssl_error("Failed to set EX data for SSL session");
         }
+        // Record alerts read from the peer, so a failure caused by a received
+        // alert can be attributed to the peer when it is reported. The reason
+        // code alone cannot make that call: the state machine also raises
+        // alert-range reasons for alerts the local side is about to send.
+        SSL_set_info_callback(_ssl.get(), [](const SSL* ssl, int where, int ret) {
+            if ((where & SSL_CB_READ_ALERT) != SSL_CB_READ_ALERT) {
+                return;
+            }
+            auto* session = static_cast<openssl_session*>(
+                SSL_get_ex_data(ssl, SSL_EX_DATA_SESSION));
+            SEASTAR_ASSERT(session != nullptr);
+            // The description is ret's low byte, the part
+            // SSL_alert_desc_string_long() reads; the high byte is the level.
+            session->_alert_read = static_cast<uint8_t>(ret);
+        });
         bio_ptr in_bio(BIO_new(get_method()));
         bio_ptr out_bio(BIO_new(get_method()));
         if (!in_bio || !out_bio) {
@@ -1107,6 +1138,10 @@ class openssl_session : public enable_shared_from_this<openssl_session>, public
             tls_log.trace("{} do_handshake: already connected", *this);
             return make_ready_future<>();
         }
+        // Scope the recorded alert to this handshake attempt: a renegotiation
+        // clears SSL_is_init_finished(), so get() can drive a second one.
+        // WANT_READ and WANT_WRITE retries stay inside the loop below.
+        _alert_read.reset();
         return do_until(
             [this] { return connected() || eof(); },
             [this] {
@@ -1160,7 +1195,7 @@ class openssl_session : public enable_shared_from_this<openssl_session>, public
                                 }
                                 // Verify did not throw, fall through and make a generic error
                             }
-                            auto err = make_openssl_error("Failed to establish SSL handshake", std::move(error_codes));
+                            auto err = make_handshake_error(std::move(error_codes));
                             return handle_output_error(std::move(err));
                         }
                         default:
@@ -1379,6 +1414,28 @@ class openssl_session : public enable_shared_from_this<openssl_session>, public
         }
     }
 
+    // Like make_openssl_error(), but labels a code carrying an alert read from
+    // the peer as such: OpenSSL words a received alert as though the local side
+    // found the fault.
+    std::system_error make_handshake_error(std::vector<openssl_errc> error_codes) {
+        static const std::string msg = "Failed to establish SSL handshake";
+        const auto is_peer_alert = [this](openssl_errc code) {
+            return _alert_read.has_value() && tls_alert_of_reason(code) == _alert_read;
+        };
+        if (std::ranges::none_of(error_codes, is_peer_alert)) {
+            return make_openssl_error(msg, std::move(error_codes));
+        }
+        auto rendered = error_codes | std::views::transform([&](openssl_errc code) {
+            return is_peer_alert(code)
+                ? fmt::format("Received TLS alert from peer: {}", code)
+                : fmt::format("{}", code);
+        });
+        return std::system_error(
+            static_cast<int>(error_codes.front()),
+            openssl_error_cat(),
+            fmt::format("{}: [{}]", msg, fmt::join(rendered, ", ")));
+    }
+
     void verify() {
         tls_log.trace("{} verify", *this);
         if (!_creds->_enable_certificate_verification) {
@@ -2163,6 +2220,9 @@ class openssl_session : public enable_shared_from_this<openssl_session>, public
     session_type _type;
     bool _eof = false;
     bool _shutdown = false;
+    // Description of the last TLS alert read from the peer, recorded by the
+    // info callback registered in the constructor.
+    std::optional<uint8_t> _alert_read;
 
     friend int bio_write_ex(BIO* b, const char * data, size_t dlen, size_t * written);
     friend int bio_read_ex(BIO* b, char * data, size_t dlen, size_t *readbytes);

tests/unit/tls_test.cc

@@ -2627,3 +2627,143 @@ SEASTAR_THREAD_TEST_CASE(test_output_pending_exception_on_destroy) {
     uint64_t after = engine().abandoned_failed_futures();
     BOOST_REQUIRE_EQUAL(after, before);
 }
+
+static void require_contains(const sstring& text, std::string_view expected) {
+    BOOST_REQUIRE_MESSAGE(text.find(expected) != sstring::npos,
+        "expected to find \"" << expected << "\" in \"" << text << "\"");
+}
+
+static void require_not_contains(const sstring& text, std::string_view unexpected) {
+    BOOST_REQUIRE_MESSAGE(text.find(unexpected) == sstring::npos,
+        "expected not to find \"" << unexpected << "\" in \"" << text << "\"");
+}
+
+// Runs a handshake expected to fail over a loopback socket pair and returns the
+// server-side exception message. The client's own failure is ignored.
+static sstring failed_handshake_server_error(
+        ::shared_ptr<tls::server_credentials> server_creds,
+        ::shared_ptr<tls::certificate_credentials> client_creds) {
+    auto b1 = ::make_lw_shared<loopback_buffer>(nullptr, loopback_buffer::type::SERVER_TX);
+    auto b2 = ::make_lw_shared<loopback_buffer>(nullptr, loopback_buffer::type::CLIENT_TX);
+
+    auto ssi = std::make_unique<loopback_connected_socket_impl>(b1, b2);
+    auto csi = std::make_unique<loopback_connected_socket_impl>(b2, b1);
+
+    auto ss = tls::wrap_server(server_creds, connected_socket(std::move(ssi))).get();
+    auto cs = tls::wrap_client(client_creds, connected_socket(std::move(csi)),
+                               tls::tls_options{.server_name = "test.scylladb.org"}).get();
+
+    auto strms = ::make_lw_shared<streams>(std::move(cs));
+    auto client_loop = strms->out.write(message)
+        .then([strms] { return strms->out.flush(); })
+        .then([strms] { return strms->in.read().discard_result(); })
+        .handle_exception([](std::exception_ptr) {});
+
+    sstring server_error;
+    try {
+        ss.input().read().get();
+        BOOST_FAIL("Expected the server side of the handshake to fail");
+    } catch (const std::exception& e) {
+        server_error = e.what();
+    }
+    client_loop.get();
+
+    BOOST_TEST_MESSAGE(fmt::format("server side error: {}", server_error).c_str());
+    return server_error;
+}
+
+// A client cert signed by a CA the server does not trust must be reported with
+// the offending certificate's DN, so an operator can tell which client
+// misbehaved. OpenSSL backend only; GnuTLS words these errors differently.
+SEASTAR_THREAD_TEST_CASE(test_x509_server_rejects_client_cert_from_unknown_ca) {
+    if (using_gnutls()) {
+        return;
+    }
+
+    tls::credentials_builder cb;
+    // other.crt is signed by caother, which the server does not trust.
+    cb.set_x509_key_file(certfile("other.crt"), certfile("other.key"), tls::x509_crt_format::PEM).get();
+    cb.set_x509_trust_file(certfile("catest.pem"), tls::x509_crt_format::PEM).get();
+    auto client_creds = cb.build_certificate_credentials();
+
+    tls::credentials_builder sb;
+    sb.set_x509_key_file(certfile("test.crt"), certfile("test.key"), tls::x509_crt_format::PEM).get();
+    sb.set_x509_trust_file(certfile("catest.pem"), tls::x509_crt_format::PEM).get();
+    sb.set_client_auth(tls::client_auth::REQUIRE);
+    sb.set_dh_level();
+    auto server_creds = sb.build_server_credentials();
+
+    auto err = failed_handshake_server_error(server_creds, client_creds);
+
+    require_contains(err, "Issuer");
+    require_contains(err, "Subject");
+    require_contains(err, "other.apa.org");
+}
+
+// A client that rejects the server's certificate chain aborts with an
+// unknown_ca(48) alert, before sending a certificate of its own. The server-side
+// error must name that received alert - not local certificate state such as
+// "no certificate presented by peer", which is a consequence of the abort.
+// OpenSSL backend only; GnuTLS words these errors differently.
+SEASTAR_THREAD_TEST_CASE(test_server_handshake_error_preserves_ssl_error_detail) {
+    if (using_gnutls()) {
+        return;
+    }
+
+    tls::credentials_builder cb;
+    // A trust store that does not contain the server's CA.
+    cb.set_x509_trust_file(certfile("tls-ca-bundle.pem"), tls::x509_crt_format::PEM).get();
+    auto client_creds = cb.build_certificate_credentials();
+
+    tls::credentials_builder sb;
+    sb.set_x509_key_file(certfile("test.crt"), certfile("test.key"), tls::x509_crt_format::PEM).get();
+    sb.set_x509_trust_file(certfile("catest.pem"), tls::x509_crt_format::PEM).get();
+    sb.set_client_auth(tls::client_auth::REQUIRE);
+    sb.set_dh_level();
+    auto server_creds = sb.build_server_credentials();
+
+    auto err = failed_handshake_server_error(server_creds, client_creds);
+
+    require_contains(err, "Received TLS alert from peer");
+    require_contains(err, "unknown ca");
+    require_not_contains(err, "no certificate presented by peer");
+}
+
+// A failure the server diagnoses itself, with no alert arriving from the peer,
+// must not be reported as a received alert.
+// OpenSSL backend only; GnuTLS words these errors differently.
+SEASTAR_THREAD_TEST_CASE(test_handshake_error_without_alert_is_not_from_peer) {
+    if (using_gnutls()) {
+        return;
+    }
+
+    tls::credentials_builder sb;
+    sb.set_x509_key_file(certfile("test.crt"), certfile("test.key"), tls::x509_crt_format::PEM).get();
+    auto server_creds = sb.build_server_credentials();
+
+    auto b1 = ::make_lw_shared<loopback_buffer>(nullptr, loopback_buffer::type::SERVER_TX);
+    auto b2 = ::make_lw_shared<loopback_buffer>(nullptr, loopback_buffer::type::CLIENT_TX);
+    auto ssi = std::make_unique<loopback_connected_socket_impl>(b1, b2);
+    auto csi = std::make_unique<loopback_connected_socket_impl>(b2, b1);
+
+    auto ss = tls::wrap_server(server_creds, connected_socket(std::move(ssi))).get();
+
+    // Plain HTTP to a TLS server: the server diagnoses the failure itself.
+    ::connected_socket cs(std::move(csi));
+    output_stream<char> out(cs.output().detach(), 1024);
+    out.write(sstring("GET / HTTP/1.1\r\n\r\n")).get();
+    out.flush().get();
+
+    sstring err;
+    try {
+        ss.input().read().get();
+        BOOST_FAIL("Expected the server side of the handshake to fail");
+    } catch (const std::exception& e) {
+        err = e.what();
+    }
+    BOOST_TEST_MESSAGE(fmt::format("server side error: {}", err).c_str());
+
+    require_contains(err, "Failed to establish SSL handshake");
+    require_contains(err, "Received HTTP request on HTTPS server");
+    require_not_contains(err, "Received TLS alert from peer");
+}