Add support for use of shared certificate chain when setting up TLS context

Actual implementation is in OpenSSL subclass. This allows a permanent vector of shared certificates to be used when creating multiple OpenSSL contexts. This replaces the existing use of a vector of unique pointers to certificates which handed over responsibility for the underlying OpenSSL certs to the OpenSSL context. To enable this to work, a new method is added to the OpenSSLCertificate class which enables the reference count on the the contained OpenSSL certificate to be incremented - this stops the OpenSSL certificate being deleted when the OpenSSL context is freed. Use of conditional compilation was necessary to get the reference counting to build with the different versions of OpenSSL in use. Modify the method in OpenSSLCertificateFactory (and stub in CertificateFactory) which generates a vector of certificates, so that it generates a vector of shared_ptrs rather than unique_ptrs. Add test of CreateCertificateChain to Swiften CertificateTest class, together with sample certificate file in PEM form. JIRA: LINK-1763 Bug: Release-notes: Manual: Test-information: Tested via development version of Mystique - created multiple TLS sessions using single certificate chain. Swift unit tests now build and run again. New Swiften TLS unit test builds and runs. Change-Id: I7fa4888b640c94b68712a6bff1f7aa334a358df2
author: Tim Costen <tim.costen@isode.com> 2019-09-06 10:32:12 (GMT)
committer: Tim Costen <tim.costen@isode.com> 2019-09-19 15:27:01 (GMT)
commit: e58cf7d5d7d3bab330bccf6a098dd476fbf4dc86 (patch)
tree: f3632c379e2d92022bdb8af5d980b44883cc2360 /Swiften/TLS/OpenSSL
parent: 8051f94932b6932a2e3eb60a26c758fbfed6d6ad (diff)
download: swift-e58cf7d5d7d3bab330bccf6a098dd476fbf4dc86.zip
swift-e58cf7d5d7d3bab330bccf6a098dd476fbf4dc86.tar.bz2
6 files changed, 23 insertions, 8 deletions
diff --git a/Swiften/TLS/OpenSSL/OpenSSLCertificate.cpp b/Swiften/TLS/OpenSSL/OpenSSLCertificate.cpp
index 8d2d965..bb51428 100644
--- a/Swiften/TLS/OpenSSL/OpenSSLCertificate.cpp
+++ b/Swiften/TLS/OpenSSL/OpenSSLCertificate.cpp
@@ -37,6 +37,14 @@ OpenSSLCertificate::OpenSSLCertificate(const ByteArray& der) {
    parse();
 }
