From b76c3c7820d9e910486d29686fc78e4c047975c3 Mon Sep 17 00:00:00 2001
From: "Roger A. Light" <roger@atchoo.org>
Date: Tue, 13 Jun 2023 17:07:43 +0100
Subject: [PATCH] Dynsec: Don't allow duplicate c/g/r when loading config

---
 ChangeLog.txt                      |  2 ++
 plugins/dynamic-security/clients.c | 20 +++++++++++-------
 plugins/dynamic-security/groups.c  | 14 ++++++++-----
 plugins/dynamic-security/roles.c   | 33 +++++++++++++++++++-----------
 4 files changed, 45 insertions(+), 24 deletions(-)

diff --git a/ChangeLog.txt b/ChangeLog.txt
index 816ba8eb..45adc1da 100644
--- a/ChangeLog.txt
+++ b/ChangeLog.txt
@@ -28,6 +28,8 @@ Broker:
 - Fix any possible case where a json string might be incorrectly loaded. This
   could have caused a crash if a textname or textdescription field of a role was
   not a string, when loading the dynsec config from file only.
+- Dynsec plugin will not allow duplicate clients/groups/roles when loading
+  config from file, which matches the behaviour for when creating them.
 
 Client library:
 - Use CLOCK_BOOTTIME when available, to keep track of time. This solves the
diff --git a/plugins/dynamic-security/clients.c b/plugins/dynamic-security/clients.c
index 07ae7a42..c21d5696 100644
--- a/plugins/dynamic-security/clients.c
+++ b/plugins/dynamic-security/clients.c
@@ -124,18 +124,24 @@ int dynsec_clients__config_load(cJSON *tree)
 
 	cJSON_ArrayForEach(j_client, j_clients){
 		if(cJSON_IsObject(j_client) == true){
+			/* Username */
+			char *username;
+			json_get_string(j_client, "username", &username, false);
+			if(!username){
+				continue;
+			}
+
+			client = dynsec_clients__find(username);
+			if(client){
+				continue;
+			}
+
 			client = mosquitto_calloc(1, sizeof(struct dynsec__client));
 			if(client == NULL){
 				return MOSQ_ERR_NOMEM;
 			}
 
-			/* Username */
-			char *username;
-			json_get_string(j_client, "username", &username, false);
-			if(!username){
-				mosquitto_free(client);
-				continue;
-			}
+
 			client->username = mosquitto_strdup(username);
 			if(client->username == NULL){
 				mosquitto_free(client);
diff --git a/plugins/dynamic-security/groups.c b/plugins/dynamic-security/groups.c
index 601b06d1..16fa76f3 100644
--- a/plugins/dynamic-security/groups.c
+++ b/plugins/dynamic-security/groups.c
@@ -214,16 +214,20 @@ int dynsec_groups__config_load(cJSON *tree)
 
 	cJSON_ArrayForEach(j_group, j_groups){
 		if(cJSON_IsObject(j_group) == true){
+			/* Group name */
+			if(json_get_string(j_group, "groupname", &groupname, false) != MOSQ_ERR_SUCCESS){
+				continue;
+			}
+			group = dynsec_groups__find(groupname);
+			if(group){
+				continue;
+			}
+
 			group = mosquitto_calloc(1, sizeof(struct dynsec__group));
 			if(group == NULL){
 				return MOSQ_ERR_NOMEM;
 			}
 
-			/* Group name */
-			if(json_get_string(j_group, "groupname", &groupname, false) != MOSQ_ERR_SUCCESS){
-				mosquitto_free(group);
-				continue;
-			}
 			group->groupname = strdup(groupname);
 			if(group->groupname == NULL){
 				mosquitto_free(group);
diff --git a/plugins/dynamic-security/roles.c b/plugins/dynamic-security/roles.c
index 13f0a6bd..09b9fe3c 100644
--- a/plugins/dynamic-security/roles.c
+++ b/plugins/dynamic-security/roles.c
@@ -220,10 +220,19 @@ static int dynsec_roles__acl_load(cJSON *j_acls, const char *key, struct dynsec_
 
 	cJSON_ArrayForEach(j_acl, j_acls){
 		char *acltype;
+		char *topic;
+
 		json_get_string(j_acl, "acltype", &acltype, false);
-		if(!acltype || strcasecmp(acltype, key) != 0){
+		json_get_string(j_acl, "topic", &topic, false);
+
+		if(!acltype || strcasecmp(acltype, key) != 0 || !topic){
 			continue;
 		}
+		HASH_FIND(hh, *acllist, topic, strlen(topic), acl);
+		if(acl){
+			continue;
+		}
+
 		acl = mosquitto_calloc(1, sizeof(struct dynsec__acl));
 		if(acl == NULL){
 			return 1;
@@ -237,11 +246,7 @@ static int dynsec_roles__acl_load(cJSON *j_acls, const char *key, struct dynsec_
 			acl->allow = allow;
 		}
 
-		char *topic;
-		if(json_get_string(j_acl, "topic", &topic, false) == MOSQ_ERR_SUCCESS){
-			acl->topic = mosquitto_strdup(topic);
-		}
-
+		acl->topic = mosquitto_strdup(topic);
 		if(acl->topic == NULL){
 			mosquitto_free(acl);
 			continue;
@@ -270,17 +275,21 @@ int dynsec_roles__config_load(cJSON *tree)
 
 	cJSON_ArrayForEach(j_role, j_roles){
 		if(cJSON_IsObject(j_role) == true){
+			/* Role name */
+			char *rolename;
+			if(json_get_string(j_role, "rolename", &rolename, false) != MOSQ_ERR_SUCCESS){
+				continue;
+			}
+			role = dynsec_roles__find(rolename);
+			if(role){
+				continue;
+			}
+
 			role = mosquitto_calloc(1, sizeof(struct dynsec__role));
 			if(role == NULL){
 				return MOSQ_ERR_NOMEM;
 			}
 
-			/* Role name */
-			char *rolename;
-			if(json_get_string(j_role, "rolename", &rolename, false) != MOSQ_ERR_SUCCESS){
-				mosquitto_free(role);
-				continue;
-			}
 			role->rolename = mosquitto_strdup(rolename);
 			if(role->rolename == NULL){
 				mosquitto_free(role);