From 6e993a5dfa38327f25c5c8eea0f0e98ffd5d2c21 Mon Sep 17 00:00:00 2001 From: Shaya Potter Date: Thu, 22 Sep 2022 16:29:00 +0300 Subject: [PATCH] Add RM_SetContextUser to support acl validation in RM_Call (and scripts) (#10966) Adds a number of user management/ACL validaiton/command execution functions to improve a Redis module's ability to enforce ACLs correctly and easily. * RM_SetContextUser - sets a RedisModuleUser on the context, which RM_Call will use to both validate ACLs (if requested and set) as well as assign to the client so that scripts executed via RM_Call will have proper ACL validation. * RM_SetModuleUserACLString - Enables one to pass an entire ACL string, not just a single OP and have it applied to the user * RM_GetModuleUserACLString - returns a stringified version of the user's ACL (same format as dump and list). Contains an optimization to cache the stringified version until the underlying ACL is modified. * Slightly re-purpose the "C" flag to RM_Call from just being about ACL check before calling the command, to actually running the command with the right user, so that it also affects commands inside EVAL scripts. see #11231 --- runtest-moduleapi | 1 + src/acl.c | 155 +++++++++++++++++++++--------- src/config.c | 6 +- src/module.c | 117 +++++++++++++++++----- src/networking.c | 7 ++ src/redismodule.h | 6 ++ src/server.h | 5 +- tests/modules/Makefile | 3 +- tests/modules/usercall.c | 129 +++++++++++++++++++++++++ tests/unit/moduleapi/usercall.tcl | 95 ++++++++++++++++++ 10 files changed, 449 insertions(+), 75 deletions(-) create mode 100644 tests/modules/usercall.c create mode 100644 tests/unit/moduleapi/usercall.tcl diff --git a/runtest-moduleapi b/runtest-moduleapi index c924a1f98..e58c58099 100755 --- a/runtest-moduleapi +++ b/runtest-moduleapi @@ -50,4 +50,5 @@ $TCLSH tests/test_helper.tcl \ --single unit/moduleapi/eventloop \ --single unit/moduleapi/timer \ --single unit/moduleapi/publish \ +--single unit/moduleapi/usercall \ "${@}" diff --git a/src/acl.c b/src/acl.c index 5f3c8ffc6..4b959433e 100644 --- a/src/acl.c +++ b/src/acl.c @@ -386,6 +386,7 @@ user *ACLCreateUser(const char *name, size_t namelen) { u->flags = USER_FLAG_DISABLED; u->flags |= USER_FLAG_SANITIZE_PAYLOAD; u->passwords = listCreate(); + u->acl_string = NULL; listSetMatchMethod(u->passwords,ACLListMatchSds); listSetFreeMethod(u->passwords,ACLListFreeSds); listSetDupMethod(u->passwords,ACLListDupSds); @@ -423,6 +424,10 @@ user *ACLCreateUnlinkedUser(void) { * will not remove the user from the Users global radix tree. */ void ACLFreeUser(user *u) { sdsfree(u->name); + if (u->acl_string) { + decrRefCount(u->acl_string); + u->acl_string = NULL; + } listRelease(u->passwords); listRelease(u->selectors); zfree(u); @@ -467,6 +472,14 @@ void ACLCopyUser(user *dst, user *src) { dst->passwords = listDup(src->passwords); dst->selectors = listDup(src->selectors); dst->flags = src->flags; + if (dst->acl_string) { + decrRefCount(dst->acl_string); + } + dst->acl_string = src->acl_string; + if (dst->acl_string) { + /* if src is NULL, we set it to NULL, if not, need to increment reference count */ + incrRefCount(dst->acl_string); + } } /* Free all the users registered in the radix tree 'users' and free the @@ -803,7 +816,12 @@ sds ACLDescribeSelector(aclSelector *selector) { * the ACLDescribeSelectorCommandRules() function. This is the function we call * when we want to rewrite the configuration files describing ACLs and * in order to show users with ACL LIST. */ -sds ACLDescribeUser(user *u) { +robj *ACLDescribeUser(user *u) { + if (u->acl_string) { + incrRefCount(u->acl_string); + return u->acl_string; + } + sds res = sdsempty(); /* Flags. */ @@ -837,7 +855,12 @@ sds ACLDescribeUser(user *u) { } sdsfree(default_perm); } - return res; + + u->acl_string = createObject(OBJ_STRING, res); + /* because we are returning it, have to increase count */ + incrRefCount(u->acl_string); + + return u->acl_string; } /* Get a command from the original command table, that is not affected @@ -1211,6 +1234,12 @@ int ACLSetSelector(aclSelector *selector, const char* op, size_t oplen) { * ECHILD: Attempt to allow a specific first argument of a subcommand */ int ACLSetUser(user *u, const char *op, ssize_t oplen) { + /* as we are changing the ACL, the old generated string is now invalid */ + if (u->acl_string) { + decrRefCount(u->acl_string); + u->acl_string = NULL; + } + if (oplen == -1) oplen = strlen(op); if (oplen == 0) return C_OK; /* Empty string is a no-operation. */ if (!strcasecmp(op,"on")) { @@ -1940,6 +1969,68 @@ sds *ACLMergeSelectorArguments(sds *argv, int argc, int *merged_argc, int *inval return acl_args; } +/* takes an acl string already split on spaces and adds it to the given user + * if the user object is NULL, will create a user with the given username + * + * Returns an error as an sds string if the ACL string is not parsable + */ +sds ACLStringSetUser(user *u, sds username, sds *argv, int argc) { + serverAssert(u != NULL || username != NULL); + + sds error = NULL; + + int merged_argc = 0, invalid_idx = 0; + sds *acl_args = ACLMergeSelectorArguments(argv, argc, &merged_argc, &invalid_idx); + + if (!acl_args) { + return sdscatfmt(sdsempty(), + "Unmatched parenthesis in acl selector starting " + "at '%s'.", (char *) argv[invalid_idx]); + } + + /* Create a temporary user to validate and stage all changes against + * before applying to an existing user or creating a new user. If all + * arguments are valid the user parameters will all be applied together. + * If there are any errors then none of the changes will be applied. */ + user *tempu = ACLCreateUnlinkedUser(); + if (u) { + ACLCopyUser(tempu, u); + } + + for (int j = 0; j < merged_argc; j++) { + if (ACLSetUser(tempu,acl_args[j],(ssize_t) sdslen(acl_args[j])) != C_OK) { + const char *errmsg = ACLSetUserStringError(); + error = sdscatfmt(sdsempty(), + "Error in ACL SETUSER modifier '%s': %s", + (char*)acl_args[j], errmsg); + goto cleanup; + } + } + + /* Existing pub/sub clients authenticated with the user may need to be + * disconnected if (some of) their channel permissions were revoked. */ + if (u) { + ACLKillPubsubClientsIfNeeded(tempu, u); + } + + /* Overwrite the user with the temporary user we modified above. */ + if (!u) { + u = ACLCreateUser(username,sdslen(username)); + } + serverAssert(u != NULL); + + ACLCopyUser(u, tempu); + +cleanup: + ACLFreeUser(tempu); + for (int i = 0; i < merged_argc; i++) { + sdsfree(acl_args[i]); + } + zfree(acl_args); + + return error; +} + /* Given an argument vector describing a user in the form: * * user ... ACL rules and flags ... @@ -2255,9 +2346,9 @@ int ACLSaveToFile(const char *filename) { sds user = sdsnew("user "); user = sdscatsds(user,u->name); user = sdscatlen(user," ",1); - sds descr = ACLDescribeUser(u); - user = sdscatsds(user,descr); - sdsfree(descr); + robj *descr = ACLDescribeUser(u); + user = sdscatsds(user,descr->ptr); + decrRefCount(descr); acl = sdscatsds(acl,user); acl = sdscatlen(acl,"\n",1); sdsfree(user); @@ -2590,50 +2681,18 @@ void aclCommand(client *c) { return; } - int merged_argc = 0, invalid_idx = 0; + user *u = ACLGetUserByName(username,sdslen(username)); + sds *temp_argv = zmalloc(c->argc * sizeof(sds)); for (int i = 3; i < c->argc; i++) temp_argv[i-3] = c->argv[i]->ptr; - sds *acl_args = ACLMergeSelectorArguments(temp_argv, c->argc - 3, &merged_argc, &invalid_idx); + + sds error = ACLStringSetUser(u, username, temp_argv, c->argc - 3); zfree(temp_argv); - - if (!acl_args) { - addReplyErrorFormat(c, - "Unmatched parenthesis in acl selector starting " - "at '%s'.", (char *) c->argv[invalid_idx]->ptr); - return; + if (error == NULL) { + addReply(c,shared.ok); + } else { + addReplyErrorSdsSafe(c, error); } - - /* Create a temporary user to validate and stage all changes against - * before applying to an existing user or creating a new user. If all - * arguments are valid the user parameters will all be applied together. - * If there are any errors then none of the changes will be applied. */ - user *tempu = ACLCreateUnlinkedUser(); - user *u = ACLGetUserByName(username,sdslen(username)); - if (u) ACLCopyUser(tempu, u); - - for (int j = 0; j < merged_argc; j++) { - if (ACLSetUser(tempu,acl_args[j],sdslen(acl_args[j])) != C_OK) { - const char *errmsg = ACLSetUserStringError(); - addReplyErrorFormat(c, - "Error in ACL SETUSER modifier '%s': %s", - (char*)acl_args[j], errmsg); - goto setuser_cleanup; - } - } - - /* Existing pub/sub clients authenticated with the user may need to be - * disconnected if (some of) their channel permissions were revoked. */ - if (u) ACLKillPubsubClientsIfNeeded(tempu, u); - - /* Overwrite the user with the temporary user we modified above. */ - if (!u) u = ACLCreateUser(username,sdslen(username)); - serverAssert(u != NULL); - ACLCopyUser(u, tempu); - addReply(c,shared.ok); -setuser_cleanup: - ACLFreeUser(tempu); - for (int i = 0; i < merged_argc; i++) sdsfree(acl_args[i]); - zfree(acl_args); return; } else if (!strcasecmp(sub,"deluser") && c->argc >= 3) { int deleted = 0; @@ -2720,9 +2779,9 @@ setuser_cleanup: sds config = sdsnew("user "); config = sdscatsds(config,u->name); config = sdscatlen(config," ",1); - sds descr = ACLDescribeUser(u); - config = sdscatsds(config,descr); - sdsfree(descr); + robj *descr = ACLDescribeUser(u); + config = sdscatsds(config,descr->ptr); + decrRefCount(descr); addReplyBulkSds(c,config); } } diff --git a/src/config.c b/src/config.c index 20a918dd8..2a34adfd3 100644 --- a/src/config.c +++ b/src/config.c @@ -1418,9 +1418,9 @@ void rewriteConfigUserOption(struct rewriteConfigState *state) { sds line = sdsnew("user "); line = sdscatsds(line,u->name); line = sdscatlen(line," ",1); - sds descr = ACLDescribeUser(u); - line = sdscatsds(line,descr); - sdsfree(descr); + robj *descr = ACLDescribeUser(u); + line = sdscatsds(line,descr->ptr); + decrRefCount(descr); rewriteConfigRewriteLine(state,"user",line,1); } raxStop(&ri); diff --git a/src/module.c b/src/module.c index 308d5716f..c3d360ec2 100644 --- a/src/module.c +++ b/src/module.c @@ -137,6 +137,7 @@ typedef struct RedisModulePoolAllocBlock { * but only the fields needed in a given context. */ struct RedisModuleBlockedClient; +struct RedisModuleUser; struct RedisModuleCtx { void *getapifuncptr; /* NOTE: Must be the first field. */ @@ -161,6 +162,9 @@ struct RedisModuleCtx { struct RedisModulePoolAllocBlock *pa_head; long long next_yield_time; + + const struct RedisModuleUser *user; /* RedisModuleUser commands executed via + RM_Call should be executed as, if set */ }; typedef struct RedisModuleCtx RedisModuleCtx; @@ -352,7 +356,7 @@ typedef struct RedisModuleServerInfoData { #define REDISMODULE_ARGV_NO_REPLICAS (1<<2) #define REDISMODULE_ARGV_RESP_3 (1<<3) #define REDISMODULE_ARGV_RESP_AUTO (1<<4) -#define REDISMODULE_ARGV_CHECK_ACL (1<<5) +#define REDISMODULE_ARGV_RUN_AS_USER (1<<5) #define REDISMODULE_ARGV_SCRIPT_MODE (1<<6) #define REDISMODULE_ARGV_NO_WRITES (1<<7) #define REDISMODULE_ARGV_CALL_REPLIES_AS_ERRORS (1<<8) @@ -566,7 +570,7 @@ void *RM_PoolAlloc(RedisModuleCtx *ctx, size_t bytes) { * Helpers for modules API implementation * -------------------------------------------------------------------------- */ -client *moduleAllocTempClient() { +client *moduleAllocTempClient(user *user) { client *c = NULL; if (moduleTempClientCount > 0) { @@ -576,8 +580,10 @@ client *moduleAllocTempClient() { } else { c = createClient(NULL); c->flags |= CLIENT_MODULE; - c->user = NULL; /* Root user */ } + + c->user = user; + return c; } @@ -761,7 +767,7 @@ void moduleCreateContext(RedisModuleCtx *out_ctx, RedisModule *module, int ctx_f out_ctx->module = module; out_ctx->flags = ctx_flags; if (ctx_flags & REDISMODULE_CTX_TEMP_CLIENT) - out_ctx->client = moduleAllocTempClient(); + out_ctx->client = moduleAllocTempClient(NULL); else if (ctx_flags & REDISMODULE_CTX_NEW_CLIENT) out_ctx->client = createClient(NULL); @@ -5619,6 +5625,11 @@ RedisModuleString *RM_CreateStringFromCallReply(RedisModuleCallReply *reply) { } } +/* Modifies the user that RM_Call will use (e.g. for ACL checks) */ +void RM_SetContextUser(RedisModuleCtx *ctx, const RedisModuleUser *user) { + ctx->user = user; +} + /* Returns an array of robj pointers, by parsing the format specifier "fmt" as described for * the RM_Call(), RM_Replicate() and other module APIs. Populates *argcp with the number of * items and *argvlenp with the length of the allocated argv. @@ -5631,7 +5642,7 @@ RedisModuleString *RM_CreateStringFromCallReply(RedisModuleCallReply *reply) { * "R" -> REDISMODULE_ARGV_NO_REPLICAS * "3" -> REDISMODULE_ARGV_RESP_3 * "0" -> REDISMODULE_ARGV_RESP_AUTO - * "C" -> REDISMODULE_ARGV_CHECK_ACL + * "C" -> REDISMODULE_ARGV_RUN_AS_USER * * On error (format specifier error) NULL is returned and nothing is * allocated. On success the argument vector is returned. */ @@ -5695,7 +5706,7 @@ robj **moduleCreateArgvFromUserFormat(const char *cmdname, const char *fmt, int } else if (*p == '0') { if (flags) (*flags) |= REDISMODULE_ARGV_RESP_AUTO; } else if (*p == 'C') { - if (flags) (*flags) |= REDISMODULE_ARGV_CHECK_ACL; + if (flags) (*flags) |= REDISMODULE_ARGV_RUN_AS_USER; } else if (*p == 'S') { if (flags) (*flags) |= REDISMODULE_ARGV_SCRIPT_MODE; } else if (*p == 'W') { @@ -5744,7 +5755,17 @@ fmterr: * * `0` -- Return the reply in auto mode, i.e. the reply format will be the * same as the client attached to the given RedisModuleCtx. This will * probably used when you want to pass the reply directly to the client. - * * `C` -- Check if command can be executed according to ACL rules. + * * `C` -- Run a command as the user attached to the context. + * User is either attached automatically via the client that directly + * issued the command and created the context or via RM_SetContextUser. + * If the context is not directly created by an issued command (such as a + * background context and no user was set on it via RM_SetContextUser, + * RM_Call will fail. + * Checks if the command can be executed according to ACL rules and causes + * the command to run as the determined user, so that any future user + * dependent activity, such as ACL checks within scripts will proceed as + * expected. + * Otherwise, the command will run as the Redis unrestricted user. * * `S` -- Run the command in a script mode, this means that it will raise * an error if a command which are not allowed inside a script * (flagged with the `deny-script` flag) is invoked (like SHUTDOWN). @@ -5779,7 +5800,7 @@ fmterr: * * ESPIPE: Command not allowed on script mode * * Example code fragment: - * + * * reply = RedisModule_Call(ctx,"INCRBY","sc",argv[1],"10"); * if (RedisModule_CallReplyType(reply) == REDISMODULE_REPLY_INTEGER) { * long long myval = RedisModule_CallReplyInteger(reply); @@ -5805,7 +5826,20 @@ RedisModuleCallReply *RM_Call(RedisModuleCtx *ctx, const char *cmdname, const ch error_as_call_replies = flags & REDISMODULE_ARGV_CALL_REPLIES_AS_ERRORS; va_end(ap); - c = moduleAllocTempClient(); + user *user = NULL; + if (flags & REDISMODULE_ARGV_RUN_AS_USER) { + user = ctx->user ? ctx->user->user : ctx->client->user; + if (!user) { + errno = ENOTSUP; + if (error_as_call_replies) { + sds msg = sdsnew("cannot run as user, no user directly attached to context or context's client"); + reply = callReplyCreateError(msg, ctx); + } + return reply; + } + } + + c = moduleAllocTempClient(user); /* We do not want to allow block, the module do not expect it */ c->flags |= CLIENT_DENY_BLOCKING; @@ -5953,20 +5987,16 @@ RedisModuleCallReply *RM_Call(RedisModuleCtx *ctx, const char *cmdname, const ch } /* Check if the user can run this command according to the current - * ACLs. */ - if (flags & REDISMODULE_ARGV_CHECK_ACL) { + * ACLs. + * + * If RM_SetContextUser has set a user, that user is used, otherwise + * use the attached client's user. If there is no attached client user and no manually + * set user, an error will be returned */ + if (flags & REDISMODULE_ARGV_RUN_AS_USER) { int acl_errpos; int acl_retval; - if (ctx->client->user == NULL) { - errno = ENOTSUP; - if (error_as_call_replies) { - sds msg = sdsnew("acl verification failed, context is not attached to a client."); - reply = callReplyCreateError(msg, ctx); - } - goto cleanup; - } - acl_retval = ACLCheckAllUserCommandPerm(ctx->client->user,c->cmd,c->argv,c->argc,&acl_errpos); + acl_retval = ACLCheckAllUserCommandPerm(user,c->cmd,c->argv,c->argc,&acl_errpos); if (acl_retval != ACL_OK) { sds object = (acl_retval == ACL_DENIED_CMD) ? sdsdup(c->cmd->fullname) : sdsdup(c->argv[acl_errpos]->ptr); addACLLogEntry(ctx->client, acl_retval, ACL_LOG_CTX_MODULE, -1, ctx->client->user->name, object); @@ -7202,8 +7232,8 @@ RedisModuleBlockedClient *moduleBlockClient(RedisModuleCtx *ctx, RedisModuleCmdF bc->disconnect_callback = NULL; /* Set by RM_SetDisconnectCallback() */ bc->free_privdata = free_privdata; bc->privdata = privdata; - bc->reply_client = moduleAllocTempClient(); - bc->thread_safe_ctx_client = moduleAllocTempClient(); + bc->reply_client = moduleAllocTempClient(NULL); + bc->thread_safe_ctx_client = moduleAllocTempClient(NULL); if (bc->client) bc->reply_client->resp = bc->client->resp; bc->dbid = c->db->id; @@ -8692,6 +8722,46 @@ int RM_SetModuleUserACL(RedisModuleUser *user, const char* acl) { return ACLSetUser(user->user, acl, -1); } +/* Sets the permission of a user with a complete ACL string, such as one + * would use on the redis ACL SETUSER command line API. This differs from + * RM_SetModuleUserACL, which only takes single ACL operations at a time. + * + * Returns REDISMODULE_OK on success and REDISMODULE_ERR on failure + * if a RedisModuleString is provided in error, a string describing the error + * will be returned */ +int RM_SetModuleUserACLString(RedisModuleCtx *ctx, RedisModuleUser *user, const char *acl, RedisModuleString **error) { + serverAssert(user != NULL); + + int argc; + sds *argv = sdssplitargs(acl, &argc); + + sds err = ACLStringSetUser(user->user, NULL, argv, argc); + + sdsfreesplitres(argv, argc); + + if (err) { + if (error) { + *error = createObject(OBJ_STRING, err); + if (ctx != NULL) autoMemoryAdd(ctx, REDISMODULE_AM_STRING, *error); + } else { + sdsfree(err); + } + + return REDISMODULE_ERR; + } + + return REDISMODULE_OK; +} + +/* Get the ACL string for a given user + * Returns a RedisModuleString + */ +RedisModuleString *RM_GetModuleUserACLString(RedisModuleUser *user) { + serverAssert(user != NULL); + + return ACLDescribeUser(user->user); +} + /* Retrieve the user name of the client connection behind the current context. * The user name can be used later, in order to get a RedisModuleUser. * See more information in RM_GetModuleUserFromUserName. @@ -12736,7 +12806,10 @@ void moduleRegisterCoreAPI(void) { REGISTER_API(Scan); REGISTER_API(ScanKey); REGISTER_API(CreateModuleUser); + REGISTER_API(SetContextUser); REGISTER_API(SetModuleUserACL); + REGISTER_API(SetModuleUserACLString); + REGISTER_API(GetModuleUserACLString); REGISTER_API(GetCurrentUserName); REGISTER_API(GetModuleUserFromUserName); REGISTER_API(ACLCheckCommandPermissions); diff --git a/src/networking.c b/src/networking.c index 932413816..3cdbe16ce 100644 --- a/src/networking.c +++ b/src/networking.c @@ -601,6 +601,13 @@ void addReplyErrorSds(client *c, sds err) { addReplyErrorSdsEx(c, err, 0); } +/* See addReplyErrorLength for expectations from the input string. */ +/* As a side effect the SDS string is freed. */ +void addReplyErrorSdsSafe(client *c, sds err) { + err = sdsmapchars(err, "\r\n", " ", 2); + addReplyErrorSdsEx(c, err, 0); +} + /* Internal function used by addReplyErrorFormat and addReplyErrorFormatEx. * Refer to afterErrorReply for more information about the flags. */ static void addReplyErrorFormatInternal(client *c, int flags, const char *fmt, va_list ap) { diff --git a/src/redismodule.h b/src/redismodule.h index 36e8bf51f..90a79a97c 100644 --- a/src/redismodule.h +++ b/src/redismodule.h @@ -1198,7 +1198,10 @@ REDISMODULE_API size_t (*RedisModule_MallocSizeString)(RedisModuleString* str) R REDISMODULE_API size_t (*RedisModule_MallocSizeDict)(RedisModuleDict* dict) REDISMODULE_ATTR; REDISMODULE_API RedisModuleUser * (*RedisModule_CreateModuleUser)(const char *name) REDISMODULE_ATTR; REDISMODULE_API void (*RedisModule_FreeModuleUser)(RedisModuleUser *user) REDISMODULE_ATTR; +REDISMODULE_API void (*RedisModule_SetContextUser)(RedisModuleCtx *ctx, const RedisModuleUser *user) REDISMODULE_ATTR; REDISMODULE_API int (*RedisModule_SetModuleUserACL)(RedisModuleUser *user, const char* acl) REDISMODULE_ATTR; +REDISMODULE_API int (*RedisModule_SetModuleUserACLString)(RedisModuleCtx * ctx, RedisModuleUser *user, const char* acl, RedisModuleString **error) REDISMODULE_ATTR; +REDISMODULE_API RedisModuleString * (*RedisModule_GetModuleUserACLString)(RedisModuleUser *user) REDISMODULE_ATTR; REDISMODULE_API RedisModuleString * (*RedisModule_GetCurrentUserName)(RedisModuleCtx *ctx) REDISMODULE_ATTR; REDISMODULE_API RedisModuleUser * (*RedisModule_GetModuleUserFromUserName)(RedisModuleString *name) REDISMODULE_ATTR; REDISMODULE_API int (*RedisModule_ACLCheckCommandPermissions)(RedisModuleUser *user, RedisModuleString **argv, int argc) REDISMODULE_ATTR; @@ -1538,7 +1541,10 @@ static int RedisModule_Init(RedisModuleCtx *ctx, const char *name, int ver, int REDISMODULE_GET_API(MallocSizeDict); REDISMODULE_GET_API(CreateModuleUser); REDISMODULE_GET_API(FreeModuleUser); + REDISMODULE_GET_API(SetContextUser); REDISMODULE_GET_API(SetModuleUserACL); + REDISMODULE_GET_API(SetModuleUserACLString); + REDISMODULE_GET_API(GetModuleUserACLString); REDISMODULE_GET_API(GetCurrentUserName); REDISMODULE_GET_API(GetModuleUserFromUserName); REDISMODULE_GET_API(ACLCheckCommandPermissions); diff --git a/src/server.h b/src/server.h index f71d7c7eb..2b0844c39 100644 --- a/src/server.h +++ b/src/server.h @@ -1043,6 +1043,7 @@ typedef struct { list *selectors; /* A list of selectors this user validates commands against. This list will always contain at least one selector for backwards compatibility. */ + robj *acl_string; /* cached string represent of ACLs */ } user; /* With multiplexing we need to take per-client state. @@ -2463,6 +2464,7 @@ void addReplyOrErrorObject(client *c, robj *reply); void afterErrorReply(client *c, const char *s, size_t len, int flags); void addReplyErrorSdsEx(client *c, sds err, int flags); void addReplyErrorSds(client *c, sds err); +void addReplyErrorSdsSafe(client *c, sds err); void addReplyError(client *c, const char *err); void addReplyErrorArity(client *c); void addReplyErrorExpireTime(client *c); @@ -2783,11 +2785,12 @@ int ACLCheckAllUserCommandPerm(user *u, struct redisCommand *cmd, robj **argv, i int ACLUserCheckCmdWithUnrestrictedKeyAccess(user *u, struct redisCommand *cmd, robj **argv, int argc, int flags); 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 ACLAppendUserForLoading(sds *argv, int argc, int *argc_err); const char *ACLSetUserStringError(void); int ACLLoadConfiguredUsers(void); -sds ACLDescribeUser(user *u); +robj *ACLDescribeUser(user *u); void ACLLoadUsersAtStartup(void); void addReplyCommandCategories(client *c, struct redisCommand *cmd); user *ACLCreateUnlinkedUser(); diff --git a/tests/modules/Makefile b/tests/modules/Makefile index ac4c3e27b..56069406b 100644 --- a/tests/modules/Makefile +++ b/tests/modules/Makefile @@ -58,7 +58,8 @@ TEST_MODULES = \ eventloop.so \ moduleconfigs.so \ moduleconfigstwo.so \ - publish.so + publish.so \ + usercall.so .PHONY: all diff --git a/tests/modules/usercall.c b/tests/modules/usercall.c new file mode 100644 index 000000000..9eb84ef39 --- /dev/null +++ b/tests/modules/usercall.c @@ -0,0 +1,129 @@ +#include "redismodule.h" + +RedisModuleUser *user = NULL; + +int call_without_user(RedisModuleCtx *ctx, RedisModuleString **argv, int argc) { + if (argc < 2) { + return RedisModule_WrongArity(ctx); + } + + const char *cmd = RedisModule_StringPtrLen(argv[1], NULL); + + RedisModuleCallReply *rep = RedisModule_Call(ctx, cmd, "Ev", argv + 2, argc - 2); + if (!rep) { + RedisModule_ReplyWithError(ctx, "NULL reply returned"); + } else { + RedisModule_ReplyWithCallReply(ctx, rep); + RedisModule_FreeCallReply(rep); + } + return REDISMODULE_OK; +} + +int call_with_user_flag(RedisModuleCtx *ctx, RedisModuleString **argv, int argc) { + if (argc < 3) { + return RedisModule_WrongArity(ctx); + } + + RedisModule_SetContextUser(ctx, user); + + /* Append Ev to the provided flags. */ + RedisModuleString *flags = RedisModule_CreateStringFromString(ctx, argv[1]); + RedisModule_StringAppendBuffer(ctx, flags, "Ev", 2); + + const char* flg = RedisModule_StringPtrLen(flags, NULL); + const char* cmd = RedisModule_StringPtrLen(argv[2], NULL); + + RedisModuleCallReply* rep = RedisModule_Call(ctx, cmd, flg, argv + 3, argc - 3); + if (!rep) { + RedisModule_ReplyWithError(ctx, "NULL reply returned"); + } else { + RedisModule_ReplyWithCallReply(ctx, rep); + RedisModule_FreeCallReply(rep); + } + RedisModule_FreeString(ctx, flags); + + return REDISMODULE_OK; +} + +int add_to_acl(RedisModuleCtx *ctx, RedisModuleString **argv, int argc) { + if (argc != 2) { + return RedisModule_WrongArity(ctx); + } + + size_t acl_len; + const char *acl = RedisModule_StringPtrLen(argv[1], &acl_len); + + RedisModuleString *error; + int ret = RedisModule_SetModuleUserACLString(ctx, user, acl, &error); + if (ret) { + size_t len; + const char * e = RedisModule_StringPtrLen(error, &len); + RedisModule_ReplyWithError(ctx, e); + return REDISMODULE_OK; + } + + RedisModule_ReplyWithSimpleString(ctx, "OK"); + + return REDISMODULE_OK; +} + +int get_acl(RedisModuleCtx *ctx, RedisModuleString **argv, int argc) { + REDISMODULE_NOT_USED(argv); + + if (argc != 1) { + return RedisModule_WrongArity(ctx); + } + + RedisModule_Assert(user != NULL); + + RedisModuleString *acl = RedisModule_GetModuleUserACLString(user); + + RedisModule_ReplyWithString(ctx, acl); + + RedisModule_FreeString(NULL, acl); + + return REDISMODULE_OK; +} + +int reset_user(RedisModuleCtx *ctx, RedisModuleString **argv, int argc) { + REDISMODULE_NOT_USED(argv); + + if (argc != 1) { + return RedisModule_WrongArity(ctx); + } + + if (user != NULL) { + RedisModule_FreeModuleUser(user); + } + + user = RedisModule_CreateModuleUser("module_user"); + + RedisModule_ReplyWithSimpleString(ctx, "OK"); + + return REDISMODULE_OK; +} + +int RedisModule_OnLoad(RedisModuleCtx *ctx, RedisModuleString **argv, int argc) { + REDISMODULE_NOT_USED(argv); + REDISMODULE_NOT_USED(argc); + + if (RedisModule_Init(ctx,"usercall",1,REDISMODULE_APIVER_1)== REDISMODULE_ERR) + return REDISMODULE_ERR; + + if (RedisModule_CreateCommand(ctx,"usercall.call_without_user", call_without_user,"write",0,0,0) == REDISMODULE_ERR) + return REDISMODULE_ERR; + + if (RedisModule_CreateCommand(ctx,"usercall.call_with_user_flag", call_with_user_flag,"write",0,0,0) == REDISMODULE_ERR) + return REDISMODULE_ERR; + + if (RedisModule_CreateCommand(ctx, "usercall.add_to_acl", add_to_acl, "write",0,0,0) == REDISMODULE_ERR) + return REDISMODULE_ERR; + + if (RedisModule_CreateCommand(ctx,"usercall.reset_user", reset_user,"write",0,0,0) == REDISMODULE_ERR) + return REDISMODULE_ERR; + + if (RedisModule_CreateCommand(ctx,"usercall.get_acl", get_acl,"write",0,0,0) == REDISMODULE_ERR) + return REDISMODULE_ERR; + + return REDISMODULE_OK; +} diff --git a/tests/unit/moduleapi/usercall.tcl b/tests/unit/moduleapi/usercall.tcl new file mode 100644 index 000000000..04f730005 --- /dev/null +++ b/tests/unit/moduleapi/usercall.tcl @@ -0,0 +1,95 @@ +set testmodule [file normalize tests/modules/usercall.so] + +set test_script_set "#!lua +redis.call('set','x',1) +return 1" + +set test_script_get "#!lua +redis.call('get','x') +return 1" + +start_server {tags {"modules usercall"}} { + r module load $testmodule + + # baseline test that module isn't doing anything weird + test {test module check regular redis command without user/acl} { + assert_equal [r usercall.reset_user] OK + assert_equal [r usercall.add_to_acl "~* &* +@all -set"] OK + assert_equal [r usercall.call_without_user set x 5] OK + assert_equal [r usercall.reset_user] OK + } + + # call with user with acl set on it, but without testing the acl + test {test module check regular redis command with user} { + assert_equal [r set x 5] OK + + assert_equal [r usercall.reset_user] OK + assert_equal [r usercall.add_to_acl "~* &* +@all -set"] OK + # off and sanitize-payload because module user / default value + assert_equal [r usercall.get_acl] "off sanitize-payload ~* &* +@all -set" + + # doesn't fail for regular commands as just testing acl here + assert_equal [r usercall.call_with_user_flag {} set x 10] OK + + assert_equal [r get x] 10 + assert_equal [r usercall.reset_user] OK + } + + # call with user with acl set on it, but with testing the acl in rm_call (for cmd itself) + test {test module check regular redis command with user and acl} { + assert_equal [r set x 5] OK + + assert_equal [r usercall.reset_user] OK + assert_equal [r usercall.add_to_acl "~* &* +@all -set"] OK + # off and sanitize-payload because module user / default value + assert_equal [r usercall.get_acl] "off sanitize-payload ~* &* +@all -set" + + # fails here as testing acl in rm call + catch {r usercall.call_with_user_flag C set x 10} e + assert_match {*ERR acl verification failed*} $e + + assert_equal [r usercall.call_with_user_flag C get x] 5 + + assert_equal [r usercall.reset_user] OK + } + + # baseline script test, call without user on script + test {test module check eval script without user} { + set sha_set [r script load $test_script_set] + set sha_get [r script load $test_script_get] + + assert_equal [r usercall.call_without_user evalsha $sha_set 0] 1 + assert_equal [r usercall.call_without_user evalsha $sha_get 0] 1 + } + + # baseline script test, call without user on script + test {test module check eval script with user being set, but not acl testing} { + set sha_set [r script load $test_script_set] + set sha_get [r script load $test_script_get] + + assert_equal [r usercall.reset_user] OK + assert_equal [r usercall.add_to_acl "~* &* +@all -set"] OK + # off and sanitize-payload because module user / default value + assert_equal [r usercall.get_acl] "off sanitize-payload ~* &* +@all -set" + + # passes as not checking ACL + assert_equal [r usercall.call_with_user_flag {} evalsha $sha_set 0] 1 + assert_equal [r usercall.call_with_user_flag {} evalsha $sha_get 0] 1 + } + + # call with user on script (without rm_call acl check) to ensure user carries through to script execution + # we already tested the check in rm_call above, here we are checking the script itself will enforce ACL + test {test module check eval script with user and acl} { + set sha_set [r script load $test_script_set] + set sha_get [r script load $test_script_get] + + assert_equal [r usercall.reset_user] OK + assert_equal [r usercall.add_to_acl "~* &* +@all -set"] OK + + # fails here in script, as rm_call will permit the eval call + catch {r usercall.call_with_user_flag C evalsha $sha_set 0} e + assert_match {*ERR The user executing the script can't run this command or subcommand script*} $e + + assert_equal [r usercall.call_with_user_flag C evalsha $sha_get 0] 1 + } +} \ No newline at end of file