dynsec: Fix role clientlists not being freed.

plugins/dynamic-security/CMakeLists.txt

@@ -17,6 +17,7 @@ add_library(mosquitto_dynamic_security SHARED
 	acl.c
 	auth.c
 	clients.c
+	clientlist.c
 	dynamic_security.h
 	groups.c
 	json_help.c

plugins/dynamic-security/Makefile

@@ -9,6 +9,7 @@ OBJS=	\
 		acl.o \
 		auth.o \
 		clients.o \
+		clientlist.o \
 		groups.o \
 		json_help.o \
 		plugin.o \
@@ -30,6 +31,9 @@ auth.o : auth.c dynamic_security.h
 clients.o : clients.c dynamic_security.h
 	${CROSS_COMPILE}${CC} $(LOCAL_CPPFLAGS) $(PLUGIN_CPPFLAGS) $(PLUGIN_CFLAGS) -c $< -o $@
 
+clientlist.o : clientlist.c dynamic_security.h
+	${CROSS_COMPILE}${CC} $(LOCAL_CPPFLAGS) $(PLUGIN_CPPFLAGS) $(PLUGIN_CFLAGS) -c $< -o $@
+
 groups.o : groups.c dynamic_security.h
 	${CROSS_COMPILE}${CC} $(LOCAL_CPPFLAGS) $(PLUGIN_CPPFLAGS) $(PLUGIN_CFLAGS) -c $< -o $@
 

plugins/dynamic-security/clientlist.c

@@ -0,0 +1,143 @@
+/*
+Copyright (c) 2020 Roger Light <roger@atchoo.org>
+
+All rights reserved. This program and the accompanying materials
+are made available under the terms of the Eclipse Public License v1.0
+and Eclipse Distribution License v1.0 which accompany this distribution.
+
+The Eclipse Public License is available at
+   http://www.eclipse.org/legal/epl-v10.html
+and the Eclipse Distribution License is available at
+  http://www.eclipse.org/org/documents/edl-v10.php.
+
+Contributors:
+   Roger Light - initial implementation and documentation.
+*/
+
+#include "config.h"
+
+#include <cJSON.h>
+#include <stdio.h>
+#include <uthash.h>
+
+#include "mosquitto.h"
+#include "mosquitto_broker.h"
+#include "json_help.h"
+
+#include "dynamic_security.h"
+
+/* ################################################################
+ * #
+ * # Plugin global variables
+ * #
+ * ################################################################ */
+
+/* ################################################################
+ * #
+ * # Function declarations
+ * #
+ * ################################################################ */
+
+/* ################################################################
+ * #
+ * # Local variables
+ * #
+ * ################################################################ */
+
+
+/* ################################################################
+ * #
+ * # Utility functions
+ * #
+ * ################################################################ */
+
+static int dynsec_clientlist__cmp(void *a, void *b)
+{
+	struct dynsec__clientlist *clientlist_a = a;
+	struct dynsec__clientlist *clientlist_b = b;
+
+	return strcmp(clientlist_a->client->username, clientlist_b->client->username);
+}
+
+
+void dynsec_clientlist__kick_all(struct dynsec__clientlist *base_clientlist)
+{
+	struct dynsec__clientlist *clientlist, *clientlist_tmp;
+
+	HASH_ITER(hh, base_clientlist, clientlist, clientlist_tmp){
+		mosquitto_kick_client_by_username(clientlist->client->username, false);
+	}
+}
+
+cJSON *dynsec_clientlist__all_to_json(struct dynsec__clientlist *base_clientlist)
+{
+	struct dynsec__clientlist *clientlist, *clientlist_tmp;
+	cJSON *j_clients, *j_client;
+
+	j_clients = cJSON_CreateArray();
+	if(j_clients == NULL) return NULL;
+
+	HASH_ITER(hh, base_clientlist, clientlist, clientlist_tmp){
+		j_client = cJSON_CreateObject();
+		if(j_client == NULL){
+			cJSON_Delete(j_clients);
+			return NULL;
+		}
+		cJSON_AddItemToArray(j_clients, j_client);
+
+		if(cJSON_AddStringToObject(j_client, "username", clientlist->client->username) == NULL
+				|| (clientlist->priority != -1 && cJSON_AddIntToObject(j_client, "priority", clientlist->priority) == NULL)
+				){
+
+			cJSON_Delete(j_clients);
+			return NULL;
+		}
+	}
+	return j_clients;
+}
+
+
+int dynsec_clientlist__add(struct dynsec__clientlist **base_clientlist, struct dynsec__client *client, int priority)
+{
+	struct dynsec__clientlist *clientlist;
+
+	HASH_FIND(hh, *base_clientlist, client->username, strlen(client->username), clientlist);
+	if(clientlist != NULL){
+		/* Client is already in the group */
+		return MOSQ_ERR_SUCCESS;
+	}
+
+	clientlist = mosquitto_malloc(sizeof(struct dynsec__clientlist));
+	if(clientlist == NULL){
+		return MOSQ_ERR_NOMEM;
+	}
+
+	clientlist->client = client;
+	clientlist->priority = priority;
+	HASH_ADD_KEYPTR_INORDER(hh, *base_clientlist, client->username, strlen(client->username), clientlist, dynsec_clientlist__cmp);
+
+	return MOSQ_ERR_SUCCESS;
+}
+
+
+void dynsec_clientlist__cleanup(struct dynsec__clientlist **base_clientlist)
+{
+	struct dynsec__clientlist *clientlist, *clientlist_tmp;
+
+	HASH_ITER(hh, *base_clientlist, clientlist, clientlist_tmp){
+		HASH_DELETE(hh, *base_clientlist, clientlist);
+		mosquitto_free(clientlist);
+	}
+}
+
+
+void dynsec_clientlist__remove(struct dynsec__clientlist **base_clientlist, struct dynsec__client *client)
+{
+	struct dynsec__clientlist *clientlist;
+
+	HASH_FIND(hh, *base_clientlist, client->username, strlen(client->username), clientlist);
+	if(clientlist){
+		HASH_DELETE(hh, *base_clientlist, clientlist);
+		mosquitto_free(clientlist);
+	}
+}

plugins/dynamic-security/clients.c

@@ -49,14 +49,6 @@ static struct dynsec__client *local_clients = NULL;
  * #
  * ################################################################ */
 
-int dynsec_clientlist__cmp(void *a, void *b)
-{
-	struct dynsec__clientlist *clientlist_a = a;
-	struct dynsec__clientlist *clientlist_b = b;
-
-	return strcmp(clientlist_a->username, clientlist_b->username);
-}
-
 static int client_cmp(void *a, void *b)
 {
 	struct dynsec__client *client_a = a;

plugins/dynamic-security/dynamic_security.h

@@ -54,7 +54,6 @@ Contributors:
 
 struct dynsec__clientlist{
 	UT_hash_handle hh;
-	char *username;
 	struct dynsec__client *client;
 	int priority;
 };
@@ -186,10 +185,18 @@ int dynsec_clients__process_set_id(cJSON *j_responses, struct mosquitto *context
 int dynsec_clients__process_set_password(cJSON *j_responses, struct mosquitto *context, cJSON *command, char *correlation_data);
 struct dynsec__client *dynsec_clients__find(const char *username);
 
-cJSON *dynsec_clientlists__all_to_json(struct dynsec__clientlist *base_clientlist);
-int dynsec_clientlist__cmp(void *a, void *b);
-void dynsec_clientlist__kick_all(struct dynsec__clientlist *base_clientlist);
 
+/* ################################################################
+ * #
+ * # Client List Functions
+ * #
+ * ################################################################ */
+
+cJSON *dynsec_clientlist__all_to_json(struct dynsec__clientlist *base_clientlist);
+int dynsec_clientlist__add(struct dynsec__clientlist **base_clientlist, struct dynsec__client *client, int priority);
+void dynsec_clientlist__cleanup(struct dynsec__clientlist **base_clientlist);
+void dynsec_clientlist__remove(struct dynsec__clientlist **base_clientlist, struct dynsec__client *client);
+void dynsec_clientlist__kick_all(struct dynsec__clientlist *base_clientlist);
 
 
 /* ################################################################

plugins/dynamic-security/groups.c

@@ -82,43 +82,6 @@ int dynsec_grouplist__cmp(void *a, void *b)
 	}
 }
 
-void dynsec_clientlist__kick_all(struct dynsec__clientlist *base_clientlist)
-{
-	struct dynsec__clientlist *clientlist, *clientlist_tmp;
-
-	HASH_ITER(hh, base_clientlist, clientlist, clientlist_tmp){
-		mosquitto_kick_client_by_username(clientlist->client->username, false);
-	}
-}
-
-cJSON *dynsec_clientlists__all_to_json(struct dynsec__clientlist *base_clientlist)
-{
-	struct dynsec__clientlist *clientlist, *clientlist_tmp;
-	cJSON *j_clients, *j_client;
-
-	j_clients = cJSON_CreateArray();
-	if(j_clients == NULL) return NULL;
-
-	HASH_ITER(hh, base_clientlist, clientlist, clientlist_tmp){
-		j_client = cJSON_CreateObject();
-		if(j_client == NULL){
-			cJSON_Delete(j_clients);
-			return NULL;
-		}
-		cJSON_AddItemToArray(j_clients, j_client);
-
-		if(cJSON_AddStringToObject(j_client, "username", clientlist->client->username) == NULL
-				|| (clientlist->priority != -1 && cJSON_AddIntToObject(j_client, "priority", clientlist->priority) == NULL)
-				){
-
-			cJSON_Delete(j_clients);
-			return NULL;
-		}
-	}
-	return j_clients;
-}
-
-
 cJSON *dynsec_grouplists__all_to_json(struct dynsec__grouplist *base_grouplist)
 {
 	struct dynsec__grouplist *grouplist, *grouplist_tmp;
@@ -374,7 +337,7 @@ static int dynsec__config_add_groups(cJSON *j_groups)
 		}
 		cJSON_AddItemToObject(j_group, "roles", j_roles);
 
-		j_clients = dynsec_clientlists__all_to_json(group->clientlist);
+		j_clients = dynsec_clientlist__all_to_json(group->clientlist);
 		if(j_clients == NULL){
 			return 1;
 		}
@@ -527,6 +490,7 @@ int dynsec_groups__add_client(const char *username, const char *groupname, int p
 	struct dynsec__clientlist *clientlist;
 	struct dynsec__group *group;
 	struct dynsec__grouplist *grouplist;
+	int rc;
 
 	client = dynsec_clients__find(username);
 	if(client == NULL){
@@ -544,19 +508,17 @@ int dynsec_groups__add_client(const char *username, const char *groupname, int p
 		return MOSQ_ERR_SUCCESS;
 	}
 
-	clientlist = mosquitto_malloc(sizeof(struct dynsec__clientlist));
+	rc = dynsec_clientlist__add(&group->clientlist, client, priority);
+	if(rc){
+		return rc;
+	}
 	grouplist = mosquitto_malloc(sizeof(struct dynsec__grouplist));
-	if(clientlist == NULL || grouplist == NULL){
-		mosquitto_free(clientlist);
+	if(grouplist == NULL){
+		dynsec_clientlist__remove(&group->clientlist, client);
 		mosquitto_free(grouplist);
 		return MOSQ_ERR_UNKNOWN;
 	}
 
-	clientlist->username = client->username;
-	clientlist->client = client;
-	clientlist->priority = priority;
-	HASH_ADD_KEYPTR_INORDER(hh, group->clientlist, clientlist->username, strlen(clientlist->username), clientlist, dynsec_clientlist__cmp);
-
 	grouplist->groupname = group->groupname;
 	grouplist->group = group;
 	grouplist->priority = priority;
@@ -614,17 +576,6 @@ int dynsec_groups__process_add_client(cJSON *j_responses, struct mosquitto *cont
 }
 
 
-static void dynsec_clientlists__remove(struct dynsec__clientlist **base_clientlist, const char *username)
-{
-	struct dynsec__clientlist *clientlist;
-
-	HASH_FIND(hh, *base_clientlist, username, strlen(username), clientlist);
-	if(clientlist){
-		HASH_DELETE(hh, *base_clientlist, clientlist);
-		mosquitto_free(clientlist);
-	}
-}
-
 static void dynsec_grouplists__remove(struct dynsec__grouplist **base_grouplist, const char *groupname)
 {
 	struct dynsec__grouplist *grouplist;
@@ -666,7 +617,7 @@ int dynsec_groups__remove_client(const char *username, const char *groupname, bo
 		return ERR_GROUP_NOT_FOUND;
 	}
 
-	dynsec_clientlists__remove(&group->clientlist, username);
+	dynsec_clientlist__remove(&group->clientlist, client);
 	dynsec_grouplists__remove(&client->grouplist, groupname);
 
 	if(update_config){
@@ -744,7 +695,7 @@ static cJSON *add_group_to_json(struct dynsec__group *group)
 		}
 		cJSON_AddItemToArray(j_clientlist, j_client);
 
-		jtmp = cJSON_CreateStringReference(clientlist->username);
+		jtmp = cJSON_CreateStringReference(clientlist->client->username);
 		if(jtmp == NULL){
 			cJSON_Delete(j_group);
 			return NULL;

plugins/dynamic-security/roles.c

@@ -29,6 +29,7 @@ Contributors:
 
 
 static cJSON *add_role_to_json(struct dynsec__role *role, bool verbose);
+static void role__remove_all_clients(struct dynsec__role *role);
 
 /* ################################################################
  * #
@@ -109,7 +110,6 @@ int dynsec_rolelists__client_remove_role(struct dynsec__client *client, struct d
 	HASH_FIND(hh, role->clientlist, client->username, strlen(client->username), found_clientlist);
 	if(found_clientlist){
 		HASH_DELETE(hh, role->clientlist, found_clientlist);
-		mosquitto_free(found_clientlist->username);
 		mosquitto_free(found_clientlist);
 		return MOSQ_ERR_SUCCESS;
 	}else{
@@ -168,7 +168,6 @@ static int dynsec_rolelists__add_role(struct dynsec__rolelist **base_rolelist, s
 int dynsec_rolelists__client_add_role(struct dynsec__client *client, struct dynsec__role *role, int priority)
 {
 	struct dynsec__rolelist *rolelist;
-	struct dynsec__clientlist *clientlist;
 	int rc;
 
 	rc = dynsec_rolelists__add_role(&client->rolelist, role, priority);
@@ -180,22 +179,7 @@ int dynsec_rolelists__client_add_role(struct dynsec__client *client, struct dyns
 		return MOSQ_ERR_UNKNOWN;
 	}
 
-	/* Add client to role clientlist */
-	clientlist = mosquitto_calloc(1, sizeof(struct dynsec__clientlist));
-	if(clientlist == NULL){
-		dynsec_rolelists__remove_role(&client->rolelist, role);
-		return MOSQ_ERR_NOMEM;
-	}
-	clientlist->client = client;
-	clientlist->username = mosquitto_strdup(client->username);
-	if(clientlist->username == NULL){
-		dynsec_rolelists__remove_role(&client->rolelist, role);
-		mosquitto_free(clientlist);
-		return MOSQ_ERR_NOMEM;
-	}
-
-	HASH_ADD_KEYPTR_INORDER(hh, role->clientlist, client->username, strlen(client->username), clientlist, dynsec_clientlist__cmp);
-	return MOSQ_ERR_SUCCESS;
+	return dynsec_clientlist__add(&role->clientlist, client, priority);
 }
 
 
@@ -310,6 +294,7 @@ static void role__free_item(struct dynsec__role *role, bool remove_from_hash)
 	if(remove_from_hash){
 		HASH_DEL(local_roles, role);
 	}
+	dynsec_clientlist__cleanup(&role->clientlist);
 	mosquitto_free(role->text_name);
 	mosquitto_free(role->text_description);
 	mosquitto_free(role->rolename);
@@ -667,7 +652,7 @@ static void role__remove_all_clients(struct dynsec__role *role)
 	struct dynsec__clientlist *clientlist, *clientlist_tmp;
 
 	HASH_ITER(hh, role->clientlist, clientlist, clientlist_tmp){
-		mosquitto_kick_client_by_username(clientlist->username, false);
+		mosquitto_kick_client_by_username(clientlist->client->username, false);
 
 		dynsec_rolelists__client_remove_role(clientlist->client, role);
 	}