In particular this is important for hosted environments; while rename-command configuration can be used to disable (or more likely: obfuscate) key commands, many of these share between uesful (necessary?) commands and harmful commands; for example:
(usually OK) / (bad)
config get / config set
client setname / client pause
debug object / debug segfault
cluster slots / cluster reset
etc; more granular control over these would be awesome - otherwise, some very useful capabilities need to be sacrificed.
IF YOU NEED TO JUSTIFY IT:
In particular, redis-cluster is kinda useless without cluster slots or cluster nodes, but in a hosted environment, random clients should not have the ability modify the cluster configuration.
Things I haven't thought about: how this impacts AOF etc.
Comment From: badboy
There's a lengthy discussion on the dev mailing list as well as a proposal for Multi users AUTH and ACLs for Redis which covers this. Please read that discussion and add any comments on the mailing list.
Comment From: mattsta
All good points, but the granular ACL problem comes down to a slight design problems with Redis about how commands are handled. The only commands Redis itself knows about are top-level commands (CONFIG, CLUSTER, DEBUG, ...). All sub-commands (CONFIG GET, CONFIG SET, ...) are manually parsed and processed by the command itself, so the server can't restrict them independently of the parent command, even though they are all actually fully independent commands without shared behavior.
At an entire server level, there's no way to automatically add nice "full subcommand" ACLs (that's also why any subcommands can't be reported by COMMAND — Redis has no way to know about all subcommands created by manual argument parsing inside individual commands.).
The only (current) way to fix the problem of config commands having both safe and unsafe operations is by adding guards around every sub-command saying "does user have auth enabled for admin commands?" It's possible, just ugly. A longer term fix would involve creating cleaner command parsing abstractions to remove manual sub-command processing while also allowing fully independent subcommands to be promoted to "top level" commands Redis could be aware of (somehow).
(The other quick/fast/dumb fix is just to turn all sub commands into top-level commands: CONFIG GET -> CONFIG-GET, CLUSTER RESET -> CLUSTER-RESET, etc. Then you could 100% disable the old commands and ACL-restrict new commands, but it's kinda ugly and does pollute the top-level command namespace (though, there are already 160+ commands, so who's counting?))
Comment From: antirez
@mattsta I don't fully agree with your analysis here. An ACL layer if we will ever want it (that is, I'm not providing an opinion about that here) would be a different layer with ability to inspect the command line just before the command is dispatched. You may ACL commands that don't even exist, for what is worth, so the two layers would be independent. As an optimization, the engine may resolve the ACLs as commands so that after the command name is resolved, there is a fast path for simple commands that are allowed, just doing bit ops, however the ACL itself could strcmp() the second string with little costs. Or even allow regrexp, for what is worth.
TLDR: There are no technical limits IMHO. Just a matter of design, if we want it to happen or not.
Comment From: antirez
Now commenting what @mgravell is proposing, I read it as an entirely different bug report, which is: CLUSTER NODES and CLUSTER SLOTS should not be sub commands of the CLUSTER command that should be only the interface for cluster admin-level changes.
In Disque there is the HELLO command that handles this stuff, and we should go the same path with Redis IMHO.
Comment From: antirez
p.s. not clear from my comment above. HELLO is the command that provides the client an interface to get informations useful to manage the interaction with the server.
Comment From: mattsta
TLDR: There are no technical limits IMHO. Just a matter of design,
Very true :)
We can restrict any arbitrary of words during command processing, but I was thinking about the implementation described in RCP1 as:
Each time an user tries to execute a command, the corresponding bit (by computing 2^command-id) is checked in the client cmdacl array. The command execution is refused with an -ACL error if the bit is not found to be set.
that approach doesn't seem to allow arbitrary sub-command blocking because the server needs a fixed map of commands to test against.
Comment From: antirez
@mattsta, yep, but you can do a two levels check. When loading the ACL, if it's a single-argument one, you can match just with a bit. For more complex ACLs you resort to slower ways. However inside the subcommand itself there is a strcasecmp() call so we are doing it already and the risk is just 2x cost of this call, but is just a small percentage. My biggest concern is instead if we don't really need this kind of granularity, but a better organization of the command space so that each command only groups subcommands with a similar level of "safeness".
Comment From: mgravell
Yes, I think antirez has summarised what I am saying very well with the HELLO example. Basically, not mixing safe operations with admin operations. CLUSTER [SLOTS|NODES] is fundamentally necessary for redis-cluster clients to function effectively (rather than discovering all 16k buckets by trial and error). The same issue affects multiple other commands, but in those cases it just loses convenient functionality (I do love client setname, for example) - but not having them available doesn't cripple the client completely.
And indeed, from a client library perspective, there are a number of config get / info etc things that sometimes get issued automatically precisely to get the things it wants to know for talking to the server (version, number of databases, connection timeout limits, role, read-only state, buckets).
I totally agree that if you were starting from the ground up, mapping the commands so that safe and dangerous weren't mixed would be a great idea. However, if you did that, you'd need to know the version number before you start negotiating, or at a minimum you need to add latency for that to respond. Or I guess a cavalier client could just issue "SAFEWHATEVER ...", "CONFIG GET ..." and if one or both fails (because command not recognised, or command renamed), just use the other, or use safe but suboptimal defaults (or discovery probes).
I have to say, though, a HELLO that returned the most commonly needed things is an interesting idea.
Comment From: itamarhaber
Heya @mgravell - do you agree that this issue can be closed now that we have ACL in v6? I feel reasonably certain that is the case so I'm doing just that, but feel free to reopen if needed :)