+void OpenSSLCertificate::incrementReferenceCount() const {
+#if OPENSSL_VERSION_NUMBER >= 0x10100000L
+    X509_up_ref(cert.get());
+#else
+    CRYPTO_add(&(cert.get()->references), 1, CRYPTO_LOCK_EVP_PKEY);
+#endif
+}
 ByteArray OpenSSLCertificate::toDER() const {
    ByteArray result;
    if (!cert) {
diff --git a/Swiften/TLS/OpenSSL/OpenSSLCertificate.h b/Swiften/TLS/OpenSSL/OpenSSLCertificate.h
index 186caea..64da82a 100644
--- a/Swiften/TLS/OpenSSL/OpenSSLCertificate.h
+++ b/Swiften/TLS/OpenSSL/OpenSSLCertificate.h
@@ -45,6 +45,8 @@ namespace Swift {
                return cert;
            }
+            void incrementReferenceCount() const;
        private:
            void parse();
diff --git a/Swiften/TLS/OpenSSL/OpenSSLCertificateFactory.cpp b/Swiften/TLS/OpenSSL/OpenSSLCertificateFactory.cpp
index 5eb626b..fd94ec8 100644
--- a/Swiften/TLS/OpenSSL/OpenSSLCertificateFactory.cpp
+++ b/Swiften/TLS/OpenSSL/OpenSSLCertificateFactory.cpp
@@ -20,8 +20,8 @@ Certificate* OpenSSLCertificateFactory::createCertificateFromDER(const ByteArray
    return new OpenSSLCertificate(der);
 }
-std::vector<std::unique_ptr<Certificate>> OpenSSLCertificateFactory::createCertificateChain(const ByteArray& data) {
+std::vector<std::shared_ptr<Certificate>> OpenSSLCertificateFactory::createCertificateChain(const ByteArray& data) {
-    std::vector<std::unique_ptr<Certificate>> certificateChain;
+    std::vector<std::shared_ptr<Certificate>> certificateChain;
    if (data.size() > std::numeric_limits<int>::max()) {
        return certificateChain;
@@ -35,11 +35,11 @@ std::vector<std::unique_ptr<Certificate>> OpenSSLCertificateFactory::createCerti
    auto x509certFromPEM = PEM_read_bio_X509(bio.get(), &openSSLCert, nullptr, nullptr);
    if (x509certFromPEM && openSSLCert) {
        std::shared_ptr<X509> x509Cert(openSSLCert, X509_free);
-        certificateChain.emplace_back(std::make_unique<OpenSSLCertificate>(x509Cert));
+        certificateChain.emplace_back(std::make_shared<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.emplace_back(std::make_unique<OpenSSLCertificate>(x509Cert));
+            certificateChain.emplace_back(std::make_shared<OpenSSLCertificate>(x509Cert));
            openSSLCert = nullptr;
        }
    }
diff --git a/Swiften/TLS/OpenSSL/OpenSSLCertificateFactory.h b/Swiften/TLS/OpenSSL/OpenSSLCertificateFactory.h
index 48e9b2c..a6974c8 100644
--- a/Swiften/TLS/OpenSSL/OpenSSLCertificateFactory.h
+++ b/Swiften/TLS/OpenSSL/OpenSSLCertificateFactory.h
@@ -16,6 +16,6 @@ namespace Swift {
            virtual ~OpenSSLCertificateFactory() override final;
            virtual Certificate* createCertificateFromDER(const ByteArray& der) override final;
-            virtual std::vector<std::unique_ptr<Certificate>> createCertificateChain(const ByteArray& data) override final;
+            virtual std::vector<std::shared_ptr<Certificate>> createCertificateChain(const ByteArray& data) override final;
    };
 }
diff --git a/Swiften/TLS/OpenSSL/OpenSSLContext.cpp b/Swiften/TLS/OpenSSL/OpenSSLContext.cpp
index 5c80976..32d6470 100644
--- a/Swiften/TLS/OpenSSL/OpenSSLContext.cpp
+++ b/Swiften/TLS/OpenSSL/OpenSSLContext.cpp
@@ -567,7 +567,7 @@ void OpenSSLContext::sendPendingDataToApplication() {
    }
 }
-bool OpenSSLContext::setCertificateChain(std::vector<std::unique_ptr<Certificate>>&& certificateChain) {
+bool OpenSSLContext::setCertificateChain(const std::vector<std::shared_ptr<Certificate>>& certificateChain) {
    if (certificateChain.size() == 0) {
        SWIFT_LOG(warning) << "Trying to load empty certificate chain." << std::endl;
        return false;
@@ -583,17 +583,22 @@ bool OpenSSLContext::setCertificateChain(std::vector<std::unique_ptr<Certificate
        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());
            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();
+            openSSLCert->incrementReferenceCount();
        }
    }
diff --git a/Swiften/TLS/OpenSSL/OpenSSLContext.h b/Swiften/TLS/OpenSSL/OpenSSLContext.h
index 885b1fe..8eb5758 100644
--- a/Swiften/TLS/OpenSSL/OpenSSLContext.h
+++ b/Swiften/TLS/OpenSSL/OpenSSLContext.h
@@ -46,7 +46,7 @@ namespace Swift {
            void connect() override final;
            void connect(const std::string& requestHostname) override final;
-            bool setCertificateChain(std::vector<std::unique_ptr<Certificate>>&& certificateChain) override final;
+            bool setCertificateChain(const std::vector<std::shared_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;
author	Tim Costen <tim.costen@isode.com>	2019-09-06 10:32:12 (GMT)
committer	Tim Costen <tim.costen@isode.com>	2019-09-19 15:27:01 (GMT)
commit	e58cf7d5d7d3bab330bccf6a098dd476fbf4dc86 (patch)
tree	f3632c379e2d92022bdb8af5d980b44883cc2360 /Swiften/TLS/OpenSSL
parent	8051f94932b6932a2e3eb60a26c758fbfed6d6ad (diff)
download	swift-e58cf7d5d7d3bab330bccf6a098dd476fbf4dc86.zip swift-e58cf7d5d7d3bab330bccf6a098dd476fbf4dc86.tar.bz2

diff --git a/Swiften/TLS/OpenSSL/OpenSSLCertificate.cpp b/Swiften/TLS/OpenSSL/OpenSSLCertificate.cpp index 8d2d965..bb51428 100644 --- a/Swiften/TLS/OpenSSL/OpenSSLCertificate.cpp +++ b/Swiften/TLS/OpenSSL/OpenSSLCertificate.cpp
@@ -37,6 +37,14 @@ OpenSSLCertificate::OpenSSLCertificate(const ByteArray& der) {
37	parse();	37	parse();
38	}	38	}
39		39
		40	void OpenSSLCertificate::incrementReferenceCount() const {
		41	#if OPENSSL_VERSION_NUMBER >= 0x10100000L
		42	X509_up_ref(cert.get());
		43	#else
		44	CRYPTO_add(&(cert.get()->references), 1, CRYPTO_LOCK_EVP_PKEY);
		45	#endif
		46	}
		47
40	ByteArray OpenSSLCertificate::toDER() const {	48	ByteArray OpenSSLCertificate::toDER() const {
41	ByteArray result;	49	ByteArray result;
42	if (!cert) {	50	if (!cert) {


diff --git a/Swiften/TLS/OpenSSL/OpenSSLCertificate.h b/Swiften/TLS/OpenSSL/OpenSSLCertificate.h index 186caea..64da82a 100644 --- a/Swiften/TLS/OpenSSL/OpenSSLCertificate.h +++ b/Swiften/TLS/OpenSSL/OpenSSLCertificate.h
@@ -45,6 +45,8 @@ namespace Swift {
45	return cert;	45	return cert;
46	}	46	}
47		47
		48	void incrementReferenceCount() const;
		49
48	private:	50	private:
49	void parse();	51	void parse();
50		52


diff --git a/Swiften/TLS/OpenSSL/OpenSSLCertificateFactory.cpp b/Swiften/TLS/OpenSSL/OpenSSLCertificateFactory.cpp index 5eb626b..fd94ec8 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
23	std::vector<std::unique_ptr<Certificate>> OpenSSLCertificateFactory::createCertificateChain(const ByteArray& data) {	23	std::vector<std::shared_ptr<Certificate>> OpenSSLCertificateFactory::createCertificateChain(const ByteArray& data) {
24	std::vector<std::unique_ptr<Certificate>> certificateChain;	24	std::vector<std::shared_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<std::unique_ptr<Certificate>> OpenSSLCertificateFactory::createCerti
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.emplace_back(std::make_unique<OpenSSLCertificate>(x509Cert));	38	certificateChain.emplace_back(std::make_shared<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.emplace_back(std::make_unique<OpenSSLCertificate>(x509Cert));	42	certificateChain.emplace_back(std::make_shared<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 48e9b2c..a6974c8 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<std::unique_ptr<Certificate>> createCertificateChain(const ByteArray& data) override final;	19	virtual std::vector<std::shared_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 5c80976..32d6470 100644 --- a/Swiften/TLS/OpenSSL/OpenSSLContext.cpp +++ b/Swiften/TLS/OpenSSL/OpenSSLContext.cpp
@@ -567,7 +567,7 @@ void OpenSSLContext::sendPendingDataToApplication() {
567	}	567	}
568	}	568	}
569		569
570	bool OpenSSLContext::setCertificateChain(std::vector<std::unique_ptr<Certificate>>&& certificateChain) {	570	bool OpenSSLContext::setCertificateChain(const std::vector<std::shared_ptr<Certificate>>& certificateChain) {
571	if (certificateChain.size() == 0) {	571	if (certificateChain.size() == 0) {
572	SWIFT_LOG(warning) << "Trying to load empty certificate chain." << std::endl;	572	SWIFT_LOG(warning) << "Trying to load empty certificate chain." << std::endl;
573	return false;	573	return false;
@@ -583,17 +583,22 @@ bool OpenSSLContext::setCertificateChain(std::vector<std::unique_ptr<Certificate
583	return false;	583	return false;
584	}	584	}
585		585
		586	// Increment reference count on certificate so that it does not get freed when the SSL context is destroyed
		587	openSSLCert->incrementReferenceCount();
		588
586	if (certificateChain.size() > 1) {	589	if (certificateChain.size() > 1) {
587	for (auto certificate = certificateChain.begin() + 1; certificate != certificateChain.end(); ++certificate) {	590	for (auto certificate = certificateChain.begin() + 1; certificate != certificateChain.end(); ++certificate) {
588	auto openSSLCert = dynamic_cast<OpenSSLCertificate*>(certificate->get());	591	auto openSSLCert = dynamic_cast<OpenSSLCertificate*>(certificate->get());
589	if (!openSSLCert) {	592	if (!openSSLCert) {
590	return false;	593	return false;
591	}	594	}
		595
592	if (SSL_CTX_add_extra_chain_cert(context_.get(), openSSLCert->getInternalX509().get()) != 1) {	596	if (SSL_CTX_add_extra_chain_cert(context_.get(), openSSLCert->getInternalX509().get()) != 1) {
593	SWIFT_LOG(warning) << "Trying to load empty certificate chain." << std::endl;	597	SWIFT_LOG(warning) << "Trying to load empty certificate chain." << std::endl;
594	return false;	598	return false;
595	}	599	}
596	certificate->release();	600
		601	openSSLCert->incrementReferenceCount();
597	}	602	}
598	}	603	}
599		604


diff --git a/Swiften/TLS/OpenSSL/OpenSSLContext.h b/Swiften/TLS/OpenSSL/OpenSSLContext.h index 885b1fe..8eb5758 100644 --- a/Swiften/TLS/OpenSSL/OpenSSLContext.h +++ b/Swiften/TLS/OpenSSL/OpenSSLContext.h
@@ -46,7 +46,7 @@ namespace Swift {
46	void connect() override final;	46	void connect() override final;
47	void connect(const std::string& requestHostname) override final;	47	void connect(const std::string& requestHostname) override final;
48		48
49	bool setCertificateChain(std::vector<std::unique_ptr<Certificate>>&& certificateChain) override final;	49	bool setCertificateChain(const std::vector<std::shared_ptr<Certificate>>& certificateChain) override final;
50	bool setPrivateKey(const PrivateKey::ref& privateKey) override final;	50	bool setPrivateKey(const PrivateKey::ref& privateKey) override final;
51	bool setClientCertificate(CertificateWithKey::ref cert) override final;	51	bool setClientCertificate(CertificateWithKey::ref cert) override final;
52	void setAbortTLSHandshake(bool abort) override final;	52	void setAbortTLSHandshake(bool abort) override final;