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 b470b81c68cab76e353203a2b2f34af152d28cf0..13ed41e0ed3d73559c29a57dab130c3f34868599 100644 (file)
--- 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 <daemon.h>
 
 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 21af912ea5f2664ab414d94b10fc7628a3525365..2740006e235866922de4e88a61c4d3d08141381c 100644 (file)
--- 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 <stroke_msg.h>
-
-#include "stroke_cred.h"
+#include <credentials/sets/mem_cred.h>
 
 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 c7d91e82c0a4ceaf645423cb34aa538c4a64e61f..42928882a84eff6e638df7bb376baa7fb6378307 100644 (file)
--- 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 9434629efa4d0abf3f4e1438c54ddab8a3cd47eb..33a0e3531c55c6d00a857ad5549c3dcd008ea1bc 100644 (file)
--- a/src/libcharon/plugins/stroke/stroke_cred.h
+++ b/src/libcharon/plugins/stroke/stroke_cred.h
@@ -29,6 +29,8 @@
 #include <credentials/certificates/certificate.h>
 #include <collections/linked_list.h>
 
+#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 db7e66f14373a6af65d2d800da946f946a74181c..29563e32f97b38b94f42c94befff84a0ace04d22 100644 (file)
--- 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);