diff --git a/src/net/ossl.cc b/src/net/ossl.cc index e293dca217e..c8e241938e7 100644 --- a/src/net/ossl.cc +++ b/src/net/ossl.cc @@ -183,6 +183,10 @@ std::system_error make_ossl_error(const std::string & msg) { return make_ossl_error(msg, get_all_ossl_errors()); } +std::runtime_error make_unknown_ossl_error(const std::string & msg) { + return std::runtime_error(fmt::format("{}: {}", msg, get_all_ossl_errors())); +} + bool contains_ossl_error(const std::vector & error_codes, int lib, int reason) { return std::any_of(error_codes.cbegin(), error_codes.cend(), [lib, reason](const ossl_errc & code) { return ERR_GET_LIB(static_cast(code)) == lib && @@ -1023,7 +1027,7 @@ class session : public enable_shared_from_this, public session_impl { bio_ptr in_bio(BIO_new(get_method())); bio_ptr out_bio(BIO_new(get_method())); if (!in_bio || !out_bio) { - throw std::runtime_error("Failed to create BIOs"); + throw make_ossl_error("Failed to create BIOs"); } if (1 != BIO_ctrl(in_bio.get(), BIO_C_SET_POINTER, 0, this)) { throw make_ossl_error("Failed to set bio ptr to in bio"); @@ -1151,7 +1155,7 @@ class session : public enable_shared_from_this, public session_impl { default: { // Some other unhandled situation - auto err = std::runtime_error( + auto err = make_unknown_ossl_error( "Unknown error encountered during SSL write"); return handle_output_error(std::move(err)).then([] { return stop_iteration::yes; @@ -1191,6 +1195,7 @@ class session : public enable_shared_from_this, public session_impl { // This do_until runs until either a renegotiation occurs or the packet is empty while (!eof() && size > 0) { size_t bytes_written = 0; + verify_clean_error_queue("SSL_write_ex"); auto write_rc = SSL_write_ex(_ssl.get(), ptr, size, &bytes_written); tls_log.trace("{} do_put: SSL_write_ex: {}", *this, write_rc); if (write_rc != 1) { @@ -1201,6 +1206,7 @@ class session : public enable_shared_from_this, public session_impl { co_return; } } else { + clear_stale_ssl_errors("SSL_write_ex"); SEASTAR_ASSERT(bytes_written <= size); tls_log.trace("{} do_put: bytes_written: {}", *this, bytes_written); ptr += bytes_written; @@ -1282,6 +1288,7 @@ class session : public enable_shared_from_this, public session_impl { [this] { return connected() || eof(); }, [this] { try { + verify_clean_error_queue("SSL_do_handshake"); auto n = SSL_do_handshake(_ssl.get()); tls_log.trace("{} do_handshake: SSL_do_handshake: {}", *this, n); if (n <= 0) { @@ -1326,11 +1333,12 @@ class session : public enable_shared_from_this, public session_impl { return handle_output_error(std::move(err)); } default: - auto err = std::runtime_error( + auto err = make_unknown_ossl_error( "Unknown error encountered during handshake"); return handle_output_error(std::move(err)); } } else { + clear_stale_ssl_errors("SSL_do_handshake"); if (_type == session_type::CLIENT || _creds->get_client_auth() != client_auth::NONE) { verify(); @@ -1393,6 +1401,7 @@ class session : public enable_shared_from_this, public session_impl { tls_log.trace("{} do_get: available: {}", *this, avail); buf_type buf(avail); size_t bytes_read = 0; + verify_clean_error_queue("SSL_read_ex"); auto read_result = SSL_read_ex( _ssl.get(), buf.get_write(), avail, &bytes_read); tls_log.trace("{} do_get: SSL_read_ex: {}", *this, read_result); @@ -1451,11 +1460,12 @@ class session : public enable_shared_from_this, public session_impl { return make_exception_future(_error); } default: - _error = std::make_exception_ptr(std::runtime_error( + _error = std::make_exception_ptr(make_unknown_ossl_error( "Unexpected error condition during SSL read")); return make_exception_future(_error); } } else { + clear_stale_ssl_errors("SSL_read_ex"); buf.trim(bytes_read); return make_ready_future(std::move(buf)); } @@ -1488,11 +1498,14 @@ class session : public enable_shared_from_this, public session_impl { return make_ready_future(); } + verify_clean_error_queue("SSL_shutdown"); auto res = SSL_shutdown(_ssl.get()); tls_log.trace("{} do_shutdown: SSL_shutdown: {}", *this, res); if (res == 1) { + clear_stale_ssl_errors("SSL_shutdown"); return wait_for_output(); } else if (res == 0) { + clear_stale_ssl_errors("SSL_shutdown"); return yield().then([this] { return do_shutdown(); }); } else { auto ssl_err = SSL_get_error(_ssl.get(), res); @@ -1536,7 +1549,7 @@ class session : public enable_shared_from_this, public session_impl { } default: { - auto err = std::runtime_error( + auto err = make_unknown_ossl_error( "Unknown error occurred during SSL shutdown"); return handle_output_error(std::move(err)); } @@ -1821,6 +1834,33 @@ SEASTAR_INTERNAL_END_IGNORE_DEPRECATIONS } private: + // Some SSL operations return success while leaving stale errors on the + // queue (e.g. from internal BIO write failures that OpenSSL absorbed). + // Drain them so they don't poison the next operation on this shard. + void clear_stale_ssl_errors(const char* operation) { + if (ERR_peek_error() == 0) [[likely]] { + return; + } + auto errors = get_all_ossl_errors(); + tls_log.debug("{} {}: ignoring stale errors on queue: {}", *this, operation, errors); + } + + // Checks that the OpenSSL per-thread error queue is clean before + // calling an SSL function. A dirty queue can cause SSL_get_error + // to misclassify results (e.g. reporting SSL_ERROR_SYSCALL instead + // of SSL_ERROR_SSL), which can affect unrelated sessions that share + // the same thread. + void verify_clean_error_queue(const char* operation) { + auto err = ERR_peek_error(); + if (err == 0) [[likely]] { + return; + } + char buf[256]; + ERR_error_string_n(err, buf, sizeof(buf)); + tls_log.warn("{} stale error on queue before {}: {}", *this, operation, buf); + SEASTAR_ASSERT(0 && "stale errors on OpenSSL error queue"); + } + std::vector do_get_alt_name_information(const x509_ptr &peer_cert, const std::unordered_set &types) const { int ext_idx = X509_get_ext_by_NID( @@ -1989,6 +2029,11 @@ SEASTAR_INTERNAL_END_IGNORE_DEPRECATIONS throw make_ossl_error( "Failed to initialize SSL context"); } + // SSL_CTX_new can return a valid context while leaving errors on the + // error queue from partially-failed system config parsing (e.g. an + // invalid Ciphersuites value in the system openssl.cnf). + // See https://github.com/openssl/openssl/issues/30760 + clear_stale_ssl_errors("SSL_CTX_new"); const auto& ck_pair = _creds->get_certkey_pair(); if (type == session_type::SERVER) { if (!ck_pair) {