Skip to content

CORE-15655 Forward port ossl changes to dev / 26.2.x-pre #273

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
pgellert merged 6 commits into from
Apr 24, 2026
Merged

CORE-15655 Forward port ossl changes to dev / 26.2.x-pre #273

Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
0cf0b04
ossl: Share BIO ptrs across shards
StephanDollberg Apr 8, 2026
eaabc39
ossl: use make_ossl_error for BIO creation failure
pgellert Apr 15, 2026
040bfc6
ossl: drain error queue on unexpected error codes
pgellert Apr 15, 2026
072f332
ossl: clear error queue after successful SSL_CTX_new
pgellert Apr 15, 2026
bd8e58d
ossl: clear error queue after successful SSL operations
pgellert Apr 15, 2026
4cd69c9
ossl: assert clean error queue before SSL operations
pgellert Apr 15, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
61 changes: 53 additions & 8 deletions src/net/ossl.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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<ossl_errc> & 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<unsigned long>(code)) == lib &&
Expand Down Expand Up @@ -1023,7 +1027,7 @@ class session : public enable_shared_from_this<session>, 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");
Expand Down Expand Up @@ -1151,7 +1155,7 @@ class session : public enable_shared_from_this<session>, 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;
Expand Down Expand Up @@ -1191,6 +1195,7 @@ class session : public enable_shared_from_this<session>, 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) {
Expand All @@ -1201,6 +1206,7 @@ class session : public enable_shared_from_this<session>, 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;
Expand Down Expand Up @@ -1282,6 +1288,7 @@ class session : public enable_shared_from_this<session>, 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) {
Expand Down Expand Up @@ -1326,11 +1333,12 @@ class session : public enable_shared_from_this<session>, 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();
Expand Down Expand Up @@ -1393,6 +1401,7 @@ class session : public enable_shared_from_this<session>, 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);
Expand Down Expand Up @@ -1451,11 +1460,12 @@ class session : public enable_shared_from_this<session>, public session_impl {
return make_exception_future<buf_type>(_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<buf_type>(_error);
}
} else {
clear_stale_ssl_errors("SSL_read_ex");
buf.trim(bytes_read);
return make_ready_future<buf_type>(std::move(buf));
}
Expand Down Expand Up @@ -1488,11 +1498,14 @@ class session : public enable_shared_from_this<session>, 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);
Expand Down Expand Up @@ -1536,7 +1549,7 @@ class session : public enable_shared_from_this<session>, 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));
}
Expand Down Expand Up @@ -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<subject_alt_name> do_get_alt_name_information(const x509_ptr &peer_cert,
const std::unordered_set<subject_alt_name_type> &types) const {
int ext_idx = X509_get_ext_by_NID(
Expand Down Expand Up @@ -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");
Copy link

Copilot AI Apr 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

make_ssl_context() calls clear_stale_ssl_errors("SSL_CTX_new") during member initialization (before _type is initialized). clear_stale_ssl_errors() logs *this, and the session formatter calls get_type_string() which reads _type, so this can access an uninitialized member (UB) and potentially crash/print garbage. Consider clearing the error queue without referencing *this here (log without session context), or ensure _type is initialized before _ctx (e.g., by reordering member declarations / splitting context creation after _type is set).

Suggested change
clear_stale_ssl_errors("SSL_CTX_new");
//
// Do not call clear_stale_ssl_errors() here: this function runs during
// member initialization, before the session object is fully
// initialized, and that helper logs *this.
ERR_clear_error();

Copilot uses AI. Check for mistakes.
const auto& ck_pair = _creds->get_certkey_pair();
if (type == session_type::SERVER) {
if (!ck_pair) {
Expand Down Expand Up @@ -2430,9 +2475,9 @@ bio_method_ptr create_bio_method() {
} // namespace tls

BIO_METHOD* get_method() {
static thread_local bio_method_ptr method_ptr = [] {
return tls::create_bio_method();
}();
// shared across shards (no actual state) to avoid running into 127 BIO
// index limit in openssl on larger shard count machines
const static bio_method_ptr method_ptr = tls::create_bio_method();

return method_ptr.get();
}
Expand Down
Loading