diff --git a/src/acl.c b/src/acl.c index 5fd956d23..900746ece 100644 --- a/src/acl.c +++ b/src/acl.c @@ -59,10 +59,12 @@ static rax *commandId = NULL; /* Command name to id mapping */ static unsigned long nextid = 0; /* Next command id that has not been assigned */ +#define ACL_MAX_CATEGORIES 64 /* Maximum number of command categories */ + struct ACLCategoryItem { - const char *name; + char *name; uint64_t flag; -} ACLCommandCategories[] = { /* See redis.conf for details on each category. */ +} ACLDefaultCommandCategories[] = { /* See redis.conf for details on each category. */ {"keyspace", ACL_CATEGORY_KEYSPACE}, {"read", ACL_CATEGORY_READ}, {"write", ACL_CATEGORY_WRITE}, @@ -87,6 +89,54 @@ struct ACLCategoryItem { {NULL,0} /* Terminator. */ }; +static struct ACLCategoryItem *ACLCommandCategories = NULL; +static size_t nextCommandCategory = 0; /* Index of the next command category to be added */ + +/* Implements the ability to add to the list of ACL categories at runtime. Since each ACL category + * also requires a bit in the acl_categories flag, there is a limit to the number that can be added. + * The new ACL categories occupy the remaining bits of acl_categories flag, other than the bits + * occupied by the default ACL command categories. + * + * The optional `flag` argument allows the assignment of the `acl_categories` flag bit to the ACL category. + * When adding a new category, except for the default ACL command categories, this arguments should be `0` + * to allow the function to assign the next available `acl_categories` flag bit to the new ACL category. + * + * returns 1 -> Added, 0 -> Failed (out of space) + * + * This function is present here to gain access to the ACLCommandCategories array and add a new ACL category. + */ +int ACLAddCommandCategory(const char *name, uint64_t flag) { + if (nextCommandCategory >= ACL_MAX_CATEGORIES) return 0; + ACLCommandCategories[nextCommandCategory].name = zstrdup(name); + ACLCommandCategories[nextCommandCategory].flag = flag != 0 ? flag : (1ULL<module->onload) { + errno = EINVAL; + return REDISMODULE_ERR; + } + + if (moduleVerifyResourceName(name) == REDISMODULE_ERR) { + errno = EINVAL; + return REDISMODULE_ERR; + } + + if (ACLGetCommandCategoryFlagByName(name)) { + errno = EBUSY; + return REDISMODULE_ERR; + } + + if (ACLAddCommandCategory(name, 0)) { + ctx->module->num_acl_categories_added++; + return REDISMODULE_OK; + } else { + errno = ENOMEM; + return REDISMODULE_ERR; + } +} + /* Helper for categoryFlagsFromString(). Attempts to find an acl flag representing the provided flag string * and adds that flag to acl_categories_flags if a match is found. * @@ -2252,6 +2295,7 @@ void RM_SetModuleAttribs(RedisModuleCtx *ctx, const char *name, int ver, int api module->loadmod = NULL; module->num_commands_with_acl_categories = 0; module->onload = 1; + module->num_acl_categories_added = 0; ctx->module = module; } @@ -11937,6 +11981,13 @@ void moduleRemoveConfigs(RedisModule *module) { } } +/* Remove ACL categories added by the module when it fails to load. */ +void moduleRemoveCateogires(RedisModule *module) { + if (module->num_acl_categories_added) { + ACLCleanupCategoriesOnFailure(module->num_acl_categories_added); + } +} + /* Load all the modules in the server.loadmodule_queue list, which is * populated by `loadmodule` directives in the configuration file. * We can't load modules directly when processing the configuration file @@ -12152,6 +12203,7 @@ int moduleLoad(const char *path, void **module_argv, int module_argc, int is_loa moduleUnregisterCommands(ctx.module); moduleUnregisterSharedAPI(ctx.module); moduleUnregisterUsedAPI(ctx.module); + moduleRemoveCateogires(ctx.module); moduleRemoveConfigs(ctx.module); moduleUnregisterAuthCBs(ctx.module); moduleFreeModuleStructure(ctx.module); @@ -12424,12 +12476,14 @@ int moduleVerifyConfigFlags(unsigned int flags, configType type) { return REDISMODULE_OK; } -int moduleVerifyConfigName(sds name) { - if (sdslen(name) == 0) { - serverLogRaw(LL_WARNING, "Module config names cannot be an empty string."); +/* Verify a module resource or name has only alphanumeric characters, underscores + * or dashes. */ +int moduleVerifyResourceName(const char *name) { + if (name[0] == '\0') { return REDISMODULE_ERR; } - for (size_t i = 0 ; i < sdslen(name) ; ++i) { + + for (size_t i = 0; name[i] != '\0'; i++) { char curr_char = name[i]; if ((curr_char >= 'a' && curr_char <= 'z') || (curr_char >= 'A' && curr_char <= 'Z') || @@ -12438,7 +12492,7 @@ int moduleVerifyConfigName(sds name) { { continue; } - serverLog(LL_WARNING, "Invalid character %c in Module Config name %s.", curr_char, name); + serverLog(LL_WARNING, "Invalid character %c in Module resource name %s.", curr_char, name); return REDISMODULE_ERR; } return REDISMODULE_OK; @@ -12597,7 +12651,7 @@ int moduleConfigValidityCheck(RedisModule *module, sds name, unsigned int flags, errno = EBUSY; return REDISMODULE_ERR; } - if (moduleVerifyConfigFlags(flags, type) || moduleVerifyConfigName(name)) { + if (moduleVerifyConfigFlags(flags, type) || moduleVerifyResourceName(name)) { errno = EINVAL; return REDISMODULE_ERR; } @@ -13505,6 +13559,7 @@ void moduleRegisterCoreAPI(void) { REGISTER_API(CreateSubcommand); REGISTER_API(SetCommandInfo); REGISTER_API(SetCommandACLCategories); + REGISTER_API(AddACLCategory); REGISTER_API(SetModuleAttribs); REGISTER_API(IsModuleNameBusy); REGISTER_API(WrongArity); diff --git a/src/redismodule.h b/src/redismodule.h index 4378126e2..de533d049 100644 --- a/src/redismodule.h +++ b/src/redismodule.h @@ -968,6 +968,7 @@ REDISMODULE_API RedisModuleCommand *(*RedisModule_GetCommand)(RedisModuleCtx *ct REDISMODULE_API int (*RedisModule_CreateSubcommand)(RedisModuleCommand *parent, const char *name, RedisModuleCmdFunc cmdfunc, const char *strflags, int firstkey, int lastkey, int keystep) REDISMODULE_ATTR; REDISMODULE_API int (*RedisModule_SetCommandInfo)(RedisModuleCommand *command, const RedisModuleCommandInfo *info) REDISMODULE_ATTR; REDISMODULE_API int (*RedisModule_SetCommandACLCategories)(RedisModuleCommand *command, const char *ctgrsflags) REDISMODULE_ATTR; +REDISMODULE_API int (*RedisModule_AddACLCategory)(RedisModuleCtx *ctx, const char *name) REDISMODULE_ATTR; REDISMODULE_API void (*RedisModule_SetModuleAttribs)(RedisModuleCtx *ctx, const char *name, int ver, int apiver) REDISMODULE_ATTR; REDISMODULE_API int (*RedisModule_IsModuleNameBusy)(const char *name) REDISMODULE_ATTR; REDISMODULE_API int (*RedisModule_WrongArity)(RedisModuleCtx *ctx) REDISMODULE_ATTR; @@ -1329,6 +1330,7 @@ static int RedisModule_Init(RedisModuleCtx *ctx, const char *name, int ver, int REDISMODULE_GET_API(CreateSubcommand); REDISMODULE_GET_API(SetCommandInfo); REDISMODULE_GET_API(SetCommandACLCategories); + REDISMODULE_GET_API(AddACLCategory); REDISMODULE_GET_API(SetModuleAttribs); REDISMODULE_GET_API(IsModuleNameBusy); REDISMODULE_GET_API(WrongArity); diff --git a/src/server.h b/src/server.h index 756435664..939da4365 100644 --- a/src/server.h +++ b/src/server.h @@ -821,6 +821,7 @@ struct RedisModule { struct moduleLoadQueueEntry *loadmod; /* Module load arguments for config rewrite. */ int num_commands_with_acl_categories; /* Number of commands in this module included in acl categories */ int onload; /* Flag to identify if the call is being made from Onload (0 or 1) */ + size_t num_acl_categories_added; /* Number of acl categories added by this module. */ }; typedef struct RedisModule RedisModule; @@ -2927,6 +2928,8 @@ int ACLCheckAllPerm(client *c, int *idxptr); int ACLSetUser(user *u, const char *op, ssize_t oplen); sds ACLStringSetUser(user *u, sds username, sds *argv, int argc); uint64_t ACLGetCommandCategoryFlagByName(const char *name); +int ACLAddCommandCategory(const char *name, uint64_t flag); +void ACLCleanupCategoriesOnFailure(size_t num_acl_categories_added); int ACLAppendUserForLoading(sds *argv, int argc, int *argc_err); const char *ACLSetUserStringError(void); int ACLLoadConfiguredUsers(void); diff --git a/tests/modules/aclcheck.c b/tests/modules/aclcheck.c index 09b525cc5..b74651804 100644 --- a/tests/modules/aclcheck.c +++ b/tests/modules/aclcheck.c @@ -197,6 +197,9 @@ int commandBlockCheck(RedisModuleCtx *ctx, RedisModuleString **argv, int argc) { int result = RedisModule_CreateCommand(ctx,"command.that.should.fail", module_test_acl_category, "", 0, 0, 0); response_ok |= (result == REDISMODULE_OK); + result = RedisModule_AddACLCategory(ctx,"blockedcategory"); + response_ok |= (result == REDISMODULE_OK); + RedisModuleCommand *parent = RedisModule_GetCommand(ctx,"block.commands.outside.onload"); result = RedisModule_SetCommandACLCategories(parent, "write"); response_ok |= (result == REDISMODULE_OK); @@ -204,7 +207,8 @@ int commandBlockCheck(RedisModuleCtx *ctx, RedisModuleString **argv, int argc) { 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, + /* This validates that it's not possible to create commands or add + * a new ACL Category outside OnLoad function. * thus returns an error if they succeed. */ if (response_ok) { RedisModule_ReplyWithError(ctx, "UNEXPECTEDOK"); @@ -215,12 +219,31 @@ int commandBlockCheck(RedisModuleCtx *ctx, RedisModuleString **argv, int argc) { } int RedisModule_OnLoad(RedisModuleCtx *ctx, RedisModuleString **argv, int argc) { - REDISMODULE_NOT_USED(argv); - REDISMODULE_NOT_USED(argc); if (RedisModule_Init(ctx,"aclcheck",1,REDISMODULE_APIVER_1)== REDISMODULE_ERR) return REDISMODULE_ERR; + if (argc > 1) return RedisModule_WrongArity(ctx); + + /* When that flag is passed, we try to create too many categories, + * and the test expects this to fail. In this case redis returns REDISMODULE_ERR + * and set errno to ENOMEM*/ + if (argc == 1) { + long long fail_flag = 0; + RedisModule_StringToLongLong(argv[0], &fail_flag); + if (fail_flag) { + for (size_t j = 0; j < 45; j++) { + RedisModuleString* name = RedisModule_CreateStringPrintf(ctx, "customcategory%zu", j); + if (RedisModule_AddACLCategory(ctx, RedisModule_StringPtrLen(name, NULL)) == REDISMODULE_ERR) { + RedisModule_Assert(errno == ENOMEM); + RedisModule_FreeString(ctx, name); + return REDISMODULE_ERR; + } + RedisModule_FreeString(ctx, name); + } + } + } + if (RedisModule_CreateCommand(ctx,"aclcheck.set.check.key", set_aclcheck_key,"write",0,0,0) == REDISMODULE_ERR) return REDISMODULE_ERR; @@ -265,5 +288,29 @@ int RedisModule_OnLoad(RedisModuleCtx *ctx, RedisModuleString **argv, int argc) "write",0,0,0) == REDISMODULE_ERR) return REDISMODULE_ERR; + /* This validates that, when module tries to add a category with invalid characters, + * redis returns REDISMODULE_ERR and set errno to `EINVAL` */ + if (RedisModule_AddACLCategory(ctx,"!nval!dch@r@cter$") == REDISMODULE_ERR) + RedisModule_Assert(errno == EINVAL); + else + return REDISMODULE_ERR; + + /* This validates that, when module tries to add a category that already exists, + * redis returns REDISMODULE_ERR and set errno to `EBUSY` */ + if (RedisModule_AddACLCategory(ctx,"write") == REDISMODULE_ERR) + RedisModule_Assert(errno == EBUSY); + else + return REDISMODULE_ERR; + + if (RedisModule_AddACLCategory(ctx,"foocategory") == REDISMODULE_ERR) + return REDISMODULE_ERR; + + if (RedisModule_CreateCommand(ctx,"aclcheck.module.command.test.add.new.aclcategories", module_test_acl_category,"",0,0,0) == REDISMODULE_ERR) + return REDISMODULE_ERR; + RedisModuleCommand *test_add_new_aclcategories = RedisModule_GetCommand(ctx,"aclcheck.module.command.test.add.new.aclcategories"); + + if (RedisModule_SetCommandACLCategories(test_add_new_aclcategories, "foocategory") == REDISMODULE_ERR) + return REDISMODULE_ERR; + return REDISMODULE_OK; } diff --git a/tests/unit/moduleapi/aclcheck.tcl b/tests/unit/moduleapi/aclcheck.tcl index ae3f67156..1ea09a232 100644 --- a/tests/unit/moduleapi/aclcheck.tcl +++ b/tests/unit/moduleapi/aclcheck.tcl @@ -104,15 +104,22 @@ start_server {tags {"modules acl"}} { assert_equal [r acl DRYRUN j2 aclcheck.module.command.aclcategories.read.only.category] OK } + test {Unload the module - aclcheck} { + assert_equal {OK} [r module unload aclcheck] + } +} + +start_server {tags {"modules acl"}} { test {test existing users to have access to module commands loaded on runtime} { - assert_equal [r module unload aclcheck] OK r acl SETUSER j3 on >password -@all +@WRITE assert_equal [r module load $testmodule] OK assert_equal [r acl DRYRUN j3 aclcheck.module.command.aclcategories.write] OK + assert_equal {OK} [r module unload aclcheck] } +} +start_server {tags {"modules acl"}} { test {test existing users without permissions, do not have access to module commands loaded on runtime.} { - assert_equal [r module unload aclcheck] OK r acl SETUSER j4 on >password -@all +@READ r acl SETUSER j5 on >password -@all +@WRITE assert_equal [r module load $testmodule] OK @@ -131,7 +138,21 @@ start_server {tags {"modules acl"}} { assert_equal {User j7 has no permissions to run the 'aclcheck.module.command.aclcategories.write.function.read.category' command} $e } - test "Unload the module - aclcheck" { + test {test if foocategory acl categories is added} { + r acl SETUSER j8 on >password -@all +@foocategory + assert_equal [r acl DRYRUN j8 aclcheck.module.command.test.add.new.aclcategories] OK + } + + test {test permission compaction and simplification for categories added by a module} { + r acl SETUSER j9 on >password -@all +@foocategory -@foocategory + catch {r ACL GETUSER j9} res + assert_equal {-@all -@foocategory} [lindex $res 5] assert_equal {OK} [r module unload aclcheck] } } + +start_server {tags {"modules acl"}} { + test {test module load fails if exceeds the maximum number of adding acl categories} { + assert_error {ERR Error loading the extension. Please check the server logs.} {r module load $testmodule 1} + } +}