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 c9955885e703d982b97225552317589008e7c501..08f6ce5c77c01f9cfd7dda4eab3861675ae201f7 100644 (file)
--- 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 a6c2c5465106d3e89827c72976f1b054584702cd..187bede0bcdff667c1bf51a52d6d6c8a1a26ced0 100644 (file)
--- 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 0f8113f9290bee885b0634ec8026d71e6a27d8c0..2a4d58eabb976807e305f3f9649020d69f2b6b12 100644 (file)
--- 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 365cce8faa62359726651fc7904366d2a4cb8e0b..17f3ff0cd8c901d58af161d79ca1afdac2d6b7e9 100644 (file)
--- 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 6ce514786d4f9061a5f9e7aa9bd6dd57dce20f96..5cfb3f7aaa09427db1b347dc77c9230777ce0fe7 100644 (file)
--- 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 <credentials/credential_set.h>
 
@@ -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 17e3a0ae204dab611d503add3c362531edd1a876..c34500ac4cb6f1ee6293b1e5c09d9675ff5ac1b6 100644 (file)
--- 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,