slipenkomk/valkey
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Disconnect pub-sub subscribers when revoking allchannels permission (#11992)

Browse Source 
The existing logic for killing pub-sub clients did not handle the `allchannels`
permission correctly. For example, if you:

    ACL SETUSER foo allchannels

Have a client authenticate as the user `foo` and subscribe to a channel, and then:

    ACL SETUSER foo resetchannels

The subscribed client would not be disconnected, though new clients under that user
would be blocked from subscribing to any channels.

This was caused by an incomplete optimization in `ACLKillPubsubClientsIfNeeded`
checking whether the new channel permissions were a strict superset of the old ones.
This commit is contained in:
Slava Koyfman 2023-04-02 16:18:28 +03:00 committed by GitHub
parent 1f76bb17dd
commit f38aa6bfb7
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 21 additions and 0 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

7
src/acl.c
View File

@ -1891,6 +1891,13 @@ void ACLKillPubsubClientsIfNeeded(user *new, user *original) {
listRewind(original->selectors,&li);
while((ln = listNext(&li)) && match) {
aclSelector *s = (aclSelector *) listNodeValue(ln);
/* If any of the original selectors has the all-channels permission, but
* the new ones don't (this is checked earlier in this function), then the
* new list is not a strict superset of the original. */
if (s->flags & SELECTOR_FLAG_ALLCHANNELS) {
match = 0;
break;
}
listRewind(s->channels, &lpi);
while((lpn = listNext(&lpi)) && match) {
if (!listSearchKey(upcoming, listNodeValue(lpn))) {

14
tests/unit/acl.tcl
View File

@ -289,6 +289,20 @@ start_server {tags {"acl external:skip"}} {
$rd close
} {0}
test {Subscribers are killed when revoked of allchannels permission} {
set rd [redis_deferring_client]
r ACL setuser psuser allchannels
$rd AUTH psuser pspass
$rd read
$rd CLIENT SETNAME deathrow
$rd read
$rd PSUBSCRIBE foo
$rd read
r ACL setuser psuser resetchannels
assert_no_match {*deathrow*} [r CLIENT LIST]
$rd close
} {0}
test {Subscribers are pardoned if literal permissions are retained and/or gaining allchannels} {
set rd [redis_deferring_client]
r ACL setuser psuser resetchannels &foo:1 &bar:* &orders