3 files changed, 16 insertions, 21 deletions
diff --git a/Swiften/QA/TLSTest/CertificateErrorTest.cpp b/Swiften/QA/TLSTest/CertificateErrorTest.cpp
index e69af0b..3b33e8e 100644
--- a/Swiften/QA/TLSTest/CertificateErrorTest.cpp
+++ b/Swiften/QA/TLSTest/CertificateErrorTest.cpp
@@ -1,47 +1,47 @@
 /*
- * Copyright (c) 2015 Isode Limited.
+ * Copyright (c) 2015-2016 Isode Limited.
  * All rights reserved.
  * See the COPYING file for more information.
  */
 
  /*
 	This file uses http://www.tls-o-matic.com/ to test the currently configured TLS backend for correct certificate validation behavior.
  */
 
 #include <cppunit/extensions/HelperMacros.h>
 #include <cppunit/extensions/TestFactoryRegistry.h>
 
 #include <Swiften/Base/Log.h>
 #include <Swiften/EventLoop/DummyEventLoop.h>
-#include <Swiften/IDN/PlatformIDNConverter.h>
 #include <Swiften/IDN/IDNConverter.h>
+#include <Swiften/IDN/PlatformIDNConverter.h>
 #include <Swiften/Network/BoostConnectionFactory.h>
 #include <Swiften/Network/BoostIOServiceThread.h>
 #include <Swiften/Network/HostAddressPort.h>
 #include <Swiften/Network/PlatformDomainNameResolver.h>
 #include <Swiften/Network/TLSConnection.h>
 #include <Swiften/Network/TLSConnectionFactory.h>
 #include <Swiften/TLS/CertificateVerificationError.h>
 #include <Swiften/TLS/PlatformTLSFactories.h>
 #include <Swiften/TLS/TLSContext.h>
 #include <Swiften/TLS/TLSContextFactory.h>
 
 using namespace Swift;
 
 class CertificateErrorTest : public CppUnit::TestFixture {
 		CPPUNIT_TEST_SUITE(CertificateErrorTest);
 
 		// These test require the TLS-O-Matic testing CA to be trusted. For more info see https://www.tls-o-matic.com/https/test1 .
 		CPPUNIT_TEST(testTLS_O_MaticTrusted);
 		CPPUNIT_TEST(testTLS_O_MaticCertificateFromTheFuture);
 		CPPUNIT_TEST(testTLS_O_MaticCertificateFromThePast);
 		CPPUNIT_TEST(testTLS_O_MaticCertificateFromUnknownCA);
 		CPPUNIT_TEST(testTLS_O_MaticCertificateWrongPurpose);
 
 #if !defined(HAVE_OPENSSL)
 		// Our OpenSSL backend does not support revocation. We excluded it from the revocation tests.
 		CPPUNIT_TEST(testRevokedCertificateRevocationDisabled);
 		CPPUNIT_TEST(testRevokedCertificateRevocationEnabled);
 #endif
 
 		CPPUNIT_TEST_SUITE_END();
@@ -95,118 +95,118 @@ class CertificateErrorTest : public CppUnit::TestFixture {
 		}
 
 		void connectToServer(boost::shared_ptr<TLSConnection> connection, const std::string& hostname, int port) {
 			connection->onConnectFinished.connect(boost::bind(&CertificateErrorTest::handleConnectFinished, this, _1));
 
 			HostAddress address = resolveName(hostname);
 
 			connection->connect(HostAddressPort(address, port));
 
 			while (!connectFinished_) {
 				eventLoop_->processEvents();
 			}
 		}
 
 		void testTLS_O_MaticTrusted() {
 			boost::shared_ptr<TLSConnection> connection = boost::dynamic_pointer_cast<TLSConnection>(tlsConnectionFactory_->createConnection());
 			TLSContext* context = connection->getTLSContext();
 
 			connectToServer(connection, "test1.tls-o-matic.com", 443);
 	
 			CPPUNIT_ASSERT_EQUAL(false, connectFinishedWithError_);
 			CPPUNIT_ASSERT_EQUAL(CertificateVerificationError::ref(), context->getPeerCertificateVerificationError());
 		}
 
 		void testTLS_O_MaticCertificateFromTheFuture() {
 			boost::shared_ptr<TLSConnection> connection = boost::dynamic_pointer_cast<TLSConnection>(tlsConnectionFactory_->createConnection());
 			TLSContext* context = connection->getTLSContext();
 
 			connectToServer(connection, "test5.tls-o-matic.com", 405);
 
-			CPPUNIT_ASSERT_EQUAL(true, connectFinishedWithError_);
+			CPPUNIT_ASSERT_EQUAL(false, connectFinishedWithError_);
 			CPPUNIT_ASSERT(context->getPeerCertificateVerificationError());
 			CPPUNIT_ASSERT_EQUAL(CertificateVerificationError::NotYetValid, context->getPeerCertificateVerificationError()->getType());
 		}
 
 		void testTLS_O_MaticCertificateFromThePast() {
 			boost::shared_ptr<TLSConnection> connection = boost::dynamic_pointer_cast<TLSConnection>(tlsConnectionFactory_->createConnection());
 			TLSContext* context = connection->getTLSContext();
 
 			connectToServer(connection, "test6.tls-o-matic.com", 406);
 
-			CPPUNIT_ASSERT_EQUAL(true, connectFinishedWithError_);
+			CPPUNIT_ASSERT_EQUAL(false, connectFinishedWithError_);
 			CPPUNIT_ASSERT(context->getPeerCertificateVerificationError());
 			CPPUNIT_ASSERT_EQUAL(CertificateVerificationError::Expired, context->getPeerCertificateVerificationError()->getType());
 		}
 
 		void testTLS_O_MaticCertificateFromUnknownCA() {
 			boost::shared_ptr<TLSConnection> connection = boost::dynamic_pointer_cast<TLSConnection>(tlsConnectionFactory_->createConnection());
 			TLSContext* context = connection->getTLSContext();
 
 			connectToServer(connection, "test7.tls-o-matic.com", 407);
 
-			CPPUNIT_ASSERT_EQUAL(true, connectFinishedWithError_);
+			CPPUNIT_ASSERT_EQUAL(false, connectFinishedWithError_);
 			CPPUNIT_ASSERT(context->getPeerCertificateVerificationError());
 			CPPUNIT_ASSERT_EQUAL(CertificateVerificationError::Untrusted, context->getPeerCertificateVerificationError()->getType());
 		}
 
 		// test14.tls-o-matic.com:414
 		void testTLS_O_MaticCertificateWrongPurpose() {
 			boost::shared_ptr<TLSConnection> connection = boost::dynamic_pointer_cast<TLSConnection>(tlsConnectionFactory_->createConnection());
 			TLSContext* context = connection->getTLSContext();
 
 			connectToServer(connection, "test14.tls-o-matic.com", 414);
 
-			CPPUNIT_ASSERT_EQUAL(true, connectFinishedWithError_);
+			CPPUNIT_ASSERT_EQUAL(false, connectFinishedWithError_);
 			CPPUNIT_ASSERT(context->getPeerCertificateVerificationError());
 			CPPUNIT_ASSERT_EQUAL(CertificateVerificationError::InvalidPurpose, context->getPeerCertificateVerificationError()->getType());
 		}
 
 		void testRevokedCertificateRevocationDisabled() {
 			tlsContextFactory_->setCheckCertificateRevocation(false);
 			boost::shared_ptr<TLSConnection> connection = boost::dynamic_pointer_cast<TLSConnection>(tlsConnectionFactory_->createConnection());
 			TLSContext* context = connection->getTLSContext();
 
 			connectToServer(connection, "revoked.grc.com", 443);
 
 			CPPUNIT_ASSERT_EQUAL(false, connectFinishedWithError_);
 			CPPUNIT_ASSERT(!context->getPeerCertificateVerificationError());
 		}
 
 		void testRevokedCertificateRevocationEnabled() {
 			tlsContextFactory_->setCheckCertificateRevocation(true);
 			boost::shared_ptr<TLSConnection> connection = boost::dynamic_pointer_cast<TLSConnection>(tlsConnectionFactory_->createConnection());
 			TLSContext* context = connection->getTLSContext();
 
 			connectToServer(connection, "revoked.grc.com", 443);
 
-			CPPUNIT_ASSERT_EQUAL(true, connectFinishedWithError_);
+			CPPUNIT_ASSERT_EQUAL(false, connectFinishedWithError_);
 			CPPUNIT_ASSERT(context->getPeerCertificateVerificationError());
 			CPPUNIT_ASSERT_EQUAL(CertificateVerificationError::Revoked, context->getPeerCertificateVerificationError()->getType());
 		}
 
 	private:
 		void handleAddressQueryResult(const std::vector<HostAddress>& address, boost::optional<DomainNameResolveError> /* error */) {
 			if (address.size() > 0) {
 				lastResoverResult_ = address[0];
 			}
 			resolvingDone_ = true;
 		}
 
 		void handleConnectFinished(bool error) {
 			connectFinished_ = true;
 			connectFinishedWithError_ = error;
 		}
 	
 	private:
 		BoostIOServiceThread* boostIOServiceThread_;
 		boost::shared_ptr<boost::asio::io_service> boostIOService_;
 		DummyEventLoop* eventLoop_;
 		ConnectionFactory* connectionFactory_;
 		PlatformTLSFactories* tlsFactories_;
 		TLSContextFactory* tlsContextFactory_;
 		TLSConnectionFactory* tlsConnectionFactory_;
 		
 		IDNConverter* idnConverter_;
 		DomainNameResolver* domainNameResolver_;
 		HostAddress lastResoverResult_;
 		bool resolvingDone_;
diff --git a/Swiften/TLS/SecureTransport/SecureTransportCertificate.mm b/Swiften/TLS/SecureTransport/SecureTransportCertificate.mm
index 4270a6f..ed409bd 100644
--- a/Swiften/TLS/SecureTransport/SecureTransportCertificate.mm
+++ b/Swiften/TLS/SecureTransport/SecureTransportCertificate.mm
@@ -1,71 +1,71 @@
 /*
- * Copyright (c) 2015 Isode Limited.
+ * Copyright (c) 2015-2016 Isode Limited.
  * All rights reserved.
  * See the COPYING file for more information.
  */
 
 #include <Swiften/TLS/SecureTransport/SecureTransportCertificate.h>
 
 #include <boost/numeric/conversion/cast.hpp>
 
 #include <Cocoa/Cocoa.h>
 #include <Security/Security.h>
 
 #include <Swiften/Base/Log.h>
 
 namespace {
 
 template <typename T, typename S>
 T bridge_cast(S source) {
 #pragma clang diagnostic push
 #pragma clang diagnostic ignored "-Wold-style-cast"
 	return (__bridge T)(source);
 #pragma clang diagnostic pop
 }
 
 }
 
 namespace Swift {
 
 SecureTransportCertificate::SecureTransportCertificate(SecCertificateRef certificate) {
 	assert(certificate);
 	CFRetain(certificate);
 	certificateHandle_ = boost::shared_ptr<SecCertificate>(certificate, CFRelease);
 	parse();
 }
 
 
 SecureTransportCertificate::SecureTransportCertificate(const ByteArray& der) {
-	CFDataRef derData = CFDataCreateWithBytesNoCopy(NULL, der.data(), static_cast<CFIndex>(der.size()), NULL); 
+	CFDataRef derData = CFDataCreateWithBytesNoCopy(NULL, der.data(), static_cast<CFIndex>(der.size()), NULL);
+	// certificate will take ownership of derData and free it on its release.
 	SecCertificateRef certificate = SecCertificateCreateWithData(NULL, derData);
-	CFRelease(derData);
 	if (certificate) {
 		certificateHandle_ = boost::shared_ptr<SecCertificate>(certificate, CFRelease);
 		parse();
 	}
 }
 
 SecureTransportCertificate::~SecureTransportCertificate() {
 
 }
 
 #define NS2STDSTRING(a) (a == nil ? std::string() : std::string([a cStringUsingEncoding:NSUTF8StringEncoding]))
 
 
 void SecureTransportCertificate::parse() {
 	assert(certificateHandle_);
 	CFErrorRef error = NULL;
 
 	// The SecCertificateCopyValues function is not part of the iOS Secure Transport API.
 	CFDictionaryRef valueDict = SecCertificateCopyValues(certificateHandle_.get(), 0, &error);
 	if (valueDict) {
 		// Handle subject.
 		CFStringRef subject = SecCertificateCopySubjectSummary(certificateHandle_.get());
 		if (subject) {
 			NSString* subjectStr = bridge_cast<NSString*>(subject);
 			subjectName_ = NS2STDSTRING(subjectStr);
 			CFRelease(subject);
 		}
 
 		// Handle a single Common Name.
 		CFStringRef commonName = NULL;
diff --git a/Swiften/TLS/SecureTransport/SecureTransportContext.mm b/Swiften/TLS/SecureTransport/SecureTransportContext.mm
index 2357579..ca6c5bb 100644
--- a/Swiften/TLS/SecureTransport/SecureTransportContext.mm
+++ b/Swiften/TLS/SecureTransport/SecureTransportContext.mm
@@ -1,32 +1,32 @@
 /*
- * Copyright (c) 2015 Isode Limited.
+ * Copyright (c) 2015-2016 Isode Limited.
  * All rights reserved.
  * See the COPYING file for more information.
  */
 
 #include <Swiften/TLS/SecureTransport/SecureTransportContext.h>
 
 #include <boost/type_traits.hpp>
 #include <boost/numeric/conversion/cast.hpp>
 
 #include <Swiften/Base/Algorithm.h>
 #include <Swiften/Base/Log.h>
 #include <Swiften/TLS/SecureTransport/SecureTransportCertificate.h>
 #include <Swiften/TLS/PKCS12Certificate.h>
 #include <Swiften/TLS/CertificateWithKey.h>
 
 #include <Cocoa/Cocoa.h>
 
 #import <Security/SecCertificate.h>
 #import <Security/SecImportExport.h>
 
 namespace {
 	typedef boost::remove_pointer<CFArrayRef>::type CFArray;
 	typedef boost::remove_pointer<SecTrustRef>::type SecTrust;
 }
 
 template <typename T, typename S>
 T bridge_cast(S source) {
 #pragma clang diagnostic push
 #pragma clang diagnostic ignored "-Wold-style-cast"
 	return (__bridge T)(source);
@@ -243,70 +243,65 @@ void SecureTransportContext::verifyServerCertificate() {
 					error = SecTrustGetResult(trustRef.get(), &trustResult, &certChain, &statusChain);
 					if (error == errSecSuccess) {
 						boost::shared_ptr<CFArray> certChainRef = boost::shared_ptr<CFArray>(certChain, CFRelease);
 						for (CFIndex index = 0; index < CFArrayGetCount(certChainRef.get()); index++) {
 							for (CFIndex n = 0; n < statusChain[index].NumStatusCodes; n++) {
 								// Even though Secure Transport reported CSSMERR_APPLETP_INCOMPLETE_REVOCATION_CHECK on the whole certificate
 								// chain, the actual cause can be that a revocation check for a specific cert returned CSSMERR_TP_CERT_REVOKED.
 								if (!verificationError_ || verificationError_->getType() == CertificateVerificationError::RevocationCheckFailed) {
 									verificationError_ = CSSMErrorToVerificationError(statusChain[index].StatusCodes[n]);
 								}
 							}
 						}
 					}
 					else {
 
 					}
 				}
 			}
 			else {
 				verificationError_ = boost::make_shared<CertificateVerificationError>(CertificateVerificationError::UnknownError);
 			}
 			break;
 		case kSecTrustResultOtherError:
 			verificationError_ = boost::make_shared<CertificateVerificationError>(CertificateVerificationError::UnknownError);
 			break;
 		default:
 			SWIFT_LOG(warning) << "Unhandled trust result " << trustResult << "." << std::endl;
 			break;
 	}
 	
-	if (verificationError_) {
-		setState(Error);
-		SSLClose(sslContext_.get());
-		sslContext_.reset();
-		onError(boost::make_shared<TLSError>());
-	}
-	else {
-		// proceed with handshake
-		processHandshake();
-	}
+	// We proceed with the TLS handshake here to give the application an opportunity
+	// to apply custom validation and trust management. The application is responsible
+	// to call \ref getPeerCertificateVerificationError directly after the \ref onConnected
+	// signal is called and before any application data is send to the context.
+	processHandshake();
 }
 
 #pragma clang diagnostic pop
 
 bool SecureTransportContext::setClientCertificate(CertificateWithKey::ref cert) {
 	CFArrayRef nativeClientChain = CreateClientCertificateChainAsCFArrayRef(cert);
 	if (nativeClientChain) {
 		clientCertificate_ = cert;
 		CFRelease(nativeClientChain);
 		return true;
 	}
 	else {
 		return false;
 	}
 }
 
 void SecureTransportContext::handleDataFromNetwork(const SafeByteArray& data) {
 	SWIFT_LOG(debug) << std::endl;
 	SWIFT_LOG_ASSERT(state_ == HandshakeDone || state_ == Handshake, error) << "current state '" << stateToString(state_) << " invalid." << std::endl;
 
 	append(readingBuffer_, data);
 
 	size_t bytesRead = 0;
 	OSStatus error = noErr;
 	SafeByteArray applicationData;
 
 	switch(state_) {
 		case None:
 			assert(false && "Invalid state 'None'.");
 			break;