Further improved ACL algorithm for picking categories

This commit is contained in:
Madelyn Olson 2020-10-26 21:23:30 -07:00 committed by Madelyn Olson
parent eda06b7231
commit 2eae7756fd
2 changed files with 60 additions and 29 deletions

View File

@ -479,36 +479,61 @@ sds ACLDescribeUserCommandRules(user *u) {
ACLSetUser(fakeuser,"-@all",-1); ACLSetUser(fakeuser,"-@all",-1);
} }
/* Try to add or subtract each category one after the other. Often a /* Attempt to find a good approximation for categories and commands
* single category will not perfectly match the set of commands into * based on the current bits used, by looping over the category list
* it, so at the end we do a final pass adding/removing the single commands * and applying the best fit each time. Often a set of categories will not
* needed to make the bitmap exactly match. A temp user is maintained to * perfectly match the set of commands into it, so at the end we do a
* keep track of categories already applied. */ * final pass adding/removing the single commands needed to make the bitmap
* exactly match. A temp user is maintained to keep track of categories
* already applied. */
user tu = {0}; user tu = {0};
user *tempuser = &tu; user *tempuser = &tu;
memcpy(tempuser->allowed_commands, memcpy(tempuser->allowed_commands,
u->allowed_commands, u->allowed_commands,
sizeof(u->allowed_commands)); sizeof(u->allowed_commands));
while (1) {
int best = -1;
unsigned long mindiff = INT_MAX, maxsame = 0;
for (int j = 0; ACLCommandCategories[j].flag != 0; j++) {
unsigned long on, off, diff, same;
ACLCountCategoryBitsForUser(tempuser,&on,&off,ACLCommandCategories[j].name);
for (int j = 0; ACLCommandCategories[j].flag != 0; j++) { /* Check if the current category is the best this loop:
unsigned long on, off; * * It has more commands in common with the user than commands
ACLCountCategoryBitsForUser(tempuser,&on,&off,ACLCommandCategories[j].name); * that are different.
if ((additive && on > off) || (!additive && off > on)) { * AND EITHER
sds op = sdsnewlen(additive ? "+@" : "-@", 2); * * It has the fewest number of differences
op = sdscat(op,ACLCommandCategories[j].name); * than the best match we have found so far.
ACLSetUser(fakeuser,op,-1); * * OR it matches the fewest number of differences
* that we've seen but it has more in common. */
sds invop = sdsnewlen(additive ? "-@" : "+@", 2); diff = additive ? off : on;
invop = sdscat(invop,ACLCommandCategories[j].name); same = additive ? on : off;
ACLSetUser(tempuser,invop,-1); if (same > diff &&
((diff < mindiff) || (diff == mindiff && same > maxsame))) {
rules = sdscatsds(rules,op); best = j;
rules = sdscatlen(rules," ",1); mindiff = diff;
sdsfree(op); maxsame = same;
sdsfree(invop); }
} }
/* We didn't find a match */
if (best == -1) break;
sds op = sdsnewlen(additive ? "+@" : "-@", 2);
op = sdscat(op,ACLCommandCategories[best].name);
ACLSetUser(fakeuser,op,-1);
sds invop = sdsnewlen(additive ? "-@" : "+@", 2);
invop = sdscat(invop,ACLCommandCategories[best].name);
ACLSetUser(tempuser,invop,-1);
rules = sdscatsds(rules,op);
rules = sdscatlen(rules," ",1);
sdsfree(op);
sdsfree(invop);
} }
/* Fix the final ACLs with single commands differences. */ /* Fix the final ACLs with single commands differences. */
dictIterator *di = dictGetIterator(server.orig_commands); dictIterator *di = dictGetIterator(server.orig_commands);
dictEntry *de; dictEntry *de;

View File

@ -138,15 +138,21 @@ start_server {tags {"acl"}} {
# A regression test make sure that as long as there is a simple # A regression test make sure that as long as there is a simple
# category defining the commands, that it will be used as is. # category defining the commands, that it will be used as is.
test {ACL GETUSER provides reasonable results} { test {ACL GETUSER provides reasonable results} {
# Test for future commands where allowed set categories [r ACL CAT]
r ACL setuser additive reset +@all -@write
set cmdstr [dict get [r ACL getuser additive] commands]
assert_match {+@all -@write} $cmdstr
# Test for future commands are disallowed # Test that adding each single category will
r ACL setuser subtractive reset -@all +@read # result in just that category with both +@all and -@all
set cmdstr [dict get [r ACL getuser subtractive] commands] foreach category $categories {
assert_match {-@all +@read} $cmdstr # Test for future commands where allowed
r ACL setuser additive reset +@all "-@$category"
set cmdstr [dict get [r ACL getuser additive] commands]
assert_equal "+@all -@$category" $cmdstr
# Test for future commands where disallowed
r ACL setuser restrictive reset -@all "+@$category"
set cmdstr [dict get [r ACL getuser restrictive] commands]
assert_equal "-@all +@$category" $cmdstr
}
} }
test {ACL #5998 regression: memory leaks adding / removing subcommands} { test {ACL #5998 regression: memory leaks adding / removing subcommands} {