Set default channel permission to resetchannels for 7.0 (#10181)

For backwards compatibility in 6.x, channels default permission was set to `allchannels` however with 7.0,
we should modify it and the default value should be `resetchannels` for better security posture.
Also, with selectors in ACL, a client doesn't have to set channel rules everytime and by default
the value will be `resetchannels`.

Before this change
```
127.0.0.1:6379> acl list
1) "user default on nopass ~* &* +@all"
127.0.0.1:6379>  acl setuser hp on nopass +@all ~*
OK
127.0.0.1:6379> acl list
1) "user default on nopass ~* &* +@all"
2) "user hp on nopass ~* &* +@all"
127.0.0.1:6379>  acl setuser hp1 on nopass -@all (%R~sales*)
OK
127.0.0.1:6379> acl list
1) "user default on nopass ~* &* +@all"
2) "user hp on nopass ~* &* +@all"
3) "user hp1 on nopass &* -@all (%R~sales* &* -@all)"
```

After this change
```
127.0.0.1:6379> acl list
1) "user default on nopass ~* &* +@all"
127.0.0.1:6379> acl setuser hp on nopass +@all ~*
OK
127.0.0.1:6379> acl list
1) "user default on nopass ~* &* +@all"
2) "user hp on nopass ~* resetchannels +@all"
127.0.0.1:6379> acl setuser hp1 on nopass -@all (%R~sales*)
OK
127.0.0.1:6379> acl list
1) "user default on nopass ~* &* +@all"
2) "user hp on nopass ~* resetchannels +@all"
3) "user hp1 on nopass resetchannels -@all (%R~sales* resetchannels -@all)"
```

5 files changed, 63 insertions, 36 deletions

diff --git a/redis.conf b/redis.conf
index b1b775358..ad37bbe33 100644
--- a/redis.conf
+++ b/redis.conf
@@ -1007,15 +1007,7 @@ acllog-max-len 128
 # allchannels: grants access to all Pub/Sub channels
 # resetchannels: revokes access to all Pub/Sub channels
 #
-# To ensure backward compatibility while upgrading Redis 6.0, acl-pubsub-default
-# defaults to the 'allchannels' permission.
-#
-# Future compatibility note: it is very likely that in a future version of Redis
-# the directive's default of 'allchannels' will be changed to 'resetchannels' in
-# order to provide better out-of-the-box Pub/Sub security. Therefore, it is
-# recommended that you explicitly define Pub/Sub permissions for all users
-# rather then rely on implicit default values. Once you've set explicit
-# Pub/Sub for all existing users, you should uncomment the following line.
+# From Redis 7.0, acl-pubsub-default defaults to 'resetchannels' permission.
 #
 # acl-pubsub-default resetchannels
 
diff --git a/src/config.c b/src/config.c
index 482a29617..a52e00f66 100644
--- a/src/config.c
+++ b/src/config.c
@@ -2811,7 +2811,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_pubsub_default, SELECTOR_FLAG_ALLCHANNELS, NULL, NULL),
+    createEnumConfig("acl-pubsub-default", NULL, MODIFIABLE_CONFIG, acl_pubsub_default_enum, server.acl_pubsub_default, 0, NULL, NULL),
     createEnumConfig("sanitize-dump-payload", NULL, DEBUG_CONFIG | MODIFIABLE_CONFIG, sanitize_dump_payload_enum, server.sanitize_dump_payload, SANITIZE_DUMP_NO, NULL, NULL),
     createEnumConfig("enable-protected-configs", NULL, IMMUTABLE_CONFIG, protected_action_enum, server.enable_protected_configs, PROTECTED_ACTION_ALLOWED_NO, NULL, NULL),
     createEnumConfig("enable-debug-command", NULL, IMMUTABLE_CONFIG, protected_action_enum, server.enable_debug_cmd, PROTECTED_ACTION_ALLOWED_NO, NULL, NULL),
diff --git a/tests/assets/user.acl b/tests/assets/user.acl
index 67303512c..926ac54f6 100644
--- a/tests/assets/user.acl
+++ b/tests/assets/user.acl
@@ -1,3 +1,3 @@
-user alice on allcommands allkeys >alice
-user bob on -@all +@set +acl ~set* >bob
-user default on nopass ~* +@all
+user alice on allcommands allkeys &* >alice
+user bob on -@all +@set +acl ~set* &* >bob
+user default on nopass ~* &* +@all
diff --git a/tests/unit/acl-v2.tcl b/tests/unit/acl-v2.tcl
index 8f4ac83cc..72ea44c3a 100644
--- a/tests/unit/acl-v2.tcl
+++ b/tests/unit/acl-v2.tcl
@@ -20,12 +20,12 @@ start_server {tags {"acl external:skip"}} {
         assert_match "*NOPERM*keys*" $err
     }
 
