Skip to content

quic: fix per-session SSL leak #66730

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

Open
pimterry wants to merge 1 commit into
base: main
Choose a base branch
from
+11 −9
Open

quic: fix per-session SSL leak #66730

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
17 changes: 9 additions & 8 deletions src/quic/tlscontext.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -188,10 +188,10 @@ OSSLContext::~OSSLContext() {

void OSSLContext::reset() {
if (ctx_) {
// The SSL object inside the ngtcp2 ctx may not have been set if
// SSL creation failed. Guard against null before clearing app data.
if (SSL* ssl = *this; ssl != nullptr) {
SSL_set_app_data(ssl, nullptr);
if (ssl_) {
SSL_set_app_data(ssl_.get(), nullptr);
ngtcp2_crypto_ossl_ctx_set_ssl(ctx_, nullptr);
ssl_.reset();
}
// connection_ is set during Initialize(). If Initialize() was
// never called (e.g. SSL creation failed), it's still nullptr.
Expand All @@ -205,19 +205,20 @@ void OSSLContext::reset() {
}

OSSLContext::operator SSL*() const {
return ngtcp2_crypto_ossl_ctx_get_ssl(ctx_);
return ssl_.get();
}

OSSLContext::operator ngtcp2_crypto_ossl_ctx*() const {
return ctx_;
}

void OSSLContext::Initialize(SSL* ssl,
void OSSLContext::Initialize(SSLPointer ssl,
ngtcp2_crypto_conn_ref* ref,
ngtcp2_conn* connection,
SSL_CTX* ssl_ctx) {
CHECK(ssl);
ngtcp2_crypto_ossl_ctx_set_ssl(ctx_, ssl);
ssl_ = std::move(ssl);
ngtcp2_crypto_ossl_ctx_set_ssl(ctx_, ssl_.get());
SSL_set_app_data(*this, ref);
// TODO(@jasnell): Later when BoringSSL is also supported, the native
// handle will be different. The ngtcp2_crypto_ossl.h impl requires
Expand Down Expand Up @@ -894,7 +895,7 @@ void TLSSession::Initialize(
EnableTrace(session_->env(), &bio_trace_, ssl);
}

ossl_context_.Initialize(ssl.release(), &ref_, session(), ctx);
ossl_context_.Initialize(std::move(ssl), &ref_, session(), ctx);

switch (ctx.side()) {
case Side::SERVER: {
Expand Down
3 changes: 2 additions & 1 deletion src/quic/tlscontext.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ class OSSLContext final {
operator SSL*() const;
operator ngtcp2_crypto_ossl_ctx*() const;

void Initialize(SSL* ssl,
void Initialize(ncrypto::SSLPointer ssl,
ngtcp2_crypto_conn_ref* ref,
ngtcp2_conn* connection,
SSL_CTX* ssl_ctx);
Expand Down Expand Up @@ -73,6 +73,7 @@ class OSSLContext final {
private:
ngtcp2_crypto_ossl_ctx* ctx_;
ngtcp2_conn* connection_ = nullptr;
ncrypto::SSLPointer ssl_;
};

// Every QUIC Session has exactly one TLSSession that maintains the state
Expand Down
Loading