Swiftdiff options
| author | Tim Costen <tim.costen@isode.com> | 2019-10-04 09:03:59 (GMT) |
|---|---|---|
| committer | Tim Costen <tim.costen@isode.com> | 2019-10-04 12:25:33 (GMT) |
| commit | 2ad1938c50f9fe57fe3dd98eb9f4bb711ac52acd (patch) | |
| tree | c18d0317b1f750bad3d413ed5bc6ec40a2e0bfbb /Swiften/TLS | |
| parent | df07a5e1e654c5fe4b513b8b0e41a392e9955cdf (diff) | |
| download | swift-2ad1938c50f9fe57fe3dd98eb9f4bb711ac52acd.zip swift-2ad1938c50f9fe57fe3dd98eb9f4bb711ac52acd.tar.bz2 | |
Correct leaks in OpenSSL interface
Remove increment of reference count on first certificate added
to a new SSL context - the call to SSL_CTX_use_certificate does
this internally. When adding extra certificates to the context
via calls to SSL_CTX_add_extra_certificate, the explicit
increment of the reference count is still required to prevent
destruction of the certificates when the SSL context is freed.
In OpenSSLContext::setPrivateKey, make sure the EVP_PKEY returned
by PEM_read_bio_PrivateKey is tidied up, by wrapping it in a
shared_ptr which calls EVP_PKEY_free.
Add a new Unit test which creates an SSL context and inserts a
multi-element certificate chain and a private key.
JIRA: SWIFT-423
Bug:
Release-notes:
Manual:
Change-Id: I82c66139a9dfe7a925eb39f73721200895a689e2
Test-information:
Leak testing performed via ASAN-compiled MLink unit tests -
now no leaks/errors reported associated with TLS Contexts and
Certificates. Swiften unit test runs as expected.
Diffstat (limited to 'Swiften/TLS')
| -rw-r--r-- | Swiften/TLS/OpenSSL/OpenSSLContext.cpp | 13 |
1 files changed, 6 insertions, 7 deletions
diff --git a/Swiften/TLS/OpenSSL/OpenSSLContext.cpp b/Swiften/TLS/OpenSSL/OpenSSLContext.cpp index 32d6470..d9560de 100644 --- a/Swiften/TLS/OpenSSL/OpenSSLContext.cpp +++ b/Swiften/TLS/OpenSSL/OpenSSLContext.cpp @@ -579,13 +579,11 @@ bool OpenSSLContext::setCertificateChain(const std::vector<std::shared_ptr<Certi return false; } + // This increments the reference count on the X509 certificate automatically if (SSL_CTX_use_certificate(context_.get(), openSSLCert->getInternalX509().get()) != 1) { return false; } - // Increment reference count on certificate so that it does not get freed when the SSL context is destroyed - openSSLCert->incrementReferenceCount(); - if (certificateChain.size() > 1) { for (auto certificate = certificateChain.begin() + 1; certificate != certificateChain.end(); ++certificate) { auto openSSLCert = dynamic_cast<OpenSSLCertificate*>(certificate->get()); @@ -597,7 +595,7 @@ bool OpenSSLContext::setCertificateChain(const std::vector<std::shared_ptr<Certi SWIFT_LOG(warning) << "Trying to load empty certificate chain." << std::endl; return false; } - + // Have to manually increment reference count as SSL_CTX_add_extra_chain_cert does not do so openSSLCert->incrementReferenceCount(); } } @@ -644,16 +642,17 @@ bool OpenSSLContext::setPrivateKey(const PrivateKey::ref& privateKey) { safePassword.push_back(0); password = safePassword.data(); } - auto resultKey = PEM_read_bio_PrivateKey(bio.get(), nullptr, empty_or_preset_password_cb, password); + // Make sure resultKey is tidied up by wrapping it in a shared_ptr + auto resultKey = std::shared_ptr<EVP_PKEY>(PEM_read_bio_PrivateKey(bio.get(), nullptr, empty_or_preset_password_cb, password), EVP_PKEY_free); if (resultKey) { if (handle_) { - auto result = SSL_use_PrivateKey(handle_.get(), resultKey);; + auto result = SSL_use_PrivateKey(handle_.get(), resultKey.get()); if (result != 1) { return false; } } else { - auto result = SSL_CTX_use_PrivateKey(context_.get(), resultKey); + auto result = SSL_CTX_use_PrivateKey(context_.get(), resultKey.get()); if (result != 1) { return false; } |