From db486520f27c941116c1d42a6a08fdb5be72f170 Mon Sep 17 00:00:00 2001 From: Jacek Piszczek Date: Wed, 24 Mar 2021 12:55:42 -0400 Subject: [PATCH] Improved certificate verification --- .../net/tls/openssl/OpenSSLInitializer.cpp | 6 ++ .../cert/defaultCertificateVerifier.cpp | 101 ++++++++++++++++++ .../cert/defaultCertificateVerifier.hpp | 7 ++ .../security/sasl/builtinSASLMechanism.cpp | 4 + 4 files changed, 118 insertions(+) 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 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 cert = dynamicCast (chain->getAt(i)); + shared_ptr next = dynamicCast (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 cert = dynamicCast (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 lastCert = dynamicCast (chain->getAt(chainLen - 1)); + + bool trusted = false; + + for (size_t i = 0 ; !trusted && i < m_x509RootCAs.size() ; ++i) { + shared_ptr 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 firstCert = dynamicCast (chain->getAt(0)); + + for (size_t i = 0 ; !trusted && i < m_x509TrustedCerts.size() ; ++i) { + shared_ptr 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 & 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 @@ -73,6 +73,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 chain, size_t chainLen); + /** Verify a chain of X.509 certificates. * * @param chain list of X.509 certificates 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 (challenge), challengeLen,