diff --git a/lib/Events/res/EventsETW.man b/lib/Events/res/EventsETW.man index 69e4dea..37c9b7a 100644 Binary files a/lib/Events/res/EventsETW.man and b/lib/Events/res/EventsETW.man differ diff --git a/lib/TTLS/include/Method.h b/lib/TTLS/include/Method.h index 555ffc8..c68cb87 100644 --- a/lib/TTLS/include/Method.h +++ b/lib/TTLS/include/Method.h @@ -225,14 +225,14 @@ namespace eap method_ttls(_In_ module &mod, _In_ config_method_ttls &cfg, _In_ credentials_ttls &cred, _In_ method *inner); /// - /// Moves an TTLS method + /// Moves a TTLS method /// /// \param[in] other TTLS method to move from /// method_ttls(_Inout_ method_ttls &&other); /// - /// Moves an TTLS method + /// Moves a TTLS method /// /// \param[in] other TTLS method to move from /// @@ -271,7 +271,7 @@ namespace eap protected: #if EAP_TLS < EAP_TLS_SCHANNEL_FULL /// - /// Verifies server's certificate if trusted by configuration + /// Verifies server certificate if trusted by configuration /// void verify_server_trust() const; #endif @@ -284,6 +284,7 @@ namespace eap winstd::sec_credentials m_sc_cred; ///< Schannel client credentials std::vector m_sc_queue; ///< TLS data queue winstd::sec_context m_sc_ctx; ///< Schannel context + winstd::cert_context m_sc_cert; ///< Server certificate /// /// Communication phase diff --git a/lib/TTLS/include/Module.h b/lib/TTLS/include/Module.h index e5831f4..eb4c0f2 100644 --- a/lib/TTLS/include/Module.h +++ b/lib/TTLS/include/Module.h @@ -155,6 +155,13 @@ namespace eap /// @} + /// + /// Spawns a new certificate revocation check thread + /// + /// \param[inout] cert Certificate context to check for revocation. `hCertStore` member should contain all certificates in chain up to and including root CA to test them for revocation too. + /// + void spawn_crl_check(_Inout_ winstd::cert_context &&cert); + protected: /// /// Checks all configured providers and tries to combine credentials. @@ -195,6 +202,53 @@ namespace eap BYTE *m_blob_cred; ///< Credentials BLOB #endif }; + + /// + ///< Post-festum server certificate revocation verify thread + /// + class crl_checker { + public: + /// + /// Constructs a thread + /// + /// \param[in ] mod EAP module to use for global services + /// \param[inout] cert Certificate context to check for revocation. `hCertStore` member should contain all certificates in chain up to and including root CA to test them for revocation too. + /// + crl_checker(_In_ module &mod, _Inout_ winstd::cert_context &&cert); + + /// + /// Moves a thread + /// + /// \param[in] other Thread to move from + /// + crl_checker(_Inout_ crl_checker &&other); + + /// + /// Moves a thread + /// + /// \param[in] other Thread to move from + /// + /// \returns Reference to this object + /// + crl_checker& operator=(_Inout_ crl_checker &&other); + + /// + /// Verifies server's certificate if it has been revoked + /// + /// \param[in] obj Pointer to the instance of this object + /// + /// \returns Thread exit code + /// + static DWORD WINAPI verify(_In_ crl_checker *obj); + + public: + module &m_module; ///< Module + winstd::win_handle m_thread; ///< Thread + winstd::win_handle m_abort; ///< Thread abort event + winstd::cert_context m_cert; ///< Server certificate + }; + + std::list m_crl_checkers; ///< List of certificate revocation check threads }; /// @} diff --git a/lib/TTLS/src/Method.cpp b/lib/TTLS/src/Method.cpp index a1a4dd0..99bd356 100644 --- a/lib/TTLS/src/Method.cpp +++ b/lib/TTLS/src/Method.cpp @@ -355,6 +355,7 @@ eap::method_ttls::method_ttls(_Inout_ method_ttls &&other) : m_sc_cred (std::move(other.m_sc_cred )), m_sc_queue (std::move(other.m_sc_queue )), m_sc_ctx (std::move(other.m_sc_ctx )), + m_sc_cert (std::move(other.m_sc_cert )), m_phase (std::move(other.m_phase )), m_packet_res (std::move(other.m_packet_res )), m_packet_res_inner(std::move(other.m_packet_res_inner)), @@ -375,6 +376,7 @@ eap::method_ttls& eap::method_ttls::operator=(_Inout_ method_ttls &&other) m_sc_cred = std::move(other.m_sc_cred ); m_sc_queue = std::move(other.m_sc_queue ); m_sc_ctx = std::move(other.m_sc_ctx ); + m_sc_cert = std::move(other.m_sc_cert ); m_phase = std::move(other.m_phase ); m_packet_res = std::move(other.m_packet_res ); m_packet_res_inner = std::move(other.m_packet_res_inner); @@ -549,10 +551,45 @@ EapPeerMethodResponseAction eap::method_ttls::process_request_packet( &buf_in_desc, &buf_out_desc); + if (status == SEC_E_OK) { + // Get server certificate. + SECURITY_STATUS status = QueryContextAttributes(m_sc_ctx, SECPKG_ATTR_REMOTE_CERT_CONTEXT, (PVOID)&m_sc_cert); + if (FAILED(status)) + throw sec_runtime_error(status, __FUNCTION__ " Error retrieving server certificate from Schannel."); + + // Add all trusted root CAs to server certificate's store. This allows CertGetIssuerCertificateFromStore() in the following CRL check to test the root CA for revocation too. + // verify_server_trust(), ignores all self-signed certificates from the server certificate's store, and rebuilds its own trusted root store, so we are safe to do this. + for (auto c = m_cfg.m_trusted_root_ca.cbegin(), c_end = m_cfg.m_trusted_root_ca.cend(); c != c_end; ++c) + CertAddCertificateContextToStore(m_sc_cert->hCertStore, *c, CERT_STORE_ADD_REPLACE_EXISTING, NULL); + + // Verify cached CRL (entire chain). + reg_key key; + if (key.open(HKEY_LOCAL_MACHINE, _T("SOFTWARE\\") _T(VENDOR_NAME_STR) _T("\\") _T(PRODUCT_NAME_STR) _T("\\TLSCRL"), 0, KEY_READ)) { + wstring hash_unicode; + vector hash, subj; + for (cert_context c(m_sc_cert); c;) { + if (CertGetCertificateContextProperty(c, CERT_HASH_PROP_ID, hash)) { + hash_unicode.clear(); + hex_enc enc; + enc.encode(hash_unicode, hash.data(), hash.size()); + if (RegQueryValueExW(key, hash_unicode.c_str(), NULL, NULL, subj) == ERROR_SUCCESS) { + // A certificate in the chain is found to be revoked as compromised. + m_cfg.m_last_status = config_method::status_server_compromised; + throw com_runtime_error(CRYPT_E_REVOKED, __FUNCTION__ " Server certificate or one of its issuer's certificate has been found revoked as compromised. Your credentials were probably sent to this server during previous connection attempts, thus changing your credentials (in a safe manner) is strongly advised. Please, contact your helpdesk immediately."); + } + } + + DWORD flags = 0; + c = CertGetIssuerCertificateFromStore(m_sc_cert->hCertStore, c, NULL, &flags); + if (!c) break; + } + } + #if EAP_TLS < EAP_TLS_SCHANNEL_FULL - if (status == SEC_E_OK) + // Verify server certificate chain. verify_server_trust(); #endif + } if (status == SEC_E_OK || status == SEC_I_CONTINUE_NEEDED) { // Send Schannel's token. @@ -830,6 +867,9 @@ void eap::method_ttls::get_result( pResult->pAttribArray = &m_eap_attr_desc; m_cfg.m_last_status = config_method::status_success; + + // Spawn certificate revocation verify thread. + dynamic_cast(m_module).spawn_crl_check(std::move(m_sc_cert)); } // Always ask EAP host to save the connection data. And it will save it *only* when we report "success". @@ -843,14 +883,9 @@ void eap::method_ttls::get_result( void eap::method_ttls::verify_server_trust() const { - cert_context cert; - SECURITY_STATUS status = QueryContextAttributes(m_sc_ctx, SECPKG_ATTR_REMOTE_CERT_CONTEXT, (PVOID)&cert); - if (FAILED(status)) - throw sec_runtime_error(status, __FUNCTION__ " Error retrieving server certificate from Schannel."); - for (auto c = m_cfg.m_trusted_root_ca.cbegin(), c_end = m_cfg.m_trusted_root_ca.cend(); c != c_end; ++c) { - if (cert->cbCertEncoded == (*c)->cbCertEncoded && - memcmp(cert->pbCertEncoded, (*c)->pbCertEncoded, cert->cbCertEncoded) == 0) + if (m_sc_cert->cbCertEncoded == (*c)->cbCertEncoded && + memcmp(m_sc_cert->pbCertEncoded, (*c)->pbCertEncoded, m_sc_cert->cbCertEncoded) == 0) { // Server certificate found directly on the trusted root CA list. m_module.log_event(&EAPMETHOD_TLS_SERVER_CERT_TRUSTED_EX1, event_data((unsigned int)eap_type_ttls), event_data::blank); @@ -865,27 +900,27 @@ void eap::method_ttls::verify_server_trust() const found = false; // Search subjectAltName2 and subjectAltName. - for (DWORD idx_ext = 0; !found && idx_ext < cert->pCertInfo->cExtension; idx_ext++) { + for (DWORD idx_ext = 0; !found && idx_ext < m_sc_cert->pCertInfo->cExtension; idx_ext++) { unique_ptr > san_info; - if (strcmp(cert->pCertInfo->rgExtension[idx_ext].pszObjId, szOID_SUBJECT_ALT_NAME2) == 0) { + if (strcmp(m_sc_cert->pCertInfo->rgExtension[idx_ext].pszObjId, szOID_SUBJECT_ALT_NAME2) == 0) { unsigned char *output = NULL; DWORD size_output; if (!CryptDecodeObjectEx( X509_ASN_ENCODING | PKCS_7_ASN_ENCODING, szOID_SUBJECT_ALT_NAME2, - cert->pCertInfo->rgExtension[idx_ext].Value.pbData, cert->pCertInfo->rgExtension[idx_ext].Value.cbData, + m_sc_cert->pCertInfo->rgExtension[idx_ext].Value.pbData, m_sc_cert->pCertInfo->rgExtension[idx_ext].Value.cbData, CRYPT_DECODE_ALLOC_FLAG | CRYPT_DECODE_ENABLE_PUNYCODE_FLAG, NULL, &output, &size_output)) throw win_runtime_error(__FUNCTION__ " Error decoding subjectAltName2 certificate extension."); san_info.reset((CERT_ALT_NAME_INFO*)output); - } else if (strcmp(cert->pCertInfo->rgExtension[idx_ext].pszObjId, szOID_SUBJECT_ALT_NAME) == 0) { + } else if (strcmp(m_sc_cert->pCertInfo->rgExtension[idx_ext].pszObjId, szOID_SUBJECT_ALT_NAME) == 0) { unsigned char *output = NULL; DWORD size_output; if (!CryptDecodeObjectEx( X509_ASN_ENCODING | PKCS_7_ASN_ENCODING, szOID_SUBJECT_ALT_NAME, - cert->pCertInfo->rgExtension[idx_ext].Value.pbData, cert->pCertInfo->rgExtension[idx_ext].Value.cbData, + m_sc_cert->pCertInfo->rgExtension[idx_ext].Value.pbData, m_sc_cert->pCertInfo->rgExtension[idx_ext].Value.cbData, CRYPT_DECODE_ALLOC_FLAG | CRYPT_DECODE_ENABLE_PUNYCODE_FLAG, NULL, &output, &size_output)) @@ -912,7 +947,7 @@ void eap::method_ttls::verify_server_trust() const if (!has_san) { // Certificate has no subjectAltName. Compare against Common Name. wstring subj; - if (!CertGetNameStringW(cert, CERT_NAME_DNS_TYPE, CERT_NAME_STR_ENABLE_PUNYCODE_FLAG, NULL, subj)) + if (!CertGetNameStringW(m_sc_cert, CERT_NAME_DNS_TYPE, CERT_NAME_STR_ENABLE_PUNYCODE_FLAG, NULL, subj)) throw win_runtime_error(__FUNCTION__ " Error retrieving server's certificate subject name."); for (auto s = m_cfg.m_server_names.cbegin(), s_end = m_cfg.m_server_names.cend(); !found && s != s_end; ++s) { @@ -927,8 +962,8 @@ void eap::method_ttls::verify_server_trust() const throw sec_runtime_error(SEC_E_WRONG_PRINCIPAL, __FUNCTION__ " Name provided in server certificate is not on the list of trusted server names."); } - if (cert->pCertInfo->Issuer.cbData == cert->pCertInfo->Subject.cbData && - memcmp(cert->pCertInfo->Issuer.pbData, cert->pCertInfo->Subject.pbData, cert->pCertInfo->Issuer.cbData) == 0) + if (m_sc_cert->pCertInfo->Issuer.cbData == m_sc_cert->pCertInfo->Subject.cbData && + memcmp(m_sc_cert->pCertInfo->Issuer.pbData, m_sc_cert->pCertInfo->Subject.pbData, m_sc_cert->pCertInfo->Issuer.cbData) == 0) throw sec_runtime_error(SEC_E_CERT_UNKNOWN, __FUNCTION__ " Server is using a self-signed certificate. Cannot trust it."); // Create temporary certificate store of our trusted root CAs. @@ -939,9 +974,9 @@ void eap::method_ttls::verify_server_trust() const CertAddCertificateContextToStore(store, *c, CERT_STORE_ADD_REPLACE_EXISTING, NULL); // Add all intermediate certificates from the server's certificate chain. - for (cert_context c(cert); c;) { + for (cert_context c(m_sc_cert); c;) { DWORD flags = 0; - c.attach(CertGetIssuerCertificateFromStore(cert->hCertStore, c, NULL, &flags)); + c.attach(CertGetIssuerCertificateFromStore(m_sc_cert->hCertStore, c, NULL, &flags)); if (!c) break; if (c->pCertInfo->Issuer.cbData == c->pCertInfo->Subject.cbData && @@ -971,7 +1006,7 @@ void eap::method_ttls::verify_server_trust() const #endif }; cert_chain_context context; - if (!context.create(NULL, cert, NULL, store, &chain_params, 0)) + if (!context.create(NULL, m_sc_cert, NULL, store, &chain_params, 0)) throw win_runtime_error(__FUNCTION__ " Error creating certificate chain context."); // Check chain validation error flags. Ignore CERT_TRUST_IS_UNTRUSTED_ROOT flag since we check root CA explicitly. diff --git a/lib/TTLS/src/Module.cpp b/lib/TTLS/src/Module.cpp index 596ed8f..b3be419 100644 --- a/lib/TTLS/src/Module.cpp +++ b/lib/TTLS/src/Module.cpp @@ -62,6 +62,15 @@ void eap::peer_ttls::initialize() void eap::peer_ttls::shutdown() { + // Signal all certificate revocation verify threads to abort and wait for them (10sec max). + vector chks; + chks.reserve(m_crl_checkers.size()); + for (auto chk = m_crl_checkers.begin(), chk_end = m_crl_checkers.end(); chk != chk_end; ++chk) { + SetEvent(chk->m_abort); + chks.push_back(chk->m_thread); + } + WaitForMultipleObjects((DWORD)chks.size(), chks.data(), TRUE, 10000); + // Uninitialize EapHost. It was initialized for EapHost based inner authentication methods. EapHostPeerUninitialize(); } @@ -379,6 +388,17 @@ void eap::peer_ttls::set_response_attributes( } +void eap::peer_ttls::spawn_crl_check(_Inout_ winstd::cert_context &&cert) +{ + // Create the thread and add it to the list. + m_crl_checkers.push_back(std::move(crl_checker(*this, std::move(cert)))); + + // Now the thread is in-place, start it. + crl_checker &chk = m_crl_checkers.back(); + chk.m_thread = CreateThread(NULL, 0, reinterpret_cast(crl_checker::verify), &chk, 0, NULL); +} + + const eap::config_method_ttls* eap::peer_ttls::combine_credentials( _In_ DWORD dwFlags, _In_ const config_connection &cfg, @@ -498,3 +518,143 @@ eap::peer_ttls::session::~session() m_module.free_memory(m_blob_cred); #endif } + + +////////////////////////////////////////////////////////////////////// +// eap::peer_ttls::crl_checker +////////////////////////////////////////////////////////////////////// + +eap::peer_ttls::crl_checker::crl_checker(_In_ module &mod, _Inout_ winstd::cert_context &&cert) : + m_module(mod), + m_cert (std::move(cert)), + m_abort (CreateEvent(NULL, TRUE, FALSE, NULL)) +{ +} + + +eap::peer_ttls::crl_checker::crl_checker(_Inout_ crl_checker &&other) : + m_module( other.m_module ), + m_thread(std::move(other.m_thread)), + m_abort (std::move(other.m_abort )), + m_cert (std::move(other.m_cert )) +{ +} + + +eap::peer_ttls::crl_checker& eap::peer_ttls::crl_checker::operator=(_Inout_ crl_checker &&other) +{ + if (this != std::addressof(other)) { + assert(std::addressof(m_module) == std::addressof(other.m_module)); // Move threads within same module only! + m_thread = std::move(other.m_thread); + m_abort = std::move(other.m_abort ); + m_cert = std::move(other.m_cert ); + } + + return *this; +} + + +DWORD WINAPI eap::peer_ttls::crl_checker::verify(_In_ crl_checker *obj) +{ + // Initialize COM. + com_initializer com_init(NULL); + + // Wait for 5sec for the link to become online. (Hopefuly!) + if (WaitForSingleObject(obj->m_abort, 5000) == WAIT_OBJECT_0) { + // Aborted. + return 1; + } + + // Prepare a list of certificates forming certificate chain. + list context_data; + for (cert_context c(obj->m_cert); c;) { + context_data.push_back(std::move(c)); + DWORD flags = 0; + c = CertGetIssuerCertificateFromStore(obj->m_cert->hCertStore, context_data.back(), NULL, &flags); + if (!c) break; + } + + // Create an array of pointers to CERT_CONTEXT required by CertVerifyRevocation(). + vector context; + context.reserve(context_data.size()); + for (auto c = context_data.cbegin(), c_end = context_data.cend(); c != c_end; ++c) + context.push_back(const_cast(c->operator PCCERT_CONTEXT())); + + CERT_REVOCATION_STATUS status_rev = { sizeof(CERT_REVOCATION_STATUS) }; + for (auto c = context.begin(), c_end = context.end(); c != c_end;) { + // Check for thread abort signal. + if (WaitForSingleObject(obj->m_abort, 0) == WAIT_OBJECT_0) + return 1; + + // Perform revocation check. + if (!CertVerifyRevocation(X509_ASN_ENCODING | PKCS_7_ASN_ENCODING, CERT_CONTEXT_REVOCATION_TYPE, + (DWORD)(c_end - c), reinterpret_cast(&*c), + CERT_VERIFY_REV_CHAIN_FLAG, NULL, &status_rev)) + { + PCCERT_CONTEXT cert = *(c + status_rev.dwIndex); + wstring subj; + if (!CertGetNameStringW(cert, CERT_NAME_DNS_TYPE, CERT_NAME_STR_ENABLE_PUNYCODE_FLAG, NULL, subj)) + sprintf(subj, L"", GetLastError()); + + switch (status_rev.dwError) { + case CRYPT_E_NO_REVOCATION_CHECK: + // Revocation check could not be performed. + c += status_rev.dwIndex + 1; + if (c == c_end) { + // This "error" is expected for the root CA certificate. + } else { + // This really was an error, as it appeared before the root CA cerficate in the chain. + obj->m_module.log_event(&EAPMETHOD_TLS_SERVER_CERT_REVOKE_SKIPPED, event_data((unsigned int)eap_type_ttls), event_data(subj), event_data::blank); + } + break; + + case CRYPT_E_REVOKED: + // One of the certificates in the chain was revoked. + switch (status_rev.dwReason) { + case CRL_REASON_AFFILIATION_CHANGED: + case CRL_REASON_SUPERSEDED: + case CRL_REASON_CESSATION_OF_OPERATION: + case CRL_REASON_CERTIFICATE_HOLD: + // The revocation was of administrative nature. No need to black-list. + obj->m_module.log_event(&EAPMETHOD_TLS_SERVER_CERT_REVOKED1, event_data((unsigned int)eap_type_ttls), event_data(subj), event_data(status_rev.dwReason), event_data::blank); + break; + + default: { + // One of the certificates in the chain was revoked as compromised. Black-list it. + obj->m_module.log_event(&EAPMETHOD_TLS_SERVER_CERT_REVOKED, event_data((unsigned int)eap_type_ttls), event_data(subj), event_data(status_rev.dwReason), event_data::blank); + reg_key key; + if (key.create(HKEY_LOCAL_MACHINE, _T("SOFTWARE\\") _T(VENDOR_NAME_STR) _T("\\") _T(PRODUCT_NAME_STR) _T("\\TLSCRL"), NULL, REG_OPTION_NON_VOLATILE, KEY_WRITE)) { + vector hash; + if (CertGetCertificateContextProperty(cert, CERT_HASH_PROP_ID, hash)) { + wstring hash_unicode; + hex_enc enc; + enc.encode(hash_unicode, hash.data(), hash.size()); + RegSetValueExW(key, hash_unicode.c_str(), NULL, REG_SZ, reinterpret_cast(subj.c_str()), (DWORD)((subj.length() + 1) * sizeof(wstring::value_type))); + } + } + }} + + // Resume checking the rest of the chain. + c += status_rev.dwIndex + 1; + break; + + case ERROR_SUCCESS: + // Odd. CertVerifyRevocation() should return TRUE then. Nevertheless, we take this as a "yes". + c = c_end; + break; + + default: + // Checking one of the certificates in the chain for revocation failed. Resume checking the rest. + obj->m_module.log_event(&EAPMETHOD_TLS_SERVER_CERT_REVOKE_FAILED, event_data((unsigned int)eap_type_ttls), event_data(subj), event_data(status_rev.dwError), event_data::blank); + c += status_rev.dwIndex + 1; + } + } else { + // Revocation check finished. + break; + } + } + + // Revocation check succeeded. + obj->m_module.log_event(&EAPMETHOD_TLS_SERVER_CERT_REVOKE_FINISHED, event_data((unsigned int)eap_type_ttls), event_data::blank); + return 0; +} diff --git a/lib/WinStd b/lib/WinStd index cea06e8..feea0fd 160000 --- a/lib/WinStd +++ b/lib/WinStd @@ -1 +1 @@ -Subproject commit cea06e8f487159798a8497a011c8f6d8db460cf7 +Subproject commit feea0fdf2a06b5746d056762b2b254ea45288527