From 27211ac2ca11c6ac259bc09bb81a7ed297a9d07d Mon Sep 17 00:00:00 2001
From: Tobias Markmann <tm@ayena.de>
Date: Mon, 8 Feb 2016 16:06:54 +0100
Subject: Treat cert verify errors as non-fatal in OS X TLS backend

Our TLS backends need to tread TLS verification errors, e.g.
outdated certificate, untrusted CA, non-matching host, etc.,
as non-fatal, so the application can apply custom key
pinning verification or similar.
This patch changes the OS X SecureTransport backend to behave
accordingly and adjusts the CertificateErrorTest to mirror
this behavior.

This commit also fixes a double-free in
SecureTransportCertificate.

Test-Information:

Connected to a host with an untrusted CA and non-matching
domain in the certificate and was prompted with the Swift
certificate trust dialog on OS X 10.11.3.

Swiften/QA/TLSTest run successfully on OS X 10.11.3.

Change-Id: I4c8ce2178540d79a5f328e2e0558d4deb4295134

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,5 +1,5 @@
 /*
- * Copyright (c) 2015 Isode Limited.
+ * Copyright (c) 2015-2016 Isode Limited.
  * All rights reserved.
  * See the COPYING file for more information.
  */
@@ -13,8 +13,8 @@
 
 #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>
@@ -122,7 +122,7 @@ class CertificateErrorTest : public CppUnit::TestFixture {
 
 			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());
 		}
@@ -133,7 +133,7 @@ class CertificateErrorTest : public CppUnit::TestFixture {
 
 			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());
 		}
@@ -144,7 +144,7 @@ class CertificateErrorTest : public CppUnit::TestFixture {
 
 			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());
 		}
@@ -156,7 +156,7 @@ class CertificateErrorTest : public CppUnit::TestFixture {
 
 			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());
 		}
@@ -179,7 +179,7 @@ class CertificateErrorTest : public CppUnit::TestFixture {
 
 			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());
 		}
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,5 +1,5 @@
 /*
- * Copyright (c) 2015 Isode Limited.
+ * Copyright (c) 2015-2016 Isode Limited.
  * All rights reserved.
  * See the COPYING file for more information.
  */
@@ -36,9 +36,9 @@ SecureTransportCertificate::SecureTransportCertificate(SecCertificateRef certifi
 
 
 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();
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,5 +1,5 @@
 /*
- * Copyright (c) 2015 Isode Limited.
+ * Copyright (c) 2015-2016 Isode Limited.
  * All rights reserved.
  * See the COPYING file for more information.
  */
@@ -270,16 +270,11 @@ void SecureTransportContext::verifyServerCertificate() {
 			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
-- 
cgit v0.10.2-6-g49f6