TLS: Fix EVP_PKEY leak in SSLPrivateKeyHandler - #13464
Merged
Merged
Conversation
SSL_CTX_use_PrivateKey() takes its own reference on the key, so the reference from PEM_read_bio_PrivateKey() belongs to the caller. SSLPrivateKeyHandler() released it only when attaching the key failed, so a successful load leaked one EVP_PKEY per certificate. This repeats at startup, on every config reload, and on every secret or certificate update. Hold the key in a scoped_PKEY so every exit releases it.
Contributor
There was a problem hiding this comment.
Pull request overview
Fixes an EVP_PKEY reference leak in the TLS private key loading path by ensuring the key returned from PEM_read_bio_PrivateKey() (or engine load) is always released, even on successful SSL_CTX_use_PrivateKey().
Changes:
- Wrap the loaded private key in an RAII handle (
scoped_PKEY) so all exit paths release the caller-owned reference. - Add an
EVP_PKEYstd::unique_ptrdeleter + alias inP_SSLUtils.hand updateSSLPrivateKeyHandler()to use.reset()/.get()accordingly.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| src/iocore/net/SSLUtils.cc | Converts SSLPrivateKeyHandler() to manage the loaded EVP_PKEY via RAII and pass the raw pointer to OpenSSL when attaching the key. |
| src/iocore/net/P_SSLUtils.h | Introduces PKEYDeleter and scoped_PKEY to standardize EVP_PKEY lifetime management alongside existing scoped OpenSSL handles. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
SSL_CTX_use_PrivateKey()takes its own reference on the key, so the reference fromPEM_read_bio_PrivateKey()belongs to the caller.SSLPrivateKeyHandler()released it only when attaching the key failed, so a successful load leaked oneEVP_PKEYper certificate. This repeats at startup, on every config reload, and on every secret or certificate update.Hold the key in a
scoped_PKEYso every exit releases it.