Improved handling of subcommands (don't allow ACL on first-arg of a sub-command) (#10147)

Recently we added extensive support for sub-commands in for redis 7.0,
this meant that the old ACL mechanism for
sub-commands wasn't needed, or actually was improved (to handle both include
and exclude control, like for commands), but only for real sub-commands.
The old mechanism in ACL was renamed to first-arg, and was able to match the
first argument of any command (including sub-commands).
We now realized that we might wanna completely delete that first-arg feature some
day, so the first step was not to give it new capabilities in 7.0 and it didn't have before.

Changes:
1. ACL: Block the first-arg mechanism on subcommands (we keep if in non-subcommands
  for backward compatibility)
2. COMMAND: When looking up a command, insist the command name doesn't contain
  extra words. Example: When a user issues `GET key` we want `lookupCommand` to return
  `getCommand` but when if COMMAND calls `lookupCommand` with `get|key` we want it to fail.

Other changes:
1. ACLSetUser: prevent a redundant command lookup

1 files changed, 16 insertions, 8 deletions

diff --git a/src/acl.c b/src/acl.c
index d4fa2c6c8..24a9e35b4 100644
--- a/src/acl.c
+++ b/src/acl.c
@@ -1110,13 +1110,20 @@ int ACLSetSelector(aclSelector *selector, const char* op, size_t oplen) {
             struct redisCommand *cmd = ACLLookupCommand(copy);
 
             /* Check if the command exists. We can't check the
-             * subcommand to see if it is valid. */
+             * first-arg to see if it is valid. */
             if (cmd == NULL) {
                 zfree(copy);
                 errno = ENOENT;
                 return C_ERR;
             }
 
+            /* We do not support allowing first-arg of a subcommand */
+            if (cmd->parent) {
+                zfree(copy);
+                errno = ECHILD;
+                return C_ERR;
+            }
+
             /* The subcommand cannot be empty, so things like DEBUG|
              * are syntax errors of course. */
             if (strlen(sub) == 0) {
@@ -1127,7 +1134,7 @@ int ACLSetSelector(aclSelector *selector, const char* op, size_t oplen) {
 
             if (cmd->subcommands_dict) {
                 /* If user is trying to allow a valid subcommand we can just add its unique ID */
-                struct redisCommand *cmd = ACLLookupCommand(op+1);
+                cmd = ACLLookupCommand(op+1);
                 if (cmd == NULL) {
                     zfree(copy);
                     errno = ENOENT;
@@ -1138,13 +1145,11 @@ int ACLSetSelector(aclSelector *selector, const char* op, size_t oplen) {
                 /* If user is trying to use the ACL mech to block SELECT except SELECT 0 or
                  * block DEBUG except DEBUG OBJECT (DEBUG subcommands are not considered
                  * subcommands for now) we use the allowed_firstargs mechanism. */
-                struct redisCommand *cmd = ACLLookupCommand(copy);
-                if (cmd == NULL) {
-                    zfree(copy);
-                    errno = ENOENT;
-                    return C_ERR;
-                }
+
                 /* Add the first-arg to the list of valid ones. */
+                serverLog(LL_WARNING, "Deprecation warning: Allowing a first arg of an otherwise "
+                                      "blocked command is a misuse of ACL and may get disabled "
+                                      "in the future (offender: +%s)", op+1);
                 ACLAddAllowedFirstArg(selector,cmd->id,sub);
             }
 
@@ -1236,6 +1241,7 @@ int ACLSetSelector(aclSelector *selector, const char* op, size_t oplen) {
  *         almost surely an error on the user side.
  * ENODEV: The password you are trying to remove from the user does not exist.
  * EBADMSG: The hash you are trying to add is not a valid hash.
+ * ECHILD: Attempt to allow a specific first argument of a subcommand
  */
 int ACLSetUser(user *u, const char *op, ssize_t oplen) {
     if (oplen == -1) oplen = strlen(op);
@@ -1360,6 +1366,8 @@ const char *ACLSetUserStringError(void) {
     else if (errno == EALREADY)
         errmsg = "Duplicate user found. A user can only be defined once in "
                  "config files";
+    else if (errno == ECHILD)
+        errmsg = "Allowing first-arg of a subcommand is not supported";
     return errmsg;
 }