Skip to content

Commit bb6513c

Browse files
authored
ACL default newly created user set USER_FLAG_SANITIZE_PAYLOAD flag (redis#11279)
Starting from 6.2, after ACL SETUSER user reset, the user will carry the sanitize-payload flag. It was added in redis#7807, and then ACL SETUSER reset is inconsistent with default newly created user which missing sanitize-payload flag. Same as `off` and `on` these two bits are mutually exclusive, the default created user needs to have sanitize-payload flag. Adds USER_FLAG_SANITIZE_PAYLOAD flag to ACLCreateUser. Note that the bug don't have any real implications, since the code in rdb.c (rdbLoadObject) checks for `USER_FLAG_SANITIZE_PAYLOAD_SKIP`, so the fact that `USER_FLAG_SANITIZE_PAYLOAD` is missing doesn't really matters. Added tests to make sure it won't be broken in the future, and updated the comment in ACLSetUser and redis.conf
1 parent eedb8b1 commit bb6513c

File tree

3 files changed

+31
-7
lines changed

3 files changed

+31
-7
lines changed

redis.conf

+3-3
Original file line numberDiff line numberDiff line change
@@ -942,9 +942,9 @@ replica-priority 100
942942
# "nopass" status. After "resetpass" the user has no associated
943943
# passwords and there is no way to authenticate without adding
944944
# some password (or setting it as "nopass" later).
945-
# reset Performs the following actions: resetpass, resetkeys, off,
946-
# -@all. The user returns to the same state it has immediately
947-
# after its creation.
945+
# reset Performs the following actions: resetpass, resetkeys, resetchannels,
946+
# allchannels (if acl-pubsub-default is set), off, clearselectors, -@all.
947+
# The user returns to the same state it has immediately after its creation.
948948
# (<options>) Create a new selector with the options specified within the
949949
# parentheses and attach it to the user. Each option should be
950950
# space separated. The first character must be ( and the last

src/acl.c

+6-3
Original file line numberDiff line numberDiff line change
@@ -384,6 +384,7 @@ user *ACLCreateUser(const char *name, size_t namelen) {
384384
user *u = zmalloc(sizeof(*u));
385385
u->name = sdsnewlen(name,namelen);
386386
u->flags = USER_FLAG_DISABLED;
387+
u->flags |= USER_FLAG_SANITIZE_PAYLOAD;
387388
u->passwords = listCreate();
388389
listSetMatchMethod(u->passwords,ACLListMatchSds);
389390
listSetFreeMethod(u->passwords,ACLListFreeSds);
@@ -1146,6 +1147,8 @@ int ACLSetSelector(aclSelector *selector, const char* op, size_t oplen) {
11461147
* off Disable the user: it's no longer possible to authenticate
11471148
* with this user, however the already authenticated connections
11481149
* will still work.
1150+
* skip-sanitize-payload RESTORE dump-payload sanitization is skipped.
1151+
* sanitize-payload RESTORE dump-payload is sanitized (default).
11491152
* ><password> Add this password to the list of valid password for the user.
11501153
* For example >mypass will add "mypass" to the list.
11511154
* This directive clears the "nopass" flag (see later).
@@ -1168,9 +1171,9 @@ int ACLSetSelector(aclSelector *selector, const char* op, size_t oplen) {
11681171
* "nopass" status. After "resetpass" the user has no associated
11691172
* passwords and there is no way to authenticate without adding
11701173
* some password (or setting it as "nopass" later).
1171-
* reset Performs the following actions: resetpass, resetkeys, off,
1172-
* -@all. The user returns to the same state it has immediately
1173-
* after its creation.
1174+
* reset Performs the following actions: resetpass, resetkeys, resetchannels,
1175+
* allchannels (if acl-pubsub-default is set), off, clearselectors, -@all.
1176+
* The user returns to the same state it has immediately after its creation.
11741177
* (<options>) Create a new selector with the options specified within the
11751178
* parentheses and attach it to the user. Each option should be
11761179
* space separated. The first character must be ( and the last

tests/unit/acl.tcl

+22-1
Original file line numberDiff line numberDiff line change
@@ -175,7 +175,7 @@ start_server {tags {"acl external:skip"}} {
175175
set curruser "hpuser"
176176
foreach user [lshuffle $users] {
177177
if {[string first $curruser $user] != -1} {
178-
assert_equal {user hpuser on nopass resetchannels &foo +@all} $user
178+
assert_equal {user hpuser on nopass sanitize-payload resetchannels &foo +@all} $user
179179
}
180180
}
181181

@@ -469,6 +469,27 @@ start_server {tags {"acl external:skip"}} {
469469
r AUTH newuser passwd1
470470
}
471471

472+
test {ACL SETUSER RESET reverting to default newly created user} {
473+
set current_user "example"
474+
r ACL DELUSER $current_user
475+
r ACL SETUSER $current_user
476+
477+
set users [r ACL LIST]
478+
foreach user [lshuffle $users] {
479+
if {[string first $current_user $user] != -1} {
480+
set current_user_output $user
481+
}
482+
}
483+
484+
r ACL SETUSER $current_user reset
485+
set users [r ACL LIST]
486+
foreach user [lshuffle $users] {
487+
if {[string first $current_user $user] != -1} {
488+
assert_equal $current_user_output $user
489+
}
490+
}
491+
}
492+
472493
# Note that the order of the generated ACL rules is not stable in Redis
473494
# so we need to match the different parts and not as a whole string.
474495
test {ACL GETUSER is able to translate back command permissions} {

0 commit comments

Comments
 (0)