Skip to content

Commit 7a0b84e

Browse files
dvkashapovhpatro
authored andcommitted
Database-level access control (valkey-io#2309)
## API changes and user behavior: - [x] Default behavior for database access. Default is `alldbs` permissions. ### Database Permissions (`db=`) - [x] Accessing particular database ``` > ACL SETUSER test1 on +@ALL ~* resetdbs db=0,1 nopass "user test1 on nopass sanitize-payload ~* resetchannels db=0,1 +@ALL" ``` - [x] (Same behavior without usage of `resetdbs`) ``` > ACL SETUSER test1 on +@ALL ~* db=0,1 nopass "user test1 on nopass sanitize-payload ~* resetchannels db=0,1 +@ALL" ``` - [x] Multiple selector can be provided ``` > ACL SETUSER test1 on nopass (db=0,1 +@Write +select ~*) (db=2,3 +@READ +select ~*) "user test1 on nopass sanitize-payload resetchannels alldbs -@ALL (~* resetchannels db=0,1 -@ALL +@Write +select) (~* resetchannels db=2,3 -@ALL +@READ +select)" ``` - [x] Restricting special commands which access databases as part of the command. The user needs to have access to both the commands and db(s) part of the command to run these commands. 1. SWAPDB 2. SELECT 3. MOVE - (Select command would have went through for the source database). Have access for the target database. 4. COPY - [x] Restricting special commands which doesn't specify database number, however, accesses multiple databases. The user needs to have access to both the commands and all databases (`alldbs`) to run these commands. 1. FLUSHALL - Access all databases 2. CLUSTER commands that access all databases: - CANCELSLOTMIGRATIONS - MIGRATESLOTS - [x] New connection establishment behavior New client connection gets established to DB 0 by default. Authentication and authorisation are decoupled and the user can connect/authenticate and further perform `SELECT` or other operation that do not access keyspace. (Do we want to extend HELLO?) Alternative suggestion by @madolson: Extend `HELLO` command to pass the dbid to which the user should get connected after authentication if they have right set of permission. I think it will become a long poll for adoption. - [x] Observability Extend `ACL LOG` to log user which received denied permission error while accessing a database. - [x] Module API * Introduce module API `int VM_ACLCheckPermissions(ValkeyModuleUser *user, ValkeyModuleString **argv, int argc, int dbid, ValkeyModuleACLLogEntryReason *denial_reason);` * Stop support of `VM_ACLCheckCommandPermissions()`. Resolves: valkey-io#1336 --------- Signed-off-by: Daniil Kashapov <daniil.kashapov.ykt@gmail.com> Signed-off-by: Harkrishn Patro <bunty.hari@gmail.com>
1 parent ffe1688 commit 7a0b84e

31 files changed

Lines changed: 2034 additions & 477 deletions

src/acl.c

Lines changed: 225 additions & 17 deletions
Large diffs are not rendered by default.

src/cli_commands.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,8 @@
33

44
/* Definitions to configure commands.c to generate the above structs. */
55
#define MAKE_CMD(name, summary, complexity, since, doc_flags, replaced, deprecated, group, group_enum, history, \
6-
num_history, tips, num_tips, function, arity, flags, acl, key_specs, key_specs_num, get_keys, \
7-
numargs) \
6+
num_history, tips, num_tips, function, arity, flags, acl, get_dbid_args, key_specs, \
7+
key_specs_num, get_keys, numargs) \
88
name, summary, group, since, numargs
99
#define MAKE_ARG(name, type, key_spec_index, token, summary, since, flags, numsubargs, deprecated_since) \
1010
name, type, token, since, flags, numsubargs

src/commands.c

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,10 +2,10 @@
22
#include "server.h"
33

