Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
61 changes: 54 additions & 7 deletions src/acl.c
Original file line number Diff line number Diff line change
Expand Up @@ -212,23 +212,30 @@ static int time_independent_strcmp(char *a, char *b, int len) {
return diff; /* If zero strings are the same. */
}

/* Given a binary digest of 'len' bytes, returns its lowercase hexadecimal
* representation as a new SDS string. */
static sds ACLHexDigest(const unsigned char *digest, size_t len) {
const char *cset = "0123456789abcdef";
sds hex = sdsnewlen(SDS_NOINIT, len * 2);

for (size_t j = 0; j < len; j++) {
hex[j * 2] = cset[((digest[j] & 0xF0) >> 4)];
hex[j * 2 + 1] = cset[(digest[j] & 0xF)];
}
return hex;
}

/* Given an SDS string, returns the SHA256 hex representation as a
* new SDS string. */
static sds ACLHashPassword(unsigned char *cleartext, size_t len) {
SHA256_CTX ctx;
unsigned char hash[SHA256_BLOCK_SIZE];
char hex[HASH_PASSWORD_LEN];
char *cset = "0123456789abcdef";

sha256_init(&ctx);
sha256_update(&ctx, (unsigned char *)cleartext, len);
sha256_final(&ctx, hash);

for (int j = 0; j < SHA256_BLOCK_SIZE; j++) {
hex[j * 2] = cset[((hash[j] & 0xF0) >> 4)];
hex[j * 2 + 1] = cset[(hash[j] & 0xF)];
}
return sdsnewlen(hex, HASH_PASSWORD_LEN);
return ACLHexDigest(hash, SHA256_BLOCK_SIZE);
}

/* Given a hash and the hash length, returns C_OK if it is a valid password
Expand Down Expand Up @@ -943,6 +950,41 @@ robj *ACLDescribeUser(user *u) {
return u->acl_string;
}

/* Return a fingerprint of the ACL rules currently in effect, as a new SDS
* string holding the hex representation of the digest.
*
* The digest is the XOR-combined SHA256("username rules") for all users. The
* username is part of what is hashed because XOR cancels out two equal values,
* which would otherwise drop a pair of users having the same rules. */
static sds ACLDigest(void) {
unsigned char digest[SHA256_BLOCK_SIZE] = {0};
raxIterator ri;

raxStart(&ri, Users);
raxSeek(&ri, "^", NULL, 0);
while (raxNext(&ri)) {
user *u = ri.data;
robj *rules = ACLDescribeUser(u);
sds rulestr = objectGetVal(rules);
unsigned char userdigest[SHA256_BLOCK_SIZE];
SHA256_CTX ctx;

/* Usernames can't contain spaces, so the separator makes the pair of
* name and rules unambiguous. */
sha256_init(&ctx);
sha256_update(&ctx, (unsigned char *)u->name, sdslen(u->name));
sha256_update(&ctx, (unsigned char *)" ", 1);
sha256_update(&ctx, (unsigned char *)rulestr, sdslen(rulestr));
sha256_final(&ctx, userdigest);
decrRefCount(rules);

for (int j = 0; j < SHA256_BLOCK_SIZE; j++) digest[j] ^= userdigest[j];
}
raxStop(&ri);

return ACLHexDigest(digest, SHA256_BLOCK_SIZE);
}

