[OpenWrt-Devel] [PATCH] ubus: fully allow wildcards in acl

[koen.cj.dergent at gmail.com]

From: Koen Dergent <koen.cj.dergent at gmail.com>

Wildcards were supported in the ACL code, but they did not work properly.

The compare function used for the tree containing the ACL's was not 
stable. It violated the expected behaviour.
( cmp(a, b) == -cmp(b, a) )
This made the ACL tree useless in case wildcards were used.

Changed so the tree now uses the normal avl_strcmp.

As a downside we have to iterate over all ACL entries in order to find
a match.

Wildcards are now supported for both object and method names.

The internal replacement of * with TAB was removed as it looked like a
complication with no real benefit.

Signed-off-by: Koen Dergent <koen.cj.dergent at gmail.com>
---
 ubusd_acl.c | 81 ++++++++++++++++++++++++++-----------------------------------
 1 file changed, 34 insertions(+), 47 deletions(-)

diff --git a/ubusd_acl.c b/ubusd_acl.c
index 4b72663..a1e52e6 100644
--- a/ubusd_acl.c
+++ b/ubusd_acl.c
@@ -12,6 +12,7 @@
  */
 
 #define _GNU_SOURCE
+#include <stdbool.h>
 #include <sys/socket.h>
 #include <sys/types.h>
 #include <sys/stat.h>
@@ -68,53 +69,48 @@ static struct avl_tree ubusd_acls;
 static int ubusd_acl_seq;
 static struct ubus_object *acl_obj;
 
-static int
-ubusd_acl_match_path(const void *k1, const void *k2, void *ptr)
+static bool
+name_matches_pattern(const char *name, const char *pattern)
 {
-	const char *name = k1;
-	const char *match = k2;
-	char *wildcard = strstr(match, "\t");
+	char *wildcard = strstr(pattern, "*");
 
 	if (wildcard)
-		return strncmp(name, match, wildcard - match);
+		return strncmp(name, pattern, wildcard - pattern)==0;
 
-	return strcmp(name, match);
+	return strcmp(name, pattern)==0;
 }
 
-static int
-ubusd_acl_match_cred(struct ubus_client *cl, struct ubusd_acl_obj *obj)
+static bool
+client_credentials_match_acl(struct ubus_client *cl, struct ubusd_acl_obj *obj)
 {
 	if (obj->user && !strcmp(cl->user, obj->user))
-		return 0;
+		return true;
 
 	if (obj->group && !strcmp(cl->group, obj->group))
-		return 0;
+		return true;
 
-	return -1;
+	return false;
+}
+
+static bool
+client_is_root(struct ubus_client *cl)
+{
+	return cl->uid==0;
 }
 
 int
-ubusd_acl_check(struct ubus_client *cl, const char *obj,
+ubusd_acl_check(struct ubus_client *cl, const char *objname,
 		const char *method, enum ubusd_acl_type type)
 {
-	struct ubusd_acl_obj *acl;
-	struct blob_attr *cur;
-	int rem;
-
-	if (!cl->uid || !obj)
-		return 0;
-
-	acl = avl_find_ge_element(&ubusd_acls, obj, acl, avl);
-	if (!acl)
-		return -1;
-
-	avl_for_element_to_last(&ubusd_acls, acl, acl, avl) {
-		int diff = ubusd_acl_match_path(obj, acl->avl.key, NULL);
+	if (!objname) return 0;
+	if (client_is_root(cl)) return 0;
 
-		if (diff)
-			break;
+	struct ubusd_acl_obj *acl;
+	avl_for_each_element(&ubusd_acls, acl, avl) {
+		if (!name_matches_pattern(objname, (const char*)acl->avl.key))
+			continue;
 
-		if (ubusd_acl_match_cred(cl, acl))
+		if (!client_credentials_match_acl(cl, acl))
 			continue;
 
 		switch (type) {
@@ -129,11 +125,14 @@ ubusd_acl_check(struct ubus_client *cl, const char *obj,
 			break;
 
 		case UBUS_ACL_ACCESS:
-			if (acl->methods)
+			if (acl->methods) {
+				struct blob_attr *cur;
+				int rem;
 				blobmsg_for_each_attr(cur, acl->methods, rem)
 					if (blobmsg_type(cur) == BLOBMSG_TYPE_STRING)
-						if (!ubusd_acl_match_path(method, blobmsg_get_string(cur), NULL))
+						if (name_matches_pattern(method, blobmsg_get_string(cur)))
 							return 0;
+			}
 			break;
 		}
 	}
@@ -220,12 +219,6 @@ ubusd_acl_alloc_obj(struct ubusd_acl_file *file, const char *obj)
 	o->avl.key = k;
 	strcpy(k, obj);
 
-	while (*k) {
-		if (*k == '*')
-			*k = '\t';
-		k++;
-	}
-
 	list_add(&o->list, &file->acl);
 	avl_insert(&ubusd_acls, &o->avl);
 
@@ -424,20 +417,14 @@ ubusd_reply_add(struct ubus_object *obj)
 	if (!obj->path.key)
 		return;
 
-	acl = avl_find_ge_element(&ubusd_acls, obj->path.key, acl, avl);
-	if (!acl)
-		return;
-
-	avl_for_element_to_last(&ubusd_acls, acl, acl, avl) {
-		void *c;
-
+	avl_for_each_element(&ubusd_acls, acl, avl) {
 		if (!acl->priv)
 			continue;
 
-		if (ubusd_acl_match_path(obj->path.key, acl->avl.key, NULL))
+		if (!name_matches_pattern((const char*)obj->path.key, (const char*)acl->avl.key))
 			continue;
 
-		c = blobmsg_open_table(&b, NULL);
+		void *c = blobmsg_open_table(&b, NULL);
 		blobmsg_add_string(&b, "obj", obj->path.key);
 		if (acl->user)
 			blobmsg_add_string(&b, "user", acl->user);
@@ -489,7 +476,7 @@ static int ubusd_acl_recv(struct ubus_client *cl, struct ubus_msg_buf *ub, const
 
 void ubusd_acl_init(void)
 {
-	avl_init(&ubusd_acls, ubusd_acl_match_path, true, NULL);
+	avl_init(&ubusd_acls, avl_strcmp, true, NULL);
 	acl_obj = ubusd_create_object_internal(NULL, UBUS_SYSTEM_OBJECT_ACL);
 	acl_obj->recv_msg = ubusd_acl_recv;
 }

_______________________________________________
openwrt-devel mailing list
openwrt-devel at lists.openwrt.org
https://lists.openwrt.org/listinfo/openwrt-devel