From: Tobias Brunner Date: Thu, 20 Aug 2015 13:29:33 +0000 (+0200) Subject: stroke: Change how CA certificates are stored X-Git-Tag: 5.3.3rc1~17^2 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=517cc501ef6a3f20278352acb825abe97b5c1263;p=thirdparty%2Fstrongswan.git stroke: Change how CA certificates are stored Since 11c14bd2f5 CA certificates referenced in ca sections were enumerated by two credential sets if they were also stored in ipsec.d/cacerts. This caused duplicate certificate requests to get sent. All CA certificates, whether loaded automatically or via a ca section, are now stored in stroke_ca_t. Certificates referenced in ca sections are now also reloaded when `ipsec rereadcacerts` is used. --- diff --git a/src/libcharon/plugins/stroke/stroke_ca.c b/src/libcharon/plugins/stroke/stroke_ca.c index b470b81c68..13ed41e0ed 100644 --- a/src/libcharon/plugins/stroke/stroke_ca.c +++ b/src/libcharon/plugins/stroke/stroke_ca.c @@ -1,5 +1,5 @@ /* - * Copyright (C) 2008 Tobias Brunner + * Copyright (C) 2008-2015 Tobias Brunner * Copyright (C) 2008 Martin Willi * Hochschule fuer Technik Rapperswil * @@ -24,6 +24,13 @@ #include typedef struct private_stroke_ca_t private_stroke_ca_t; +typedef struct ca_section_t ca_section_t; +typedef struct ca_cert_t ca_cert_t; + +/** + * Provided by stroke_cred.c + */ +certificate_t *stroke_load_ca_cert(char *filename); /** * private data of stroke_ca @@ -41,17 +48,16 @@ struct private_stroke_ca_t { rwlock_t *lock; /** - * list of starters CA sections and its certificates (ca_section_t) + * list of CA sections and their certificates (ca_section_t) */ linked_list_t *sections; /** - * stroke credentials, stores our CA certificates + * list of all loaded CA certificates (ca_cert_t) */ - stroke_cred_t *cred; + linked_list_t *certs; }; -typedef struct ca_section_t ca_section_t; /** * loaded ipsec.conf CA sections @@ -64,7 +70,12 @@ struct ca_section_t { char *name; /** - * reference to cert in trusted_credential_t + * path/name of the certificate + */ + char *path; + + /** + * reference to cert */ certificate_t *cert; @@ -89,17 +100,38 @@ struct ca_section_t { char *certuribase; }; +/** + * loaded CA certificate + */ +struct ca_cert_t { + + /** + * reference to cert + */ + certificate_t *cert; + + /** + * The number of CA sections referring to this certificate + */ + u_int count; + + /** + * TRUE if this certificate was automatically loaded + */ + bool automatic; +}; + /** * create a new CA section */ -static ca_section_t *ca_section_create(char *name, certificate_t *cert) +static ca_section_t *ca_section_create(char *name, char *path) { ca_section_t *ca = malloc_thing(ca_section_t); ca->name = strdup(name); + ca->path = strdup(path); ca->crl = linked_list_create(); ca->ocsp = linked_list_create(); - ca->cert = cert; ca->hashes = linked_list_create(); ca->certuribase = NULL; return ca; @@ -115,10 +147,20 @@ static void ca_section_destroy(ca_section_t *this) this->hashes->destroy_offset(this->hashes, offsetof(identification_t, destroy)); this->cert->destroy(this->cert); free(this->certuribase); + free(this->path); free(this->name); free(this); } +/** + * Destroy a ca cert entry + */ +static void ca_cert_destroy(ca_cert_t *this) +{ + this->cert->destroy(this->cert); + free(this); +} + /** * Data for the certificate enumerator */ @@ -141,7 +183,7 @@ static void cert_data_destroy(cert_data_t *data) /** * filter function for certs enumerator */ -static bool certs_filter(cert_data_t *data, ca_section_t **in, +static bool certs_filter(cert_data_t *data, ca_cert_t **in, certificate_t **out) { public_key_t *public; @@ -192,7 +234,7 @@ METHOD(credential_set_t, create_cert_enumerator, enumerator_t*, ); this->lock->read_lock(this->lock); - enumerator = this->sections->create_enumerator(this->sections); + enumerator = this->certs->create_enumerator(this->certs); return enumerator_create_filter(enumerator, (void*)certs_filter, data, (void*)cert_data_destroy); } @@ -312,6 +354,81 @@ METHOD(credential_set_t, create_cdp_enumerator, enumerator_t*, data, (void*)cdp_data_destroy); } +/** + * Compare the given certificate to the ca_cert_t items in the list + */ +static bool match_cert(ca_cert_t *item, certificate_t *cert) +{ + return cert->equals(cert, item->cert); +} + +/** + * Match automatically added certificates and remove/destroy them if they are + * not referenced by CA sections. + */ +static bool remove_auto_certs(ca_cert_t *item, void *not_used) +{ + if (item->automatic) + { + item->automatic = FALSE; + if (!item->count) + { + ca_cert_destroy(item); + return TRUE; + } + } + return FALSE; +} + +/** + * Find the given certificate that was referenced by a section and remove it + * unless it was also loaded automatically or is used by other CA sections. + */ +static bool remove_cert(ca_cert_t *item, certificate_t *cert) +{ + if (item->count && cert->equals(cert, item->cert)) + { + if (--item->count == 0 && !item->automatic) + { + ca_cert_destroy(item); + return TRUE; + } + } + return FALSE; +} + +/** + * Adds a certificate to the certificate store + */ +static certificate_t *add_cert_internal(private_stroke_ca_t *this, + certificate_t *cert, bool automatic) +{ + ca_cert_t *found; + + if (this->certs->find_first(this->certs, (linked_list_match_t)match_cert, + (void**)&found, cert) == SUCCESS) + { + 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 (automatic) + { + found->automatic = TRUE; + } + else + { + found->count++; + } + return cert; +} + METHOD(stroke_ca_t, add, void, private_stroke_ca_t *this, stroke_msg_t *msg) { @@ -323,10 +440,10 @@ METHOD(stroke_ca_t, add, void, DBG1(DBG_CFG, "missing cacert parameter"); return; } - cert = this->cred->load_ca(this->cred, msg->add_ca.cacert); + cert = stroke_load_ca_cert(msg->add_ca.cacert); if (cert) { - ca = ca_section_create(msg->add_ca.name, cert); + ca = ca_section_create(msg->add_ca.name, msg->add_ca.cacert); if (msg->add_ca.crluri) { ca->crl->insert_last(ca->crl, strdup(msg->add_ca.crluri)); @@ -348,6 +465,7 @@ METHOD(stroke_ca_t, add, void, ca->certuribase = strdup(msg->add_ca.certuribase); } this->lock->write_lock(this->lock); + ca->cert = add_cert_internal(this, cert, FALSE); this->sections->insert_last(this->sections, ca); this->lock->unlock(this->lock); DBG1(DBG_CFG, "added ca '%s'", msg->add_ca.name); @@ -372,8 +490,12 @@ METHOD(stroke_ca_t, del, void, ca = NULL; } enumerator->destroy(enumerator); + if (ca) + { + this->certs->remove(this->certs, ca->cert, (void*)remove_cert); + } this->lock->unlock(this->lock); - if (ca == NULL) + if (!ca) { DBG1(DBG_CFG, "no ca named '%s' found\n", msg->del_ca.name); return; @@ -383,6 +505,88 @@ METHOD(stroke_ca_t, del, void, lib->credmgr->flush_cache(lib->credmgr, CERT_ANY); } +METHOD(stroke_ca_t, get_cert_ref, certificate_t*, + private_stroke_ca_t *this, certificate_t *cert) +{ + ca_cert_t *found; + + this->lock->read_lock(this->lock); + if (this->certs->find_first(this->certs, (linked_list_match_t)match_cert, + (void**)&found, cert) == SUCCESS) + { + cert->destroy(cert); + cert = found->cert->get_ref(found->cert); + } + this->lock->unlock(this->lock); + return cert; +} + +METHOD(stroke_ca_t, reload_certs, void, + private_stroke_ca_t *this) +{ + enumerator_t *enumerator; + certificate_t *cert; + ca_section_t *ca; + certificate_type_t type = CERT_X509; + + /* holding the write lock while loading/parsing certificates is not optimal, + * however, there usually are not that many ca sections configured */ + this->lock->write_lock(this->lock); + if (this->sections->get_count(this->sections)) + { + DBG1(DBG_CFG, "rereading ca certificates in ca sections"); + } + enumerator = this->sections->create_enumerator(this->sections); + while (enumerator->enumerate(enumerator, &ca)) + { + cert = stroke_load_ca_cert(ca->path); + if (cert) + { + if (cert->equals(cert, ca->cert)) + { + cert->destroy(cert); + } + else + { + this->certs->remove(this->certs, ca->cert, (void*)remove_cert); + ca->cert->destroy(ca->cert); + ca->cert = add_cert_internal(this, cert, FALSE); + } + } + else + { + DBG1(DBG_CFG, "failed to reload certificate '%s', removing ca '%s'", + ca->path, ca->name); + this->sections->remove_at(this->sections, enumerator); + this->certs->remove(this->certs, ca->cert, (void*)remove_cert); + ca_section_destroy(ca); + type = CERT_ANY; + } + } + enumerator->destroy(enumerator); + this->lock->unlock(this->lock); + lib->credmgr->flush_cache(lib->credmgr, type); +} + +METHOD(stroke_ca_t, replace_certs, void, + private_stroke_ca_t *this, mem_cred_t *certs) +{ + enumerator_t *enumerator; + certificate_t *cert; + + enumerator = certs->set.create_cert_enumerator(&certs->set, CERT_X509, + KEY_ANY, NULL, TRUE); + this->lock->write_lock(this->lock); + this->certs->remove(this->certs, NULL, (void*)remove_auto_certs); + while (enumerator->enumerate(enumerator, &cert)) + { + cert = add_cert_internal(this, cert->get_ref(cert), TRUE); + cert->destroy(cert); + } + this->lock->unlock(this->lock); + enumerator->destroy(enumerator); + lib->credmgr->flush_cache(lib->credmgr, CERT_X509); +} /** * list crl or ocsp URIs */ @@ -501,6 +705,7 @@ METHOD(stroke_ca_t, destroy, void, private_stroke_ca_t *this) { this->sections->destroy_function(this->sections, (void*)ca_section_destroy); + this->certs->destroy_function(this->certs, (void*)ca_cert_destroy); this->lock->destroy(this->lock); free(this); } @@ -508,7 +713,7 @@ METHOD(stroke_ca_t, destroy, void, /* * see header file */ -stroke_ca_t *stroke_ca_create(stroke_cred_t *cred) +stroke_ca_t *stroke_ca_create() { private_stroke_ca_t *this; @@ -524,12 +729,15 @@ stroke_ca_t *stroke_ca_create(stroke_cred_t *cred) .add = _add, .del = _del, .list = _list, + .get_cert_ref = _get_cert_ref, + .reload_certs = _reload_certs, + .replace_certs = _replace_certs, .check_for_hash_and_url = _check_for_hash_and_url, .destroy = _destroy, }, .sections = linked_list_create(), + .certs = linked_list_create(), .lock = rwlock_create(RWLOCK_TYPE_DEFAULT), - .cred = cred, ); return &this->public; diff --git a/src/libcharon/plugins/stroke/stroke_ca.h b/src/libcharon/plugins/stroke/stroke_ca.h index 21af912ea5..2740006e23 100644 --- a/src/libcharon/plugins/stroke/stroke_ca.h +++ b/src/libcharon/plugins/stroke/stroke_ca.h @@ -1,5 +1,5 @@ /* - * Copyright (C) 2008 Tobias Brunner + * Copyright (C) 2008-2015 Tobias Brunner * Copyright (C) 2008 Martin Willi * Hochschule fuer Technik Rapperswil * @@ -23,8 +23,7 @@ #define STROKE_CA_H_ #include - -#include "stroke_cred.h" +#include typedef struct stroke_ca_t stroke_ca_t; @@ -66,6 +65,29 @@ struct stroke_ca_t { */ void (*check_for_hash_and_url)(stroke_ca_t *this, certificate_t* cert); + /** + * Get a reference to a CA certificate if it is already stored, + * otherwise returns the same certificate. + * + * @param cert certificate to check + * @return reference to stored CA certifiate, or original + */ + certificate_t *(*get_cert_ref)(stroke_ca_t *this, certificate_t *cert); + + /** + * Reload CA certificates referenced in CA sections. Flushes the certificate + * cache. + */ + void (*reload_certs)(stroke_ca_t *this); + + /** + * Replace automatically loaded CA certificates. Flushes the certificate + * cache. + * + * @param certs credential set to take certificates from (not modified) + */ + void (*replace_certs)(stroke_ca_t *this, mem_cred_t *certs); + /** * Destroy a stroke_ca instance. */ @@ -75,6 +97,6 @@ struct stroke_ca_t { /** * Create a stroke_ca instance. */ -stroke_ca_t *stroke_ca_create(stroke_cred_t *cred); +stroke_ca_t *stroke_ca_create(); #endif /** STROKE_CA_H_ @}*/ diff --git a/src/libcharon/plugins/stroke/stroke_cred.c b/src/libcharon/plugins/stroke/stroke_cred.c index c7d91e82c0..42928882a8 100644 --- a/src/libcharon/plugins/stroke/stroke_cred.c +++ b/src/libcharon/plugins/stroke/stroke_cred.c @@ -74,11 +74,6 @@ struct private_stroke_cred_t { */ mem_cred_t *creds; - /** - * CA certificates - */ - mem_cred_t *cacerts; - /** * Attribute Authority certificates */ @@ -94,6 +89,11 @@ struct private_stroke_cred_t { * cache CRLs to disk? */ bool cachecrl; + + /** + * CA certificate store + */ + stroke_ca_t *ca; }; /** Length of smartcard specifier parts (module, keyid) */ @@ -385,17 +385,17 @@ static certificate_t *load_ca_cert(char *filename, bool force_ca_cert) return NULL; } -METHOD(stroke_cred_t, load_ca, certificate_t*, - private_stroke_cred_t *this, char *filename) +/** + * Used by stroke_ca.c + */ +certificate_t *stroke_load_ca_cert(char *filename) { - certificate_t *cert; + bool force_ca_cert; - cert = load_ca_cert(filename, this->force_ca_cert); - if (cert) - { - return this->cacerts->get_cert_ref(this->cacerts, cert); - } - return NULL; + force_ca_cert = lib->settings->get_bool(lib->settings, + "%s.plugins.stroke.ignore_missing_ca_basic_constraint", + FALSE, lib->ns); + return load_ca_cert(filename, force_ca_cert); } /** @@ -409,6 +409,7 @@ static void load_x509_ca(private_stroke_cred_t *this, char *file, cert = load_ca_cert(file, this->force_ca_cert); if (cert) { + cert = this->ca->get_cert_ref(this->ca, cert); creds->add_cert(creds, TRUE, cert); } else @@ -1346,9 +1347,14 @@ static void load_secrets(private_stroke_cred_t *this, mem_cred_t *secrets, */ static void load_certs(private_stroke_cred_t *this) { + mem_cred_t *creds; + DBG1(DBG_CFG, "loading ca certificates from '%s'", CA_CERTIFICATE_DIR); - load_certdir(this, CA_CERTIFICATE_DIR, CERT_X509, X509_CA, this->cacerts); + creds = mem_cred_create(); + load_certdir(this, CA_CERTIFICATE_DIR, CERT_X509, X509_CA, creds); + this->ca->replace_certs(this->ca, creds); + creds->destroy(creds); DBG1(DBG_CFG, "loading aa certificates from '%s'", AA_CERTIFICATE_DIR); @@ -1378,24 +1384,28 @@ METHOD(stroke_cred_t, reread, void, DBG1(DBG_CFG, "rereading secrets"); load_secrets(this, NULL, this->secrets_file, 0, prompt); } - creds = mem_cred_create(); if (msg->reread.flags & REREAD_CACERTS) { + /* first reload certificates in ca sections, so we can refer to them */ + this->ca->reload_certs(this->ca); + DBG1(DBG_CFG, "rereading ca certificates from '%s'", CA_CERTIFICATE_DIR); + creds = mem_cred_create(); load_certdir(this, CA_CERTIFICATE_DIR, CERT_X509, X509_CA, creds); - this->cacerts->replace_certs(this->cacerts, creds, FALSE); - lib->credmgr->flush_cache(lib->credmgr, CERT_X509); + this->ca->replace_certs(this->ca, creds); + creds->destroy(creds); } if (msg->reread.flags & REREAD_AACERTS) { DBG1(DBG_CFG, "rereading aa certificates from '%s'", AA_CERTIFICATE_DIR); + creds = mem_cred_create(); load_certdir(this, AA_CERTIFICATE_DIR, CERT_X509, X509_AA, creds); this->aacerts->replace_certs(this->aacerts, creds, FALSE); + creds->destroy(creds); lib->credmgr->flush_cache(lib->credmgr, CERT_X509); } - creds->destroy(creds); if (msg->reread.flags & REREAD_OCSPCERTS) { DBG1(DBG_CFG, "rereading ocsp signer certificates from '%s'", @@ -1427,10 +1437,8 @@ METHOD(stroke_cred_t, destroy, void, private_stroke_cred_t *this) { lib->credmgr->remove_set(lib->credmgr, &this->aacerts->set); - lib->credmgr->remove_set(lib->credmgr, &this->cacerts->set); lib->credmgr->remove_set(lib->credmgr, &this->creds->set); this->aacerts->destroy(this->aacerts); - this->cacerts->destroy(this->cacerts); this->creds->destroy(this->creds); free(this); } @@ -1438,7 +1446,7 @@ METHOD(stroke_cred_t, destroy, void, /* * see header file */ -stroke_cred_t *stroke_cred_create() +stroke_cred_t *stroke_cred_create(stroke_ca_t *ca) { private_stroke_cred_t *this; @@ -1452,7 +1460,6 @@ stroke_cred_t *stroke_cred_create() .cache_cert = (void*)_cache_cert, }, .reread = _reread, - .load_ca = _load_ca, .load_peer = _load_peer, .load_pubkey = _load_pubkey, .add_shared = _add_shared, @@ -1463,12 +1470,11 @@ stroke_cred_t *stroke_cred_create() "%s.plugins.stroke.secrets_file", SECRETS_FILE, lib->ns), .creds = mem_cred_create(), - .cacerts = mem_cred_create(), .aacerts = mem_cred_create(), + .ca = ca, ); lib->credmgr->add_set(lib->credmgr, &this->creds->set); - lib->credmgr->add_set(lib->credmgr, &this->cacerts->set); lib->credmgr->add_set(lib->credmgr, &this->aacerts->set); this->force_ca_cert = lib->settings->get_bool(lib->settings, diff --git a/src/libcharon/plugins/stroke/stroke_cred.h b/src/libcharon/plugins/stroke/stroke_cred.h index 9434629efa..33a0e3531c 100644 --- a/src/libcharon/plugins/stroke/stroke_cred.h +++ b/src/libcharon/plugins/stroke/stroke_cred.h @@ -29,6 +29,8 @@ #include #include +#include "stroke_ca.h" + typedef struct stroke_cred_t stroke_cred_t; /** @@ -49,17 +51,6 @@ struct stroke_cred_t { */ void (*reread)(stroke_cred_t *this, stroke_msg_t *msg, FILE *prompt); - /** - * Load a CA certificate. - * - * This method does not add the loaded CA certificate to the internal - * credentail set, but returns it only. - * - * @param filename file to load CA cert from - * @return loaded certificate, or NULL - */ - certificate_t* (*load_ca)(stroke_cred_t *this, char *filename); - /** * Load a peer certificate and serve it through the credential_set. * @@ -103,6 +94,6 @@ struct stroke_cred_t { /** * Create a stroke_cred instance. */ -stroke_cred_t *stroke_cred_create(); +stroke_cred_t *stroke_cred_create(stroke_ca_t *ca); #endif /** STROKE_CRED_H_ @}*/ diff --git a/src/libcharon/plugins/stroke/stroke_socket.c b/src/libcharon/plugins/stroke/stroke_socket.c index db7e66f143..29563e32f9 100644 --- a/src/libcharon/plugins/stroke/stroke_socket.c +++ b/src/libcharon/plugins/stroke/stroke_socket.c @@ -779,10 +779,10 @@ stroke_socket_t *stroke_socket_create() "%s.plugins.stroke.prevent_loglevel_changes", FALSE, lib->ns), ); - this->cred = stroke_cred_create(); + this->ca = stroke_ca_create(); + this->cred = stroke_cred_create(this->ca); this->attribute = stroke_attribute_create(); this->handler = stroke_handler_create(); - this->ca = stroke_ca_create(this->cred); this->config = stroke_config_create(this->ca, this->cred, this->attribute); this->control = stroke_control_create(); this->list = stroke_list_create(this->attribute);