Merge pull request #257 from jacadcaps/defaultCertificateVerifier

Improved certificate verification.

4 files changed, 118 insertions, 0 deletions

diff --git a/src/vmime/net/tls/openssl/OpenSSLInitializer.cpp b/src/vmime/net/tls/openssl/OpenSSLInitializer.cpp
index c7b1013d..49b34b54 100644
--- a/src/vmime/net/tls/openssl/OpenSSLInitializer.cpp
+++ b/src/vmime/net/tls/openssl/OpenSSLInitializer.cpp
@@ -115,6 +115,12 @@ void OpenSSLInitializer::initialize() {
 	OPENSSL_config(NULL);
 #endif
 
+#if OPENSSL_VERSION_NUMBER >=0x10100000L
+	OPENSSL_init_crypto(OPENSSL_INIT_ADD_ALL_CIPHERS | OPENSSL_INIT_ADD_ALL_DIGESTS |
+		OPENSSL_INIT_LOAD_CONFIG | OPENSSL_INIT_ENGINE_OPENSSL | OPENSSL_INIT_ENGINE_ALL_BUILTIN , NULL);
+	OPENSSL_init_ssl(OPENSSL_INIT_LOAD_SSL_STRINGS, NULL);
+#endif
+
 #if OPENSSL_VERSION_NUMBER < 0x10100000L
 	SSL_load_error_strings();
 	SSL_library_init();
diff --git a/src/vmime/security/cert/defaultCertificateVerifier.cpp b/src/vmime/security/cert/defaultCertificateVerifier.cpp
index 33162a0a..c41396f1 100644
--- a/src/vmime/security/cert/defaultCertificateVerifier.cpp
+++ b/src/vmime/security/cert/defaultCertificateVerifier.cpp
@@ -75,6 +75,69 @@ void defaultCertificateVerifier::verify(
 }
 
 
+void defaultCertificateVerifier::verifyX509chain
+	(shared_ptr <certificateChain> chain, size_t chainLen)
+{
+	// For every certificate in the chain, verify that the certificate
+	// has been issued by the next certificate in the chain
+	if (chainLen >= 2) {
+		for (size_t i = 0 ; i < chainLen - 1 ; ++i) {
+			shared_ptr <X509Certificate> cert = dynamicCast <X509Certificate>(chain->getAt(i));
+			shared_ptr <X509Certificate> next = dynamicCast <X509Certificate>(chain->getAt(i + 1));
+
+			if (!cert->checkIssuer(next)) {
+				certificateIssuerVerificationException ex;
+				ex.setCertificate(cert);
+
+				throw ex;
+			}
+		}
+	}
+
+	// For every certificate in the chain, verify that the certificate
+	// is valid at the current time
+	for (size_t i = 0 ; i < chainLen ; ++i) {
+		shared_ptr <X509Certificate> cert = dynamicCast <X509Certificate>(chain->getAt(i));
+		cert->checkValidity();
+	}
+
+	// Check whether the certificate can be trusted
+
+	// -- First, verify that the the last certificate in the chain was
+	// -- issued by a third-party that we trust
+	shared_ptr <X509Certificate> lastCert = dynamicCast <X509Certificate>(chain->getAt(chainLen - 1));
+
+	bool trusted = false;
+
+	for (size_t i = 0 ; !trusted && i < m_x509RootCAs.size() ; ++i) {
+		shared_ptr <X509Certificate> rootCa = m_x509RootCAs[i];
+
+		if (lastCert->verify(rootCa)) {
+			trusted = true;
+		}
+	}
+
+	// -- Next, if the issuer certificate cannot be verified against
+	// -- root CAs, compare the subject's certificate against the
+	// -- trusted certificates
+	shared_ptr <X509Certificate> firstCert = dynamicCast <X509Certificate>(chain->getAt(0));
+
+	for (size_t i = 0 ; !trusted && i < m_x509TrustedCerts.size() ; ++i) {
+		shared_ptr <X509Certificate> cert = m_x509TrustedCerts[i];
+
+		if (firstCert->equals(cert)) {
+			trusted = true;
+		}
+	}
+
+	if (!trusted) {
+		certificateNotTrustedException ex;
+		ex.setCertificate(firstCert);
+
+		throw ex;
+	}
+}
+
 void defaultCertificateVerifier::verifyX509(
 	const shared_ptr <certificateChain>& chain,
 	const string& hostname
@@ -161,6 +224,44 @@ void defaultCertificateVerifier::verifyX509(
 
 		throw ex;
 	}
+
+	// Perform verification on all possible subchains in order to work
+	// around a server that sends extranous certificates in the chain
+	// after the valid one.
+	//
+	// Note: The TLS 1.3 draft says the following:
+	//
+	//   Note: Prior to TLS 1.3, "certificate_list" ordering required each
+	//   certificate to certify the one immediately preceding it; however,
+	//   some implementations allowed some flexibility.  Servers sometimes
+	//   send both a current and deprecated intermediate for transitional
+	//   purposes, and others are simply configured incorrectly, but these
+	//   cases can nonetheless be validated properly.  For maximum
+	//   compatibility, all implementations SHOULD be prepared to handle
+	//   potentially extraneous certificates and arbitrary orderings from any
+	//   TLS version, with the exception of the end-entity certificate which
+	//   MUST be first.
+	//
+	// This code does NOT yet handle arbitrary ordering.
+	//
+	for (size_t chainLen = chain->getCount(); chainLen > 0; chainLen--) {
+
+		try {
+			verifyX509chain(chain, chainLen);
+			return;
+		}
+		catch(...) {
+			if (chainLen == 1) {
+				throw;
+			}
+		}
+	}
+
+	// We really should never get here from the above loop
+	certificateNotTrustedException ex;
+	ex.setCertificate(firstCert);
+
+	throw ex;
 }
 
 
diff --git a/src/vmime/security/cert/defaultCertificateVerifier.hpp b/src/vmime/security/cert/defaultCertificateVerifier.hpp
index 4aa4445c..b90283b2 100644
--- a/src/vmime/security/cert/defaultCertificateVerifier.hpp
+++ b/src/vmime/security/cert/defaultCertificateVerifier.hpp
@@ -74,6 +74,13 @@ public:
 private:
 
 	/** Verify a chain of X.509 certificates.
+	 *
+	 * @param chain list of X.509 certificates
+	 * @param chainLen number of certificates to verify
+	 */
+	void verifyX509chain(shared_ptr <certificateChain> chain, size_t chainLen);
+
+	/** Verify a chain of X.509 certificates.
 	  *
 	  * @param chain list of X.509 certificates
 	  * @param hostname server hostname
diff --git a/src/vmime/security/sasl/builtinSASLMechanism.cpp b/src/vmime/security/sasl/builtinSASLMechanism.cpp
index 846e2ccf..020cae95 100644
--- a/src/vmime/security/sasl/builtinSASLMechanism.cpp
+++ b/src/vmime/security/sasl/builtinSASLMechanism.cpp
@@ -78,6 +78,10 @@ bool builtinSASLMechanism::step(
 	char* output = 0;
 	size_t outputLen = 0;
 
+	if (nullptr == sess->m_gsaslSession) {
+		throw exceptions::sasl_exception("Invalid SASL session");
+	}
+
 	const int result = gsasl_step(
 		sess->m_gsaslSession,
 		reinterpret_cast <const char*>(challenge), challengeLen,