From 16eecfcbc5ba4931105fd54802d8ee5153f34b5d Mon Sep 17 00:00:00 2001 From: Brandt Hill Date: Mon, 24 Feb 2020 17:00:36 -0600 Subject: [PATCH] Add 'deny' as an option for topics/patterns in acl file to allow certain topics to be explicitly denied when they might otherwise be allowed through a more open read/write/readwrite option. Example: 'topic readwrite test/#' and 'topic deny test/hello/#' may be added so that a user can read/write to all test/# topics, except for test/hello/#. Signed-off-by: Brandt Hill Change variable name for clarity. Remember to initialize bool (I'm bad at C). Signed-off-by: Brandt Hill Add documentation to config man page Signed-off-by: Brandt Hill Add test case for deny option Signed-off-by: Brandt Hill Add deny acls to top of the list to preserve early exit Signed-off-by: Brandt Hill change comments Signed-off-by: Brandt Hill --- man/mosquitto.conf.5.xml | 12 +++++---- src/security_default.c | 38 +++++++++++++++++++++------ test/broker/09-acl-access-variants.py | 12 ++++++--- 3 files changed, 46 insertions(+), 16 deletions(-) diff --git a/man/mosquitto.conf.5.xml b/man/mosquitto.conf.5.xml index 01913e29..75ccde65 100644 --- a/man/mosquitto.conf.5.xml +++ b/man/mosquitto.conf.5.xml @@ -107,14 +107,16 @@ listed will have access. Topic access is added with lines of the format: - topic [read|write|readwrite] <topic> + topic [read|write|readwrite|deny] <topic> - The access type is controlled using "read", "write" or - "readwrite". This parameter is optional (unless + The access type is controlled using "read", "write", + "readwrite" or "deny". This parameter is optional (unless <topic> includes a space character) - if not given then the access is read/write. <topic> can contain the + or # wildcards as in - subscriptions. + subscriptions. The "deny" option can used to explicity + deny access to a topic that would otherwise be granted + by a broader read/write/readwrite statement. The first set of topics are applied to anonymous clients, assuming is @@ -131,7 +133,7 @@ substitution within the topic. The form is the same as for the topic keyword, but using pattern as the keyword. - pattern [read|write|readwrite] <topic> + pattern [read|write|readwrite|deny] <topic> The patterns available for substition are: diff --git a/src/security_default.c b/src/security_default.c index 80638bb4..8b0fe53f 100644 --- a/src/security_default.c +++ b/src/security_default.c @@ -217,10 +217,16 @@ int add__acl(struct mosquitto__security_options *security_opts, const char *user /* Add acl to user acl list */ if(acl_user->acl){ acl_tail = acl_user->acl; - while(acl_tail->next){ - acl_tail = acl_tail->next; + if(access == MOSQ_ACL_NONE){ + /* Put "deny" acls at front of the list */ + acl->next = acl_tail; + acl_user->acl = acl; + }else{ + while(acl_tail->next){ + acl_tail = acl_tail->next; + } + acl_tail->next = acl; } - acl_tail->next = acl; }else{ acl_user->acl = acl; } @@ -291,10 +297,16 @@ int add__acl_pattern(struct mosquitto__security_options *security_opts, const ch if(security_opts->acl_patterns){ acl_tail = security_opts->acl_patterns; - while(acl_tail->next){ - acl_tail = acl_tail->next; + if(access == MOSQ_ACL_NONE){ + /* Put "deny" acls at front of the list */ + acl->next = acl_tail; + security_opts->acl_patterns = acl; + }else{ + while(acl_tail->next){ + acl_tail = acl_tail->next; + } + acl_tail->next = acl; } - acl_tail->next = acl; }else{ security_opts->acl_patterns = acl; } @@ -334,7 +346,7 @@ int mosquitto_acl_check_default(struct mosquitto_db *db, struct mosquitto *conte acl_root = NULL; } - /* Loop through all ACLs for this client. */ + /* Loop through all ACLs for this client. ACL denials are iterated over first. */ while(acl_root){ /* Loop through the topic looking for matches to this ACL. */ @@ -345,6 +357,10 @@ int mosquitto_acl_check_default(struct mosquitto_db *db, struct mosquitto *conte } mosquitto_topic_matches_sub(acl_root->topic, topic, &result); if(result){ + if(acl_root->access == MOSQ_ACL_NONE){ + /* Access was explicitly denied for this topic. */ + return MOSQ_ERR_ACL_DENIED; + } if(access & acl_root->access){ /* And access is allowed. */ return MOSQ_ERR_SUCCESS; @@ -374,7 +390,7 @@ int mosquitto_acl_check_default(struct mosquitto_db *db, struct mosquitto *conte } } - /* Loop through all pattern ACLs. */ + /* Loop through all pattern ACLs. ACL denial patterns are iterated over first. */ if(!context->id) return MOSQ_ERR_ACL_DENIED; clen = strlen(context->id); @@ -418,6 +434,10 @@ int mosquitto_acl_check_default(struct mosquitto_db *db, struct mosquitto *conte mosquitto_topic_matches_sub(local_acl, topic, &result); mosquitto__free(local_acl); if(result){ + if(acl_root->access == MOSQ_ACL_NONE){ + /* Access was explicitly denied for this topic pattern. */ + return MOSQ_ERR_ACL_DENIED; + } if(access & acl_root->access){ /* And access is allowed. */ return MOSQ_ERR_SUCCESS; @@ -501,6 +521,8 @@ static int aclfile__parse(struct mosquitto_db *db, struct mosquitto__security_op access = MOSQ_ACL_WRITE; }else if(!strcmp(access_s, "readwrite")){ access = MOSQ_ACL_READ | MOSQ_ACL_WRITE; + }else if(!strcmp(access_s, "deny")){ + access = MOSQ_ACL_NONE; }else{ log__printf(NULL, MOSQ_LOG_ERR, "Error: Invalid topic access type \"%s\" in acl_file \"%s\".", access_s, security_opts->acl_file); mosquitto__free(user); diff --git a/test/broker/09-acl-access-variants.py b/test/broker/09-acl-access-variants.py index f20a5eed..c9b8849b 100755 --- a/test/broker/09-acl-access-variants.py +++ b/test/broker/09-acl-access-variants.py @@ -13,12 +13,15 @@ def write_config(filename, port, per_listener): def write_acl(filename, global_en, user_en, pattern_en): with open(filename, 'w') as f: if global_en: - f.write('topic readwrite topic/global\n') + f.write('topic readwrite topic/global/#\n') + f.write('topic deny topic/global/except\n') if user_en: f.write('user username\n') - f.write('topic readwrite topic/username\n') + f.write('topic readwrite topic/username/#\n') + f.write('topic deny topic/username/except\n') if pattern_en: - f.write('pattern readwrite pattern/%u\n') + f.write('pattern readwrite pattern/%u/#\n') + f.write('pattern deny pattern/%u/except\n') @@ -73,12 +76,15 @@ def acl_test(port, per_listener, global_en, user_en, pattern_en): if global_en: single_test(port, per_listener, username=None, topic="topic/global", expect_deny=False) single_test(port, per_listener, username="username", topic="topic/global", expect_deny=True) + single_test(port, per_listener, username=None, topic="topic/global/except", expect_deny=True) if user_en: single_test(port, per_listener, username=None, topic="topic/username", expect_deny=True) single_test(port, per_listener, username="username", topic="topic/username", expect_deny=False) + single_test(port, per_listener, username="username", topic="topic/username/except", expect_deny=True) if pattern_en: single_test(port, per_listener, username=None, topic="pattern/username", expect_deny=True) single_test(port, per_listener, username="username", topic="pattern/username", expect_deny=False) + single_test(port, per_listener, username="username", topic="pattern/username/except", expect_deny=True) def do_test(port, per_listener): try: