Swift
summaryrefslogtreecommitdiffstats
path: root/Swiften/TLS/OpenSSL
diff options
context:
space:
mode:
authorEdwin Mons <edwin.mons@isode.com>2019-01-18 15:25:58 (GMT)
committerEdwin Mons <edwin.mons@isode.com>2019-01-18 20:27:03 (GMT)
commit68dd665d51c925a118cfced4583942b7157b59de (patch)
treefc4144d4a3284fdd68c34b8d3bf6c0d107998a6b /Swiften/TLS/OpenSSL
parent9b12c9751cf8fd1658dfd948c4d854b0e1407b0d (diff)
downloadswift-68dd665d51c925a118cfced4583942b7157b59de.zip
swift-68dd665d51c925a118cfced4583942b7157b59de.tar.bz2
Allow ownership transfer of certificates
OpenSSL TLS contexts assume ownership of any additional certificate passed into it. The CertificateFactory now returns a vector of unique_ptrs, and OpenSSLContext will do the needful with releasing ownership at the right moment. A unit test has been added that uses a chained certificate in client/server context. Before the fix, this test would either fail, or result in a segmentation fault, depending on the mood of OpenSSL. Test-Information: Unit tests pass on Debian 9 Ran manual tests with server test code, tested both chained and single certificates, and no longer observed crashes when accepting a connection. Change-Id: I21814969e45c7d77e9a1af14f2c958c4c0311cd0
Diffstat (limited to 'Swiften/TLS/OpenSSL')
-rw-r--r--Swiften/TLS/OpenSSL/OpenSSLCertificateFactory.cpp8
-rw-r--r--Swiften/TLS/OpenSSL/OpenSSLCertificateFactory.h2
-rw-r--r--Swiften/TLS/OpenSSL/OpenSSLContext.cpp9
-rw-r--r--Swiften/TLS/OpenSSL/OpenSSLContext.h2
4 files changed, 11 insertions, 10 deletions
diff --git a/Swiften/TLS/OpenSSL/OpenSSLCertificateFactory.cpp b/Swiften/TLS/OpenSSL/OpenSSLCertificateFactory.cpp
index c94702c..5eb626b 100644
--- a/Swiften/TLS/OpenSSL/OpenSSLCertificateFactory.cpp
+++ b/Swiften/TLS/OpenSSL/OpenSSLCertificateFactory.cpp
@@ -17,14 +17,14 @@ OpenSSLCertificateFactory::~OpenSSLCertificateFactory() {
}
Certificate* OpenSSLCertificateFactory::createCertificateFromDER(const ByteArray& der) {
return new OpenSSLCertificate(der);
}
-std::vector<Certificate::ref> OpenSSLCertificateFactory::createCertificateChain(const ByteArray& data) {
- std::vector<Certificate::ref> certificateChain;
+std::vector<std::unique_ptr<Certificate>> OpenSSLCertificateFactory::createCertificateChain(const ByteArray& data) {
+ std::vector<std::unique_ptr<Certificate>> certificateChain;
if (data.size() > std::numeric_limits<int>::max()) {
return certificateChain;
}
auto bio = std::shared_ptr<BIO>(BIO_new(BIO_s_mem()), BIO_free);
@@ -32,17 +32,17 @@ std::vector<Certificate::ref> OpenSSLCertificateFactory::createCertificateChain(
// Attempt parsing data as PEM
X509* openSSLCert = nullptr;
auto x509certFromPEM = PEM_read_bio_X509(bio.get(), &openSSLCert, nullptr, nullptr);
if (x509certFromPEM && openSSLCert) {
std::shared_ptr<X509> x509Cert(openSSLCert, X509_free);
- certificateChain.push_back(std::make_shared<OpenSSLCertificate>(x509Cert));
+ certificateChain.emplace_back(std::make_unique<OpenSSLCertificate>(x509Cert));
openSSLCert = nullptr;
while ((x509certFromPEM = PEM_read_bio_X509(bio.get(), &openSSLCert, nullptr, nullptr)) != nullptr) {
std::shared_ptr<X509> x509Cert(openSSLCert, X509_free);
- certificateChain.push_back(std::make_shared<OpenSSLCertificate>(x509Cert));
+ certificateChain.emplace_back(std::make_unique<OpenSSLCertificate>(x509Cert));
openSSLCert = nullptr;
}
}
return certificateChain;
}
diff --git a/Swiften/TLS/OpenSSL/OpenSSLCertificateFactory.h b/Swiften/TLS/OpenSSL/OpenSSLCertificateFactory.h
index af45a33..48e9b2c 100644
--- a/Swiften/TLS/OpenSSL/OpenSSLCertificateFactory.h
+++ b/Swiften/TLS/OpenSSL/OpenSSLCertificateFactory.h
@@ -13,9 +13,9 @@ namespace Swift {
class OpenSSLCertificateFactory : public CertificateFactory {
public:
OpenSSLCertificateFactory();
virtual ~OpenSSLCertificateFactory() override final;
virtual Certificate* createCertificateFromDER(const ByteArray& der) override final;
- virtual std::vector<Certificate::ref> createCertificateChain(const ByteArray& data) override final;
+ virtual std::vector<std::unique_ptr<Certificate>> createCertificateChain(const ByteArray& data) override final;
};
}
diff --git a/Swiften/TLS/OpenSSL/OpenSSLContext.cpp b/Swiften/TLS/OpenSSL/OpenSSLContext.cpp
index 968ef8f..e9889bc 100644
--- a/Swiften/TLS/OpenSSL/OpenSSLContext.cpp
+++ b/Swiften/TLS/OpenSSL/OpenSSLContext.cpp
@@ -335,38 +335,39 @@ void OpenSSLContext::sendPendingDataToApplication() {
if (ret < 0 && SSL_get_error(handle_.get(), ret) != SSL_ERROR_WANT_READ) {
state_ = State::Error;
onError(std::make_shared<TLSError>(TLSError::UnknownError, openSSLInternalErrorToString()));
}
}
-bool OpenSSLContext::setCertificateChain(const std::vector<Certificate::ref>& certificateChain) {
+bool OpenSSLContext::setCertificateChain(std::vector<std::unique_ptr<Certificate>>&& certificateChain) {
if (certificateChain.size() == 0) {
SWIFT_LOG(warning) << "Trying to load empty certificate chain." << std::endl;
return false;
}
// load endpoint certificate
- auto openSSLCert = std::dynamic_pointer_cast<OpenSSLCertificate>(certificateChain[0]);
+ auto openSSLCert = dynamic_cast<OpenSSLCertificate*>(certificateChain[0].get());
if (!openSSLCert) {
return false;
}
if (SSL_CTX_use_certificate(context_.get(), openSSLCert->getInternalX509().get()) != 1) {
return false;
}
if (certificateChain.size() > 1) {
- for (auto certificate : range(certificateChain.begin() + 1, certificateChain.end())) {
- auto openSSLCert = std::dynamic_pointer_cast<OpenSSLCertificate>(certificate);
+ for (auto certificate = certificateChain.begin() + 1; certificate != certificateChain.end(); ++certificate) {
+ auto openSSLCert = dynamic_cast<OpenSSLCertificate*>(certificate->get());
if (!openSSLCert) {
return false;
}
if (SSL_CTX_add_extra_chain_cert(context_.get(), openSSLCert->getInternalX509().get()) != 1) {
SWIFT_LOG(warning) << "Trying to load empty certificate chain." << std::endl;
return false;
}
+ certificate->release();
}
}
if (handle_) {
// This workaround is needed as OpenSSL has a shortcut to not do anything
// if you set the SSL_CTX to the existing SSL_CTX and not reloading the
diff --git a/Swiften/TLS/OpenSSL/OpenSSLContext.h b/Swiften/TLS/OpenSSL/OpenSSLContext.h
index cfa852a..c18a6f4 100644
--- a/Swiften/TLS/OpenSSL/OpenSSLContext.h
+++ b/Swiften/TLS/OpenSSL/OpenSSLContext.h
@@ -42,13 +42,13 @@ namespace Swift {
virtual ~OpenSSLContext() override final;
void accept() override final;
void connect() override final;
void connect(const std::string& requestHostname) override final;
- bool setCertificateChain(const std::vector<Certificate::ref>& certificateChain) override final;
+ bool setCertificateChain(std::vector<std::unique_ptr<Certificate>>&& certificateChain) override final;
bool setPrivateKey(const PrivateKey::ref& privateKey) override final;
bool setClientCertificate(CertificateWithKey::ref cert) override final;
void setAbortTLSHandshake(bool abort) override final;
bool setDiffieHellmanParameters(const ByteArray& parametersInOpenSslDer) override final;
void handleDataFromNetwork(const SafeByteArray&) override final;