44
#define MAKE_CMD(name, summary, complexity, since, doc_flags, replaced, deprecated, group, group_enum, history, \
5-
num_history, tips, num_tips, function, arity, flags, acl, key_specs, key_specs_num, get_keys, \
6-
numargs) \
5+
num_history, tips, num_tips, function, arity, flags, acl, get_dbid_args, key_specs, \
6+
key_specs_num, get_keys, numargs) \
77
name, summary, complexity, since, doc_flags, replaced, deprecated, group_enum, history, num_history, tips, \
8-
num_tips, function, arity, flags, acl, key_specs, key_specs_num, get_keys, numargs
8+
num_tips, function, arity, flags, acl, get_dbid_args, key_specs, key_specs_num, get_keys, numargs
99
#define MAKE_ARG(name, type, key_spec_index, token, summary, since, flags, numsubargs, deprecated_since) \
1010
name, type, key_spec_index, token, summary, since, flags, deprecated_since, numsubargs
1111
#define COMMAND_STRUCT serverCommand

src/commands.def

Lines changed: 425 additions & 423 deletions
Large diffs are not rendered by default.

src/commands/README.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,7 @@ following keys. To be safe, assume all of them are optional.
5454
the command. (Don't use it for anything else.)
5555
* `"command_flags"`: An array of flags represented as strings. Command flags:
5656
* `"ADMIN"`
57+
* `"ALL_DBS"`
5758
* `"ALLOW_BUSY"`
5859
* `"ASKING"`
5960
* `"BLOCKING"`
@@ -93,6 +94,9 @@ following keys. To be safe, assume all of them are optional.
9394
* `"STREAM"`
9495
* `"STRING"`
9596
* `"TRANSACTION"`
97+
* `"get_dbid_args"`: The name of the C function in Valkey's source code
98+
implementing retrieval of database ID arguments from commands that accept
99+
database ID as an argument.
96100
* `"command_tips"`: Optional. A list of one or more of these strings:
97101
* `"NONDETERMINISTIC_OUTPUT"`
98102
* `"NONDETERMINISTIC_OUTPUT_ORDER"`

src/commands/acl-getuser.json

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,10 @@
1515
[
1616
"7.0.0",
1717
"Added selectors and changed the format of key and channel patterns from a list to their rule representation."
18+
],
19+
[
20+
"9.1.0",
21+
"Added database permission rules."
1822
]
1923
],
2024
"command_flags": [
@@ -61,6 +65,10 @@
6165
"description": "Root selector's channels.",
6266
"type": "string"
6367
},
68+
"databases": {
69+
"description": "Root selector's databases.",
70+
"type": "string"
71+
},
6472
"selectors": {
6573
"type": "array",
6674
"items": {
@@ -75,6 +83,9 @@
7583
},
7684
"channels": {
7785
"type": "string"
86+
},
87+
"databases": {
88+
"type": "string"
7889
}
7990
}
8091
}

src/commands/acl-setuser.json

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,10 @@
1515
[
1616
"7.0.0",
1717
"Added selectors and key based permissions."
18+
],
19+
[
20+
"9.1.0",
21+
"Added database permission rules."
1822
]
1923
],
2024
"command_flags": [

src/commands/cluster-cancelslotmigrations.json

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,8 @@
1010
"command_flags": [
1111
"NO_ASYNC_LOADING",
1212
"ADMIN",
13-
"STALE"
13+
"STALE",
14+
"ALL_DBS"
1415
],
1516
"reply_schema": {
1617
"const": "OK"

src/commands/cluster-migrateslots.json

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,8 @@
1010
"command_flags": [
1111
"NO_ASYNC_LOADING",
1212
"ADMIN",
13-
"STALE"
13+
"STALE",
14+
"ALL_DBS"
1415
],
1516
"arguments": [
1617
{

src/commands/copy.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
"SLOW",
1616
"WRITE"
1717
],
18+
"get_dbid_args": "copyDbIdArgs",
1819
"key_specs": [
1920
{
2021
"flags": [

0 commit comments

Comments
 (0)