Related to issue #11083

Recently, we added support for modules to include commands to existing ACL categories #11708. However, it is still not possible to provide access/restrict users to only module commands or set of commands except by explicitly adding each command to the ACL Rules for each user. In this issue, we will discuss how we can allow modules to create new ACL categories.

Creating new ACL Categories: This can be achieved by implementing a new Module API int RM_AddModuleACLCategories(RedisModuleCtx *ctx, const char *category_name).

Allocating space/bits for the new ACL categories: Redis presently has 21 ACL categories, and we will allow modules to declare new categories by allocating space for 43 new categories . We will set acl_categories flag bits for the categories added by the modules. For ensuring ownership of the categories we will add pointer to the module, who declares the category, to the struct ACLCategoryItem of ACLCommandCategories array. The default categories will have this pointer set to null.

Ownership and permissions: ACL categories ownership will be verified while setting a ACL category to module commands. The modules will be restricted to register commands only to the categories created by them. The modules can add ACL categories only while loading the module. Modules will not be allowed to unload, if any of the users have the module ACL categories in its ACL Rules. Category names can only be combination of case insensitive alphabetic characters.

For example,

Module foo_module creates a new category foo_aclcategory and creates two command foo_command_1 and foo_command_2. The module sets the command ACL category of foo_command_1 as foo_aclcategory and sets write ACL category for foo_command_2

> MODULE LOAD foo_module
> OK
> ACL SETUSER user1 on >password -@all +@foo_aclcategory
> OK
> AUTH user1 password
> OK
> foo_command_1
> OK
> foo_command_2
> (error) NOPERM

Here, the user1 will only have access to all the commands that are added to foo_aclcategory ACL categories like foo_command_1 and will not be permitted to access foo_command_2 as it does not belongs to foo_aclcategory

Comment From: hpatro

Thanks @roshkhatri for bringing this up. I think this will be helpful to define custom categories based on the different data structures/type introduced by modules.

@roshkhatri Would you be able to add some example/use case for easier understanding of others ?

Comment From: roshkhatri

Hey @hpatro, Thanks, I have updated the top comment with an example.

Comment From: hpatro

@madolson @oranagra What are your thoughts about this ?

Comment From: madolson

I think this approach makes sense. Enumerating some additional constraints I don't see outlined here: 1. Modules can only register commands to categories they create. 2. Categories should be constrained to alphabetic characters and case insensitive.

Redis presently has 21 ACL categories, and we will allow modules to declare new categories by allocating space for 43 new categories .

Ideally this would be more of a soft limit, as we just need more bits to extend it. However, 43 should be practically sufficient for most cases I can think of.

One open question I also have if we should require categories to look like <module_name>.<category> to prevent overlap, in that the module name is prepended onto the introduced category. I'm inclined to say that we shouldn't, and that module categories are exposed verbatim. It seems unlikely two separate modules would expose the same category, unless they are trying to be compatible.

Comment From: roshkhatri

I think this approach makes sense. Enumerating some additional constraints I don't see outlined here:

  1. Modules can only register commands to categories they create.
  2. Categories should be constrained to alphabetic characters and case insensitive.

Yes, these constraints are true for ACL categories declared by modules. Updated the comment above.

Comment From: oranagra

For ensuring ownership of the categories we will add pointer to the module, who declares the category

I don't think i understand why we need this.

The modules will be restricted to register commands only to the categories created by them

I don't think this is desired, what if two different modules want to participate in the same category? i.e. we already allow them to register for built-in categories (e.g. write), so why not also let them create a new general categories (e.g. introspection or whatever)?

@yossigo @MeirShpilraien FYI.

Comment From: madolson

I don't think i understand why we need this.

Presumably to support unloading the categories if the module is unloaded. There are multiple ways this could be implemented though.

I don't think this is desired, what if two different modules want to participate in the same category? (e.g. introspection or whatever)?

From a usability perspective I mostly agree. From a security perspective I have a concern that loading a new module would introduce unexpected permissions since users will now have access to new commands.

I suppose the question is if we believe modules will introduce "general categories". If there is interest, if there is a small finite number, maybe we can just add them into the core.

Comment From: oranagra

Presumably to support unloading the categories if the module is unloaded. There are multiple ways this could be implemented though.

I don't think we have to do that... i can argue that when a module created a new category, it is here to stay (like adding a key to the keyspace or changing a config, and unlike registering a command). in any case, i don't think unloading modules is an important use case. i think many modules are impossible to unload anyway.

From a usability perspective I mostly agree. From a security perspective I have a concern that loading a new module would introduce unexpected permissions since users will now have access to new commands.

if modules can introduce new commands into core categories (as they already do), this concern isn't relevant anymore anyway. unless i'm missing something. personally, i don't see why we should have any restriction, and i think that if there are two modules that have some common topic (maybe they're complementing each other), it's ok to let them both participate in the same category.

Comment From: madolson

personally, i don't see why we should have any restriction, and i think that if there are two modules that have some common topic (maybe they're complementing each other), it's ok to let them both participate in the same category.

Ok, I think I'm convinced of this. So the category register API is idempotent, and categories are not removed when the module is unloaded. Arguably this makes everything a lot simpler, as the API is basically just adding a category.

Comment From: madolson

if modules can introduce new commands into core categories (as they already do), this concern isn't relevant anymore anyway. unless i'm missing something.

The only difference is that module developers might disagree on what a category means, whereas I think there is a lot less ambiguity in what the core categories mean. I can't think of an immediate example, but maybe some module might overload a word for their commands while at the same time another module is using the literal meaning of it. Something like, "Search" might be for search index creation commands while "Search" might be commands that are O(N) and require searching.

Comment From: hpatro

@oranagra proposal makes the API lot simpler. "Provide a new API to create a new category (custom)". The ownership of that category doesn't belong to any particular module and would forever exist in the system.

One thing I'm unable to foresee as of now if a delete category (custom) API would be required at any point in the future.