From be7632881677da5267eb711c1f2823ac82d43d09 Mon Sep 17 00:00:00 2001 From: Tim Costen Date: Mon, 28 Oct 2019 11:15:57 +0000 Subject: Allow use of system TAs to be disabled via TLSOptions Add new boolean flag to TLSOptions which when set to true prevents system Trust Anchors being loaded into new TLS contexts created using OpenSSL. Add new test to Swiften QA with appropriate comment. JIRA: SWIFT-425 Test-information: Checked logic of change under debugger while running the tests in CertificateTest.cpp which create TLS contexts. Change-Id: I2d4a8410ce9cc752e6774e1d1cdb84dcd37b01d7 diff --git a/Swiften/QA/TLSTest/CertificateTest.cpp b/Swiften/QA/TLSTest/CertificateTest.cpp index 624d953..463ef9e 100644 --- a/Swiften/QA/TLSTest/CertificateTest.cpp +++ b/Swiften/QA/TLSTest/CertificateTest.cpp @@ -35,6 +35,7 @@ class CertificateTest : public CppUnit::TestFixture { CPPUNIT_TEST(testGetXMPPAddresses); CPPUNIT_TEST(testCreateCertificateChain); CPPUNIT_TEST(testCreateTlsContext); + CPPUNIT_TEST(testCreateTlsContextDisableSystemTAs); CPPUNIT_TEST_SUITE_END(); public: @@ -130,6 +131,29 @@ class CertificateTest : public CppUnit::TestFixture { context->setCertificateChain(chain); context->setPrivateKey(key); } + + /** + * This test does not actually verify that use of system TAs has been disabled, it just provides + * a convenient mechanism for testing via a debugger. + **/ + void testCreateTlsContextDisableSystemTAs() { + // Create 2-certificate chain as in previous test + std::vector> chain = certificateFactory->createCertificateChain(chainData); + CPPUNIT_ASSERT_EQUAL(2,static_cast(chain.size())); + + // Load private key from string + PrivateKey::ref key = certificateFactory->createPrivateKey(Swift::createSafeByteArray(keyData)); + CPPUNIT_ASSERT(key); + + // Turn off use of system TAs + TLSOptions options; + options.ignoreSystemTrustAnchors = true; + auto context = tlsContextFactory_->createTLSContext(options, TLSContext::Mode::Server); + CPPUNIT_ASSERT(context); + + context->setCertificateChain(chain); + context->setPrivateKey(key); + } private: std::unique_ptr pathProvider; ByteArray certificateData; diff --git a/Swiften/TLS/OpenSSL/OpenSSLContext.cpp b/Swiften/TLS/OpenSSL/OpenSSLContext.cpp index 490a361..b7cf178 100644 --- a/Swiften/TLS/OpenSSL/OpenSSLContext.cpp +++ b/Swiften/TLS/OpenSSL/OpenSSLContext.cpp @@ -121,52 +121,57 @@ OpenSSLContext::OpenSSLContext(const TLSOptions& options, Mode mode) : mode_(mod // TODO: implement OCSP support // TODO: handle OCSP stapling see https://www.rfc-editor.org/rfc/rfc4366.txt - // Load system certs + + // Default for ignoreSystemTrustAnchors is false, i.e. load System TAs by default, + // to preserve previous behaviour + if (!options.ignoreSystemTrustAnchors) { + // Load system certs #if defined(SWIFTEN_PLATFORM_WINDOWS) - X509_STORE* store = SSL_CTX_get_cert_store(context_.get()); - HCERTSTORE systemStore = CertOpenSystemStore(0, "ROOT"); - if (systemStore) { - PCCERT_CONTEXT certContext = nullptr; - while (true) { - certContext = CertFindCertificateInStore(systemStore, X509_ASN_ENCODING | PKCS_7_ASN_ENCODING, 0, CERT_FIND_ANY, nullptr, certContext); - if (!certContext) { - break; - } - OpenSSLCertificate cert(createByteArray(certContext->pbCertEncoded, certContext->cbCertEncoded)); - if (store && cert.getInternalX509()) { - X509_STORE_add_cert(store, cert.getInternalX509().get()); + X509_STORE* store = SSL_CTX_get_cert_store(context_.get()); + HCERTSTORE systemStore = CertOpenSystemStore(0, "ROOT"); + if (systemStore) { + PCCERT_CONTEXT certContext = nullptr; + while (true) { + certContext = CertFindCertificateInStore(systemStore, X509_ASN_ENCODING | PKCS_7_ASN_ENCODING, 0, CERT_FIND_ANY, nullptr, certContext); + if (!certContext) { + break; + } + OpenSSLCertificate cert(createByteArray(certContext->pbCertEncoded, certContext->cbCertEncoded)); + if (store && cert.getInternalX509()) { + X509_STORE_add_cert(store, cert.getInternalX509().get()); + } } } - } #elif !defined(SWIFTEN_PLATFORM_MACOSX) - SSL_CTX_set_default_verify_paths(context_.get()); + SSL_CTX_set_default_verify_paths(context_.get()); #elif defined(SWIFTEN_PLATFORM_MACOSX) && !defined(SWIFTEN_PLATFORM_IPHONE) - // On Mac OS X 10.5 (OpenSSL < 0.9.8), OpenSSL does not automatically look in the system store. - // On Mac OS X 10.6 (OpenSSL >= 0.9.8), OpenSSL *does* look in the system store to determine trust. - // However, if there is a certificate error, it will always emit the "Invalid CA" error if we didn't add - // the certificates first. See - // http://opensource.apple.com/source/OpenSSL098/OpenSSL098-27/src/crypto/x509/x509_vfy_apple.c - // to understand why. We therefore add all certs from the system store ourselves. - X509_STORE* store = SSL_CTX_get_cert_store(context_.get()); - CFArrayRef anchorCertificates; - if (SecTrustCopyAnchorCertificates(&anchorCertificates) == 0) { - for (int i = 0; i < CFArrayGetCount(anchorCertificates); ++i) { - SecCertificateRef cert = reinterpret_cast(const_cast(CFArrayGetValueAtIndex(anchorCertificates, i))); - CSSM_DATA certCSSMData; - if (SecCertificateGetData(cert, &certCSSMData) != 0 || certCSSMData.Length == 0) { - continue; - } - std::vector certData; - certData.resize(certCSSMData.Length); - memcpy(&certData[0], certCSSMData.Data, certCSSMData.Length); - OpenSSLCertificate certificate(certData); - if (store && certificate.getInternalX509()) { - X509_STORE_add_cert(store, certificate.getInternalX509().get()); + // On Mac OS X 10.5 (OpenSSL < 0.9.8), OpenSSL does not automatically look in the system store. + // On Mac OS X 10.6 (OpenSSL >= 0.9.8), OpenSSL *does* look in the system store to determine trust. + // However, if there is a certificate error, it will always emit the "Invalid CA" error if we didn't add + // the certificates first. See + // http://opensource.apple.com/source/OpenSSL098/OpenSSL098-27/src/crypto/x509/x509_vfy_apple.c + // to understand why. We therefore add all certs from the system store ourselves. + X509_STORE* store = SSL_CTX_get_cert_store(context_.get()); + CFArrayRef anchorCertificates; + if (SecTrustCopyAnchorCertificates(&anchorCertificates) == 0) { + for (int i = 0; i < CFArrayGetCount(anchorCertificates); ++i) { + SecCertificateRef cert = reinterpret_cast(const_cast(CFArrayGetValueAtIndex(anchorCertificates, i))); + CSSM_DATA certCSSMData; + if (SecCertificateGetData(cert, &certCSSMData) != 0 || certCSSMData.Length == 0) { + continue; + } + std::vector certData; + certData.resize(certCSSMData.Length); + memcpy(&certData[0], certCSSMData.Data, certCSSMData.Length); + OpenSSLCertificate certificate(certData); + if (store && certificate.getInternalX509()) { + X509_STORE_add_cert(store, certificate.getInternalX509().get()); + } } + CFRelease(anchorCertificates); } - CFRelease(anchorCertificates); - } #endif + } configure(options); } diff --git a/Swiften/TLS/TLSOptions.h b/Swiften/TLS/TLSOptions.h index 4109096..e3faaf9 100644 --- a/Swiften/TLS/TLSOptions.h +++ b/Swiften/TLS/TLSOptions.h @@ -68,5 +68,10 @@ namespace Swift { * Allows specification of application-specific Trust Anchors */ boost::optional>> trustAnchors; + + /** + * Turns off automatic loading of system Trust Anchors + */ + bool ignoreSystemTrustAnchors = false; }; } -- cgit v0.10.2-6-g49f6