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
@@ -20,8 +20,8 @@ Certificate* OpenSSLCertificateFactory::createCertificateFromDER(const ByteArray
20 return new OpenSSLCertificate(der); 20 return new OpenSSLCertificate(der);
21} 21}
22 22
23std::vector<Certificate::ref> OpenSSLCertificateFactory::createCertificateChain(const ByteArray& data) { 23std::vector<std::unique_ptr<Certificate>> OpenSSLCertificateFactory::createCertificateChain(const ByteArray& data) {
24 std::vector<Certificate::ref> certificateChain; 24 std::vector<std::unique_ptr<Certificate>> certificateChain;
25 25
26 if (data.size() > std::numeric_limits<int>::max()) { 26 if (data.size() > std::numeric_limits<int>::max()) {
27 return certificateChain; 27 return certificateChain;
@@ -35,11 +35,11 @@ std::vector<Certificate::ref> OpenSSLCertificateFactory::createCertificateChain(
35 auto x509certFromPEM = PEM_read_bio_X509(bio.get(), &openSSLCert, nullptr, nullptr); 35 auto x509certFromPEM = PEM_read_bio_X509(bio.get(), &openSSLCert, nullptr, nullptr);
36 if (x509certFromPEM && openSSLCert) { 36 if (x509certFromPEM && openSSLCert) {
37 std::shared_ptr<X509> x509Cert(openSSLCert, X509_free); 37 std::shared_ptr<X509> x509Cert(openSSLCert, X509_free);
38 certificateChain.push_back(std::make_shared<OpenSSLCertificate>(x509Cert)); 38 certificateChain.emplace_back(std::make_unique<OpenSSLCertificate>(x509Cert));
39 openSSLCert = nullptr; 39 openSSLCert = nullptr;
40 while ((x509certFromPEM = PEM_read_bio_X509(bio.get(), &openSSLCert, nullptr, nullptr)) != nullptr) { 40 while ((x509certFromPEM = PEM_read_bio_X509(bio.get(), &openSSLCert, nullptr, nullptr)) != nullptr) {
41 std::shared_ptr<X509> x509Cert(openSSLCert, X509_free); 41 std::shared_ptr<X509> x509Cert(openSSLCert, X509_free);
42 certificateChain.push_back(std::make_shared<OpenSSLCertificate>(x509Cert)); 42 certificateChain.emplace_back(std::make_unique<OpenSSLCertificate>(x509Cert));
43 openSSLCert = nullptr; 43 openSSLCert = nullptr;
44 } 44 }
45 } 45 }
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
@@ -16,6 +16,6 @@ namespace Swift {
16 virtual ~OpenSSLCertificateFactory() override final; 16 virtual ~OpenSSLCertificateFactory() override final;
17 17
18 virtual Certificate* createCertificateFromDER(const ByteArray& der) override final; 18 virtual Certificate* createCertificateFromDER(const ByteArray& der) override final;
19 virtual std::vector<Certificate::ref> createCertificateChain(const ByteArray& data) override final; 19 virtual std::vector<std::unique_ptr<Certificate>> createCertificateChain(const ByteArray& data) override final;
20 }; 20 };
21} 21}
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
@@ -338,14 +338,14 @@ void OpenSSLContext::sendPendingDataToApplication() {
338 } 338 }
339} 339}
340 340
341bool OpenSSLContext::setCertificateChain(const std::vector<Certificate::ref>& certificateChain) { 341bool OpenSSLContext::setCertificateChain(std::vector<std::unique_ptr<Certificate>>&& certificateChain) {
342 if (certificateChain.size() == 0) { 342 if (certificateChain.size() == 0) {
343 SWIFT_LOG(warning) << "Trying to load empty certificate chain." << std::endl; 343 SWIFT_LOG(warning) << "Trying to load empty certificate chain." << std::endl;
344 return false; 344 return false;
345 } 345 }
346 346
347 // load endpoint certificate 347 // load endpoint certificate
348 auto openSSLCert = std::dynamic_pointer_cast<OpenSSLCertificate>(certificateChain[0]); 348 auto openSSLCert = dynamic_cast<OpenSSLCertificate*>(certificateChain[0].get());
349 if (!openSSLCert) { 349 if (!openSSLCert) {
350 return false; 350 return false;
351 } 351 }
@@ -355,8 +355,8 @@ bool OpenSSLContext::setCertificateChain(const std::vector<Certificate::ref>& ce
355 } 355 }
356 356
357 if (certificateChain.size() > 1) { 357 if (certificateChain.size() > 1) {
358 for (auto certificate : range(certificateChain.begin() + 1, certificateChain.end())) { 358 for (auto certificate = certificateChain.begin() + 1; certificate != certificateChain.end(); ++certificate) {
359 auto openSSLCert = std::dynamic_pointer_cast<OpenSSLCertificate>(certificate); 359 auto openSSLCert = dynamic_cast<OpenSSLCertificate*>(certificate->get());
360 if (!openSSLCert) { 360 if (!openSSLCert) {
361 return false; 361 return false;
362 } 362 }
@@ -364,6 +364,7 @@ bool OpenSSLContext::setCertificateChain(const std::vector<Certificate::ref>& ce
364 SWIFT_LOG(warning) << "Trying to load empty certificate chain." << std::endl; 364 SWIFT_LOG(warning) << "Trying to load empty certificate chain." << std::endl;
365 return false; 365 return false;
366 } 366 }
367 certificate->release();
367 } 368 }
368 } 369 }
369 370
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
@@ -45,7 +45,7 @@ namespace Swift {
45 void connect() override final; 45 void connect() override final;
46 void connect(const std::string& requestHostname) override final; 46 void connect(const std::string& requestHostname) override final;
47 47
48 bool setCertificateChain(const std::vector<Certificate::ref>& certificateChain) override final; 48 bool setCertificateChain(std::vector<std::unique_ptr<Certificate>>&& certificateChain) override final;
49 bool setPrivateKey(const PrivateKey::ref& privateKey) override final; 49 bool setPrivateKey(const PrivateKey::ref& privateKey) override final;
50 bool setClientCertificate(CertificateWithKey::ref cert) override final; 50 bool setClientCertificate(CertificateWithKey::ref cert) override final;
51 void setAbortTLSHandshake(bool abort) override final; 51 void setAbortTLSHandshake(bool abort) override final;