From: Tobias Brunner Date: Wed, 20 May 2020 14:50:11 +0000 (+0200) Subject: vici: Keep track of all CA certificates in vici_authority_t X-Git-Tag: 5.9.0rc1~8^2 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=3c5e7eaa88b0068d62a9be139616fb1291e36902;p=thirdparty%2Fstrongswan.git vici: Keep track of all CA certificates in vici_authority_t This way we only have one reference for each CA certificate, whether it is loaded in an authority section, a connection or via load-certs() command. It also avoids enumerating CA certificates multiple times if they are loaded in different ways. --- diff --git a/src/libcharon/plugins/vici/vici_authority.c b/src/libcharon/plugins/vici/vici_authority.c index c9955885e7..08f6ce5c77 100644 --- a/src/libcharon/plugins/vici/vici_authority.c +++ b/src/libcharon/plugins/vici/vici_authority.c @@ -44,10 +44,15 @@ struct private_vici_authority_t { vici_dispatcher_t *dispatcher; /** - * List of certification authorities + * List of certification authorities (authority_t*) */ linked_list_t *authorities; + /** + * List of CA certificates (ca_cert_t*) + */ + linked_list_t *certs; + /** * rwlock to lock access to certification authorities */ @@ -104,10 +109,8 @@ static authority_t *authority_create(char *name) return authority; } -/** - * destroy a certification authority - */ -static void authority_destroy(authority_t *this) +CALLBACK(authority_destroy, void, + authority_t *this) { this->crl_uris->destroy_function(this->crl_uris, free); this->ocsp_uris->destroy_function(this->ocsp_uris, free); @@ -117,6 +120,108 @@ static void authority_destroy(authority_t *this) free(this); } +typedef struct ca_cert_t ca_cert_t; + +/** + * Loaded CA certificate. + */ +struct ca_cert_t { + + /** + * Reference to certificate. + */ + certificate_t *cert; + + /** + * The number of authority sections referring to this certificate. + */ + u_int count; + + /** + * TRUE if this certificate was (also) added externally. + */ + bool external; +}; + +/** + * Destroy a CA certificate entry + */ +CALLBACK(ca_cert_destroy, void, + ca_cert_t *this) +{ + this->cert->destroy(this->cert); + free(this); +} + +CALLBACK(match_cert, bool, + ca_cert_t *item, va_list args) +{ + certificate_t *cert; + + VA_ARGS_VGET(args, cert); + return cert->equals(cert, item->cert); +} + +/** + * Add a CA certificate to the local store + */ +static certificate_t *add_cert_internal(private_vici_authority_t *this, + certificate_t *cert, bool external) +{ + ca_cert_t *found; + + if (this->certs->find_first(this->certs, match_cert, (void**)&found, cert)) + { + cert->destroy(cert); + cert = found->cert->get_ref(found->cert); + } + else + { + INIT(found, + .cert = cert->get_ref(cert) + ); + this->certs->insert_first(this->certs, found); + } + if (external) + { + found->external = TRUE; + } + else + { + found->count++; + } + return cert; +} + +CALLBACK(remove_external_certs, bool, + ca_cert_t *item, void *unused) +{ + if (item->external) + { + item->external = FALSE; + + if (!item->count) + { + ca_cert_destroy(item); + return TRUE; + } + } + return FALSE; +} + +CALLBACK2(remove_cert, bool, + ca_cert_t *item, certificate_t *cert) +{ + if (cert == item->cert) + { + if (--item->count == 0 && !item->external) + { + ca_cert_destroy(item); + return TRUE; + } + } + return FALSE; +} /** * Create a (error) reply message @@ -431,6 +536,9 @@ CALLBACK(authority_sn, bool, request->this->lock->write_lock(request->this->lock); + data->authority->cert = add_cert_internal(request->this, + data->authority->cert, FALSE); + authorities = request->this->authorities; enumerator = authorities->create_enumerator(authorities); while (enumerator->enumerate(enumerator, &authority)) @@ -492,6 +600,7 @@ CALLBACK(unload_authority, vici_message_t*, if (streq(authority->name, authority_name)) { this->authorities->remove_at(this->authorities, enumerator); + this->certs->remove(this->certs, authority->cert, remove_cert); authority_destroy(authority); found = TRUE; break; @@ -638,17 +747,16 @@ CALLBACK(cert_data_destroy, void, CALLBACK(certs_filter, bool, cert_data_t *data, enumerator_t *orig, va_list args) { - authority_t *authority; + ca_cert_t *ca; certificate_t **out; VA_ARGS_VGET(args, out); - while (orig->enumerate(orig, &authority)) + while (orig->enumerate(orig, &ca)) { - if (certificate_matches(authority->cert, data->type, data->key, - data->id)) + if (certificate_matches(ca->cert, data->type, data->key, data->id)) { - *out = authority->cert; + *out = ca->cert; return TRUE; } } @@ -670,7 +778,7 @@ METHOD(credential_set_t, create_cert_enumerator, enumerator_t*, ); this->lock->read_lock(this->lock); - enumerator = this->authorities->create_enumerator(this->authorities); + enumerator = this->certs->create_enumerator(this->certs); return enumerator_create_filter(enumerator, certs_filter, data, cert_data_destroy); } @@ -760,6 +868,23 @@ METHOD(credential_set_t, create_cdp_enumerator, enumerator_t*, (void*)create_inner_cdp, data, cert_data_destroy); } +METHOD(vici_authority_t, add_ca_cert, certificate_t*, + private_vici_authority_t *this, certificate_t *cert) +{ + this->lock->write_lock(this->lock); + cert = add_cert_internal(this, cert, TRUE); + this->lock->unlock(this->lock); + return cert; +} + +METHOD(vici_authority_t, clear_ca_certs, void, + private_vici_authority_t *this) +{ + this->lock->write_lock(this->lock); + this->certs->remove(this->certs, NULL, remove_external_certs); + this->lock->unlock(this->lock); +} + METHOD(vici_authority_t, destroy, void, private_vici_authority_t *this) { @@ -767,6 +892,7 @@ METHOD(vici_authority_t, destroy, void, this->authorities->destroy_function(this->authorities, (void*)authority_destroy); + this->certs->destroy_function(this->certs, ca_cert_destroy); this->lock->destroy(this->lock); free(this); } @@ -787,10 +913,13 @@ vici_authority_t *vici_authority_create(vici_dispatcher_t *dispatcher) .create_cdp_enumerator = _create_cdp_enumerator, .cache_cert = (void*)nop, }, + .add_ca_cert = _add_ca_cert, + .clear_ca_certs = _clear_ca_certs, .destroy = _destroy, }, .dispatcher = dispatcher, .authorities = linked_list_create(), + .certs = linked_list_create(), .lock = rwlock_create(RWLOCK_TYPE_DEFAULT), ); diff --git a/src/libcharon/plugins/vici/vici_authority.h b/src/libcharon/plugins/vici/vici_authority.h index a6c2c54651..187bede0bc 100644 --- a/src/libcharon/plugins/vici/vici_authority.h +++ b/src/libcharon/plugins/vici/vici_authority.h @@ -1,4 +1,5 @@ /* + * Copyright (C) 2020 Tobias Brunner * Copyright (C) 2015 Andreas Steffen * HSR Hochschule fuer Technik Rapperswil * @@ -22,7 +23,6 @@ #define VICI_AUTHORITY_H_ #include "vici_dispatcher.h" -#include "vici_cred.h" typedef struct vici_authority_t vici_authority_t; @@ -36,6 +36,20 @@ struct vici_authority_t { */ credential_set_t set; + /** + * Add a CA certificate and return a reference if it is already stored, + * otherwise returns the same certificate. + * + * @param cert certificate to check + * @return reference to stored CA certificate, or original + */ + certificate_t *(*add_ca_cert)(vici_authority_t *this, certificate_t *cert); + + /** + * Remove CA certificates added via add_ca_cert(). + */ + void (*clear_ca_certs)(vici_authority_t *this); + /** * Destroy a vici_authority_t. */ diff --git a/src/libcharon/plugins/vici/vici_config.c b/src/libcharon/plugins/vici/vici_config.c index 0f8113f929..2a4d58eabb 100644 --- a/src/libcharon/plugins/vici/vici_config.c +++ b/src/libcharon/plugins/vici/vici_config.c @@ -1444,10 +1444,19 @@ CALLBACK(parse_cert_policy, bool, */ static bool add_cert(auth_data_t *auth, auth_rule_t rule, certificate_t *cert) { + vici_authority_t *authority; vici_cred_t *cred; - cred = auth->request->this->cred; - cert = cred->add_cert(cred, cert); + if (rule == AUTH_RULE_CA_CERT) + { + authority = auth->request->this->authority; + cert = authority->add_ca_cert(authority, cert); + } + else + { + cred = auth->request->this->cred; + cert = cred->add_cert(cred, cert); + } auth->cfg->add(auth->cfg, rule, cert); return TRUE; } @@ -1492,17 +1501,13 @@ CALLBACK(parse_cacerts, bool, CALLBACK(parse_pubkeys, bool, auth_data_t *auth, chunk_t v) { - vici_cred_t *cred; certificate_t *cert; cert = lib->creds->create(lib->creds, CRED_CERTIFICATE, CERT_TRUSTED_PUBKEY, BUILD_BLOB_PEM, v, BUILD_END); if (cert) { - cred = auth->request->this->cred; - cert = cred->add_cert(cred, cert); - auth->cfg->add(auth->cfg, AUTH_RULE_SUBJECT_CERT, cert); - return TRUE; + return add_cert(auth, AUTH_RULE_SUBJECT_CERT, cert); } return FALSE; } diff --git a/src/libcharon/plugins/vici/vici_cred.c b/src/libcharon/plugins/vici/vici_cred.c index 365cce8faa..17f3ff0cd8 100644 --- a/src/libcharon/plugins/vici/vici_cred.c +++ b/src/libcharon/plugins/vici/vici_cred.c @@ -51,6 +51,11 @@ struct private_vici_cred_t { */ vici_dispatcher_t *dispatcher; + /** + * CA certificate store + */ + vici_authority_t *authority; + /** * credentials */ @@ -178,19 +183,24 @@ CALLBACK(load_cert, vici_message_t*, } DBG1(DBG_CFG, "loaded certificate '%Y'", cert->get_subject(cert)); - /* check if CA certificate has CA basic constraint set */ - if (flag & X509_CA) + if (type == CERT_X509) { - char err_msg[] = "ca certificate lacks CA basic constraint, rejected"; x509 = (x509_t*)cert; - - if (!(x509->get_flags(x509) & X509_CA)) + if (x509->get_flags(x509) & X509_CA) { + cert = this->authority->add_ca_cert(this->authority, cert); cert->destroy(cert); - DBG1(DBG_CFG, " %s", err_msg); - return create_reply(err_msg); + return create_reply(NULL); + } + else if (flag & X509_CA) + { + char msg[] = "ca certificate lacks CA basic constraint, rejected"; + cert->destroy(cert); + DBG1(DBG_CFG, " %s", msg); + return create_reply(msg); } } + if (type == CERT_X509_CRL) { this->creds->add_crl(this->creds, (crl_t*)cert); @@ -535,6 +545,7 @@ CALLBACK(clear_creds, vici_message_t*, private_vici_cred_t *this, char *name, u_int id, vici_message_t *message) { this->creds->clear(this->creds); + this->authority->clear_ca_certs(this->authority); lib->credmgr->flush_cache(lib->credmgr, CERT_ANY); return create_reply(NULL); @@ -603,7 +614,8 @@ METHOD(vici_cred_t, destroy, void, /** * See header */ -vici_cred_t *vici_cred_create(vici_dispatcher_t *dispatcher) +vici_cred_t *vici_cred_create(vici_dispatcher_t *dispatcher, + vici_authority_t *authority) { private_vici_cred_t *this; @@ -620,6 +632,7 @@ vici_cred_t *vici_cred_create(vici_dispatcher_t *dispatcher) .destroy = _destroy, }, .dispatcher = dispatcher, + .authority = authority, .creds = mem_cred_create(), .pins = mem_cred_create(), ); diff --git a/src/libcharon/plugins/vici/vici_cred.h b/src/libcharon/plugins/vici/vici_cred.h index 6ce514786d..5cfb3f7aaa 100644 --- a/src/libcharon/plugins/vici/vici_cred.h +++ b/src/libcharon/plugins/vici/vici_cred.h @@ -25,6 +25,7 @@ #define VICI_CRED_H_ #include "vici_dispatcher.h" +#include "vici_authority.h" #include @@ -58,8 +59,10 @@ struct vici_cred_t { * Create a vici_cred instance. * * @param dispatcher dispatcher to receive requests from + * @param authority CA certificate storage * @return credential backend */ -vici_cred_t *vici_cred_create(vici_dispatcher_t *dispatcher); +vici_cred_t *vici_cred_create(vici_dispatcher_t *dispatcher, + vici_authority_t *authority); #endif /** VICI_CRED_H_ @}*/ diff --git a/src/libcharon/plugins/vici/vici_plugin.c b/src/libcharon/plugins/vici/vici_plugin.c index 17e3a0ae20..c34500ac4c 100644 --- a/src/libcharon/plugins/vici/vici_plugin.c +++ b/src/libcharon/plugins/vici/vici_plugin.c @@ -127,8 +127,8 @@ static bool register_vici(private_vici_plugin_t *this, { this->query = vici_query_create(this->dispatcher); this->control = vici_control_create(this->dispatcher); - this->cred = vici_cred_create(this->dispatcher); this->authority = vici_authority_create(this->dispatcher); + this->cred = vici_cred_create(this->dispatcher, this->authority); lib->credmgr->add_set(lib->credmgr, &this->cred->set); lib->credmgr->add_set(lib->credmgr, &this->authority->set); this->config = vici_config_create(this->dispatcher, this->authority,