Skip to content
This repository was archived by the owner on Nov 29, 2024. It is now read-only.

Commit d82e22d

Browse files
rozmansirozmansi
committed
Post-festum CRL checking introduced
1 parent 303350d commit d82e22d

File tree

6 files changed

+273
-23
lines changed

6 files changed

+273
-23
lines changed

‎lib/Events/res/EventsETW.man

7.66 KB
Binary file not shown.

‎lib/TTLS/include/Method.h

+4-3
Original file line numberDiff line numberDiff line change
@@ -225,14 +225,14 @@ namespace eap
225225
method_ttls(_In_ module &mod, _In_ config_method_ttls &cfg, _In_ credentials_ttls &cred, _In_ method *inner);
226226

227227
///
228-
/// Moves an TTLS method
228+
/// Moves a TTLS method
229229
///
230230
/// \param[in] other TTLS method to move from
231231
///
232232
method_ttls(_Inout_ method_ttls &&other);
233233

234234
///
235-
/// Moves an TTLS method
235+
/// Moves a TTLS method
236236
///
237237
/// \param[in] other TTLS method to move from
238238
///
@@ -271,7 +271,7 @@ namespace eap
271271
protected:
272272
#if EAP_TLS < EAP_TLS_SCHANNEL_FULL
273273
///
274-
/// Verifies server's certificate if trusted by configuration
274+
/// Verifies server certificate if trusted by configuration
275275
///
276276
void verify_server_trust() const;
277277
#endif
@@ -284,6 +284,7 @@ namespace eap
284284
winstd::sec_credentials m_sc_cred; ///< Schannel client credentials
285285
std::vector<unsigned char> m_sc_queue; ///< TLS data queue
286286
winstd::sec_context m_sc_ctx; ///< Schannel context
287+
winstd::cert_context m_sc_cert; ///< Server certificate
287288

288289
///
289290
/// Communication phase

‎lib/TTLS/include/Module.h

+54
Original file line numberDiff line numberDiff line change
@@ -155,6 +155,13 @@ namespace eap
155155

156156
/// @}
157157

158+
///
159+
/// Spawns a new certificate revocation check thread
160+
///
161+
/// \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.
162+
///
163+
void spawn_crl_check(_Inout_ winstd::cert_context &&cert);
164+
158165
protected:
159166
///
160167
/// Checks all configured providers and tries to combine credentials.
@@ -195,6 +202,53 @@ namespace eap
195202
BYTE *m_blob_cred; ///< Credentials BLOB
196203
#endif
197204
};
205+
206+
///
207+
///< Post-festum server certificate revocation verify thread
208+
///
209+
class crl_checker {
210+
public:
211+
///
212+
/// Constructs a thread
213+
///
214+
/// \param[in ] mod EAP module to use for global services
215+
/// \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.
216+
///
217+
crl_checker(_In_ module &mod, _Inout_ winstd::cert_context &&cert);
218+
219+
///
220+
/// Moves a thread
221+
///
222+
/// \param[in] other Thread to move from
223+
///
224+
crl_checker(_Inout_ crl_checker &&other);
225+
226+
///
227+
/// Moves a thread
228+
///
229+
/// \param[in] other Thread to move from
230+
///
231+
/// \returns Reference to this object
232+
///
233+
crl_checker& operator=(_Inout_ crl_checker &&other);
234+
235+
///
236+
/// Verifies server's certificate if it has been revoked
237+
///
238+
/// \param[in] obj Pointer to the instance of this object
239+
///
240+
/// \returns Thread exit code
241+
///
242+
static DWORD WINAPI verify(_In_ crl_checker *obj);
243+
244+
public:
245+
module &m_module; ///< Module
246+
winstd::win_handle m_thread; ///< Thread
247+
winstd::win_handle m_abort; ///< Thread abort event
248+
winstd::cert_context m_cert; ///< Server certificate
249+
};
250+
251+
std::list<crl_checker> m_crl_checkers; ///< List of certificate revocation check threads
198252
};
199253

200254
/// @}

‎lib/TTLS/src/Method.cpp

+54-19
Original file line numberDiff line numberDiff line change
@@ -355,6 +355,7 @@ eap::method_ttls::method_ttls(_Inout_ method_ttls &&other) :
355355
m_sc_cred (std::move(other.m_sc_cred )),
356356
m_sc_queue (std::move(other.m_sc_queue )),
357357
m_sc_ctx (std::move(other.m_sc_ctx )),
358+
m_sc_cert (std::move(other.m_sc_cert )),
358359
m_phase (std::move(other.m_phase )),
359360
m_packet_res (std::move(other.m_packet_res )),
360361
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)
375376
m_sc_cred = std::move(other.m_sc_cred );
376377
m_sc_queue = std::move(other.m_sc_queue );
377378
m_sc_ctx = std::move(other.m_sc_ctx );
379+
m_sc_cert = std::move(other.m_sc_cert );
378380
m_phase = std::move(other.m_phase );
379381
m_packet_res = std::move(other.m_packet_res );
380382
m_packet_res_inner = std::move(other.m_packet_res_inner);
@@ -549,10 +551,45 @@ EapPeerMethodResponseAction eap::method_ttls::process_request_packet(
549551
&buf_in_desc,
550552
&buf_out_desc);
551553

