From 1f870ae1892de1e58d92422761b9047c86c2e0fe Mon Sep 17 00:00:00 2001 From: Tobias Brunner Date: Wed, 21 Sep 2022 10:22:16 +0200 Subject: [PATCH] cert-validator: Use a separate method for online revocation checking This avoids having to repeat offline checks after basic trust chain validation. --- .../plugins/addrblock/addrblock_validator.c | 3 +- .../plugins/coupling/coupling_validator.c | 5 +- .../credentials/cert_validator.h | 28 +++++++++-- .../credentials/credential_manager.c | 46 +++++++++++++------ .../plugins/acert/acert_validator.c | 3 +- .../constraints/constraints_validator.c | 3 +- .../plugins/revocation/revocation_validator.c | 9 ++-- 7 files changed, 67 insertions(+), 30 deletions(-) diff --git a/src/libcharon/plugins/addrblock/addrblock_validator.c b/src/libcharon/plugins/addrblock/addrblock_validator.c index 3fbabbca57..3fa37a2f3b 100644 --- a/src/libcharon/plugins/addrblock/addrblock_validator.c +++ b/src/libcharon/plugins/addrblock/addrblock_validator.c @@ -110,8 +110,7 @@ static bool check_addrblock(private_addrblock_validator_t *this, METHOD(cert_validator_t, validate, bool, private_addrblock_validator_t *this, certificate_t *subject, - certificate_t *issuer, bool online, u_int pathlen, bool anchor, - auth_cfg_t *auth) + certificate_t *issuer, u_int pathlen, bool anchor, auth_cfg_t *auth) { if (subject->get_type(subject) == CERT_X509 && issuer->get_type(issuer) == CERT_X509) diff --git a/src/libcharon/plugins/coupling/coupling_validator.c b/src/libcharon/plugins/coupling/coupling_validator.c index 6cbec4b8f3..e5261e2d5a 100644 --- a/src/libcharon/plugins/coupling/coupling_validator.c +++ b/src/libcharon/plugins/coupling/coupling_validator.c @@ -136,9 +136,8 @@ static bool add_entry(private_coupling_validator_t *this, char *hash, } METHOD(cert_validator_t, validate, bool, - private_coupling_validator_t *this, - certificate_t *subject, certificate_t *issuer, - bool online, u_int pathlen, bool anchor, auth_cfg_t *auth) + private_coupling_validator_t *this, certificate_t *subject, + certificate_t *issuer, u_int pathlen, bool anchor, auth_cfg_t *auth) { bool valid = FALSE; char hash[MAX_HASH_SIZE]; diff --git a/src/libstrongswan/credentials/cert_validator.h b/src/libstrongswan/credentials/cert_validator.h index c5857958f0..e6ea2465f5 100644 --- a/src/libstrongswan/credentials/cert_validator.h +++ b/src/libstrongswan/credentials/cert_validator.h @@ -1,4 +1,5 @@ /* + * Copyright (C) 2022 Tobias Brunner * Copyright (C) 2010 Martin Willi * * Copyright (C) secunet Security Networks AG @@ -51,6 +52,7 @@ struct cert_validator_t { */ status_t (*check_lifetime)(cert_validator_t *this, certificate_t *cert, int pathlen, bool anchor, auth_cfg_t *auth); + /** * Validate a subject certificate in relation to its issuer. * @@ -59,15 +61,35 @@ struct cert_validator_t { * * @param subject subject certificate to check * @param issuer issuer of subject - * @param online whether to do online revocation checking * @param pathlen the current length of the path bottom-up * @param anchor is issuer trusted root anchor * @param auth container for resulting authentication info * @return TRUE if subject certificate valid */ bool (*validate)(cert_validator_t *this, certificate_t *subject, - certificate_t *issuer, bool online, u_int pathlen, - bool anchor, auth_cfg_t *auth); + certificate_t *issuer, u_int pathlen, bool anchor, + auth_cfg_t *auth); + + /** + * Do extended online revocation checking for the given subject certificate + * in relation to its issuer. + * + * If FALSE is returned, the validator should call_hook() on the + * credential manager with an appropriate type and the certificate. + * + * @note This is called after successful basic validation of the complete + * trust chain including validation via validate(). + * + * @param subject subject certificate to check + * @param issuer issuer of subject + * @param pathlen the current length of the path bottom-up + * @param anchor is issuer trusted root anchor + * @param auth container for resulting authentication info + * @return TRUE if subject certificate valid + */ + bool (*validate_online)(cert_validator_t *this, certificate_t *subject, + certificate_t *issuer, u_int pathlen, bool anchor, + auth_cfg_t *auth); }; #endif /** CERT_VALIDATOR_H_ @}*/ diff --git a/src/libstrongswan/credentials/credential_manager.c b/src/libstrongswan/credentials/credential_manager.c index 798785544e..cf26889160 100644 --- a/src/libstrongswan/credentials/credential_manager.c +++ b/src/libstrongswan/credentials/credential_manager.c @@ -1,5 +1,5 @@ /* - * Copyright (C) 2015 Tobias Brunner + * Copyright (C) 2015-2022 Tobias Brunner * Copyright (C) 2007 Martin Willi * * Copyright (C) secunet Security Networks AG @@ -600,11 +600,11 @@ static bool check_lifetime(private_credential_manager_t *this, } /** - * check a certificate for its lifetime + * Check a certificate's lifetime and consult plugins */ static bool check_certificate(private_credential_manager_t *this, - certificate_t *subject, certificate_t *issuer, bool online, - int pathlen, bool anchor, auth_cfg_t *auth) + certificate_t *subject, certificate_t *issuer, + int pathlen, bool anchor, auth_cfg_t *auth) { cert_validator_t *validator; enumerator_t *enumerator; @@ -618,12 +618,34 @@ static bool check_certificate(private_credential_manager_t *this, enumerator = this->validators->create_enumerator(this->validators); while (enumerator->enumerate(enumerator, &validator)) { - if (!validator->validate) + if (validator->validate && + !validator->validate(validator, subject, issuer, + pathlen, anchor, auth)) { - continue; + enumerator->destroy(enumerator); + return FALSE; } - if (!validator->validate(validator, subject, issuer, - online, pathlen, anchor, auth)) + } + enumerator->destroy(enumerator); + return TRUE; +} + +/** + * Do online revocation checking + */ +static bool check_certificate_online(private_credential_manager_t *this, + certificate_t *subject, certificate_t *issuer, + int pathlen, bool anchor, auth_cfg_t *auth) +{ + cert_validator_t *validator; + enumerator_t *enumerator; + + enumerator = this->validators->create_enumerator(this->validators); + while (enumerator->enumerate(enumerator, &validator)) + { + if (validator->validate_online && + !validator->validate_online(validator, subject, issuer, + pathlen, anchor, auth)) { enumerator->destroy(enumerator); return FALSE; @@ -788,9 +810,7 @@ static bool verify_trust_chain(private_credential_manager_t *this, break; } } - /* don't do online verification here */ - if (!check_certificate(this, current, issuer, FALSE, - pathlen, is_anchor, auth)) + if (!check_certificate(this, current, issuer, pathlen, is_anchor, auth)) { trusted = FALSE; issuer->destroy(issuer); @@ -828,8 +848,8 @@ static bool verify_trust_chain(private_credential_manager_t *this, { if (rule == AUTH_RULE_CA_CERT || rule == AUTH_RULE_IM_CERT) { - if (!check_certificate(this, current, issuer, TRUE, pathlen++, - rule == AUTH_RULE_CA_CERT, auth)) + if (!check_certificate_online(this, current, issuer, pathlen++, + rule == AUTH_RULE_CA_CERT, auth)) { trusted = FALSE; break; diff --git a/src/libstrongswan/plugins/acert/acert_validator.c b/src/libstrongswan/plugins/acert/acert_validator.c index a46f11a92d..a9e116cc20 100644 --- a/src/libstrongswan/plugins/acert/acert_validator.c +++ b/src/libstrongswan/plugins/acert/acert_validator.c @@ -91,8 +91,7 @@ static void apply(private_acert_validator_t *this, ac_t *ac, auth_cfg_t *auth) METHOD(cert_validator_t, validate, bool, private_acert_validator_t *this, certificate_t *subject, - certificate_t *issuer, bool online, u_int pathlen, bool anchor, - auth_cfg_t *auth) + certificate_t *issuer, u_int pathlen, bool anchor, auth_cfg_t *auth) { /* for X.509 end entity certs only */ if (pathlen == 0 && subject->get_type(subject) == CERT_X509) diff --git a/src/libstrongswan/plugins/constraints/constraints_validator.c b/src/libstrongswan/plugins/constraints/constraints_validator.c index fd8390d6cf..0f074c4f57 100644 --- a/src/libstrongswan/plugins/constraints/constraints_validator.c +++ b/src/libstrongswan/plugins/constraints/constraints_validator.c @@ -655,8 +655,7 @@ static bool check_policy_constraints(x509_t *issuer, u_int pathlen, METHOD(cert_validator_t, validate, bool, private_constraints_validator_t *this, certificate_t *subject, - certificate_t *issuer, bool online, u_int pathlen, bool anchor, - auth_cfg_t *auth) + certificate_t *issuer, u_int pathlen, bool anchor, auth_cfg_t *auth) { if (issuer->get_type(issuer) == CERT_X509 && subject->get_type(subject) == CERT_X509) diff --git a/src/libstrongswan/plugins/revocation/revocation_validator.c b/src/libstrongswan/plugins/revocation/revocation_validator.c index b554f406f4..7d867d57a1 100644 --- a/src/libstrongswan/plugins/revocation/revocation_validator.c +++ b/src/libstrongswan/plugins/revocation/revocation_validator.c @@ -826,10 +826,9 @@ static cert_validation_t check_crl(x509_t *subject, x509_t *issuer, return valid; } -METHOD(cert_validator_t, validate, bool, +METHOD(cert_validator_t, validate_online, bool, private_revocation_validator_t *this, certificate_t *subject, - certificate_t *issuer, bool online, u_int pathlen, bool anchor, - auth_cfg_t *auth) + certificate_t *issuer, u_int pathlen, bool anchor, auth_cfg_t *auth) { bool enable_ocsp, enable_crl; u_int timeout; @@ -840,7 +839,7 @@ METHOD(cert_validator_t, validate, bool, timeout = this->timeout; this->lock->unlock(this->lock); - if (online && (enable_ocsp || enable_crl) && + if ((enable_ocsp || enable_crl) && subject->get_type(subject) == CERT_X509 && issuer->get_type(issuer) == CERT_X509) { @@ -956,7 +955,7 @@ revocation_validator_t *revocation_validator_create() INIT(this, .public = { - .validator.validate = _validate, + .validator.validate_online = _validate_online, .reload = _reload, .destroy = _destroy, }, -- 2.47.2