gvsafronov/futriix
1
0
Fork 0
Code Issues Pull Requests Actions 8 Packages Projects Releases Wiki Activity

Remove acl subcommand validation if fully added command exists. (#8483)

Browse Source 
This validation was only done for sub-commands and not for commands.

These would have been valid (not produce any error)
ACL SETUSER bob +@all +client
ACL SETUSER bob +client +client

so no reason for this one to fail:
ACL SETUSER bob +client +client|id

One example why this is needed is that pfdebug wasn't part of the @hyperloglog
group and now it is. so something like:
acl setuser user1 +@hyperloglog +pfdebug|test
would have succeeded in early 6.0.x, and fail in 6.2 RC3

Co-authored-by: Harkrishn Patro <harkrisp@amazon.com>
Co-authored-by: Madelyn Olson <madelyneolson@gmail.com>
Co-authored-by: Oran Agra <oran@redislabs.com>
This commit is contained in:
Harkrishn Patro 2021-02-22 14:22:25 +01:00 committed by GitHub
parent b164c4dec3
commit 4739131ca6
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 22 additions and 19 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

22
src/acl.c
View File

@ -814,8 +814,6 @@ void ACLAddAllowedSubcommand(user *u, unsigned long id, const char *sub) {
* invalid (contains non allowed characters).
* ENOENT: The command name or command category provided with + or - is not
* known.
* EBUSY: The subcommand you want to add is about a command that is currently
* fully added.
* EEXIST: You are adding a key pattern after "*" was already added. This is
* almost surely an error on the user side.
* EISDIR: You are adding a channel pattern after "*" was already added. This is
@ -976,22 +974,12 @@ int ACLSetUser(user *u, const char *op, ssize_t oplen) {
return C_ERR;
}
/* The command should not be set right now in the command
* bitmap, because adding a subcommand of a fully added
* command is probably an error on the user side. */
unsigned long id = ACLGetCommandID(copy);
if (ACLGetUserCommandBit(u,id) == 1) {
zfree(copy);
errno = EBUSY;
return C_ERR;
/* Add the subcommand to the list of valid ones, if the command is not set. */
if (ACLGetUserCommandBit(u,id) == 0) {
ACLAddAllowedSubcommand(u,id,sub);
}
/* Add the subcommand to the list of valid ones. */
ACLAddAllowedSubcommand(u,id,sub);
/* We have to clear the command bit so that we force the
* subcommand check. */
ACLSetUserCommandBit(u,id,0);
zfree(copy);
}
} else if (op[0] == '-' && op[1] != '@') {
@ -1030,10 +1018,6 @@ const char *ACLSetUserStringError(void) {
errmsg = "Unknown command or category name in ACL";
else if (errno == EINVAL)
errmsg = "Syntax error";
else if (errno == EBUSY)
errmsg = "Adding a subcommand of a command already fully "
"added is not allowed. Remove the command to start. "
"Example: -DEBUG +DEBUG|DIGEST";
else if (errno == EEXIST)
errmsg = "Adding a pattern after the * pattern (or the "
"'allkeys' flag) is not valid and does not have any "

19
tests/unit/acl.tcl
View File

@ -217,6 +217,25 @@ start_server {tags {"acl"}} {
set e
} {*NOPERM*}
test {ACLs set can include subcommands, if already full command exists} {
r ACL setuser bob +memory|doctor
set cmdstr [dict get [r ACL getuser bob] commands]
assert_equal {-@all +memory|doctor} $cmdstr
# Validate the commands have got engulfed to +memory.
r ACL setuser bob +memory
set cmdstr [dict get [r ACL getuser bob] commands]
assert_equal {-@all +memory} $cmdstr
# Appending to the existing access string of bob.
r ACL setuser bob +@all +client|id
# Validate the new commands has got engulfed to +@all.
set cmdstr [dict get [r ACL getuser bob] commands]
assert_equal {+@all} $cmdstr
r CLIENT ID; # Should not fail
r MEMORY DOCTOR; # Should not fail
}
# Note that the order of the generated ACL rules is not stable in Redis
# so we need to match the different parts and not as a whole string.
test {ACL GETUSER is able to translate back command permissions} {