554+
if (status == SEC_E_OK) {
555+
// Get server certificate.
556+
SECURITY_STATUS status = QueryContextAttributes(m_sc_ctx, SECPKG_ATTR_REMOTE_CERT_CONTEXT, (PVOID)&m_sc_cert);
557+
if (FAILED(status))
558+
throw sec_runtime_error(status, __FUNCTION__ " Error retrieving server certificate from Schannel.");
559+
560+
// 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.
561+
// 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.
562+
for (auto c = m_cfg.m_trusted_root_ca.cbegin(), c_end = m_cfg.m_trusted_root_ca.cend(); c != c_end; ++c)
563+
CertAddCertificateContextToStore(m_sc_cert->hCertStore, *c, CERT_STORE_ADD_REPLACE_EXISTING, NULL);
564+
565+
// Verify cached CRL (entire chain).
566+
reg_key key;
567+
if (key.open(HKEY_LOCAL_MACHINE, _T("SOFTWARE\\") _T(VENDOR_NAME_STR) _T("\\") _T(PRODUCT_NAME_STR) _T("\\TLSCRL"), 0, KEY_READ)) {
568+
wstring hash_unicode;
569+
vector<unsigned char> hash, subj;
570+
for (cert_context c(m_sc_cert); c;) {
571+
if (CertGetCertificateContextProperty(c, CERT_HASH_PROP_ID, hash)) {
572+
hash_unicode.clear();
573+
hex_enc enc;
574+
enc.encode(hash_unicode, hash.data(), hash.size());
575+
if (RegQueryValueExW(key, hash_unicode.c_str(), NULL, NULL, subj) == ERROR_SUCCESS) {
576+
// A certificate in the chain is found to be revoked as compromised.
577+
m_cfg.m_last_status = config_method::status_server_compromised;
578+
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.");
579+
}
580+
}
581+
582+
DWORD flags = 0;
583+
c = CertGetIssuerCertificateFromStore(m_sc_cert->hCertStore, c, NULL, &flags);
584+
if (!c) break;
585+
}
586+
}
587+
552588
#if EAP_TLS < EAP_TLS_SCHANNEL_FULL
553-
if (status == SEC_E_OK)
589+
// Verify server certificate chain.
554590
verify_server_trust();
555591
#endif
592+
}
556593

