mirrors/valkey
1
0
Fork 0
mirror of http://github.com/valkey-io/valkey synced 2024-11-22 18:54:58 +00:00
Code Issues Packages Projects Releases Wiki Activity

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

Browse Source 
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)"
```
This commit is contained in:
Harkrishn Patro 2022-01-30 11:02:55 +01:00 committed by GitHub
parent eedec155ac
commit a43b6922d1
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 67 additions and 40 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

10
redis.conf
View File

@ -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

2
src/config.c
View File

@ -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),

6
tests/assets/user.acl
View File

@ -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

6
tests/unit/acl-v2.tcl
View File

@ -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]

83
tests/unit/acl.tcl
View File

@ -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 close
} {0}
test {By default users are able to subscribe to any shard channel} {
set rd [redis_deferring_client]
$rd UNSUBSCRIBE
$rd read
$rd AUTH psuser pspass
$rd read
$rd SUBSCRIBE foo
catch {$rd read} e
$rd close
set e
} {*NOPERM*channels*}
test {By default, only default user is able to subscribe to any shard channel} {
set rd [redis_deferring_client]
$rd AUTH default pwd
$rd read
$rd SSUBSCRIBE foo
assert_match {ssubscribe foo 1} [$rd read]
$rd close
} {0}
test {By default users are able to subscribe to any pattern} {
set rd [redis_deferring_client]
$rd SUNSUBSCRIBE
$rd read
$rd AUTH psuser pspass
$rd read
$rd SSUBSCRIBE foo
catch {$rd read} e
$rd close
set e
} {*NOPERM*channels*}
test {By default, only default user is able to subscribe to any pattern} {
set rd [redis_deferring_client]
$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