/* Get a command from the original command table, that is not affected
* by the command renaming operations: we base all the ACL work from that
* table, so that ACLs are valid regardless of command renaming. */
Expand Down Expand Up @@ -3105,6 +3147,7 @@ static int aclAddReplySelectorDescription(client *c, aclSelector *s) {
* ACL SAVE
* ACL LIST
* ACL USERS
* ACL DIGEST
* ACL CAT [<category>]
* ACL SETUSER <username> ... acl rules ...
* ACL DELUSER <username> [...]
Expand Down Expand Up @@ -3236,6 +3279,8 @@ void aclCommand(client *c) {
}
}
raxStop(&ri);
} else if (!strcasecmp(sub, "digest") && c->argc == 2) {
addReplyBulkSds(c, ACLDigest());
} else if (!strcasecmp(sub, "whoami") && c->argc == 2) {
if (c->user != NULL) {
addReplyBulkCBuffer(c, c->user->name, sdslen(c->user->name));
Expand Down Expand Up @@ -3406,6 +3451,8 @@ void aclCommand(client *c) {
" when no category is specified.",
"DELUSER <username> [<username> ...]",
" Delete a list of users.",
"DIGEST",
" Return a fingerprint of the rules currently in effect.",
"DRYRUN <username> <command> [<arg> ...]",
" Returns whether the user can execute the given command without executing the command.",
"GETUSER <username>",
Expand Down
18 changes: 18 additions & 0 deletions src/commands.def
Original file line number Diff line number Diff line change
Expand Up @@ -6877,6 +6877,23 @@ struct COMMAND_ARG ACL_DELUSER_Args[] = {
{MAKE_ARG("username",ARG_TYPE_STRING,-1,NULL,NULL,NULL,CMD_ARG_MULTIPLE,0,NULL)},
};

/********** ACL DIGEST ********************/

#ifndef SKIP_CMD_HISTORY_TABLE
/* ACL DIGEST history */
#define ACL_DIGEST_History NULL
#endif

#ifndef SKIP_CMD_TIPS_TABLE
/* ACL DIGEST tips */
#define ACL_DIGEST_Tips NULL
#endif

#ifndef SKIP_CMD_KEY_SPECS_TABLE
/* ACL DIGEST key specs */
#define ACL_DIGEST_Keyspecs NULL
#endif

/********** ACL DRYRUN ********************/

#ifndef SKIP_CMD_HISTORY_TABLE
Expand Down Expand Up @@ -7118,6 +7135,7 @@ struct COMMAND_ARG ACL_SETUSER_Args[] = {
struct COMMAND_STRUCT ACL_Subcommands[] = {
{MAKE_CMD("cat","Lists the ACL categories, or the commands inside a category.","O(1) since the categories and commands are a fixed set.","6.0.0",CMD_DOC_NONE,NULL,NULL,"server",COMMAND_GROUP_SERVER,ACL_CAT_History,0,ACL_CAT_Tips,0,aclCommand,-2,CMD_NOSCRIPT|CMD_LOADING|CMD_STALE|CMD_SENTINEL,ACL_CATEGORY_SLOW,NULL,ACL_CAT_Keyspecs,0,NULL,1),.args=ACL_CAT_Args},
{MAKE_CMD("deluser","Deletes ACL users, and terminates their connections.","O(1) amortized time considering the typical user.","6.0.0",CMD_DOC_NONE,NULL,NULL,"server",COMMAND_GROUP_SERVER,ACL_DELUSER_History,0,ACL_DELUSER_Tips,2,aclCommand,-3,CMD_ADMIN|CMD_NOSCRIPT|CMD_LOADING|CMD_STALE|CMD_SENTINEL,ACL_CATEGORY_ADMIN|ACL_CATEGORY_DANGEROUS|ACL_CATEGORY_SLOW,NULL,ACL_DELUSER_Keyspecs,0,NULL,1),.args=ACL_DELUSER_Args},
{MAKE_CMD("digest","Returns a fingerprint of the ACL rules currently in effect.","O(N). Where N is the number of configured users.","9.2.0",CMD_DOC_NONE,NULL,NULL,"server",COMMAND_GROUP_SERVER,ACL_DIGEST_History,0,ACL_DIGEST_Tips,0,aclCommand,2,CMD_ADMIN|CMD_NOSCRIPT|CMD_LOADING|CMD_STALE|CMD_SENTINEL,ACL_CATEGORY_ADMIN|ACL_CATEGORY_DANGEROUS|ACL_CATEGORY_SLOW,NULL,ACL_DIGEST_Keyspecs,0,NULL,0)},
{MAKE_CMD("dryrun","Simulates the execution of a command by a user, without executing the command.","O(1).","7.0.0",CMD_DOC_NONE,NULL,NULL,"server",COMMAND_GROUP_SERVER,ACL_DRYRUN_History,0,ACL_DRYRUN_Tips,0,aclCommand,-4,CMD_ADMIN|CMD_NOSCRIPT|CMD_LOADING|CMD_STALE|CMD_SENTINEL,ACL_CATEGORY_ADMIN|ACL_CATEGORY_DANGEROUS|ACL_CATEGORY_SLOW,NULL,ACL_DRYRUN_Keyspecs,0,NULL,3),.args=ACL_DRYRUN_Args},
{MAKE_CMD("genpass","Generates a pseudorandom, secure password that can be used to identify ACL users.","O(1)","6.0.0",CMD_DOC_NONE,NULL,NULL,"server",COMMAND_GROUP_SERVER,ACL_GENPASS_History,0,ACL_GENPASS_Tips,0,aclCommand,-2,CMD_NOSCRIPT|CMD_LOADING|CMD_STALE|CMD_SENTINEL,ACL_CATEGORY_SLOW,NULL,ACL_GENPASS_Keyspecs,0,NULL,1),.args=ACL_GENPASS_Args},
{MAKE_CMD("getuser","Lists the ACL rules of a user.","O(N). Where N is the number of password, command and pattern rules that the user has.","6.0.0",CMD_DOC_NONE,NULL,NULL,"server",COMMAND_GROUP_SERVER,ACL_GETUSER_History,3,ACL_GETUSER_Tips,0,aclCommand,3,CMD_ADMIN|CMD_NOSCRIPT|CMD_LOADING|CMD_STALE|CMD_SENTINEL,ACL_CATEGORY_ADMIN|ACL_CATEGORY_DANGEROUS|ACL_CATEGORY_SLOW,NULL,ACL_GETUSER_Keyspecs,0,NULL,1),.args=ACL_GETUSER_Args},
Expand Down
27 changes: 27 additions & 0 deletions src/commands/acl-digest.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
{
"DIGEST": {
"summary": "Returns a fingerprint of the ACL rules currently in effect.",
"complexity": "O(N). Where N is the number of configured users.",
"group": "server",
"since": "9.2.0",
"arity": 2,
"container": "ACL",
"function": "aclCommand",
"command_flags": [
"ADMIN",
"NOSCRIPT",
"LOADING",
"STALE",
"SENTINEL"
],

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is node-local state, but there is no request-policy tip telling cluster clients to query every node. ACL SETUSER, ACL DELUSER, and ACL SAVE all declare REQUEST_POLICY:ALL_NODES, and read-only node-local introspection such as SLOWLOG GET pairs that with NONDETERMINISTIC_OUTPUT. Without the same metadata here, a cluster-aware client can route ACL DIGEST to one arbitrary node and miss an ACL revision that differs elsewhere. Add command_tips with REQUEST_POLICY:ALL_NODES and NONDETERMINISTIC_OUTPUT.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The comparisons are accurate: ACL SETUSER, ACL DELUSER and ACL SAVE do carry REQUEST_POLICY:ALL_NODES, and SLOWLOG GET does pair it with NONDETERMINISTIC_OUTPUT. I left both tips off on purpose.

NONDETERMINISTIC_OUTPUT would say the wrong thing here. The definition in command-tips is that calls "may yield different results with the same arguments and data", which is what INFO, CLIENT LIST and TTL do. ACL DIGEST is the opposite: the same ACL state always produces the same reply. That is the contract of the command and there is a test asserting it. The tip tells a client the reply is not meant to be compared, and comparing it is the only thing the command is for.

REQUEST_POLICY:ALL_NODES is a fairer question, but I do not think it belongs on this command alone. The rest of the ACL read family carries no tips at all: ACL LIST, ACL USERS, ACL GETUSER, ACL WHOAMI, and ACL LOAD too. ACL DIGEST is node local in exactly the way ACL LIST is. Tagging only the digest puts a cluster client in an odd spot, where it fans out ACL DIGEST, finds two nodes disagreeing, and then cannot fan out ACL LIST the same way to see what differs. There is also no useful RESPONSE_POLICY for a set of hex strings, so it would end up as SPECIAL.

If node-local ACL introspection should be fanned out, I would rather do it as one change across the whole family than on the single new command, and I can open that separately. If a maintainer wants the tip here now, say so and I will add it.

"reply_schema": {
"type": "string",
"description": "The hex representation of the digest of the rules currently in effect."
},
"acl_categories": [
"ADMIN",
"DANGEROUS",
"SLOW"
]
}
}
133 changes: 133 additions & 0 deletions tests/unit/acl.tcl
Original file line number Diff line number Diff line change
Expand Up @@ -992,6 +992,88 @@ start_server {tags {"acl external:skip"}} {
assert {[s acl_access_denied_key] eq $current_invalid_key_accesses}
assert {[s acl_access_denied_channel] eq [expr $current_invalid_channel_accesses + 1]}
}

test {ACL DIGEST returns a stable digest of a fixed size} {
set digest [r ACL DIGEST]
assert_equal 1 [regexp {^[0-9a-f]{64}$} $digest]
assert_equal $digest [r ACL DIGEST]
}

test {ACL DIGEST follows the rules of a user} {
set before [r ACL DIGEST]
r ACL setuser digestuser on >digestpass ~key:a* +get
set created [r ACL DIGEST]
assert {$created ne $before}

# Changing a rule moves the digest, and putting the rule back moves it
# to the very same value: the digest describes the state, not how many
# times the state was edited.
r ACL setuser digestuser resetkeys ~key:b*
assert {[r ACL DIGEST] ne $created}
r ACL setuser digestuser resetkeys ~key:a*
assert_equal $created [r ACL DIGEST]

r ACL deluser digestuser
assert_equal $before [r ACL DIGEST]
}

test {ACL DIGEST follows the passwords of a user} {
set before [r ACL DIGEST]
r ACL setuser digestuser on >digestpass ~* +get
set created [r ACL DIGEST]
r ACL setuser digestuser >anotherpass
assert {[r ACL DIGEST] ne $created}
r ACL setuser digestuser <anotherpass
assert_equal $created [r ACL DIGEST]

r ACL deluser digestuser
assert_equal $before [r ACL DIGEST]
}

test {ACL DIGEST does not cancel out users sharing the same rules} {
# Per user digests are combined with XOR, which cancels out two equal
# values, so users having the very same rules are only kept apart by
# their names being hashed along with the rules.
set before [r ACL DIGEST]
r ACL setuser digesttwin1 on >twinpass ~twin:* +get
r ACL setuser digesttwin2 on >twinpass ~twin:* +get

# Assert the rules really are identical, or the pair would not
# exercise the cancellation at all.
set twin1 ""
set twin2 ""
foreach line [r ACL LIST] {
if {[string match "user digesttwin1 *" $line]} {
set twin1 [string range $line [string length "user digesttwin1 "] end]
} elseif {[string match "user digesttwin2 *" $line]} {
set twin2 [string range $line [string length "user digesttwin2 "] end]
}
}
assert {$twin1 ne ""}
assert_equal $twin1 $twin2

set both [r ACL DIGEST]
assert {$both ne $before}
r ACL deluser digesttwin2
set one [r ACL DIGEST]
assert {$one ne $both}
assert {$one ne $before}

r ACL deluser digesttwin1
assert_equal $before [r ACL DIGEST]
}

test {ACL DIGEST rejects extra arguments} {
assert_error "*wrong number of arguments for 'acl|digest' command" {r ACL DIGEST extra}
}

test {ACL DIGEST needs permission to run} {
r ACL setuser digestnoperm on >digestpass ~* +acl|whoami
r AUTH digestnoperm digestpass
assert_error "*has no permissions to run the 'acl|digest' command*" {r ACL DIGEST}
r AUTH default ""
r ACL deluser digestnoperm
}
}

set server_path [tmpdir "server.acl"]
Expand Down Expand Up @@ -1240,6 +1322,57 @@ start_server [list overrides [list "dir" $server_path "acl-pubsub-default" "allc
}
}

set server_path [tmpdir "digest.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"]] {
set aclfile [file join $server_path "user.acl"]

proc write_digest_acl_file {path users} {
set fd [open $path w]
foreach user $users {
puts $fd $user
}
close $fd
}

# Every file below keeps the default user as it already is, so that the
# client running the test is never disconnected by the reload.
test {ACL DIGEST is unchanged by an ACL LOAD of an equivalent file} {
write_digest_acl_file $aclfile {
"user default on nopass ~* &* +@all"
"user alice on >alice ~key:* &chan:* +@all"
"user bob on >bob ~* &* +@all"
}
r ACL LOAD
set digest [r ACL DIGEST]

# The same users with the same effective rules, listed in a different
# order and written with the aliases of those rules. Both files load
# into the same state, which is what the digest reports.
write_digest_acl_file $aclfile {
"user bob on >bob allkeys allchannels allcommands"
"user default on nopass allkeys allchannels allcommands"
"user alice on >alice ~key:* &chan:* allcommands"
}
r ACL LOAD
assert_equal $digest [r ACL DIGEST]
}

test {ACL DIGEST changes after an ACL LOAD of a file with different rules} {
set digest [r ACL DIGEST]

# A permission only edit: same users, same passwords, one command
# taken away from bob.
write_digest_acl_file $aclfile {
"user default on nopass ~* &* +@all"
"user alice on >alice ~key:* &chan:* +@all"
"user bob on >bob ~* &* +@all -get"
}
r ACL LOAD
assert {[r ACL DIGEST] ne $digest}
}
}

set server_path [tmpdir "resetchannels.acl"]
exec cp -f tests/assets/nodefaultuser.acl $server_path
exec cp -f tests/assets/default.conf $server_path
Expand Down
Loading