557594
if (status == SEC_E_OK || status == SEC_I_CONTINUE_NEEDED) {
558595
// Send Schannel's token.
@@ -830,6 +867,9 @@ void eap::method_ttls::get_result(
830867
pResult->pAttribArray = &m_eap_attr_desc;
831868

832869
m_cfg.m_last_status = config_method::status_success;
870+
871+
// Spawn certificate revocation verify thread.
872+
dynamic_cast<peer_ttls&>(m_module).spawn_crl_check(std::move(m_sc_cert));
833873
}
834874

835875
// 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(
843883

844884
void eap::method_ttls::verify_server_trust() const
845885
{
846-
cert_context cert;
847-
SECURITY_STATUS status = QueryContextAttributes(m_sc_ctx, SECPKG_ATTR_REMOTE_CERT_CONTEXT, (PVOID)&cert);
848-
if (FAILED(status))
849-
throw sec_runtime_error(status, __FUNCTION__ " Error retrieving server certificate from Schannel.");
850-
851886
for (auto c = m_cfg.m_trusted_root_ca.cbegin(), c_end = m_cfg.m_trusted_root_ca.cend(); c != c_end; ++c) {
852-
if (cert->cbCertEncoded == (*c)->cbCertEncoded &&
853-
memcmp(cert->pbCertEncoded, (*c)->pbCertEncoded, cert->cbCertEncoded) == 0)
887+
if (m_sc_cert->cbCertEncoded == (*c)->cbCertEncoded &&
888+
memcmp(m_sc_cert->pbCertEncoded, (*c)->pbCertEncoded, m_sc_cert->cbCertEncoded) == 0)
854889
{
855890
// Server certificate found directly on the trusted root CA list.
856891
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
865900
found = false;
866901

867902
// Search subjectAltName2 and subjectAltName.
868-
for (DWORD idx_ext = 0; !found && idx_ext < cert->pCertInfo->cExtension; idx_ext++) {
903+
for (DWORD idx_ext = 0; !found && idx_ext < m_sc_cert->pCertInfo->cExtension; idx_ext++) {
869904
unique_ptr<CERT_ALT_NAME_INFO, LocalFree_delete<CERT_ALT_NAME_INFO> > san_info;
870-
if (strcmp(cert->pCertInfo->rgExtension[idx_ext].pszObjId, szOID_SUBJECT_ALT_NAME2) == 0) {
905+
if (strcmp(m_sc_cert->pCertInfo->rgExtension[idx_ext].pszObjId, szOID_SUBJECT_ALT_NAME2) == 0) {
871906
unsigned char *output = NULL;
872907
DWORD size_output;
873908
if (!CryptDecodeObjectEx(
874909
X509_ASN_ENCODING | PKCS_7_ASN_ENCODING,
875910
szOID_SUBJECT_ALT_NAME2,
876-
cert->pCertInfo->rgExtension[idx_ext].Value.pbData, cert->pCertInfo->rgExtension[idx_ext].Value.cbData,
911+
m_sc_cert->pCertInfo->rgExtension[idx_ext].Value.pbData, m_sc_cert->pCertInfo->rgExtension[idx_ext].Value.cbData,
877912
CRYPT_DECODE_ALLOC_FLAG | CRYPT_DECODE_ENABLE_PUNYCODE_FLAG,
878913
NULL,
879914
&output, &size_output))
880915
throw win_runtime_error(__FUNCTION__ " Error decoding subjectAltName2 certificate extension.");
881916
san_info.reset((CERT_ALT_NAME_INFO*)output);
882-
} else if (strcmp(cert->pCertInfo->rgExtension[idx_ext].pszObjId, szOID_SUBJECT_ALT_NAME) == 0) {
917+
} else if (strcmp(m_sc_cert->pCertInfo->rgExtension[idx_ext].pszObjId, szOID_SUBJECT_ALT_NAME) == 0) {
883918
unsigned char *output = NULL;
884919
DWORD size_output;
885920
if (!CryptDecodeObjectEx(
886921
X509_ASN_ENCODING | PKCS_7_ASN_ENCODING,
887922
szOID_SUBJECT_ALT_NAME,
888-
cert->pCertInfo->rgExtension[idx_ext].Value.pbData, cert->pCertInfo->rgExtension[idx_ext].Value.cbData,
923+
m_sc_cert->pCertInfo->rgExtension[idx_ext].Value.pbData, m_sc_cert->pCertInfo->rgExtension[idx_ext].Value.cbData,
889924
CRYPT_DECODE_ALLOC_FLAG | CRYPT_DECODE_ENABLE_PUNYCODE_FLAG,
890925
NULL,
891926
&output, &size_output))
@@ -912,7 +947,7 @@ void eap::method_ttls::verify_server_trust() const
912947
if (!has_san) {
913948
// Certificate has no subjectAltName. Compare against Common Name.
914949
wstring subj;
915-
if (!CertGetNameStringW(cert, CERT_NAME_DNS_TYPE, CERT_NAME_STR_ENABLE_PUNYCODE_FLAG, NULL, subj))
950+
if (!CertGetNameStringW(m_sc_cert, CERT_NAME_DNS_TYPE, CERT_NAME_STR_ENABLE_PUNYCODE_FLAG, NULL, subj))
916951
throw win_runtime_error(__FUNCTION__ " Error retrieving server's certificate subject name.");
917952

918953
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
927962
throw sec_runtime_error(SEC_E_WRONG_PRINCIPAL, __FUNCTION__ " Name provided in server certificate is not on the list of trusted server names.");
928963
}
929964

930-
if (cert->pCertInfo->Issuer.cbData == cert->pCertInfo->Subject.cbData &&
931-
memcmp(cert->pCertInfo->Issuer.pbData, cert->pCertInfo->Subject.pbData, cert->pCertInfo->Issuer.cbData) == 0)
965+
if (m_sc_cert->pCertInfo->Issuer.cbData == m_sc_cert->pCertInfo->Subject.cbData &&
966+
memcmp(m_sc_cert->pCertInfo->Issuer.pbData, m_sc_cert->pCertInfo->Subject.pbData, m_sc_cert->pCertInfo->Issuer.cbData) == 0)
932967
throw sec_runtime_error(SEC_E_CERT_UNKNOWN, __FUNCTION__ " Server is using a self-signed certificate. Cannot trust it.");
933968

934969
// Create temporary certificate store of our trusted root CAs.
@@ -939,9 +974,9 @@ void eap::method_ttls::verify_server_trust() const
939974
CertAddCertificateContextToStore(store, *c, CERT_STORE_ADD_REPLACE_EXISTING, NULL);
940975

941976
// Add all intermediate certificates from the server's certificate chain.
942-
for (cert_context c(cert); c;) {
977+
for (cert_context c(m_sc_cert); c;) {
943978
DWORD flags = 0;
944-
c.attach(CertGetIssuerCertificateFromStore(cert->hCertStore, c, NULL, &flags));
979+
c.attach(CertGetIssuerCertificateFromStore(m_sc_cert->hCertStore, c, NULL, &flags));
945980
if (!c) break;
946981

947982
if (c->pCertInfo->Issuer.cbData == c->pCertInfo->Subject.cbData &&
@@ -971,7 +1006,7 @@ void eap::method_ttls::verify_server_trust() const
9711006
#endif
9721007
};
9731008
cert_chain_context context;
974-
if (!context.create(NULL, cert, NULL, store, &chain_params, 0))
1009+
if (!context.create(NULL, m_sc_cert, NULL, store, &chain_params, 0))
9751010
throw win_runtime_error(__FUNCTION__ " Error creating certificate chain context.");
9761011

9771012
// Check chain validation error flags. Ignore CERT_TRUST_IS_UNTRUSTED_ROOT flag since we check root CA explicitly.

0 commit comments

Comments
 (0)