From 4a3419acfcaa178237a09e769aa34163d1391d09 Mon Sep 17 00:00:00 2001
From: antirez <antirez@gmail.com>
Date: Wed, 16 Jan 2019 18:31:05 +0100
Subject: [PATCH] ACL: fix and improve ACL key checking.

---
 src/acl.c    | 22 ++++++++++++++--------
 src/server.c | 12 ++++++++++--
 src/server.h |  4 ++++
 3 files changed, 28 insertions(+), 10 deletions(-)

diff --git a/src/acl.c b/src/acl.c
index c656b3f6..851439aa 100644
--- a/src/acl.c
+++ b/src/acl.c
@@ -218,6 +218,7 @@ void ACLInit(void) {
     Users = raxNew();
     DefaultUser = ACLCreateUser("default",7);
     ACLSetUser(DefaultUser,"+@all",-1);
+    ACLSetUser(DefaultUser,"~*",-1);
     ACLSetUser(DefaultUser,"on",-1);
     ACLSetUser(DefaultUser,"nopass",-1);
 }
@@ -288,18 +289,21 @@ user *ACLGetUserByName(const char *name, size_t namelen) {
  * referenced by c->cmd, can be executed by this client according to the
  * ACls associated to the client user c->user.
  *
- * If the user can execute the command C_OK is returned, otherwise
- * C_ERR is returned. */
+ * If the user can execute the command ACL_OK is returned, otherwise
+ * ACL_DENIED_CMD or ACL_DENIED_KEY is returned: the first in case the
+ * command cannot be executed because the user is not allowed to run such
+ * command, the second if the command is denied because the user is trying
+ * to access keys that are not among the specified patterns. */
 int ACLCheckCommandPerm(client *c) {
     user *u = c->user;
     uint64_t id = c->cmd->id;
 
     /* If there is no associated user, the connection can run anything. */
-    if (u == NULL) return C_OK;
+    if (u == NULL) return ACL_OK;
 
     /* We have to deny every command with an ID that overflows the Redis
      * internal structures. Very unlikely to happen. */
-    if (c->cmd->id >= USER_MAX_COMMAND_BIT) return C_ERR;
+    if (c->cmd->id >= USER_MAX_COMMAND_BIT) return ACL_DENIED_CMD;
 
     /* Check if the user can execute this command. */
     if (!(u->flags & USER_FLAG_ALLCOMMANDS) &&
@@ -311,10 +315,12 @@ int ACLCheckCommandPerm(client *c) {
          * command is allowed just with that specific subcommand. */
         if (!(u->allowed_commands[wordid] & bit)) {
             /* Check if the subcommand matches. */
-            if (u->allowed_subcommands == NULL || c->argc < 2) return C_ERR;
+            if (u->allowed_subcommands == NULL || c->argc < 2)
+                return ACL_DENIED_CMD;
             long subid = 0;
             while (1) {
-                if (u->allowed_subcommands[id][subid] == NULL) return C_ERR;
+                if (u->allowed_subcommands[id][subid] == NULL)
+                    return ACL_DENIED_CMD;
                 if (!strcasecmp(c->argv[1]->ptr,
                                 u->allowed_subcommands[id][subid]))
                     break; /* Subcommand match found. Stop here. */
@@ -348,14 +354,14 @@ int ACLCheckCommandPerm(client *c) {
                     break;
                 }
             }
-            if (!match) return C_ERR;
+            if (!match) return ACL_DENIED_KEY;
         }
         getKeysFreeResult(keyidx);
     }
 
     /* If we survived all the above checks, the user can execute the
      * command. */
-    return C_OK;
+    return ACL_OK;
 }
 
 /* =============================================================================
diff --git a/src/server.c b/src/server.c
index edf16917..a4cd5620 100644
--- a/src/server.c
+++ b/src/server.c
@@ -2598,9 +2598,17 @@ int processCommand(client *c) {
 
     /* Check if the user can run this command according to the current
      * ACLs. */
-    if (ACLCheckCommandPerm(c) == C_ERR) {
+    int acl_retval = ACLCheckCommandPerm(c);
+    if (acl_retval != ACL_OK) {
         flagTransaction(c);
-        addReplyErrorFormat(c,"-NOPERM this user has no permissions to run the %s command", c->cmd->name);
+        if (acl_retval == ACL_DENIED_CMD)
+            addReplyErrorFormat(c,
+                "-NOPERM this user has no permissions to run "
+                "the '%s' command", c->cmd->name);
+        else
+            addReplyErrorFormat(c,
+                "-NOPERM this user has no permissions to access "
+                "one of the keys used as arguments");
         return C_OK;
     }
 
diff --git a/src/server.h b/src/server.h
index 95331c6d..58a4501b 100644
--- a/src/server.h
+++ b/src/server.h
@@ -1698,6 +1698,10 @@ void receiveChildInfo(void);
 /* acl.c -- Authentication related prototypes. */
 extern user *DefaultUser;
 void ACLInit(void);
+/* Return values for ACLCheckUserCredentials(). */
+#define ACL_OK 0
+#define ACL_DENIED_CMD 1
+#define ACL_DENIED_KEY 2
 int ACLCheckUserCredentials(robj *username, robj *password);
 unsigned long ACLGetCommandID(const char *cmdname);
 user *ACLGetUserByName(const char *name, size_t namelen);