From 832e51cb571c13ce9c5a42d954e352a6d769460e Mon Sep 17 00:00:00 2001 From: "Roger A. Light" Date: Fri, 5 Aug 2022 13:03:34 +0100 Subject: [PATCH] dynsec: reduce memory allocations --- plugins/dynamic-security/clients.c | 41 ++++---- plugins/dynamic-security/dynamic_security.h | 10 +- plugins/dynamic-security/groups.c | 40 ++++---- plugins/dynamic-security/rolelist.c | 16 ++-- plugins/dynamic-security/roles.c | 100 +++++++++----------- 5 files changed, 100 insertions(+), 107 deletions(-) diff --git a/plugins/dynamic-security/clients.c b/plugins/dynamic-security/clients.c index fc3f176d..87057c9b 100644 --- a/plugins/dynamic-security/clients.c +++ b/plugins/dynamic-security/clients.c @@ -87,7 +87,6 @@ static void client__free_item(struct dynsec__data *data, struct dynsec__client * mosquitto_free(client->text_name); mosquitto_free(client->text_description); mosquitto_free(client->clientid); - mosquitto_free(client->username); mosquitto_free(client); } @@ -116,6 +115,7 @@ int dynsec_clients__config_load(struct dynsec__data *data, cJSON *tree) unsigned int buf_len; int priority; int iterations; + size_t username_len; j_clients = cJSON_GetObjectItem(tree, "clients"); if(j_clients == NULL){ @@ -128,23 +128,22 @@ int dynsec_clients__config_load(struct dynsec__data *data, cJSON *tree) cJSON_ArrayForEach(j_client, j_clients){ if(cJSON_IsObject(j_client) == true){ - client = mosquitto_calloc(1, sizeof(struct dynsec__client)); - if(client == NULL){ - return MOSQ_ERR_NOMEM; - } - /* Username */ jtmp = cJSON_GetObjectItem(j_client, "username"); if(jtmp == NULL || !cJSON_IsString(jtmp)){ - mosquitto_free(client); continue; } - client->username = mosquitto_strdup(jtmp->valuestring); - if(client->username == NULL){ - mosquitto_free(client); + username_len = strlen(jtmp->valuestring); + if(username_len == 0){ continue; } + client = mosquitto_calloc(1, sizeof(struct dynsec__client) + username_len + 1); + if(client == NULL){ + return MOSQ_ERR_NOMEM; + } + strncpy(client->username, jtmp->valuestring, username_len); + jtmp = cJSON_GetObjectItem(j_client, "disabled"); if(jtmp && cJSON_IsBool(jtmp)){ client->disabled = cJSON_IsTrue(jtmp); @@ -244,7 +243,7 @@ int dynsec_clients__config_load(struct dynsec__data *data, cJSON *tree) } } - HASH_ADD_KEYPTR(hh, data->clients, client->username, strlen(client->username), client); + HASH_ADD(hh, data->clients, username, username_len, client); } } HASH_SORT(data->clients, client_cmp); @@ -332,12 +331,18 @@ int dynsec_clients__process_create(struct dynsec__data *data, struct plugin_cmd cJSON *j_groups, *j_group, *jtmp; int priority; const char *admin_clientid, *admin_username; + size_t username_len; if(json_get_string(cmd->j_command, "username", &username, false) != MOSQ_ERR_SUCCESS){ plugin__command_reply(cmd, "Invalid/missing username"); return MOSQ_ERR_INVAL; } - if(mosquitto_validate_utf8(username, (int)strlen(username)) != MOSQ_ERR_SUCCESS){ + username_len = strlen(username); + if(username_len == 0){ + plugin__command_reply(cmd, "Empty username"); + return MOSQ_ERR_INVAL; + } + if(mosquitto_validate_utf8(username, (int)username_len) != MOSQ_ERR_SUCCESS){ plugin__command_reply(cmd, "Username not valid UTF-8"); return MOSQ_ERR_INVAL; } @@ -373,17 +378,13 @@ int dynsec_clients__process_create(struct dynsec__data *data, struct plugin_cmd return MOSQ_ERR_SUCCESS; } - client = mosquitto_calloc(1, sizeof(struct dynsec__client)); + client = mosquitto_calloc(1, sizeof(struct dynsec__client) + username_len + 1); if(client == NULL){ plugin__command_reply(cmd, "Internal error"); return MOSQ_ERR_NOMEM; } - client->username = mosquitto_strdup(username); - if(client->username == NULL){ - plugin__command_reply(cmd, "Internal error"); - client__free_item(data, client); - return MOSQ_ERR_NOMEM; - } + strncpy(client->username, username, username_len); + if(text_name){ client->text_name = mosquitto_strdup(text_name); if(client->text_name == NULL){ @@ -435,7 +436,7 @@ int dynsec_clients__process_create(struct dynsec__data *data, struct plugin_cmd } /* Must add user before groups, otherwise adding groups will fail */ - HASH_ADD_KEYPTR_INORDER(hh, data->clients, client->username, strlen(client->username), client, client_cmp); + HASH_ADD_INORDER(hh, data->clients, username, username_len, client, client_cmp); j_groups = cJSON_GetObjectItem(cmd->j_command, "groups"); if(j_groups && cJSON_IsArray(j_groups)){ diff --git a/plugins/dynamic-security/dynamic_security.h b/plugins/dynamic-security/dynamic_security.h index 5ff8bc2b..a236ae23 100644 --- a/plugins/dynamic-security/dynamic_security.h +++ b/plugins/dynamic-security/dynamic_security.h @@ -70,9 +70,9 @@ struct dynsec__grouplist{ struct dynsec__rolelist{ UT_hash_handle hh; - char *rolename; struct dynsec__role *role; int priority; + char rolename[]; }; struct dynsec__kicklist{ @@ -85,28 +85,28 @@ struct dynsec__client{ struct mosquitto_pw pw; struct dynsec__rolelist *rolelist; struct dynsec__grouplist *grouplist; - char *username; char *clientid; char *text_name; char *text_description; bool disabled; + char username[]; }; struct dynsec__group{ UT_hash_handle hh; struct dynsec__rolelist *rolelist; struct dynsec__clientlist *clientlist; - char *groupname; char *text_name; char *text_description; + char groupname[]; }; struct dynsec__acl{ UT_hash_handle hh; - char *topic; int priority; bool allow; + char topic[]; }; struct dynsec__acls{ @@ -123,10 +123,10 @@ struct dynsec__role{ struct dynsec__acls acls; struct dynsec__clientlist *clientlist; struct dynsec__grouplist *grouplist; - char *rolename; char *text_name; char *text_description; bool allow_wildcard_subs; + char rolename[]; }; struct dynsec__acl_default_access{ diff --git a/plugins/dynamic-security/groups.c b/plugins/dynamic-security/groups.c index 0cfe73c3..56739aa4 100644 --- a/plugins/dynamic-security/groups.c +++ b/plugins/dynamic-security/groups.c @@ -98,7 +98,6 @@ static void group__free_item(struct dynsec__data *data, struct dynsec__group *gr dynsec__remove_all_clients_from_group(group); mosquitto_free(group->text_name); mosquitto_free(group->text_description); - mosquitto_free(group->groupname); dynsec_rolelist__cleanup(&group->rolelist); mosquitto_free(group); } @@ -197,6 +196,7 @@ int dynsec_groups__config_load(struct dynsec__data *data, cJSON *tree) struct dynsec__role *role; char *str; int priority; + size_t groupname_len; j_groups = cJSON_GetObjectItem(tree, "groups"); if(j_groups == NULL){ @@ -209,22 +209,21 @@ int dynsec_groups__config_load(struct dynsec__data *data, cJSON *tree) cJSON_ArrayForEach(j_group, j_groups){ if(cJSON_IsObject(j_group) == true){ - group = mosquitto_calloc(1, sizeof(struct dynsec__group)); - if(group == NULL){ - return MOSQ_ERR_NOMEM; - } - /* Group name */ if(json_get_string(j_group, "groupname", &str, false) != MOSQ_ERR_SUCCESS){ - mosquitto_free(group); continue; } - group->groupname = strdup(str); - if(group->groupname == NULL){ - mosquitto_free(group); + groupname_len = strlen(str); + if(groupname_len == 0){ continue; } + group = mosquitto_calloc(1, sizeof(struct dynsec__group) + groupname_len + 1); + if(group == NULL){ + return MOSQ_ERR_NOMEM; + } + strncpy(group->groupname, str, groupname_len+1); + /* Text name */ if(json_get_string(j_group, "textname", &str, false) == MOSQ_ERR_SUCCESS){ if(str){ @@ -266,7 +265,7 @@ int dynsec_groups__config_load(struct dynsec__data *data, cJSON *tree) } /* This must go before clients are loaded, otherwise the group won't be found */ - HASH_ADD_KEYPTR(hh, data->groups, group->groupname, strlen(group->groupname), group); + HASH_ADD(hh, data->groups, groupname, groupname_len, group); /* Clients */ j_clientlist = cJSON_GetObjectItem(j_group, "clients"); @@ -365,12 +364,18 @@ int dynsec_groups__process_create(struct dynsec__data *data, struct plugin_cmd * struct dynsec__group *group = NULL; int rc = MOSQ_ERR_SUCCESS; const char *admin_clientid, *admin_username; + size_t groupname_len; if(json_get_string(cmd->j_command, "groupname", &groupname, false) != MOSQ_ERR_SUCCESS){ plugin__command_reply(cmd, "Invalid/missing groupname"); return MOSQ_ERR_INVAL; } - if(mosquitto_validate_utf8(groupname, (int)strlen(groupname)) != MOSQ_ERR_SUCCESS){ + groupname_len = strlen(groupname); + if(groupname_len == 0){ + plugin__command_reply(cmd, "Empty groupname"); + return MOSQ_ERR_INVAL; + } + if(mosquitto_validate_utf8(groupname, (int)groupname_len) != MOSQ_ERR_SUCCESS){ plugin__command_reply(cmd, "Group name not valid UTF-8"); return MOSQ_ERR_INVAL; } @@ -391,17 +396,12 @@ int dynsec_groups__process_create(struct dynsec__data *data, struct plugin_cmd * return MOSQ_ERR_SUCCESS; } - group = mosquitto_calloc(1, sizeof(struct dynsec__group)); + group = mosquitto_calloc(1, sizeof(struct dynsec__group) + groupname_len + 1); if(group == NULL){ plugin__command_reply(cmd, "Internal error"); return MOSQ_ERR_NOMEM; } - group->groupname = strdup(groupname); - if(group->groupname == NULL){ - plugin__command_reply(cmd, "Internal error"); - group__free_item(data, group); - return MOSQ_ERR_NOMEM; - } + strncpy(group->groupname, groupname, groupname_len+1); if(text_name){ group->text_name = strdup(text_name); if(group->text_name == NULL){ @@ -431,7 +431,7 @@ int dynsec_groups__process_create(struct dynsec__data *data, struct plugin_cmd * return MOSQ_ERR_INVAL; } - HASH_ADD_KEYPTR_INORDER(hh, data->groups, group->groupname, strlen(group->groupname), group, group_cmp); + HASH_ADD_INORDER(hh, data->groups, groupname, groupname_len, group, group_cmp); admin_clientid = mosquitto_client_id(context); admin_username = mosquitto_client_username(context); diff --git a/plugins/dynamic-security/rolelist.c b/plugins/dynamic-security/rolelist.c index 5adbd41d..6a5f3664 100644 --- a/plugins/dynamic-security/rolelist.c +++ b/plugins/dynamic-security/rolelist.c @@ -54,7 +54,6 @@ static int rolelist_cmp(void *a, void *b) static void dynsec_rolelist__free_item(struct dynsec__rolelist **base_rolelist, struct dynsec__rolelist *rolelist) { HASH_DELETE(hh, *base_rolelist, rolelist); - mosquitto_free(rolelist->rolename); mosquitto_free(rolelist); } @@ -110,24 +109,23 @@ void dynsec_rolelist__group_remove(struct dynsec__group *group, struct dynsec__r static int dynsec_rolelist__add(struct dynsec__rolelist **base_rolelist, struct dynsec__role *role, int priority) { struct dynsec__rolelist *rolelist; + size_t rolename_len; if(role == NULL) return MOSQ_ERR_INVAL; + rolename_len = strlen(role->rolename); + if(rolename_len == 0) return MOSQ_ERR_INVAL; - HASH_FIND(hh, *base_rolelist, role->rolename, strlen(role->rolename), rolelist); + HASH_FIND(hh, *base_rolelist, role->rolename, rolename_len, rolelist); if(rolelist){ return MOSQ_ERR_ALREADY_EXISTS; }else{ - rolelist = mosquitto_calloc(1, sizeof(struct dynsec__rolelist)); + rolelist = mosquitto_calloc(1, sizeof(struct dynsec__rolelist) + rolename_len + 1); if(rolelist == NULL) return MOSQ_ERR_NOMEM; rolelist->role = role; rolelist->priority = priority; - rolelist->rolename = mosquitto_strdup(role->rolename); - if(rolelist->rolename == NULL){ - mosquitto_free(rolelist); - return MOSQ_ERR_NOMEM; - } - HASH_ADD_KEYPTR_INORDER(hh, *base_rolelist, role->rolename, strlen(role->rolename), rolelist, rolelist_cmp); + strncpy(rolelist->rolename, role->rolename, rolename_len+1); + HASH_ADD_INORDER(hh, *base_rolelist, rolename, rolename_len, rolelist, rolelist_cmp); return MOSQ_ERR_SUCCESS; } } diff --git a/plugins/dynamic-security/roles.c b/plugins/dynamic-security/roles.c index c058b845..fc7e0e8f 100644 --- a/plugins/dynamic-security/roles.c +++ b/plugins/dynamic-security/roles.c @@ -51,7 +51,6 @@ static int role_cmp(void *a, void *b) static void role__free_acl(struct dynsec__acl **acl, struct dynsec__acl *item) { HASH_DELETE(hh, *acl, item); - mosquitto_free(item->topic); mosquitto_free(item); } @@ -73,7 +72,6 @@ static void role__free_item(struct dynsec__data *data, struct dynsec__role *role dynsec_grouplist__cleanup(&role->grouplist); mosquitto_free(role->text_name); mosquitto_free(role->text_description); - mosquitto_free(role->rolename); role__free_all_acls(&role->acls.publish_c_send); role__free_all_acls(&role->acls.publish_c_recv); role__free_all_acls(&role->acls.subscribe_literal); @@ -204,16 +202,28 @@ static int dynsec_roles__acl_load(cJSON *j_acls, const char *key, struct dynsec_ { cJSON *j_acl, *j_type, *jtmp; struct dynsec__acl *acl; + size_t topic_len; cJSON_ArrayForEach(j_acl, j_acls){ j_type = cJSON_GetObjectItem(j_acl, "acltype"); if(j_type == NULL || !cJSON_IsString(j_type) || strcasecmp(j_type->valuestring, key) != 0){ continue; } - acl = mosquitto_calloc(1, sizeof(struct dynsec__acl)); + jtmp = cJSON_GetObjectItem(j_acl, "topic"); + if(!jtmp || !cJSON_IsString(jtmp)){ + continue; + } + + topic_len = strlen(jtmp->valuestring); + if(topic_len == 0){ + continue; + } + + acl = mosquitto_calloc(1, sizeof(struct dynsec__acl) + topic_len + 1); if(acl == NULL){ return 1; } + strncpy(acl->topic, jtmp->valuestring, topic_len+1); json_get_int(j_acl, "priority", &acl->priority, true, 0); json_get_bool(j_acl, "allow", &acl->allow, true, false); @@ -223,17 +233,7 @@ static int dynsec_roles__acl_load(cJSON *j_acls, const char *key, struct dynsec_ acl->allow = cJSON_IsTrue(jtmp); } - jtmp = cJSON_GetObjectItem(j_acl, "topic"); - if(jtmp && cJSON_IsString(jtmp)){ - acl->topic = mosquitto_strdup(jtmp->valuestring); - } - - if(acl->topic == NULL){ - mosquitto_free(acl); - continue; - } - - HASH_ADD_KEYPTR_INORDER(hh, *acllist, acl->topic, strlen(acl->topic), acl, insert_acl_cmp); + HASH_ADD_INORDER(hh, *acllist, topic, topic_len, acl, insert_acl_cmp); } return 0; @@ -244,6 +244,7 @@ int dynsec_roles__config_load(struct dynsec__data *data, cJSON *tree) { cJSON *j_roles, *j_role, *jtmp, *j_acls; struct dynsec__role *role; + size_t rolename_len; j_roles = cJSON_GetObjectItem(tree, "roles"); if(j_roles == NULL){ @@ -256,23 +257,22 @@ int dynsec_roles__config_load(struct dynsec__data *data, cJSON *tree) cJSON_ArrayForEach(j_role, j_roles){ if(cJSON_IsObject(j_role) == true){ - role = mosquitto_calloc(1, sizeof(struct dynsec__role)); - if(role == NULL){ - return MOSQ_ERR_NOMEM; - } - /* Role name */ jtmp = cJSON_GetObjectItem(j_role, "rolename"); - if(jtmp == NULL){ - mosquitto_free(role); + if(jtmp == NULL || !cJSON_IsString(jtmp)){ continue; } - role->rolename = mosquitto_strdup(jtmp->valuestring); - if(role->rolename == NULL){ - mosquitto_free(role); + rolename_len = strlen(jtmp->valuestring); + if(rolename_len == 0){ continue; } + role = mosquitto_calloc(1, sizeof(struct dynsec__role) + rolename_len + 1); + if(role == NULL){ + return MOSQ_ERR_NOMEM; + } + strncpy(role->rolename, jtmp->valuestring, rolename_len+1); + /* Text name */ jtmp = cJSON_GetObjectItem(j_role, "textname"); if(jtmp != NULL){ @@ -315,13 +315,12 @@ int dynsec_roles__config_load(struct dynsec__data *data, cJSON *tree) || dynsec_roles__acl_load(j_acls, ACL_TYPE_UNSUB_PATTERN, &role->acls.unsubscribe_pattern) != 0 ){ - mosquitto_free(role->rolename); mosquitto_free(role); continue; } } - HASH_ADD_KEYPTR(hh, data->roles, role->rolename, strlen(role->rolename), role); + HASH_ADD(hh, data->roles, rolename, rolename_len, role); } } HASH_SORT(data->roles, role_cmp); @@ -339,12 +338,18 @@ int dynsec_roles__process_create(struct dynsec__data *data, struct plugin_cmd *c int rc = MOSQ_ERR_SUCCESS; cJSON *j_acls; const char *admin_clientid, *admin_username; + size_t rolename_len; if(json_get_string(cmd->j_command, "rolename", &rolename, false) != MOSQ_ERR_SUCCESS){ plugin__command_reply(cmd, "Invalid/missing rolename"); return MOSQ_ERR_INVAL; } - if(mosquitto_validate_utf8(rolename, (int)strlen(rolename)) != MOSQ_ERR_SUCCESS){ + rolename_len = strlen(rolename); + if(rolename_len == 0){ + plugin__command_reply(cmd, "Empty rolename"); + return MOSQ_ERR_INVAL; + } + if(mosquitto_validate_utf8(rolename, (int)rolename_len) != MOSQ_ERR_SUCCESS){ plugin__command_reply(cmd, "Role name not valid UTF-8"); return MOSQ_ERR_INVAL; } @@ -370,17 +375,12 @@ int dynsec_roles__process_create(struct dynsec__data *data, struct plugin_cmd *c return MOSQ_ERR_SUCCESS; } - role = mosquitto_calloc(1, sizeof(struct dynsec__role)); + role = mosquitto_calloc(1, sizeof(struct dynsec__role) + rolename_len + 1); if(role == NULL){ plugin__command_reply(cmd, "Internal error"); return MOSQ_ERR_NOMEM; } - role->rolename = mosquitto_strdup(rolename); - if(role->rolename == NULL){ - plugin__command_reply(cmd, "Internal error"); - rc = MOSQ_ERR_NOMEM; - goto error; - } + strncpy(role->rolename, rolename, rolename_len+1); if(text_name){ role->text_name = mosquitto_strdup(text_name); if(role->text_name == NULL){ @@ -417,7 +417,7 @@ int dynsec_roles__process_create(struct dynsec__data *data, struct plugin_cmd *c } - HASH_ADD_KEYPTR_INORDER(hh, data->roles, role->rolename, strlen(role->rolename), role, role_cmp); + HASH_ADD_INORDER(hh, data->roles, rolename, rolename_len, role, role_cmp); dynsec__config_save(data); @@ -595,12 +595,12 @@ int dynsec_roles__process_list(struct dynsec__data *data, struct plugin_cmd *cmd int dynsec_roles__process_add_acl(struct dynsec__data *data, struct plugin_cmd *cmd, struct mosquitto *context) { char *rolename; - char *topic; struct dynsec__role *role; - cJSON *jtmp, *j_acltype; + cJSON *j_acltype, *j_topic; struct dynsec__acl **acllist, *acl; int rc; const char *admin_clientid, *admin_username; + size_t topic_len; if(json_get_string(cmd->j_command, "rolename", &rolename, false) != MOSQ_ERR_SUCCESS){ plugin__command_reply(cmd, "Invalid/missing rolename"); @@ -639,46 +639,40 @@ int dynsec_roles__process_add_acl(struct dynsec__data *data, struct plugin_cmd * return MOSQ_ERR_SUCCESS; } - jtmp = cJSON_GetObjectItem(cmd->j_command, "topic"); - if(jtmp && cJSON_IsString(jtmp)){ - if(mosquitto_validate_utf8(jtmp->valuestring, (int)strlen(jtmp->valuestring)) != MOSQ_ERR_SUCCESS){ + j_topic = cJSON_GetObjectItem(cmd->j_command, "topic"); + if(j_topic && cJSON_IsString(j_topic)){ + topic_len = strlen(j_topic->valuestring); + if(mosquitto_validate_utf8(j_topic->valuestring, (int)topic_len) != MOSQ_ERR_SUCCESS){ plugin__command_reply(cmd, "Topic not valid UTF-8"); return MOSQ_ERR_INVAL; } - rc = mosquitto_sub_topic_check(jtmp->valuestring); + rc = mosquitto_sub_topic_check(j_topic->valuestring); if(rc != MOSQ_ERR_SUCCESS){ plugin__command_reply(cmd, "Invalid ACL topic"); return MOSQ_ERR_INVAL; } - topic = mosquitto_strdup(jtmp->valuestring); - if(topic == NULL){ - plugin__command_reply(cmd, "Internal error"); - return MOSQ_ERR_SUCCESS; - } }else{ plugin__command_reply(cmd, "Invalid/missing topic"); return MOSQ_ERR_SUCCESS; } - HASH_FIND(hh, *acllist, topic, strlen(topic), acl); + HASH_FIND(hh, *acllist, j_topic->valuestring, topic_len, acl); if(acl){ - mosquitto_free(topic); plugin__command_reply(cmd, "ACL with this topic already exists"); return MOSQ_ERR_SUCCESS; } - acl = mosquitto_calloc(1, sizeof(struct dynsec__acl)); + acl = mosquitto_calloc(1, sizeof(struct dynsec__acl) + topic_len + 1); if(acl == NULL){ - mosquitto_free(topic); plugin__command_reply(cmd, "Internal error"); return MOSQ_ERR_SUCCESS; } - acl->topic = topic; + strncpy(acl->topic, j_topic->valuestring, topic_len+1); json_get_int(cmd->j_command, "priority", &acl->priority, true, 0); json_get_bool(cmd->j_command, "allow", &acl->allow, true, false); - HASH_ADD_KEYPTR_INORDER(hh, *acllist, acl->topic, strlen(acl->topic), acl, insert_acl_cmp); + HASH_ADD_INORDER(hh, *acllist, topic, topic_len, acl, insert_acl_cmp); dynsec__config_save(data); plugin__command_reply(cmd, NULL); @@ -687,7 +681,7 @@ int dynsec_roles__process_add_acl(struct dynsec__data *data, struct plugin_cmd * admin_clientid = mosquitto_client_id(context); admin_username = mosquitto_client_username(context); mosquitto_log_printf(MOSQ_LOG_INFO, "dynsec: %s/%s | addRoleACL | rolename=%s | acltype=%s | topic=%s | priority=%d | allow=%s", - admin_clientid, admin_username, rolename, j_acltype->valuestring, topic, acl->priority, acl->allow?"true":"false"); + admin_clientid, admin_username, rolename, j_acltype->valuestring, j_topic->valuestring, acl->priority, acl->allow?"true":"false"); return MOSQ_ERR_SUCCESS; }