From 0433b8ee9bbcdb05a17b09a3dfac5be91c139a53 Mon Sep 17 00:00:00 2001 From: "Roger A. Light" Date: Thu, 10 Mar 2022 10:47:49 +0000 Subject: [PATCH] Update changelog, fix usage of m_set_clientid(), add test. --- ChangeLog.txt | 2 + src/plugin_public.c | 42 +++++++++--- test/broker/09-plugin-change-id.py | 63 ++++++++++++++++++ test/broker/Makefile | 1 + test/broker/c/Makefile | 3 +- test/broker/c/auth_plugin_id_change.c | 94 +++++++++++++++++++++++++++ test/broker/test.py | 1 + 7 files changed, 195 insertions(+), 11 deletions(-) create mode 100755 test/broker/09-plugin-change-id.py create mode 100644 test/broker/c/auth_plugin_id_change.c diff --git a/ChangeLog.txt b/ChangeLog.txt index 2aa23ee3..7684e861 100644 --- a/ChangeLog.txt +++ b/ChangeLog.txt @@ -97,6 +97,8 @@ Plugins / plugin interface: listClients. - The dynamic security plugin now generates an initial configuration if none is present, including a set of default roles. +- Add `mosquitto_set_clientid()` to allow plugins to force a client id for a + client. Client library: - Add MOSQ_OPT_DISABLE_SOCKETPAIR to allow the disabling of the socketpair diff --git a/src/plugin_public.c b/src/plugin_public.c index 01a7eff3..b55f150e 100644 --- a/src/plugin_public.c +++ b/src/plugin_public.c @@ -320,24 +320,46 @@ int mosquitto_set_username(struct mosquitto *client, const char *username) int mosquitto_set_clientid(struct mosquitto *client, const char *clientid) { - char *u_dup; - char *old; - - if(!client) return MOSQ_ERR_INVAL; + struct mosquitto *found_client; + char *id_dup; + bool in_by_id; + + if(!client || !clientid) return MOSQ_ERR_INVAL; + + in_by_id = client->in_by_id; + /* If in_by_id is true, then this client has already authenticated and + * completed the connection flow. This means it *cannot* take over an + * existing session, and we must remove/add it to the by_id hash table. + * + * If in_by_id is false, then this client is currently going through + * authentication and so it is safe to change the client id to any value + * because it will be checked after authentication. + */ + + if(in_by_id){ + HASH_FIND(hh_id, db.contexts_by_id, clientid, strlen(clientid), found_client); + if(found_client){ + return MOSQ_ERR_ALREADY_EXISTS; + } + } int clientid_len = (int)strlen(clientid); if(mosquitto_validate_utf8(clientid, clientid_len)){ return MOSQ_ERR_INVAL; } - u_dup = mosquitto__strdup(clientid); - if(!u_dup) return MOSQ_ERR_NOMEM; + id_dup = mosquitto__strdup(clientid); + if(!id_dup) return MOSQ_ERR_NOMEM; + if(in_by_id){ + context__remove_from_by_id(client); + } + mosquitto__free(client->id); + client->id = id_dup; + if(in_by_id){ + context__add_to_by_id(client); + } - old = client->id; - client->id = u_dup; - - mosquitto__free(old); return MOSQ_ERR_SUCCESS; } diff --git a/test/broker/09-plugin-change-id.py b/test/broker/09-plugin-change-id.py new file mode 100755 index 00000000..6fabfec4 --- /dev/null +++ b/test/broker/09-plugin-change-id.py @@ -0,0 +1,63 @@ +#!/usr/bin/env python3 + +from mosq_test_helper import * + +def write_config(filename, port, plugin_ver): + with open(filename, 'w') as f: + f.write("listener %d\n" % (port)) + f.write("auth_plugin c/auth_plugin_id_change.so\n") + f.write("allow_anonymous true\n") + +def do_test(plugin_ver): + port = mosq_test.get_port() + conf_file = os.path.basename(__file__).replace('.py', '.conf') + write_config(conf_file, port, plugin_ver) + + rc = 1 + connect1_packet = mosq_test.gen_connect("already-exists") + connack1_packet = mosq_test.gen_connack(rc=0) + + connect2_packet = mosq_test.gen_connect("id-change-test") + connack2_packet = mosq_test.gen_connack(rc=0) + + mid = 1 + subscribe_packet = mosq_test.gen_subscribe(mid, "#", 0) + # Only subs by client id == allowed is allowed + suback_packet_denied = mosq_test.gen_suback(mid, 128) + suback_packet_ok = mosq_test.gen_suback(mid, 0) + + mid = 2 + publish1_packet = mosq_test.gen_publish("publish/topic", qos=2, mid=mid, payload="message") + pubrec1_packet = mosq_test.gen_pubrec(mid) + pubrel1_packet = mosq_test.gen_pubrel(mid) + pubcomp1_packet = mosq_test.gen_pubcomp(mid) + + broker = mosq_test.start_broker(filename=os.path.basename(__file__), use_conf=True, port=port) + + try: + sock1 = mosq_test.do_client_connect(connect1_packet, connack1_packet, timeout=20, port=port) + sock2 = mosq_test.do_client_connect(connect2_packet, connack2_packet, timeout=20, port=port) + + mosq_test.do_send_receive(sock1, subscribe_packet, suback_packet_denied, "suback denied") + mosq_test.do_send_receive(sock2, subscribe_packet, suback_packet_ok, "suback ok") + + mosq_test.do_ping(sock1) + mosq_test.do_ping(sock2) + sock1.close() + sock2.close() + + rc = 0 + except mosq_test.TestError: + pass + except Exception as err: + print(err) + finally: + os.remove(conf_file) + broker.terminate() + broker.wait() + (stdo, stde) = broker.communicate() + if rc: + print(stde.decode('utf-8')) + exit(rc) + +do_test(4) diff --git a/test/broker/Makefile b/test/broker/Makefile index 3158db9f..c9f3d808 100644 --- a/test/broker/Makefile +++ b/test/broker/Makefile @@ -200,6 +200,7 @@ endif ./09-plugin-auth-v4-unpwd-success.py ./09-plugin-auth-v5-unpwd-fail.py ./09-plugin-auth-v5-unpwd-success.py + ./09-plugin-change-id.py ./09-plugin-delayed-auth.py ./09-plugin-publish.py ./09-pwfile-parse-invalid.py diff --git a/test/broker/c/Makefile b/test/broker/c/Makefile index 31fa9034..e35ffce0 100644 --- a/test/broker/c/Makefile +++ b/test/broker/c/Makefile @@ -13,6 +13,7 @@ PLUGIN_SRC = \ auth_plugin_extended_reauth.c \ auth_plugin_extended_single.c \ auth_plugin_extended_single2.c \ + auth_plugin_id_change.c \ auth_plugin_msg_params.c \ auth_plugin_publish.c \ auth_plugin_pwd.c \ @@ -20,8 +21,8 @@ PLUGIN_SRC = \ auth_plugin_v3.c \ auth_plugin_v4.c \ auth_plugin_v5.c \ - auth_plugin_v5_handle_message.c \ auth_plugin_v5_control.c \ + auth_plugin_v5_handle_message.c \ plugin_control.c PLUGINS = ${PLUGIN_SRC:.c=.so} diff --git a/test/broker/c/auth_plugin_id_change.c b/test/broker/c/auth_plugin_id_change.c new file mode 100644 index 00000000..1709676a --- /dev/null +++ b/test/broker/c/auth_plugin_id_change.c @@ -0,0 +1,94 @@ +#include +#include +#include +#include +#include +#include + +int mosquitto_auth_acl_check_v5(int event, void *event_data, void *user_data); +int mosquitto_auth_unpwd_check_v5(int event, void *event_data, void *user_data); + +static mosquitto_plugin_id_t *plg_id; + +MOSQUITTO_PLUGIN_DECLARE_VERSION(5); + +int mosquitto_plugin_init(mosquitto_plugin_id_t *identifier, void **user_data, struct mosquitto_opt *auth_opts, int auth_opt_count) +{ + (void)user_data; + (void)auth_opts; + (void)auth_opt_count; + + plg_id = identifier; + + mosquitto_callback_register(plg_id, MOSQ_EVT_ACL_CHECK, mosquitto_auth_acl_check_v5, NULL, NULL); + mosquitto_callback_register(plg_id, MOSQ_EVT_BASIC_AUTH, mosquitto_auth_unpwd_check_v5, NULL, NULL); + + return MOSQ_ERR_SUCCESS; +} + +int mosquitto_plugin_cleanup(void *user_data, struct mosquitto_opt *auth_opts, int auth_opt_count) +{ + (void)user_data; + (void)auth_opts; + (void)auth_opt_count; + + mosquitto_callback_unregister(plg_id, MOSQ_EVT_ACL_CHECK, mosquitto_auth_acl_check_v5, NULL); + mosquitto_callback_unregister(plg_id, MOSQ_EVT_BASIC_AUTH, mosquitto_auth_unpwd_check_v5, NULL); + + return MOSQ_ERR_SUCCESS; +} + +int mosquitto_auth_acl_check_v5(int event, void *event_data, void *user_data) +{ + struct mosquitto_evt_acl_check *ed = event_data; + int rc; + + (void)user_data; + + if(event != MOSQ_EVT_ACL_CHECK){ + abort(); + } + + if(!strcmp(mosquitto_client_id(ed->client), "allowed")){ + /* Attempt to change to a different existing ID after we have + * authenticated - should fail */ + rc = mosquitto_set_clientid(ed->client, "already-exists"); + if(rc != MOSQ_ERR_ALREADY_EXISTS){ + exit(1); + } + } + + + if(!strcmp(mosquitto_client_id(ed->client), "allowed")){ + return MOSQ_ERR_SUCCESS; + }else{ + return MOSQ_ERR_ACL_DENIED; + } +} + +int mosquitto_auth_unpwd_check_v5(int event, void *event_data, void *user_data) +{ + struct mosquitto_evt_basic_auth *ed = event_data; + const char *clientid; + int rc; + + (void)user_data; + + clientid = mosquitto_client_id(ed->client); + + if(event != MOSQ_EVT_BASIC_AUTH){ + abort(); + } + + if(!strcmp(clientid, "id-change-test")){ + rc = mosquitto_set_clientid(ed->client, "allowed"); + if(strcmp(mosquitto_client_id(ed->client), "allowed")){ + rc = MOSQ_ERR_INVAL; + } + if(rc != MOSQ_ERR_SUCCESS){ + exit(1); + } + } + + return MOSQ_ERR_SUCCESS; +} diff --git a/test/broker/test.py b/test/broker/test.py index bda92105..7bdd84ef 100755 --- a/test/broker/test.py +++ b/test/broker/test.py @@ -169,6 +169,7 @@ tests = [ (1, './09-plugin-auth-v4-unpwd-success.py'), (1, './09-plugin-auth-v5-unpwd-fail.py'), (1, './09-plugin-auth-v5-unpwd-success.py'), + (1, './09-plugin-change-id.py'), (1, './09-plugin-delayed-auth.py'), (1, './09-plugin-publish.py'), (1, './09-pwfile-parse-invalid.py'),