Redis [NEW] allow set binary prefix for acl

The problem/use-case that the feature addresses

I tryed failed as below

127.0.0.1:9998> ACL SETUSER t on nopass ~'\x00*' &* +@all
(error) ERR Error in ACL SETUSER modifier '~\x00*': Adding a pattern after the * pattern (or the 'allkeys' flag) is not valid and does not have any effect. Try 'resetkeys' to start with an empty list of patterns
127.0.0.1:9998> ACL SETUSER t on nopass ~\x00* &* +@all
(error) ERR Error in ACL SETUSER modifier '~\x00*': Adding a pattern after the * pattern (or the 'allkeys' flag) is not valid and does not have any effect. Try 'resetkeys' to start with an empty list of patterns
127.0.0.1:9998> ACL SETUSER t on nopass "~\x00*" &* +@all
(error) ERR Error in ACL SETUSER modifier '~': Adding a pattern after the * pattern (or the 'allkeys' flag) is not valid and does not have any effect. Try 'resetkeys' to start with an empty list of patterns

Description of the feature

something like

ACL SETUSER t on nopass ~"\x00\x01"* &* +@all

there maybe * in "\x00\x01" , for example "\x00\x01" means the user only can access prefix with "\x00\x01*"

Comment From: zeekling

its ok on redis i build using unstable branch

Comment From: usrtax

i use redis_version:7.0.4 , I will wait the unstable branch release a version and have a try

Comment From: usrtax

I found this is bug for RedisInsight-v2 not redis

https://github.com/RedisInsight/RedisInsight/issues/1370

RedisInsight 2.12.0 (2.12.0.64844)

Comment From: oranagra

i don't think you're really able to achieve what you're aiming for.

$ src/redis-cli ACL SETUSER ttt on nopass resetkeys '~\x42\x41' "&*" +@all
OK
$ src/redis-cli acl getuser ttt
 1) "flags"
 2) 1) "on"
    2) "nopass"
    3) "sanitize-payload"
 3) "passwords"
 4) (empty array)
 5) "commands"
 6) "+@all"
 7) "keys"
 8) "~\\x42\\x41"
 9) "channels"
10) "&*"
11) "selectors"
12) (empty array)

what you did is create a string that actually contains \x00 (i.e. the x char). you need to pass --quoted-input to redis-cli to get what you want:

$ src/redis-cli --quoted-input  ACL SETUSER ttt on nopass resetkeys '"~\x42\x41"' "&*" +@all
OK
$ src/redis-cli acl getuser ttt
^[[15~ 1) "flags"
 2) 1) "on"
    2) "nopass"
    3) "sanitize-payload"
 3) "passwords"
 4) (empty array)
 5) "commands"
 6) "+@all"
 7) "keys"
 8) "~BA"
 9) "channels"
10) "&*"
11) "selectors"
12) (empty array)

however, when using the NULL char we get

$ src/redis-cli --quoted-input  ACL SETUSER ttt on nopass resetkeys '"~\x00\x41"' "&*" +@all
(error) ERR Error in ACL SETUSER modifier '~': Syntax error

see #6418

Comment From: usrtax

@oranagra you are right

for now , redis can't use binary prefix for acl

Comment From: oranagra

yes, it also doesn't support spaces. since the ACL configuration is persisted to an acl.conf (text) file, also because of the response of ACL LIST, it's not easy to support and i guess it was decided not to support that. see https://github.com/redis/redis/issues/6418#issuecomment-614083097 and 3519a5a026be50022fb4e103ddc602ffd59daf42

Comment From: madolson

Hmm, I think we should investigate supporting binary string. Theoretically we support binary keys for all other structures, I'm not sure why we wouldn't want to support it here.

Comment From: oranagra

we could certainly improve. i think the limitation was added intentionally (understanding it's a limitation), but not because it's undesired, but rather just because it was harder (and maybe unnecessary).

despite being able to store binary data in values, and even key names, in the vast majority of cases people don't use non textual key names, and even if they do, they still have some meaningful textual prefix on which you can apply filters such as ACL. my point is that i'm not certain anyone is really realistically affected by this limitation. maybe the exception is that maybe redis is used as base for something else, and the users of the layers above don't even know they're using keys and ACL. but i'm still not sure it's a realistic concern.

Comment From: madolson

We've had one user at AWS complain about it. They used compressed key names to save on space, and weren't able to adopt ACLs easily. They ended up rewriting their application to support it though, so perhaps an indicator it's not the highest priority.

Comment From: oranagra

i think that even if you compress your key names (e.g. really deflate them), if you want different keys to fall into different categories (e.g. for either ACL or cluster slot tags), you'll need some prefix or suffix to do that, and that prefix can / should be plain printable text.

i don't mind "fixing" this and making sure we escape things when we store them to the config file, but we might hit some complications with backwards compatibility, and if this limitation is not a high priority one, i guess we better spend out time elsewhere.

Comment From: madolson

Just to add one last little detail, they didn't compress it with something like deflate, they used a 2 byte symbol table with a custom codex. Imagine something like foo:: -> \x00, bar:: -> \x01 etc etc. They had some ordering in the codex, something like \x00 - \x040 was common values. I don't know how compressing with deflate would work well with ACLs either.

Complete aside, it would be cool if RESP supported a symbol table.