Skip to content

Commit 6e993a5

Browse files
authored
Add RM_SetContextUser to support acl validation in RM_Call (and scripts) (redis#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 redis#11231
1 parent 6d21560 commit 6e993a5

File tree

10 files changed

+449
-75
lines changed

10 files changed

+449
-75
lines changed

runtest-moduleapi

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,4 +50,5 @@ $TCLSH tests/test_helper.tcl \
5050
--single unit/moduleapi/eventloop \
5151
--single unit/moduleapi/timer \
5252
--single unit/moduleapi/publish \
53+
--single unit/moduleapi/usercall \
5354
"${@}"

src/acl.c

Lines changed: 107 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -386,6 +386,7 @@ user *ACLCreateUser(const char *name, size_t namelen) {
386386
u->flags = USER_FLAG_DISABLED;
387387
u->flags |= USER_FLAG_SANITIZE_PAYLOAD;
388388
u->passwords = listCreate();
389+
u->acl_string = NULL;
389390
listSetMatchMethod(u->passwords,ACLListMatchSds);
390391
listSetFreeMethod(u->passwords,ACLListFreeSds);
391392
listSetDupMethod(u->passwords,ACLListDupSds);
@@ -423,6 +424,10 @@ user *ACLCreateUnlinkedUser(void) {
423424
* will not remove the user from the Users global radix tree. */
424425
void ACLFreeUser(user *u) {
425426
sdsfree(u->name);
427+
if (u->acl_string) {
428+
decrRefCount(u->acl_string);
429+
u->acl_string = NULL;
430+
}
426431
listRelease(u->passwords);
427432
listRelease(u->selectors);
428433
zfree(u);
@@ -467,6 +472,14 @@ void ACLCopyUser(user *dst, user *src) {
467472
dst->passwords = listDup(src->passwords);
468473
dst->selectors = listDup(src->selectors);
469474
dst->flags = src->flags;
475+
if (dst->acl_string) {
476+
decrRefCount(dst->acl_string);
477+
}
478+
dst->acl_string = src->acl_string;
479+
if (dst->acl_string) {
480+
/* if src is NULL, we set it to NULL, if not, need to increment reference count */
481+
incrRefCount(dst->acl_string);
482+
}
470483
}
471484

472485
/* Free all the users registered in the radix tree 'users' and free the
@@ -803,7 +816,12 @@ sds ACLDescribeSelector(aclSelector *selector) {
803816
* the ACLDescribeSelectorCommandRules() function. This is the function we call
804817
* when we want to rewrite the configuration files describing ACLs and
805818
* in order to show users with ACL LIST. */
806-
sds ACLDescribeUser(user *u) {
819+
robj *ACLDescribeUser(user *u) {
820+
if (u->acl_string) {
821+
incrRefCount(u->acl_string);
822+
return u->acl_string;
823+
}
824+
807825
sds res = sdsempty();
808826

809827
/* Flags. */
@@ -837,7 +855,12 @@ sds ACLDescribeUser(user *u) {
837855
}
838856
sdsfree(default_perm);
839857
}
840-
return res;
858+
859+
u->acl_string = createObject(OBJ_STRING, res);
860+
/* because we are returning it, have to increase count */
861+
incrRefCount(u->acl_string);
862+
863+
return u->acl_string;
841864
}
842865

843866
/* 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) {
12111234
* ECHILD: Attempt to allow a specific first argument of a subcommand
12121235
*/
12131236
int ACLSetUser(user *u, const char *op, ssize_t oplen) {
1237+
/* as we are changing the ACL, the old generated string is now invalid */
1238+
if (u->acl_string) {
1239+
decrRefCount(u->acl_string);
1240+
u->acl_string = NULL;
1241+
}
1242+
12141243
if (oplen == -1) oplen = strlen(op);
12151244
if (oplen == 0) return C_OK; /* Empty string is a no-operation. */
12161245
if (!strcasecmp(op,"on")) {
@@ -1940,6 +1969,68 @@ sds *ACLMergeSelectorArguments(sds *argv, int argc, int *merged_argc, int *inval
19401969
return acl_args;
19411970
}
19421971

1972+
/* takes an acl string already split on spaces and adds it to the given user
1973+
* if the user object is NULL, will create a user with the given username
1974+
*
1975+
* Returns an error as an sds string if the ACL string is not parsable
1976+
*/
1977+
sds ACLStringSetUser(user *u, sds username, sds *argv, int argc) {
1978+
serverAssert(u != NULL || username != NULL);
1979+
1980+
sds error = NULL;
1981+
1982+
int merged_argc = 0, invalid_idx = 0;
1983+
sds *acl_args = ACLMergeSelectorArguments(argv, argc, &merged_argc, &invalid_idx);
1984+
1985+
if (!acl_args) {
1986+
return sdscatfmt(sdsempty(),
1987+
"Unmatched parenthesis in acl selector starting "
1988+
"at '%s'.", (char *) argv[invalid_idx]);
1989+
}
1990+
1991+
/* Create a temporary user to validate and stage all changes against
1992+
* before applying to an existing user or creating a new user. If all
1993+
* arguments are valid the user parameters will all be applied together.
1994+
* If there are any errors then none of the changes will be applied. */
1995+
user *tempu = ACLCreateUnlinkedUser();
1996+
if (u) {
1997+
ACLCopyUser(tempu, u);
1998+
}
1999+
2000+
for (int j = 0; j < merged_argc; j++) {
2001+
if (ACLSetUser(tempu,acl_args[j],(ssize_t) sdslen(acl_args[j])) != C_OK) {
2002+
const char *errmsg = ACLSetUserStringError();
2003+
error = sdscatfmt(sdsempty(),
2004+
"Error in ACL SETUSER modifier '%s': %s",
2005+
(char*)acl_args[j], errmsg);
2006+
goto cleanup;
2007+
}
2008+
}
2009+
2010+
/* Existing pub/sub clients authenticated with the user may need to be
2011+
* disconnected if (some of) their channel permissions were revoked. */
2012+
if (u) {
2013+
ACLKillPubsubClientsIfNeeded(tempu, u);
2014+
}
2015+
2016+
/* Overwrite the user with the temporary user we modified above. */
2017+
if (!u) {
2018+
u = ACLCreateUser(username,sdslen(username));
2019+
}
2020+
serverAssert(u != NULL);
2021+
2022+
ACLCopyUser(u, tempu);
2023+
2024+
cleanup:
2025+
ACLFreeUser(tempu);
2026+
for (int i = 0; i < merged_argc; i++) {
2027+
sdsfree(acl_args[i]);
2028+
}
2029+
zfree(acl_args);
2030+
2031+
return error;
2032+
}
2033+
19432034
/* Given an argument vector describing a user in the form:
19442035
*
19452036
* user <username> ... ACL rules and flags ...
@@ -2255,9 +2346,9 @@ int ACLSaveToFile(const char *filename) {
22552346
sds user = sdsnew("user ");
22562347
user = sdscatsds(user,u->name);
22572348
user = sdscatlen(user," ",1);
2258-
sds descr = ACLDescribeUser(u);
2259-
user = sdscatsds(user,descr);
2260-
sdsfree(descr);
2349+
robj *descr = ACLDescribeUser(u);
2350+
user = sdscatsds(user,descr->ptr);
2351+
decrRefCount(descr);
22612352
acl = sdscatsds(acl,user);
22622353
acl = sdscatlen(acl,"\n",1);
22632354
sdsfree(user);
@@ -2590,50 +2681,18 @@ void aclCommand(client *c) {
25902681
return;
25912682
}
25922683

2593-
int merged_argc = 0, invalid_idx = 0;
2684+
user *u = ACLGetUserByName(username,sdslen(username));
2685+
25942686
sds *temp_argv = zmalloc(c->argc * sizeof(sds));
25952687
for (int i = 3; i < c->argc; i++) temp_argv[i-3] = c->argv[i]->ptr;
2596-
sds *acl_args = ACLMergeSelectorArguments(temp_argv, c->argc - 3, &merged_argc, &invalid_idx);
2597-
zfree(temp_argv);
2598-
2599-
if (!acl_args) {
2600-
addReplyErrorFormat(c,
2601-
"Unmatched parenthesis in acl selector starting "
2602-
"at '%s'.", (char *) c->argv[invalid_idx]->ptr);
2603-
return;
2604-
}
2605-
2606-
/* Create a temporary user to validate and stage all changes against
2607-
* before applying to an existing user or creating a new user. If all
2608-
* arguments are valid the user parameters will all be applied together.
2609-
* If there are any errors then none of the changes will be applied. */
2610-
user *tempu = ACLCreateUnlinkedUser();
2611-
user *u = ACLGetUserByName(username,sdslen(username));
2612-
if (u) ACLCopyUser(tempu, u);
26132688

2614-
for (int j = 0; j < merged_argc; j++) {
2615-
if (ACLSetUser(tempu,acl_args[j],sdslen(acl_args[j])) != C_OK) {
2616-
const char *errmsg = ACLSetUserStringError();
2617-
addReplyErrorFormat(c,
2618-
"Error in ACL SETUSER modifier '%s': %s",
2619-
(char*)acl_args[j], errmsg);
2620-
goto setuser_cleanup;
2621-
}
2689+
sds error = ACLStringSetUser(u, username, temp_argv, c->argc - 3);
2690+
zfree(temp_argv);
2691+
if (error == NULL) {
2692+
addReply(c,shared.ok);
2693+
} else {
2694+
addReplyErrorSdsSafe(c, error);
26222695
}
2623-
2624-
/* Existing pub/sub clients authenticated with the user may need to be
2625-
* disconnected if (some of) their channel permissions were revoked. */
2626-
if (u) ACLKillPubsubClientsIfNeeded(tempu, u);
2627-
2628-
/* Overwrite the user with the temporary user we modified above. */
2629-
if (!u) u = ACLCreateUser(username,sdslen(username));
2630-
serverAssert(u != NULL);
2631-
ACLCopyUser(u, tempu);
2632-
addReply(c,shared.ok);
2633-
setuser_cleanup:
2634-
ACLFreeUser(tempu);
2635-
for (int i = 0; i < merged_argc; i++) sdsfree(acl_args[i]);
2636-
zfree(acl_args);
26372696
return;
26382697
} else if (!strcasecmp(sub,"deluser") && c->argc >= 3) {
26392698
int deleted = 0;
@@ -2720,9 +2779,9 @@ void aclCommand(client *c) {
27202779
sds config = sdsnew("user ");
27212780
config = sdscatsds(config,u->name);
27222781
config = sdscatlen(config," ",1);
2723-
sds descr = ACLDescribeUser(u);
2724-
config = sdscatsds(config,descr);
2725-
sdsfree(descr);
2782+
robj *descr = ACLDescribeUser(u);
2783+
config = sdscatsds(config,descr->ptr);
2784+
decrRefCount(descr);
27262785
addReplyBulkSds(c,config);
27272786
}
27282787
}

src/config.c

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1418,9 +1418,9 @@ void rewriteConfigUserOption(struct rewriteConfigState *state) {
14181418
sds line = sdsnew("user ");
14191419
line = sdscatsds(line,u->name);
14201420
line = sdscatlen(line," ",1);
1421-
sds descr = ACLDescribeUser(u);
1422-
line = sdscatsds(line,descr);
1423-
sdsfree(descr);
1421+
robj *descr = ACLDescribeUser(u);
1422+
line = sdscatsds(line,descr->ptr);
1423+
decrRefCount(descr);
14241424
rewriteConfigRewriteLine(state,"user",line,1);
14251425
}
14261426
raxStop(&ri);

0 commit comments

Comments
 (0)