Fix "default" and overwritten / reset users will not have pubsub channels permissions by default. (#8723)

Background:
Redis 6.2 added ACL control for pubsub channels (#7993), which were supposed
to be permissive by default to retain compatibility with redis 6.0 ACL. 
But due to a bug, only newly created users got this `acl-pubsub-default` applied,
while overwritten (updated) users got reset to `resetchannels` (denied).

Since the "default" user exists before loading the config file,
any ACL change to it, results in an update / overwrite.

So when a "default" user is loaded from config file or include ACL
file with no channels related rules, the user will not have any
permissions to any channels. But other users will have default
permissions to any channels.

When upgraded from 6.0 with config rewrite, this will lead to
"default" user channels permissions lost.
When users are loaded from include file, then call "acl load", users
will also lost channels permissions.

Similarly, the `reset` ACL rule, would have reset the user to be denied
access to any channels, ignoring `acl-pubsub-default` and breaking
compatibility with redis 6.0.

The implication of this fix is that it regains compatibility with redis 6.0,
but breaks compatibility with redis 6.2.0 and 2.0.1. e.g. after the upgrade,
the default user will regain access to pubsub channels.

Other changes:
Additionally this commit rename server.acl_pubusub_default to
server.acl_pubsub_default and fix typo in acl tests.

src/acl.c

@@ -245,7 +245,7 @@ user *ACLCreateUser(const char *name, size_t namelen) {
     if (raxFind(Users,(unsigned char*)name,namelen) != raxNotFound) return NULL;
     user *u = zmalloc(sizeof(*u));
     u->name = sdsnewlen(name,namelen);
-    u->flags = USER_FLAG_DISABLED | server.acl_pubusub_default;
+    u->flags = USER_FLAG_DISABLED | server.acl_pubsub_default;
     u->allowed_subcommands = NULL;
     u->passwords = listCreate();
     u->patterns = listCreate();
@@ -1000,6 +1000,8 @@ int ACLSetUser(user *u, const char *op, ssize_t oplen) {
         serverAssert(ACLSetUser(u,"resetpass",-1) == C_OK);
         serverAssert(ACLSetUser(u,"resetkeys",-1) == C_OK);
         serverAssert(ACLSetUser(u,"resetchannels",-1) == C_OK);
+        if (server.acl_pubsub_default & USER_FLAG_ALLCHANNELS)
+            serverAssert(ACLSetUser(u,"allchannels",-1) == C_OK);
         serverAssert(ACLSetUser(u,"off",-1) == C_OK);
         serverAssert(ACLSetUser(u,"sanitize-payload",-1) == C_OK);
         serverAssert(ACLSetUser(u,"-@all",-1) == C_OK);

src/config.c

@@ -2470,7 +2470,7 @@ standardConfig configs[] = {
     createEnumConfig("maxmemory-policy", NULL, MODIFIABLE_CONFIG, maxmemory_policy_enum, server.maxmemory_policy, MAXMEMORY_NO_EVICTION, NULL, NULL),
     createEnumConfig("appendfsync", NULL, MODIFIABLE_CONFIG, aof_fsync_enum, server.aof_fsync, AOF_FSYNC_EVERYSEC, NULL, NULL),
     createEnumConfig("oom-score-adj", NULL, MODIFIABLE_CONFIG, oom_score_adj_enum, server.oom_score_adj, OOM_SCORE_ADJ_NO, NULL, updateOOMScoreAdj),
-    createEnumConfig("acl-pubsub-default", NULL, MODIFIABLE_CONFIG, acl_pubsub_default_enum, server.acl_pubusub_default, USER_FLAG_ALLCHANNELS, NULL, NULL),
+    createEnumConfig("acl-pubsub-default", NULL, MODIFIABLE_CONFIG, acl_pubsub_default_enum, server.acl_pubsub_default, USER_FLAG_ALLCHANNELS, NULL, NULL),
     createEnumConfig("sanitize-dump-payload", NULL, MODIFIABLE_CONFIG, sanitize_dump_payload_enum, server.sanitize_dump_payload, SANITIZE_DUMP_NO, NULL, NULL),
 
     /* Integer configs */

src/server.h

@@ -1596,7 +1596,7 @@ struct redisServer {
     sds requirepass;              /* Remember the cleartext password set with
                                      the old "requirepass" directive for
                                      backward compatibility with Redis <= 5. */
-    int acl_pubusub_default;      /* Default ACL pub/sub channels flag */
+    int acl_pubsub_default;      /* Default ACL pub/sub channels flag */
     /* Assert & bug reporting */
     int watchdog_period;  /* Software watchdog period in ms. 0 = off */
     /* System hardware info */

tests/assets/user.acl

@@ -1,2 +1,3 @@
 user alice on allcommands allkeys >alice
-user bob on -@all +@set +acl ~set* >bob
+user bob on -@all +@set +acl ~set* >bob
+user default on nopass ~* +@all

tests/unit/acl.tcl

@@ -461,14 +461,44 @@ exec cp -f tests/assets/user.acl $server_path
 start_server [list overrides [list "dir" $server_path "aclfile" "user.acl"]] {
     # user alice on allcommands allkeys >alice
     # user bob on -@all +@set +acl ~set* >bob
+    # user default on nopass ~* +@all
 
-    test "Alice: can excute all command" {
+    test {default: load from include file, can access any channels} {
+        r SUBSCRIBE foo
+        r PSUBSCRIBE bar*
+        r UNSUBSCRIBE
+        r PUNSUBSCRIBE
+        r PUBLISH hello world
+    }
+
+    test {default: with config acl-pubsub-default allchannels after reset, can access any channels} {
+        r ACL setuser default reset on nopass ~* +@all
+        r SUBSCRIBE foo
+        r PSUBSCRIBE bar*
+        r UNSUBSCRIBE
+        r PUNSUBSCRIBE
+        r PUBLISH hello world
+    }
+
+    test {default: with config acl-pubsub-default resetchannels after reset, can not access any channels} {
+        r CONFIG SET acl-pubsub-default resetchannels
+        r ACL setuser default reset on nopass ~* +@all
+        catch {r SUBSCRIBE foo} e
+        assert_match {*NOPERM*} $e
+        catch {r PSUBSCRIBE bar*} e
+        assert_match {*NOPERM*} $e
+        catch {r PUBLISH hello world} e
+        assert_match {*NOPERM*} $e
+        r CONFIG SET acl-pubsub-default resetchannels
+    }
+
+    test {Alice: can execute all command} {
         r AUTH alice alice
         assert_equal "alice" [r acl whoami]
         r SET key value
     }
 
-    test "Bob: just excute @set and acl command" {
+    test {Bob: just execute @set and acl command} {
         r AUTH bob bob
         assert_equal "bob" [r acl whoami]
         assert_equal "3" [r sadd set 1 2 3]
@@ -476,7 +506,7 @@ start_server [list overrides [list "dir" $server_path "aclfile" "user.acl"]] {
         set e
     } {*NOPERM*}
 
-    test "ACL load and save" {
+    test {ACL load and save} {
         r ACL setuser eve +get allkeys >eve on
         r ACL save
 
@@ -494,3 +524,13 @@ start_server [list overrides [list "dir" $server_path "aclfile" "user.acl"]] {
         set e
     } {*NOPERM*}
 }
+
+start_server {overrides {user "default on nopass ~* +@all"}} {
+    test {default: load from config file, can access any channels} {
+        r SUBSCRIBE foo
+        r PSUBSCRIBE bar*
+        r UNSUBSCRIBE
+        r PUNSUBSCRIBE
+        r PUBLISH hello world
+    }
+}