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.

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;
 }
 
-/**
+CALLBACK(authority_destroy, void,
- * destroy a certification authority
+	authority_t *this)
- */
-static void authority_destroy(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,
+		if (certificate_matches(ca->cert, data->type, data->key, data->id))
-								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),
 	);
 

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.
 	 */

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;
+	if (rule == AUTH_RULE_CA_CERT)
-	cert = cred->add_cert(cred, 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;
+		return add_cert(auth, AUTH_RULE_SUBJECT_CERT, cert);
-		cert = cred->add_cert(cred, cert);
-		auth->cfg->add(auth->cfg, AUTH_RULE_SUBJECT_CERT, cert);
-		return TRUE;
 	}
 	return FALSE;
 }

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 (type == CERT_X509)
-	if (flag & X509_CA)
 	{
-		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(NULL);
-			return create_reply(err_msg);
+		}
+		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(),
 	);

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_ @}*/

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,