Added support for controlling UNSUBSCRIBE calls in v5 plugin ACL checks.

eclipse · Oct 9, 2020 · 4472c1f · 4472c1f
1 parent 6c6542a
commit 4472c1f
Show file tree

Hide file tree

Showing 6 changed files with 47 additions and 17 deletions.
diff --git a/ChangeLog.txt b/ChangeLog.txt
@@ -49,6 +49,7 @@ Broker:
 - Add support for PBKDF2-SHA512 password hashing.
 - Enabling certificate based TLS encryption is now through certfile and
  keyfile, not capath or cafile.
+- Added support for controlling UNSUBSCRIBE calls in v5 plugin ACL checks.
 
 Client library:
 - Client no longer generates random client ids for v3.1.1 clients, these are

diff --git a/include/mosquitto_plugin.h b/include/mosquitto_plugin.h
@@ -28,6 +28,7 @@ extern "C" {
 #define MOSQ_ACL_READ 0x01
 #define MOSQ_ACL_WRITE 0x02
 #define MOSQ_ACL_SUBSCRIBE 0x04
+#define MOSQ_ACL_UNSUBSCRIBE 0x08
 
 #include <stdbool.h>
 #include <stdint.h>

diff --git a/src/handle_subscribe.c b/src/handle_subscribe.c
@@ -43,6 +43,7 @@ int handle__subscribe(struct mosquitto_db *db, struct mosquitto *context)
  int slen;
  char *sub_mount;
  mosquitto_property *properties = NULL;
+ bool allowed;
 
  if(!context) return MOSQ_ERR_INVAL;
 
@@ -160,21 +161,25 @@ int handle__subscribe(struct mosquitto_db *db, struct mosquitto *context)
  }
  log__printf(NULL, MOSQ_LOG_DEBUG, "\t%s (QoS %d)", sub, qos);
 
- if(context->protocol != mosq_p_mqtt31){
- rc2 = mosquitto_acl_check(db, context, sub, 0, NULL, qos, false, MOSQ_ACL_SUBSCRIBE);
- switch(rc2){
- case MOSQ_ERR_SUCCESS:
- break;
- case MOSQ_ERR_ACL_DENIED:
+ allowed = true;
+ rc2 = mosquitto_acl_check(db, context, sub, 0, NULL, qos, false, MOSQ_ACL_SUBSCRIBE);
+ switch(rc2){
+ case MOSQ_ERR_SUCCESS:
+ break;
+ case MOSQ_ERR_ACL_DENIED:
+ allowed = false;
+ if(context->protocol == mosq_p_mqtt5){
+ qos = MQTT_RC_NOT_AUTHORIZED;
+ }else if(context->protocol == mosq_p_mqtt311){
  qos = 0x80;
-  break;
- default:
-  mosquitto__free(sub);
-  return rc2;
- }
+ }
+ break;
+ default:
+ mosquitto__free(sub);
+  return rc2;
  }
 
- if(qos != 0x80){
+ if(allowed){
  rc2 = sub__add(db, context, sub, qos, subscription_identifier, subscription_options, &db->subs);
  if(rc2 > 0){
  mosquitto__free(sub);

diff --git a/src/handle_unsubscribe.c b/src/handle_unsubscribe.c
@@ -36,6 +36,7 @@ int handle__unsubscribe(struct mosquitto_db *db, struct mosquitto *context)
  int reason_code_max;
  uint8_t *reason_codes = NULL, *reason_tmp;
  mosquitto_property *properties = NULL;
+ bool allowed;
 
  if(!context) return MOSQ_ERR_INVAL;
 
@@ -105,8 +106,25 @@ int handle__unsubscribe(struct mosquitto_db *db, struct mosquitto *context)
  return MOSQ_ERR_MALFORMED_PACKET;
  }
 
+ /* ACL check */
+ allowed = true;
+ rc = mosquitto_acl_check(db, context, sub, 0, NULL, 0, false, MOSQ_ACL_UNSUBSCRIBE);
+ switch(rc){
+ case MOSQ_ERR_SUCCESS:
+ break;
+ case MOSQ_ERR_ACL_DENIED:
+ allowed = false;
+ reason = MQTT_RC_NOT_AUTHORIZED;
+ break;
+ default:
+ mosquitto__free(sub);
+ return rc;
+ }
+
  log__printf(NULL, MOSQ_LOG_DEBUG, "\t%s", sub);
- rc = sub__remove(db, context, sub, db->subs, &reason);
+ if(allowed){
+ rc = sub__remove(db, context, sub, db->subs, &reason);
+ }
  log__printf(NULL, MOSQ_LOG_UNSUBSCRIBE, "%s %s", context->id, sub);
  mosquitto__free(sub);
  if(rc){

diff --git a/src/security.c b/src/security.c
@@ -614,11 +614,17 @@ static int acl__check_single(struct mosquitto__auth_plugin_config *auth_plugin,
  }
 
  if(auth_plugin->plugin.version == 4){
+ if(access == MOSQ_ACL_UNSUBSCRIBE){
+ return MOSQ_ERR_SUCCESS;
+ }
  return auth_plugin->plugin.acl_check_v4(auth_plugin->plugin.user_data, access, context, msg);
  }else if(auth_plugin->plugin.version == 3){
+ if(access == MOSQ_ACL_UNSUBSCRIBE){
+ return MOSQ_ERR_SUCCESS;
+ }
  return auth_plugin->plugin.acl_check_v3(auth_plugin->plugin.user_data, access, context, msg);
  }else if(auth_plugin->plugin.version == 2){
- if(access == MOSQ_ACL_SUBSCRIBE){
+ if(access == MOSQ_ACL_SUBSCRIBE || access == MOSQ_ACL_UNSUBSCRIBE){
  return MOSQ_ERR_SUCCESS;
  }
  return auth_plugin->plugin.acl_check_v2(auth_plugin->plugin.user_data, context->id, username, topic, access);
@@ -649,8 +655,7 @@ static int acl__check_dollar(const char *topic, int access)
  }
  }else if(!strncmp(topic, "$share", 6)){
  /* Only allow sub/unsub to shared subscriptions */
- if(access == MOSQ_ACL_SUBSCRIBE){
- /* FIXME if(access == MOSQ_ACL_SUBSCRIBE || access == MOSQ_ACL_UNSUBSCRIBE){ */
+ if(access == MOSQ_ACL_SUBSCRIBE || access == MOSQ_ACL_UNSUBSCRIBE){
  return MOSQ_ERR_SUCCESS;
  }else{
  return MOSQ_ERR_ACL_DENIED;

diff --git a/src/security_default.c b/src/security_default.c
@@ -327,7 +327,7 @@ int mosquitto_acl_check_default(struct mosquitto_db *db, struct mosquitto *conte
  return MOSQ_ERR_PLUGIN_DEFER;
  }
 
- if(access == MOSQ_ACL_SUBSCRIBE) return MOSQ_ERR_SUCCESS; /* FIXME - implement ACL subscription strings. */
+ if(access == MOSQ_ACL_SUBSCRIBE || access == MOSQ_ACL_UNSUBSCRIBE) return MOSQ_ERR_SUCCESS; /* FIXME - implement ACL subscription strings. */
  if(!context->acl_list && !security_opts->acl_patterns) return MOSQ_ERR_ACL_DENIED;
 
  if(context->acl_list){