-    test {Test ACL selectors by default have no permissions (except channels)} {
+    test {Test ACL selectors by default have no permissions} {
         r ACL SETUSER selector-default reset ()
         set user [r ACL GETUSER "selector-default"]
         assert_equal 1 [llength [dict get $user selectors]]
         assert_equal "" [dict get [lindex [dict get $user selectors] 0] keys]
-        assert_equal "&*" [dict get [lindex [dict get $user selectors] 0] channels]
+        assert_equal "" [dict get [lindex [dict get $user selectors] 0] channels]
         assert_equal "-@all" [dict get [lindex [dict get $user selectors] 0] commands]
     }
 
@@ -44,7 +44,7 @@ start_server {tags {"acl external:skip"}} {
         catch {r ACL SETUSER selector-syntax on (this-is-invalid)} e
         assert_match "*ERR Error in ACL SETUSER modifier '(*)*Syntax*" $e
 
-        catch {r ACL SETUSER selector-syntax on (&fail)} e
+        catch {r ACL SETUSER selector-syntax on (&* &fail)} e
         assert_match "*ERR Error in ACL SETUSER modifier '(*)*Adding a pattern after the*" $e
 
         assert_equal "" [r ACL GETUSER selector-syntax]
diff --git a/tests/unit/acl.tcl b/tests/unit/acl.tcl
index 24f069313..494c3847e 100644
--- a/tests/unit/acl.tcl
+++ b/tests/unit/acl.tcl
@@ -81,42 +81,70 @@ start_server {tags {"acl external:skip"}} {
         set e
     } {*NOPERM*key*}
 
-    test {By default users are able to publish to any channel} {
+    test {By default, only default user is able to publish to any channel} {
+        r AUTH default pwd
+        r PUBLISH foo bar
         r ACL setuser psuser on >pspass +acl +client +@pubsub
         r AUTH psuser pspass
-        r PUBLISH foo bar
-    } {0}
+        catch {r PUBLISH foo bar} e
+        set e
+    } {*NOPERM*channels*}
 
-    test {By default users are able to publish to any shard channel} {
+    test {By default, only default user is not able to publish to any shard channel} {
+        r AUTH default pwd
         r SPUBLISH foo bar
-    } {0}
+        r AUTH psuser pspass
+        catch {r SPUBLISH foo bar} e
+        set e
+    } {*NOPERM*channels*}
 
-    test {By default users are able to subscribe to any channel} {
+    test {By default, only default user is able to subscribe to any channel} {
         set rd [redis_deferring_client]
-        $rd AUTH psuser pspass
+        $rd AUTH default pwd
         $rd read
         $rd SUBSCRIBE foo
         assert_match {subscribe foo 1} [$rd read]
+        $rd UNSUBSCRIBE
+        $rd read
+        $rd AUTH psuser pspass
+        $rd read
+        $rd SUBSCRIBE foo
+        catch {$rd read} e
         $rd close
-    } {0}
+        set e
+    } {*NOPERM*channels*}
 
-    test {By default users are able to subscribe to any shard channel} {
+    test {By default, only default user is able to subscribe to any shard channel} {
         set rd [redis_deferring_client]
-        $rd AUTH psuser pspass
+        $rd AUTH default pwd
         $rd read
         $rd SSUBSCRIBE foo
         assert_match {ssubscribe foo 1} [$rd read]
+        $rd SUNSUBSCRIBE
+        $rd read
+        $rd AUTH psuser pspass
+        $rd read
+        $rd SSUBSCRIBE foo
+        catch {$rd read} e
         $rd close
-    } {0}
+        set e
+    } {*NOPERM*channels*}
 
-    test {By default users are able to subscribe to any pattern} {
+    test {By default, only default user is able to subscribe to any pattern} {
         set rd [redis_deferring_client]
-        $rd AUTH psuser pspass
+        $rd AUTH default pwd
         $rd read
         $rd PSUBSCRIBE bar*
         assert_match {psubscribe bar\* 1} [$rd read]
+        $rd PUNSUBSCRIBE
+        $rd read
+        $rd AUTH psuser pspass
+        $rd read
+        $rd PSUBSCRIBE bar*
+        catch {$rd read} e
         $rd close
-    } {0}
+        set e
+    } {*NOPERM*channels*}
 
     test {It's possible to allow publishing to a subset of channels} {
         r ACL setuser psuser resetchannels &foo:1 &bar:*
@@ -675,10 +703,10 @@ start_server {tags {"acl external:skip"}} {
 
 set server_path [tmpdir "server.acl"]
 exec cp -f tests/assets/user.acl $server_path
-start_server [list overrides [list "dir" $server_path "aclfile" "user.acl"] tags [list "external:skip"]] {
-    # user alice on allcommands allkeys >alice
-    # user bob on -@all +@set +acl ~set* >bob
-    # user default on nopass ~* +@all
+start_server [list overrides [list "dir" $server_path "acl-pubsub-default" "allchannels" "aclfile" "user.acl"] tags [list "external:skip"]] {
+    # user alice on allcommands allkeys &* >alice
+    # user bob on -@all +@set +acl ~set* &* >bob
+    # user default on nopass ~* &* +@all
 
     test {default: load from include file, can access any channels} {
         r SUBSCRIBE foo
@@ -760,7 +788,7 @@ start_server [list overrides [list "dir" $server_path "aclfile" "user.acl"] tags
 set server_path [tmpdir "resetchannels.acl"]
 exec cp -f tests/assets/nodefaultuser.acl $server_path
 exec cp -f tests/assets/default.conf $server_path
-start_server [list overrides [list "dir" $server_path "acl-pubsub-default" "resetchannels" "aclfile" "nodefaultuser.acl"] tags [list "external:skip"]] {
+start_server [list overrides [list "dir" $server_path "aclfile" "nodefaultuser.acl"] tags [list "external:skip"]] {
 
     test {Default user has access to all channels irrespective of flag} {
         set channelinfo [dict get [r ACL getuser default] channels]
@@ -811,7 +839,14 @@ start_server [list overrides [list "dir" $server_path "acl-pubsub-default" "rese
 
 
 start_server {overrides {user "default on nopass ~* +@all"} tags {"external:skip"}} {
-    test {default: load from config file, can access any channels} {
+    test {default: load from config file, without channel permission default user can't access any channels} {
+        catch {r SUBSCRIBE foo} e
+        set e
+    } {*NOPERM*channel*}
+}
+
+start_server {overrides {user "default on nopass ~* &* +@all"} tags {"external:skip"}} {
+    test {default: load from config file with all channels permissions} {
         r SUBSCRIBE foo
         r PSUBSCRIBE bar*
         r UNSUBSCRIBE