From 6948dacaf63415c6cabce207cd7d23dcb37dd5e1 Mon Sep 17 00:00:00 2001 From: Roshan Khatri <117414976+roshkhatri@users.noreply.github.com> Date: Tue, 21 Mar 2023 10:07:11 -0700 Subject: Module commands to have ACL categories. (#11708) This allows modules to register commands to existing ACL categories and blocks the creation of [sub]commands, datatypes and registering the configs outside of the OnLoad function. For allowing modules to register commands to existing ACL categories, This PR implements a new API int RM_SetCommandACLCategories() which takes a pointer to a RedisModuleCommand and a C string aclflags containing the set of space separated ACL categories. Example, 'write slow' marks the command as part of the write and slow ACL categories. The C string aclflags is tokenized by implementing a helper function categoryFlagsFromString(). Theses tokens are matched and the corresponding ACL categories flags are set by a helper function matchAclCategoriesFlags. The helper function categoryFlagsFromString() returns the corresponding categories_flags or returns -1 if some token not processed correctly. If the module contains commands which are registered to existing ACL categories, the number of [sub]commands are tracked by num_commands_with_acl_categories in struct RedisModule. Further, the allowed command bit-map of the existing users are recomputed from the command_rules list, by implementing a function called ACLRecomputeCommandBitsFromCommandRulesAllUsers() for the existing users to have access to the module commands on runtime. ## Breaking change This change requires that registering commands and subcommands only occur during a modules "OnLoad" function, in order to allow efficient recompilation of ACL bits. We also chose to block registering configs and types, since we believe it's only valid for those to be created during onLoad. We check for this onload flag in struct RedisModule to check if the call is made from the OnLoad function. Co-authored-by: Madelyn Olson --- tests/modules/aclcheck.c | 55 ++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 55 insertions(+) (limited to 'tests/modules/aclcheck.c') diff --git a/tests/modules/aclcheck.c b/tests/modules/aclcheck.c index 9f4564d27..09b525cc5 100644 --- a/tests/modules/aclcheck.c +++ b/tests/modules/aclcheck.c @@ -183,6 +183,37 @@ int rm_call_aclcheck(RedisModuleCtx *ctx, RedisModuleString **argv, int argc){ return REDISMODULE_OK; } +int module_test_acl_category(RedisModuleCtx *ctx, RedisModuleString **argv, int argc) { + REDISMODULE_NOT_USED(argv); + REDISMODULE_NOT_USED(argc); + RedisModule_ReplyWithSimpleString(ctx, "OK"); + return REDISMODULE_OK; +} + +int commandBlockCheck(RedisModuleCtx *ctx, RedisModuleString **argv, int argc) { + REDISMODULE_NOT_USED(argv); + REDISMODULE_NOT_USED(argc); + int response_ok = 0; + int result = RedisModule_CreateCommand(ctx,"command.that.should.fail", module_test_acl_category, "", 0, 0, 0); + response_ok |= (result == REDISMODULE_OK); + + RedisModuleCommand *parent = RedisModule_GetCommand(ctx,"block.commands.outside.onload"); + result = RedisModule_SetCommandACLCategories(parent, "write"); + response_ok |= (result == REDISMODULE_OK); + + result = RedisModule_CreateSubcommand(parent,"subcommand.that.should.fail",module_test_acl_category,"",0,0,0); + response_ok |= (result == REDISMODULE_OK); + + /* This validates that it's not possible to create commands outside OnLoad, + * thus returns an error if they succeed. */ + if (response_ok) { + RedisModule_ReplyWithError(ctx, "UNEXPECTEDOK"); + } else { + RedisModule_ReplyWithSimpleString(ctx, "OK"); + } + return REDISMODULE_OK; +} + int RedisModule_OnLoad(RedisModuleCtx *ctx, RedisModuleString **argv, int argc) { REDISMODULE_NOT_USED(argv); REDISMODULE_NOT_USED(argc); @@ -193,6 +224,30 @@ int RedisModule_OnLoad(RedisModuleCtx *ctx, RedisModuleString **argv, int argc) if (RedisModule_CreateCommand(ctx,"aclcheck.set.check.key", set_aclcheck_key,"write",0,0,0) == REDISMODULE_ERR) return REDISMODULE_ERR; + if (RedisModule_CreateCommand(ctx,"block.commands.outside.onload", commandBlockCheck,"write",0,0,0) == REDISMODULE_ERR) + return REDISMODULE_ERR; + + if (RedisModule_CreateCommand(ctx,"aclcheck.module.command.aclcategories.write", module_test_acl_category,"write",0,0,0) == REDISMODULE_ERR) + return REDISMODULE_ERR; + RedisModuleCommand *aclcategories_write = RedisModule_GetCommand(ctx,"aclcheck.module.command.aclcategories.write"); + + if (RedisModule_SetCommandACLCategories(aclcategories_write, "write") == REDISMODULE_ERR) + return REDISMODULE_ERR; + + if (RedisModule_CreateCommand(ctx,"aclcheck.module.command.aclcategories.write.function.read.category", module_test_acl_category,"write",0,0,0) == REDISMODULE_ERR) + return REDISMODULE_ERR; + RedisModuleCommand *read_category = RedisModule_GetCommand(ctx,"aclcheck.module.command.aclcategories.write.function.read.category"); + + if (RedisModule_SetCommandACLCategories(read_category, "read") == REDISMODULE_ERR) + return REDISMODULE_ERR; + + if (RedisModule_CreateCommand(ctx,"aclcheck.module.command.aclcategories.read.only.category", module_test_acl_category,"",0,0,0) == REDISMODULE_ERR) + return REDISMODULE_ERR; + RedisModuleCommand *read_only_category = RedisModule_GetCommand(ctx,"aclcheck.module.command.aclcategories.read.only.category"); + + if (RedisModule_SetCommandACLCategories(read_only_category, "read") == REDISMODULE_ERR) + return REDISMODULE_ERR; + if (RedisModule_CreateCommand(ctx,"aclcheck.publish.check.channel", publish_aclcheck_channel,"",0,0,0) == REDISMODULE_ERR) return REDISMODULE_ERR; -- cgit v1.2.1