From 02f4e087d7ad19a92464daefc68563f2271290ec Mon Sep 17 00:00:00 2001
From: antirez <antirez@gmail.com>
Date: Tue, 29 Jan 2019 10:12:22 +0100
Subject: [PATCH 01/32] ACL: enforce ACLs in Lua scripts as well.

---
 src/scripting.c | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/src/scripting.c b/src/scripting.c
index f6df38400..cbbf43fb1 100644
--- a/src/scripting.c
+++ b/src/scripting.c
@@ -460,6 +460,7 @@ int luaRedisGenericCommand(lua_State *lua, int raise_error) {
     /* Setup our fake client for command execution */
     c->argv = argv;
     c->argc = argc;
+    c->user = server.lua_caller->user;
 
     /* Log the command if debugging is active. */
     if (ldb.active && ldb.step) {
@@ -497,6 +498,19 @@ int luaRedisGenericCommand(lua_State *lua, int raise_error) {
         goto cleanup;
     }
 
+    /* Check the ACLs. */
+    int acl_retval = ACLCheckCommandPerm(c);
+    if (acl_retval != ACL_OK) {
+        if (acl_retval == ACL_DENIED_CMD)
+            luaPushError(lua, "The user executing the script can't run this "
+                              "command or subcommand");
+        else
+            luaPushError(lua, "The user executing the script can't access "
+                              "at least one of the keys mentioned in the "
+                              "command arguments");
+        goto cleanup;
+    }
+
     /* Write commands are forbidden against read-only slaves, or if a
      * command marked as non-deterministic was already called in the context
      * of this script. */
@@ -655,6 +669,8 @@ cleanup:
         argv_size = 0;
     }
 
+    c->user = NULL;
+
     if (raise_error) {
         /* If we are here we should have an error in the stack, in the
          * form of a table with an "err" field. Extract the string to

From 66714ee44922201a89bb43ca16794cb62be35cc2 Mon Sep 17 00:00:00 2001
From: antirez <antirez@gmail.com>
Date: Tue, 29 Jan 2019 17:25:02 +0100
Subject: [PATCH 02/32] ACL: initial design for ACLDescribeUserCommandRules()
 and helpers.

---
 src/acl.c | 88 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 88 insertions(+)

diff --git a/src/acl.c b/src/acl.c
index ecaae9268..ebfb5a066 100644
--- a/src/acl.c
+++ b/src/acl.c
@@ -228,6 +228,94 @@ int ACLSetUserCommandBitsForCategory(user *u, const char *category, int value) {
     return C_OK;
 }
 
+/* Return the number of commands allowed (on) and denied (off) for the user 'u'
+ * in the subset of commands flagged with the specified category name.
+ * If the categoty name is not valid, C_ERR is returend, otherwise C_OK is
+ * returned and on and off are populated by reference. */
+int ACLCountCategoryBitsForUser(user *u, unsigned long *on, unsigned long *off,
+                                const char *category)
+{
+    uint64_t cflag = ACLGetCommandCategoryFlagByName(category);
+    if (!cflag) return C_ERR;
+
+    *on = *off = 0;
+    dictIterator *di = dictGetIterator(server.orig_commands);
+    dictEntry *de;
+    while ((de = dictNext(di)) != NULL) {
+        struct redisCommand *cmd = dictGetVal(de);
+        if (cmd->flags & cflag) {
+            if (ACLGetUserCommandBit(u,cmd->id))
+                (*on)++;
+            else
+                (*off)++;
+        }
+    }
+    dictReleaseIterator(di);
+    return C_OK;
+}
+
+/* This function returns an SDS string representing the specified user ACL
+ * rules related to command execution, in the same format you could set them
+ * back using ACL SETUSER. The function will return just the set of rules needed
+ * to recreate the user commands bitmap, without including other user flags such
+ * as on/off, passwords and so forth. The returned string always starts with
+ * the +@all or -@all rule, depending on the user bitmap, and is followed, if
+ * needed, by the other rules needed to narrow or extend what the user can do. */
+sds ACLDescribeUserCommandRules(user *u) {
+    sds rules = sdsempty();
+    int additive;   /* If true we start from -@all and add, otherwise if
+                       false we start from +@all and remove. */
+
+    /* This code is based on a trick: as we generate the rules, we apply
+     * them to a fake user, so that as we go we still know what are the
+     * bit differences we should try to address by emitting more rules. */
+    user fu = {0};
+    user *fakeuser = &fu;
+
+    /* Here we want to understand if we should start with +@all and remove
+     * the commands corresponding to the bits that are not set in the user
+     * commands bitmap, or the contrary. Note that semantically the two are
+     * different. For instance starting with +@all and subtracting, the user
+     * will be able to execute future commands, while -@all and adding will just
+     * allow the user the run the selected commands and/or categories.
+     * How do we test for that? We use the trick of a reserved command ID bit
+     * that is set only by +@all (and its alias "allcommands"). */
+    if (ACLUserCanExecuteFutureCommands(u)) {
+        additive = 0;
+        rules = sdscat(rules,"+@all ");
+    } else {
+        additive = 1;
+        rules = sdscat(rules,"-@all ");
+    }
+
+    /* Try to add or subtract each category one after the other. Often a
+     * single category will not perfectly match the set of commands into
+     * it, so at the end we do a final pass adding/removing the single commands
+     * needed to make the bitmap exactly match. */
+    for (int j = 0; ACLCommandCategories[j].flag != 0; j++) {
+        unsigned long on, off;
+        ACLCountCategoryBitsForUser(u,&on,&off,ACLCommandCategories[j].name);
+        if ((additive && on > off) || (!additive && off > on)) {
+            rules = sdscatlen(rules, additive ? "+@" : "-@", 2);
+            rules = sdscat(rules,ACLCommandCategories[j].name);
+            rules = sdscatlen(rules," ",1);
+        }
+    }
+
+    /* Fix the final ACLs with single commands differences. */
+
+    /* Trim the final useless space. */
+
+    /* This is technically not needed, but we want to verify that now the
+     * predicted bitmap is exactly the same as the user bitmap, and abort
+     * otherwise, because aborting is better than a security risk in this
+     * code path. */
+    serverAssert(memcmp(fakeuser->allowed_commands,
+                        u->allowed_commands,
+                        sizeof(u->allowed_commands)) == 0);
+    return rules;
+}
+
 /* Get a command from the original command table, that is not affected
  * by the command renaming operations: we base all the ACL work from that
  * table, so that ACLs are valid regardless of command renaming. */

From 40e619cff2682a7cbfe789bf84a59b0f94e2c9a4 Mon Sep 17 00:00:00 2001
From: antirez <antirez@gmail.com>
Date: Tue, 29 Jan 2019 18:41:11 +0100
Subject: [PATCH 03/32] ACL: ACLDescribeUserCommandRules(): add final commands.

---
 src/acl.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/src/acl.c b/src/acl.c
index ebfb5a066..70e3d0d63 100644
--- a/src/acl.c
+++ b/src/acl.c
@@ -303,8 +303,22 @@ sds ACLDescribeUserCommandRules(user *u) {
     }
 
     /* Fix the final ACLs with single commands differences. */
+    dictIterator *di = dictGetIterator(server.orig_commands);
+    dictEntry *de;
+    while ((de = dictNext(di)) != NULL) {
+        struct redisCommand *cmd = dictGetVal(de);
+        int userbit = ACLGetUserCommandBit(u,cmd->id);
+        int fakebit = ACLGetUserCommandBit(u,cmd->id);
+        if (userbit != fakebit) {
+            rules = sdscatlen(rules, userbit ? "+" : "-", 1);
+            rules = sdscat(rules,cmd->name);
+            rules = sdscatlen(rules," ",1);
+        }
+    }
+    dictReleaseIterator(di);
 
     /* Trim the final useless space. */
+    sdsrange(rules,0,-2);
 
     /* This is technically not needed, but we want to verify that now the
      * predicted bitmap is exactly the same as the user bitmap, and abort

From 3293d1c2e72d86645e761a2876af621c6950236c Mon Sep 17 00:00:00 2001
From: antirez <antirez@gmail.com>
Date: Tue, 29 Jan 2019 18:54:21 +0100
Subject: [PATCH 04/32] ACL: finish/fix ACLDescribeUserCommandRules() first
 version.

---
 src/acl.c | 29 +++++++++++++++++++++++------
 1 file changed, 23 insertions(+), 6 deletions(-)

diff --git a/src/acl.c b/src/acl.c
index 70e3d0d63..d90ef8df7 100644
--- a/src/acl.c
+++ b/src/acl.c
@@ -283,9 +283,11 @@ sds ACLDescribeUserCommandRules(user *u) {
     if (ACLUserCanExecuteFutureCommands(u)) {
         additive = 0;
         rules = sdscat(rules,"+@all ");
+        ACLSetUser(fakeuser,"+@all",-1);
     } else {
         additive = 1;
         rules = sdscat(rules,"-@all ");
+        ACLSetUser(fakeuser,"-@all",-1);
     }
 
     /* Try to add or subtract each category one after the other. Often a
@@ -296,9 +298,12 @@ sds ACLDescribeUserCommandRules(user *u) {
         unsigned long on, off;
         ACLCountCategoryBitsForUser(u,&on,&off,ACLCommandCategories[j].name);
         if ((additive && on > off) || (!additive && off > on)) {
-            rules = sdscatlen(rules, additive ? "+@" : "-@", 2);
-            rules = sdscat(rules,ACLCommandCategories[j].name);
+            sds op = sdsnewlen(additive ? "+@" : "-@", 2);
+            op = sdscat(op,ACLCommandCategories[j].name);
+            ACLSetUser(fakeuser,op,-1);
+            rules = sdscatsds(rules,op);
             rules = sdscatlen(rules," ",1);
+            sdsfree(op);
         }
     }
 
@@ -308,11 +313,12 @@ sds ACLDescribeUserCommandRules(user *u) {
     while ((de = dictNext(di)) != NULL) {
         struct redisCommand *cmd = dictGetVal(de);
         int userbit = ACLGetUserCommandBit(u,cmd->id);
-        int fakebit = ACLGetUserCommandBit(u,cmd->id);
+        int fakebit = ACLGetUserCommandBit(fakeuser,cmd->id);
         if (userbit != fakebit) {
             rules = sdscatlen(rules, userbit ? "+" : "-", 1);
             rules = sdscat(rules,cmd->name);
             rules = sdscatlen(rules," ",1);
+            ACLSetUserCommandBit(fakeuser,cmd->id,userbit);
         }
     }
     dictReleaseIterator(di);
@@ -324,9 +330,15 @@ sds ACLDescribeUserCommandRules(user *u) {
      * predicted bitmap is exactly the same as the user bitmap, and abort
      * otherwise, because aborting is better than a security risk in this
      * code path. */
-    serverAssert(memcmp(fakeuser->allowed_commands,
+    if (memcmp(fakeuser->allowed_commands,
                         u->allowed_commands,
-                        sizeof(u->allowed_commands)) == 0);
+                        sizeof(u->allowed_commands)) != 0)
+    {
+        serverLog(LL_WARNING,
+            "CRITICAL ERROR: User ACLs don't match final bitmap: '%s'",
+            rules);
+        serverPanic("No bitmap match in ACLDescribeUserCommandRules()");
+    }
     return rules;
 }
 
@@ -798,7 +810,7 @@ void aclCommand(client *c) {
             return;
         }
 
-        addReplyMapLen(c,2);
+        addReplyMapLen(c,3);
 
         /* Flags */
         addReplyBulkCString(c,"flags");
@@ -835,6 +847,11 @@ void aclCommand(client *c) {
             sds thispass = listNodeValue(ln);
             addReplyBulkCBuffer(c,thispass,sdslen(thispass));
         }
+
+        /* Commands */
+        addReplyBulkCString(c,"commands");
+        sds cmddescr = ACLDescribeUserCommandRules(u);
+        addReplyBulkSds(c,cmddescr);
     } else if (!strcasecmp(sub,"help")) {
         const char *help[] = {
 "LIST                              -- List all the registered users.",

From 20106cfa04bb909fe46a07beed7efdeb0fc15be3 Mon Sep 17 00:00:00 2001
From: antirez <antirez@gmail.com>
Date: Wed, 30 Jan 2019 08:09:05 +0100
Subject: [PATCH 05/32] ACL: clear the subcommands array when setting category
 bits.

---
 src/acl.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/src/acl.c b/src/acl.c
index d90ef8df7..b7426be9b 100644
--- a/src/acl.c
+++ b/src/acl.c
@@ -67,6 +67,8 @@ struct ACLCategoryItem {
     {"",0} /* Terminator. */
 };
 
+void ACLResetSubcommandsForCommand(user *u, unsigned long id);
+
 /* =============================================================================
  * Helper functions for the rest of the ACL implementation
  * ==========================================================================*/
@@ -222,7 +224,10 @@ int ACLSetUserCommandBitsForCategory(user *u, const char *category, int value) {
     dictEntry *de;
     while ((de = dictNext(di)) != NULL) {
         struct redisCommand *cmd = dictGetVal(de);
-        if (cmd->flags & cflag) ACLSetUserCommandBit(u,cmd->id,value);
+        if (cmd->flags & cflag) {
+            ACLSetUserCommandBit(u,cmd->id,value);
+            ACLResetSubcommandsForCommand(u,cmd->id);
+        }
     }
     dictReleaseIterator(di);
     return C_OK;

From 1f645638018e7c8543740e47e4bcf19644d7f644 Mon Sep 17 00:00:00 2001
From: antirez <antirez@gmail.com>
Date: Wed, 30 Jan 2019 08:17:33 +0100
Subject: [PATCH 06/32] ACL: ACLDescribeUserCommandRules(): emit subcommands.

---
 src/acl.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/src/acl.c b/src/acl.c
index b7426be9b..674d39213 100644
--- a/src/acl.c
+++ b/src/acl.c
@@ -325,6 +325,19 @@ sds ACLDescribeUserCommandRules(user *u) {
             rules = sdscatlen(rules," ",1);
             ACLSetUserCommandBit(fakeuser,cmd->id,userbit);
         }
+
+        /* Emit the subcommands if there are any. */
+        if (userbit == 0 && u->allowed_subcommands &&
+            u->allowed_subcommands[cmd->id])
+        {
+            for (int j = 0; u->allowed_subcommands[cmd->id][j]; j++) {
+                rules = sdscatlen(rules,"+",1);
+                rules = sdscat(rules,cmd->name);
+                rules = sdscatlen(rules,"|",1);
+                rules = sdscatsds(rules,u->allowed_subcommands[cmd->id][j]);
+                rules = sdscatlen(rules," ",1);
+            }
+        }
     }
     dictReleaseIterator(di);
 

From ef252265d9adbe680cd460df6d951ac4cbb5056d Mon Sep 17 00:00:00 2001
From: antirez <antirez@gmail.com>
Date: Wed, 30 Jan 2019 08:20:31 +0100
Subject: [PATCH 07/32] ACL: remove leak in ACLSetUser().

---
 src/acl.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/src/acl.c b/src/acl.c
index 674d39213..cc5f17cbf 100644
--- a/src/acl.c
+++ b/src/acl.c
@@ -554,6 +554,7 @@ int ACLSetUser(user *u, const char *op, ssize_t oplen) {
             /* Check if the command exists. We can't check the
              * subcommand to see if it is valid. */
             if (ACLLookupCommand(copy) == NULL) {
+                zfree(copy);
                 errno = ENOENT;
                 return C_ERR;
             }

From 0fa16cbd4be486b2eeb9030c813260ed02216ffd Mon Sep 17 00:00:00 2001
From: antirez <antirez@gmail.com>
Date: Wed, 30 Jan 2019 08:25:08 +0100
Subject: [PATCH 08/32] ACL: return error when adding subcommands of fully
 added commands.

It's almost certainly an error from the user side.
---
 src/acl.c | 18 +++++++++++++++++-
 1 file changed, 17 insertions(+), 1 deletion(-)

diff --git a/src/acl.c b/src/acl.c
index cc5f17cbf..5f4185170 100644
--- a/src/acl.c
+++ b/src/acl.c
@@ -485,7 +485,10 @@ void ACLAddAllowedSubcommand(user *u, unsigned long id, const char *sub) {
  *
  * EINVAL: The specified opcode is not understood.
  * ENOENT: The command name or command category provided with + or - is not
- * known. */
+ *         known.
+ * EBUSY:  The subcommand you want to add is about a command that is currently
+ *         fully added.
+ */
 int ACLSetUser(user *u, const char *op, ssize_t oplen) {
     if (oplen == -1) oplen = strlen(op);
     if (!strcasecmp(op,"on")) {
@@ -568,6 +571,15 @@ int ACLSetUser(user *u, const char *op, ssize_t oplen) {
                 return C_ERR;
             }
 
+            /* The command should not be set right now in the command
+             * bitmap, because adding a subcommand of a fully added
+             * command is probably an error on the user side. */
+            if (ACLGetUserCommandBit(u,id) == 1) {
+                zfree(copy);
+                errno = EBUSY;
+                return C_ERR;
+            }
+
             /* Add the subcommand to the list of valid ones. */
             ACLAddAllowedSubcommand(u,id,sub);
 
@@ -809,6 +821,10 @@ void aclCommand(client *c) {
                     errmsg = "unknown command or category name in ACL";
                 else if (errno == EINVAL)
                     errmsg = "syntax error";
+                else if (errno == EBUSY)
+                    errmsg = "adding a subcommand of a command already fully "
+                             "added is not allowed. Remove the command to start. "
+                             "Example: -DEBUG +DEBUG|DIGEST";
                 addReplyErrorFormat(c,
                     "Error in ACL SETUSER modifier '%s': %s",
                     (char*)c->argv[j]->ptr, errmsg);

From cd60bcc4273e669a6961680b98677cdfc12b8fd4 Mon Sep 17 00:00:00 2001
From: antirez <antirez@gmail.com>
Date: Wed, 30 Jan 2019 11:50:30 +0100
Subject: [PATCH 09/32] ACL: free memory leak when freeing subcommands array.

---
 src/acl.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/src/acl.c b/src/acl.c
index 5f4185170..da7c0bd49 100644
--- a/src/acl.c
+++ b/src/acl.c
@@ -385,8 +385,13 @@ void ACLResetSubcommandsForCommand(user *u, unsigned long id) {
  * for the user. */
 void ACLResetSubcommands(user *u) {
     if (u->allowed_subcommands == NULL) return;
-    for (int j = 0; j < USER_COMMAND_BITS_COUNT; j++)
-        if (u->allowed_subcommands[j]) zfree(u->allowed_subcommands[j]);
+    for (int j = 0; j < USER_COMMAND_BITS_COUNT; j++) {
+        if (u->allowed_subcommands[j]) {
+            for (int i = 0; u->allowed_subcommands[j][i]; i++)
+                sdsfree(u->allowed_subcommands[j][i]);
+            zfree(u->allowed_subcommands[j]);
+        }
+    }
     zfree(u->allowed_subcommands);
     u->allowed_subcommands = NULL;
 }

From 7deb1e90d9f609e517b6315ba2ab00a16367afec Mon Sep 17 00:00:00 2001
From: antirez <antirez@gmail.com>
Date: Wed, 30 Jan 2019 12:01:23 +0100
Subject: [PATCH 10/32] Acl: Test: check command rules synthesis.

---
 tests/unit/acl.tcl | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)

diff --git a/tests/unit/acl.tcl b/tests/unit/acl.tcl
index 4469f6535..82c75f82d 100644
--- a/tests/unit/acl.tcl
+++ b/tests/unit/acl.tcl
@@ -86,4 +86,26 @@ start_server {tags {"acl"}} {
         catch {r CLIENT KILL type master} e
         set e
     } {*NOPERM*}
+
+    # Note that the order of the generated ACL rules is not stable in Redis
+    # so we need to match the different parts and not as a whole string.
+    test {ACL GETUSER is able to translate back command permissions} {
+        # Subtractive
+        r ACL setuser newuser reset +@all ~* -@string +incr -debug +debug|digest
+        set cmdstr [dict get [r ACL getuser newuser] commands]
+        assert_match {*+@all*} $cmdstr
+        assert_match {*-@string*} $cmdstr
+        assert_match {*+incr*} $cmdstr
+        assert_match {*-debug +debug|digest**} $cmdstr
+
+        # Additive
+        r ACL setuser newuser reset +@string -incr +acl +debug|digest +debug|segfault
+        set cmdstr [dict get [r ACL getuser newuser] commands]
+        assert_match {*-@all*} $cmdstr
+        assert_match {*+@string*} $cmdstr
+        assert_match {*-incr*} $cmdstr
+        assert_match {*+debug|digest*} $cmdstr
+        assert_match {*+debug|segfault*} $cmdstr
+        assert_match {*+acl*} $cmdstr
+    }
 }

From e1d14fb943645043f69219d46e80e5743421bb3e Mon Sep 17 00:00:00 2001
From: antirez <antirez@gmail.com>
Date: Wed, 30 Jan 2019 15:52:36 +0100
Subject: [PATCH 11/32] ACL: implement keys field in ACL GETUSER.

---
 src/acl.c | 18 +++++++++++++++++-
 1 file changed, 17 insertions(+), 1 deletion(-)

diff --git a/src/acl.c b/src/acl.c
index da7c0bd49..cb054a7e5 100644
--- a/src/acl.c
+++ b/src/acl.c
@@ -850,7 +850,7 @@ void aclCommand(client *c) {
             return;
         }
 
-        addReplyMapLen(c,3);
+        addReplyMapLen(c,4);
 
         /* Flags */
         addReplyBulkCString(c,"flags");
@@ -892,6 +892,22 @@ void aclCommand(client *c) {
         addReplyBulkCString(c,"commands");
         sds cmddescr = ACLDescribeUserCommandRules(u);
         addReplyBulkSds(c,cmddescr);
+
+        /* Key patterns */
+        addReplyBulkCString(c,"keys");
+        if (u->flags & USER_FLAG_ALLKEYS) {
+            addReplyArrayLen(c,1);
+            addReplyBulkCBuffer(c,"*",1);
+        } else {
+            addReplyArrayLen(c,listLength(u->patterns));
+            listIter li;
+            listNode *ln;
+            listRewind(u->patterns,&li);
+            while((ln = listNext(&li))) {
+                sds thispat = listNodeValue(ln);
+                addReplyBulkCBuffer(c,thispat,sdslen(thispat));
+            }
+        }
     } else if (!strcasecmp(sub,"help")) {
         const char *help[] = {
 "LIST                              -- List all the registered users.",

From 88af62aca2c078bd50481e8914f1377c2573560a Mon Sep 17 00:00:00 2001
From: antirez <antirez@gmail.com>
Date: Wed, 30 Jan 2019 15:59:45 +0100
Subject: [PATCH 12/32] ACL: don't allow patterns after the * pattern.

---
 src/acl.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/src/acl.c b/src/acl.c
index cb054a7e5..c4f4984a9 100644
--- a/src/acl.c
+++ b/src/acl.c
@@ -493,6 +493,8 @@ void ACLAddAllowedSubcommand(user *u, unsigned long id, const char *sub) {
  *         known.
  * EBUSY:  The subcommand you want to add is about a command that is currently
  *         fully added.
+ * EEXIST: You are adding a key pattern after "*" was already added. This is
+ *         almost surely an error on the user side.
  */
 int ACLSetUser(user *u, const char *op, ssize_t oplen) {
     if (oplen == -1) oplen = strlen(op);
@@ -538,6 +540,10 @@ int ACLSetUser(user *u, const char *op, ssize_t oplen) {
         if (ln) listDelNode(u->passwords,ln);
         sdsfree(delpass);
     } else if (op[0] == '~') {
+        if (u->flags & USER_FLAG_ALLKEYS) {
+            errno = EEXIST;
+            return C_ERR;
+        }
         sds newpat = sdsnewlen(op+1,oplen-1);
         listNode *ln = listSearchKey(u->patterns,newpat);
         /* Avoid re-adding the same pattern multiple times. */
@@ -830,6 +836,11 @@ void aclCommand(client *c) {
                     errmsg = "adding a subcommand of a command already fully "
                              "added is not allowed. Remove the command to start. "
                              "Example: -DEBUG +DEBUG|DIGEST";
+                else if (errno == EEXIST)
+                    errmsg = "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";
                 addReplyErrorFormat(c,
                     "Error in ACL SETUSER modifier '%s': %s",
                     (char*)c->argv[j]->ptr, errmsg);

From 2458e62da4eff0bc80e832b2aaa0f4491a3d3651 Mon Sep 17 00:00:00 2001
From: antirez <antirez@gmail.com>
Date: Wed, 30 Jan 2019 16:02:25 +0100
Subject: [PATCH 13/32] ACL: add function to return ACLSetUser() string errors.

---
 src/acl.c | 35 +++++++++++++++++++++--------------
 1 file changed, 21 insertions(+), 14 deletions(-)

diff --git a/src/acl.c b/src/acl.c
index c4f4984a9..f112d8708 100644
--- a/src/acl.c
+++ b/src/acl.c
@@ -625,6 +625,26 @@ int ACLSetUser(user *u, const char *op, ssize_t oplen) {
     return C_OK;
 }
 
+/* Return a description of the error that occurred in ACLSetUser() according to
+ * the errno value set by the function on error. */
+char *ACLSetUserStringError(void) {
+    char *errmsg = "Wrong format";
+    if (errno == ENOENT)
+        errmsg = "Unknown command or category name in ACL";
+    else if (errno == EINVAL)
+        errmsg = "Syntax error";
+    else if (errno == EBUSY)
+        errmsg = "Adding a subcommand of a command already fully "
+                 "added is not allowed. Remove the command to start. "
+                 "Example: -DEBUG +DEBUG|DIGEST";
+    else if (errno == EEXIST)
+        errmsg = "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";
+    return errmsg;
+}
+
 /* Return the first password of the default user or NULL.
  * This function is needed for backward compatibility with the old
  * directive "requirepass" when Redis supported a single global
@@ -827,20 +847,7 @@ void aclCommand(client *c) {
         serverAssert(u != NULL);
         for (int j = 3; j < c->argc; j++) {
             if (ACLSetUser(u,c->argv[j]->ptr,sdslen(c->argv[j]->ptr)) != C_OK) {
-                char *errmsg = "wrong format";
-                if (errno == ENOENT)
-                    errmsg = "unknown command or category name in ACL";
-                else if (errno == EINVAL)
-                    errmsg = "syntax error";
-                else if (errno == EBUSY)
-                    errmsg = "adding a subcommand of a command already fully "
-                             "added is not allowed. Remove the command to start. "
-                             "Example: -DEBUG +DEBUG|DIGEST";
-                else if (errno == EEXIST)
-                    errmsg = "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";
+                char *errmsg = ACLSetUserStringError();
                 addReplyErrorFormat(c,
                     "Error in ACL SETUSER modifier '%s': %s",
                     (char*)c->argv[j]->ptr, errmsg);

From 110c8caf8a85e476c33444c17aefb5d072f4d18c Mon Sep 17 00:00:00 2001
From: antirez <antirez@gmail.com>
Date: Thu, 31 Jan 2019 16:49:22 +0100
Subject: [PATCH 14/32] ACL: flags refactoring, function to describe user.

---
 src/acl.c    | 91 ++++++++++++++++++++++++++++++++++++++++------------
 src/server.h |  7 ++--
 2 files changed, 74 insertions(+), 24 deletions(-)

diff --git a/src/acl.c b/src/acl.c
index f112d8708..dce164f21 100644
--- a/src/acl.c
+++ b/src/acl.c
@@ -64,7 +64,19 @@ struct ACLCategoryItem {
     {"connection", CMD_CATEGORY_CONNECTION},
     {"transaction", CMD_CATEGORY_TRANSACTION},
     {"scripting", CMD_CATEGORY_SCRIPTING},
-    {"",0} /* Terminator. */
+    {NULL,0} /* Terminator. */
+};
+
+struct ACLUserFlag {
+    const char *name;
+    uint64_t flag;
+} ACLUserFlags[] = {
+    {"on", USER_FLAG_ENABLED},
+    {"off", USER_FLAG_DISABLED},
+    {"allkeys", USER_FLAG_ALLKEYS},
+    {"allcommands", USER_FLAG_ALLCOMMANDS},
+    {"nopass", USER_FLAG_NOPASS},
+    {NULL,0} /* Terminator. */
 };
 
 void ACLResetSubcommandsForCommand(user *u, unsigned long id);
@@ -150,7 +162,7 @@ user *ACLCreateUser(const char *name, size_t namelen) {
     if (raxFind(Users,(unsigned char*)name,namelen) != raxNotFound) return NULL;
     user *u = zmalloc(sizeof(*u));
     u->name = sdsnewlen(name,namelen);
-    u->flags = 0;
+    u->flags = USER_FLAG_DISABLED;
     u->allowed_subcommands = NULL;
     u->passwords = listCreate();
     u->patterns = listCreate();
@@ -360,6 +372,54 @@ sds ACLDescribeUserCommandRules(user *u) {
     return rules;
 }
 
+/* This is similar to ACLDescribeUserCommandRules(), however instead of
+ * describing just the user command rules, everything is described: user
+ * flags, keys, passwords and finally the command rules obtained via
+ * the ACLDescribeUserCommandRules() function. This is the function we call
+ * when we want to rewrite the configuration files describing ACLs and
+ * in order to show users with ACL LIST. */
+sds ACLDescribeUser(user *u) {
+    sds res = sdsempty();
+
+    /* Flags. */
+    for (int j = 0; ACLUserFlags[j].flag; j++) {
+        if (u->flags & ACLUserFlags[j].flag) {
+            res = sdscat(res,ACLUserFlags[j].name);
+            res = sdscatlen(res," ",1);
+        }
+    }
+
+    /* Passwords. */
+    listIter li;
+    listNode *ln;
+    listRewind(u->passwords,&li);
+    while((ln = listNext(&li))) {
+        sds thispass = listNodeValue(ln);
+        res = sdscatlen(res,">",1);
+        res = sdscatsds(res,thispass);
+        res = sdscatlen(res," ",1);
+    }
+
+    /* Key patterns. */
+    if (u->flags & USER_FLAG_ALLKEYS) {
+        res = sdscatlen(res,"~* ",3);
+    } else {
+        listRewind(u->patterns,&li);
+        while((ln = listNext(&li))) {
+            sds thispat = listNodeValue(ln);
+            res = sdscatlen(res,"~",1);
+            res = sdscatsds(res,thispat);
+            res = sdscatlen(res," ",1);
+        }
+    }
+
+    /* Command rules. */
+    sds rules = ACLDescribeUserCommandRules(u);
+    res = sdscatsds(res,rules);
+    sdsfree(rules);
+    return res;
+}
+
 /* Get a command from the original command table, that is not affected
  * by the command renaming operations: we base all the ACL work from that
  * table, so that ACLs are valid regardless of command renaming. */
@@ -500,7 +560,9 @@ int ACLSetUser(user *u, const char *op, ssize_t oplen) {
     if (oplen == -1) oplen = strlen(op);
     if (!strcasecmp(op,"on")) {
         u->flags |= USER_FLAG_ENABLED;
+        u->flags &= ~USER_FLAG_DISABLED;
     } else if (!strcasecmp(op,"off")) {
+        u->flags |= USER_FLAG_DISABLED;
         u->flags &= ~USER_FLAG_ENABLED;
     } else if (!strcasecmp(op,"allkeys") ||
                !strcasecmp(op,"~*"))
@@ -679,7 +741,7 @@ int ACLCheckUserCredentials(robj *username, robj *password) {
     }
 
     /* Disabled users can't login. */
-    if ((u->flags & USER_FLAG_ENABLED) == 0) {
+    if (u->flags & USER_FLAG_DISABLED) {
         errno = EINVAL;
         return C_ERR;
     }
@@ -874,24 +936,11 @@ void aclCommand(client *c) {
         addReplyBulkCString(c,"flags");
         void *deflen = addReplyDeferredLen(c);
         int numflags = 0;
-        if (u->flags & USER_FLAG_ENABLED) {
-            addReplyBulkCString(c,"on");
-            numflags++;
-        } else {
-            addReplyBulkCString(c,"off");
-            numflags++;
-        }
-        if (u->flags & USER_FLAG_ALLKEYS) {
-            addReplyBulkCString(c,"allkeys");
-            numflags++;
-        }
-        if (u->flags & USER_FLAG_ALLCOMMANDS) {
-            addReplyBulkCString(c,"allcommands");
-            numflags++;
-        }
-        if (u->flags & USER_FLAG_NOPASS) {
-            addReplyBulkCString(c,"nopass");
-            numflags++;
+        for (int j = 0; ACLUserFlags[j].flag; j++) {
+            if (u->flags & ACLUserFlags[j].flag) {
+                addReplyBulkCString(c,ACLUserFlags[j].name);
+                numflags++;
+            }
         }
         setDeferredSetLen(c,deflen,numflags);
 
diff --git a/src/server.h b/src/server.h
index 1e38b8ae6..13d29308f 100644
--- a/src/server.h
+++ b/src/server.h
@@ -740,9 +740,10 @@ typedef struct readyList {
                                            command ID we can set in the user
                                            is USER_COMMAND_BITS_COUNT-1. */
 #define USER_FLAG_ENABLED (1<<0)        /* The user is active. */
-#define USER_FLAG_ALLKEYS (1<<1)        /* The user can mention any key. */
-#define USER_FLAG_ALLCOMMANDS (1<<2)    /* The user can run all commands. */
-#define USER_FLAG_NOPASS      (1<<3)    /* The user requires no password, any
+#define USER_FLAG_DISABLED (1<<1)       /* The user is disabled. */
+#define USER_FLAG_ALLKEYS (1<<2)        /* The user can mention any key. */
+#define USER_FLAG_ALLCOMMANDS (1<<3)    /* The user can run all commands. */
+#define USER_FLAG_NOPASS      (1<<4)    /* The user requires no password, any
                                            provided password will work. For the
                                            default user, this also means that
                                            no AUTH is needed, and every

From 1cb9b66aaae5ce9a2464fd67cc1ac9c584ebe27b Mon Sep 17 00:00:00 2001
From: antirez <antirez@gmail.com>
Date: Thu, 31 Jan 2019 17:01:28 +0100
Subject: [PATCH 15/32] ACL: implement LIST and USERS subcommands.

---
 src/acl.c | 41 ++++++++++++++++++++++++++++++++---------
 1 file changed, 32 insertions(+), 9 deletions(-)

diff --git a/src/acl.c b/src/acl.c
index dce164f21..74efbcc33 100644
--- a/src/acl.c
+++ b/src/acl.c
@@ -917,12 +917,6 @@ void aclCommand(client *c) {
             }
         }
         addReply(c,shared.ok);
-    } else if (!strcasecmp(sub,"whoami")) {
-        if (c->user != NULL) {
-            addReplyBulkCBuffer(c,c->user->name,sdslen(c->user->name));
-        } else {
-            addReplyNull(c);
-        }
     } else if (!strcasecmp(sub,"getuser") && c->argc == 3) {
         user *u = ACLGetUserByName(c->argv[2]->ptr,sdslen(c->argv[2]->ptr));
         if (u == NULL) {
@@ -975,13 +969,42 @@ void aclCommand(client *c) {
                 addReplyBulkCBuffer(c,thispat,sdslen(thispat));
             }
         }
+    } else if (!strcasecmp(sub,"list") || !strcasecmp(sub,"users")) {
+        int justnames = !strcasecmp(sub,"users");
+        addReplyArrayLen(c,raxSize(Users));
+        raxIterator ri;
+        raxStart(&ri,Users);
+        raxSeek(&ri,"^",NULL,0);
+        while(raxNext(&ri)) {
+            user *u = ri.data;
+            if (justnames) {
+                addReplyBulkCBuffer(c,u->name,sdslen(u->name));
+            } else {
+                /* Return information in the configuration file format. */
+                sds config = sdsnew("user ");
+                config = sdscatsds(config,u->name);
+                config = sdscatlen(config," ",1);
+                sds descr = ACLDescribeUser(u);
+                config = sdscatsds(config,descr);
+                sdsfree(descr);
+                addReplyBulkSds(c,config);
+            }
+        }
+        raxStop(&ri);
+    } else if (!strcasecmp(sub,"whoami")) {
+        if (c->user != NULL) {
+            addReplyBulkCBuffer(c,c->user->name,sdslen(c->user->name));
+        } else {
+            addReplyNull(c);
+        }
     } else if (!strcasecmp(sub,"help")) {
         const char *help[] = {
-"LIST                              -- List all the registered users.",
+"LIST                              -- Show user details in config file format.",
+"USERS                             -- List all the registered usernames.",
 "SETUSER <username> [attribs ...]  -- Create or modify a user.",
-"DELUSER <username>                -- Delete a user.",
 "GETUSER <username>                -- Get the user details.",
-"WHOAMI                            -- Return the current username.",
+"DELUSER <username>                -- Delete a user.",
+"WHOAMI                            -- Return the current connection username.",
 NULL
         };
         addReplyHelp(c,help);

From 3ec2dc3a6d5763adb8cf319327b960814049b83b Mon Sep 17 00:00:00 2001
From: antirez <antirez@gmail.com>
Date: Thu, 31 Jan 2019 17:04:42 +0100
Subject: [PATCH 16/32] ACL: don't emit useless flags in ACLDescribeUser().

---
 src/acl.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/src/acl.c b/src/acl.c
index 74efbcc33..cf46ef812 100644
--- a/src/acl.c
+++ b/src/acl.c
@@ -383,6 +383,10 @@ sds ACLDescribeUser(user *u) {
 
     /* Flags. */
     for (int j = 0; ACLUserFlags[j].flag; j++) {
+        /* Skip the allcommands and allkeys flags because they'll be emitted
+         * later as ~* and +@all. */
+        if (ACLUserFlags[j].flag == USER_FLAG_ALLKEYS ||
+            ACLUserFlags[j].flag == USER_FLAG_ALLCOMMANDS) continue;
         if (u->flags & ACLUserFlags[j].flag) {
             res = sdscat(res,ACLUserFlags[j].name);
             res = sdscatlen(res," ",1);

From b807d63f928d710aa1139cb8180ad668b9d431ad Mon Sep 17 00:00:00 2001
From: antirez <antirez@gmail.com>
Date: Thu, 31 Jan 2019 18:32:49 +0100
Subject: [PATCH 17/32] ACL: check arity of LIST / USERS subcommand.

---
 src/acl.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/src/acl.c b/src/acl.c
index cf46ef812..58ffece34 100644
--- a/src/acl.c
+++ b/src/acl.c
@@ -973,7 +973,9 @@ void aclCommand(client *c) {
                 addReplyBulkCBuffer(c,thispat,sdslen(thispat));
             }
         }
-    } else if (!strcasecmp(sub,"list") || !strcasecmp(sub,"users")) {
+    } else if ((!strcasecmp(sub,"list") || !strcasecmp(sub,"users")) &&
+               c->argc == 2)
+    {
         int justnames = !strcasecmp(sub,"users");
         addReplyArrayLen(c,raxSize(Users));
         raxIterator ri;

From 91f55d3206ec946853d78716dc1c445a2d923a8f Mon Sep 17 00:00:00 2001
From: antirez <antirez@gmail.com>
Date: Thu, 31 Jan 2019 18:33:14 +0100
Subject: [PATCH 18/32] ACL: implement DELUSER.

---
 src/acl.c | 49 +++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 49 insertions(+)

diff --git a/src/acl.c b/src/acl.c
index 58ffece34..1df511329 100644
--- a/src/acl.c
+++ b/src/acl.c
@@ -80,6 +80,7 @@ struct ACLUserFlag {
 };
 
 void ACLResetSubcommandsForCommand(user *u, unsigned long id);
+void ACLResetSubcommands(user *u);
 
 /* =============================================================================
  * Helper functions for the rest of the ACL implementation
@@ -175,6 +176,16 @@ user *ACLCreateUser(const char *name, size_t namelen) {
     return u;
 }
 
+/* Release the memory used by the user structure. Note that this function
+ * will not remove the user from the Users global radix tree. */
+void ACLFreeUser(user *u) {
+    sdsfree(u->name);
+    listRelease(u->passwords);
+    listRelease(u->patterns);
+    ACLResetSubcommands(u);
+    zfree(u);
+}
+
 /* Given a command ID, this function set by reference 'word' and 'bit'
  * so that user->allowed_commands[word] will address the right word
  * where the corresponding bit for the provided ID is stored, and
@@ -921,6 +932,44 @@ void aclCommand(client *c) {
             }
         }
         addReply(c,shared.ok);
+    } else if (!strcasecmp(sub,"deluser") && c->argc >= 3) {
+        int deleted = 0;
+        for (int j = 2; j < c->argc; j++) {
+            sds username = c->argv[j]->ptr;
+            if (!strcmp(username,"default")) {
+                addReplyError(c,"The 'default' user cannot be removed");
+                return;
+            }
+            user *u;
+            if (raxRemove(Users,(unsigned char*)username,
+                          sdslen(username),
+                          (void**)&u))
+            {
+                /* When a user is deleted we need to cycle the active
+                 * connections in order to kill all the pending ones that
+                 * are authenticated with such user. */
+                ACLFreeUser(u);
+                listIter li;
+                listNode *ln;
+                listRewind(server.clients,&li);
+                while ((ln = listNext(&li)) != NULL) {
+                    client *c = listNodeValue(ln);
+                    if (c->user == u) {
+                        /* We'll free the conenction asynchronously, so
+                         * in theory to set a different user is not needed.
+                         * However if there are bugs in Redis, soon or later
+                         * this may result in some security hole: it's much
+                         * more defensive to set the default user and put
+                         * it in non authenticated mode. */
+                        c->user = DefaultUser;
+                        c->authenticated = 0;
+                        freeClientAsync(c);
+                    }
+                }
+                deleted++;
+            }
+        }
+        addReplyLongLong(c,deleted);
     } else if (!strcasecmp(sub,"getuser") && c->argc == 3) {
         user *u = ACLGetUserByName(c->argv[2]->ptr,sdslen(c->argv[2]->ptr));
         if (u == NULL) {

From 15385d4d68f56ea96407f94939933ab54b2c750d Mon Sep 17 00:00:00 2001
From: antirez <antirez@gmail.com>
Date: Fri, 1 Feb 2019 08:17:24 +0100
Subject: [PATCH 19/32] ACL: assign ACL command ID to modules commands.

---
 src/module.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/src/module.c b/src/module.c
index 8954fcdf0..9c24bfe2b 100644
--- a/src/module.c
+++ b/src/module.c
@@ -684,6 +684,7 @@ int RM_CreateCommand(RedisModuleCtx *ctx, const char *name, RedisModuleCmdFunc c
     cp->rediscmd->calls = 0;
     dictAdd(server.commands,sdsdup(cmdname),cp->rediscmd);
     dictAdd(server.orig_commands,sdsdup(cmdname),cp->rediscmd);
+    cp->rediscmd->id = ACLGetCommandID(cmdname); /* ID used for ACL. */
     return REDISMODULE_OK;
 }
 

From 38c60302784123f1ec2402346779f0288d141067 Mon Sep 17 00:00:00 2001
From: antirez <antirez@gmail.com>
Date: Fri, 1 Feb 2019 11:37:20 +0100
Subject: [PATCH 20/32] ACL: set modules help clients to the root user.

It does not make much sense to limit what modules can do: the admin
should instead limit what module commnads an user may call. So
RedisModule_Call() and other module operations should be able to execute
everything they want: the limitation should be posed by the API exported
by the module itself.
---
 src/module.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/src/module.c b/src/module.c
index 9c24bfe2b..81982ba76 100644
--- a/src/module.c
+++ b/src/module.c
@@ -2697,6 +2697,7 @@ RedisModuleCallReply *RM_Call(RedisModuleCtx *ctx, const char *cmdname, const ch
     /* Create the client and dispatch the command. */
     va_start(ap, fmt);
     c = createClient(-1);
+    c->user = NULL; /* Root user. */
     argv = moduleCreateArgvFromUserFormat(cmdname,fmt,&argc,&flags,ap);
     replicate = flags & REDISMODULE_ARGV_REPLICATE;
     va_end(ap);
@@ -4660,6 +4661,7 @@ void moduleInitModulesSystem(void) {
     moduleKeyspaceSubscribers = listCreate();
     moduleFreeContextReusedClient = createClient(-1);
     moduleFreeContextReusedClient->flags |= CLIENT_MODULE;
+    moduleFreeContextReusedClient->user = NULL; /* root user. */
 
     moduleRegisterCoreAPI();
     if (pipe(server.module_blocked_pipe) == -1) {

From 816f2fce080e74c325c0118530e7db9a0741d02e Mon Sep 17 00:00:00 2001
From: antirez <antirez@gmail.com>
Date: Fri, 1 Feb 2019 12:20:09 +0100
Subject: [PATCH 21/32] ACL: skeleton and first ideas for postponed user
 loading.

---
 src/acl.c | 39 +++++++++++++++++++++++++++++++++++----
 1 file changed, 35 insertions(+), 4 deletions(-)

diff --git a/src/acl.c b/src/acl.c
index 1df511329..5ad891cbd 100644
--- a/src/acl.c
+++ b/src/acl.c
@@ -34,10 +34,19 @@
  * ==========================================================================*/
 
 rax *Users; /* Table mapping usernames to user structures. */
-user *DefaultUser;   /* Global reference to the default user.
-                        Every new connection is associated to it, if no
-                        AUTH or HELLO is used to authenticate with a
-                        different user. */
+
+user *DefaultUser;  /* Global reference to the default user.
+                       Every new connection is associated to it, if no
+                       AUTH or HELLO is used to authenticate with a
+                       different user. */
+
+list *UsersToLoad;  /* This is a list of users found in the configuration file
+                       that we'll need to load in the final stage of Redis
+                       initialization, after all the modules are already
+                       loaded. Every list element is a NULL terminated
+                       array of SDS pointers: the first is the user name,
+                       all the remaining pointers are ACL rules in the same
+                       format as ACLSetUser(). */
 
 struct ACLCategoryItem {
     const char *name;
@@ -735,6 +744,7 @@ sds ACLDefaultUserFirstPassword(void) {
 /* Initialization of the ACL subsystem. */
 void ACLInit(void) {
     Users = raxNew();
+    UsersToLoad = listCreate();
     DefaultUser = ACLCreateUser("default",7);
     ACLSetUser(DefaultUser,"+@all",-1);
     ACLSetUser(DefaultUser,"~*",-1);
@@ -904,6 +914,27 @@ int ACLCheckCommandPerm(client *c) {
     return ACL_OK;
 }
 
+/* =============================================================================
+ * ACL loading / saving functions
+ * ==========================================================================*/
+
+/* Given an argument vector describing a user in the form:
+ *
+ *      user <username> ... ACL rules and flags ...
+ *
+ * this function validates, and if the syntax is valid, appends
+ * the user definition to a list for later loading.
+ *
+ * The rules are tested for validity and if there obvious syntax errors
+ * the function returns C_ERR and does nothing, otherwise C_OK is returned
+ * and the user is appended to the list.
+ *
+ * Note that this function cannot stop in case of commands that are not found
+ * and, in that case, the error will be emitted later, because certain
+ * commands may be defined later once modules are loaded. */
+int ACLAppendUserForLoading(sds *argv, int argc) {
+}
+
 /* =============================================================================
  * ACL related commands
  * ==========================================================================*/

From a23e926924d646ea8af4e5270fe0379bb5136e68 Mon Sep 17 00:00:00 2001
From: antirez <antirez@gmail.com>
Date: Fri, 1 Feb 2019 13:02:41 +0100
Subject: [PATCH 22/32] ACL: implement ACLAppendUserForLoading().

---
 src/acl.c | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/src/acl.c b/src/acl.c
index 5ad891cbd..105bcfc95 100644
--- a/src/acl.c
+++ b/src/acl.c
@@ -933,6 +933,24 @@ int ACLCheckCommandPerm(client *c) {
  * and, in that case, the error will be emitted later, because certain
  * commands may be defined later once modules are loaded. */
 int ACLAppendUserForLoading(sds *argv, int argc) {
+    if (argc < 2 || strcasecmp(argv[0],"user")) return C_ERR;
+
+    /* Try to apply the user rules in a fake user to see if they
+     * are actually valid. */
+    user fu = {0};
+    user *fakeuser = &fu;
+    for (int j = 2; j < argc; j++) {
+        if (ACLSetUser(fakeuser,argv[j],sdslen(argv[j])) == C_ERR) {
+            if (errno != ENOENT) return C_ERR;
+        }
+    }
+
+    /* Rules look valid, let's append the user to the list. */
+    sds *copy = zmalloc(sizeof(sds)*argc);
+    for (int j = 1; j < argc; j++) copy[j-1] = sdsdup(argv[j]);
+    copy[argc-1] = NULL;
+    listAddNodeTail(UsersToLoad,copy);
+    return C_OK;
 }
 
 /* =============================================================================

From 132753822111897e4452c1a720fcf49d1d5bb820 Mon Sep 17 00:00:00 2001
From: antirez <antirez@gmail.com>
Date: Mon, 4 Feb 2019 12:55:26 +0100
Subject: [PATCH 23/32] ACL: initial appending of users in user loading list.

---
 src/acl.c    | 14 +++++++++++---
 src/config.c |  5 +++++
 src/server.h |  1 +
 3 files changed, 17 insertions(+), 3 deletions(-)

diff --git a/src/acl.c b/src/acl.c
index 105bcfc95..43517300b 100644
--- a/src/acl.c
+++ b/src/acl.c
@@ -937,11 +937,18 @@ int ACLAppendUserForLoading(sds *argv, int argc) {
 
     /* Try to apply the user rules in a fake user to see if they
      * are actually valid. */
-    user fu = {0};
-    user *fakeuser = &fu;
+    char *funame = "__fakeuser__";
+    user *fakeuser = ACLCreateUser(funame,strlen(funame));
+    serverAssert(fakeuser != NULL);
+    int retval = raxRemove(Users,(unsigned char*) funame,strlen(funame),NULL);
+    serverAssert(retval != 0);
+
     for (int j = 2; j < argc; j++) {
         if (ACLSetUser(fakeuser,argv[j],sdslen(argv[j])) == C_ERR) {
-            if (errno != ENOENT) return C_ERR;
+            if (errno != ENOENT) {
+                ACLFreeUser(fakeuser);
+                return C_ERR;
+            }
         }
     }
 
@@ -950,6 +957,7 @@ int ACLAppendUserForLoading(sds *argv, int argc) {
     for (int j = 1; j < argc; j++) copy[j-1] = sdsdup(argv[j]);
     copy[argc-1] = NULL;
     listAddNodeTail(UsersToLoad,copy);
+    ACLFreeUser(fakeuser);
     return C_OK;
 }
 
diff --git a/src/config.c b/src/config.c
index 91bbdee72..b2600d2ea 100644
--- a/src/config.c
+++ b/src/config.c
@@ -791,6 +791,11 @@ void loadServerConfigFromString(char *config) {
                     "Allowed values: 'upstart', 'systemd', 'auto', or 'no'";
                 goto loaderr;
             }
+        } else if (!strcasecmp(argv[0],"user") && argc >= 2) {
+            if (ACLAppendUserForLoading(argv,argc) == C_ERR) {
+                err = "Syntax error in user declaration";
+                goto loaderr;
+            }
         } else if (!strcasecmp(argv[0],"loadmodule") && argc >= 2) {
             queueLoadModule(argv[1],&argv[2],argc-2);
         } else if (!strcasecmp(argv[0],"sentinel")) {
diff --git a/src/server.h b/src/server.h
index 13d29308f..3f23cee20 100644
--- a/src/server.h
+++ b/src/server.h
@@ -1738,6 +1738,7 @@ int ACLCheckCommandPerm(client *c);
 int ACLSetUser(user *u, const char *op, ssize_t oplen);
 sds ACLDefaultUserFirstPassword(void);
 uint64_t ACLGetCommandCategoryFlagByName(const char *name);
+int ACLAppendUserForLoading(sds *argv, int argc);
 
 /* Sorted sets data type */
 

From 0d6e0f8d23f0e39adf4d8f383b213629f515cb07 Mon Sep 17 00:00:00 2001
From: antirez <antirez@gmail.com>
Date: Mon, 4 Feb 2019 13:00:38 +0100
Subject: [PATCH 24/32] ACL: make ACLAppendUserForLoading() able to report bad
 argument.

---
 src/acl.c    | 14 +++++++++++---
 src/config.c |  3 ++-
 src/server.h |  2 +-
 3 files changed, 14 insertions(+), 5 deletions(-)

diff --git a/src/acl.c b/src/acl.c
index 43517300b..991be210e 100644
--- a/src/acl.c
+++ b/src/acl.c
@@ -931,9 +931,16 @@ int ACLCheckCommandPerm(client *c) {
  *
  * Note that this function cannot stop in case of commands that are not found
  * and, in that case, the error will be emitted later, because certain
- * commands may be defined later once modules are loaded. */
-int ACLAppendUserForLoading(sds *argv, int argc) {
-    if (argc < 2 || strcasecmp(argv[0],"user")) return C_ERR;
+ * commands may be defined later once modules are loaded.
+ *
+ * When an error is detected and C_ERR is returned, the function populates
+ * by reference (if not set to NULL) the argc_err argument with the index
+ * of the argv vector that caused the error. */
+int ACLAppendUserForLoading(sds *argv, int argc, int *argc_err) {
+    if (argc < 2 || strcasecmp(argv[0],"user")) {
+        if (argc_err) *argc_err = 0;
+        return C_ERR;
+    }
 
     /* Try to apply the user rules in a fake user to see if they
      * are actually valid. */
@@ -947,6 +954,7 @@ int ACLAppendUserForLoading(sds *argv, int argc) {
         if (ACLSetUser(fakeuser,argv[j],sdslen(argv[j])) == C_ERR) {
             if (errno != ENOENT) {
                 ACLFreeUser(fakeuser);
+                if (argc_err) *argc_err = j;
                 return C_ERR;
             }
         }
diff --git a/src/config.c b/src/config.c
index b2600d2ea..c852b01b7 100644
--- a/src/config.c
+++ b/src/config.c
@@ -792,7 +792,8 @@ void loadServerConfigFromString(char *config) {
                 goto loaderr;
             }
         } else if (!strcasecmp(argv[0],"user") && argc >= 2) {
-            if (ACLAppendUserForLoading(argv,argc) == C_ERR) {
+            int argc_err;
+            if (ACLAppendUserForLoading(argv,argc,&argc_err) == C_ERR) {
                 err = "Syntax error in user declaration";
                 goto loaderr;
             }
diff --git a/src/server.h b/src/server.h
index 3f23cee20..cbf995d4e 100644
--- a/src/server.h
+++ b/src/server.h
@@ -1738,7 +1738,7 @@ int ACLCheckCommandPerm(client *c);
 int ACLSetUser(user *u, const char *op, ssize_t oplen);
 sds ACLDefaultUserFirstPassword(void);
 uint64_t ACLGetCommandCategoryFlagByName(const char *name);
-int ACLAppendUserForLoading(sds *argv, int argc);
+int ACLAppendUserForLoading(sds *argv, int argc, int *argc_err);
 
 /* Sorted sets data type */
 

From 20fa89d093e405ac712b8dbc092d7b42a44ca57e Mon Sep 17 00:00:00 2001
From: antirez <antirez@gmail.com>
Date: Mon, 4 Feb 2019 13:04:35 +0100
Subject: [PATCH 25/32] ACL: better error reporting in users configuration
 errors.

---
 src/config.c | 6 +++++-
 src/server.h | 1 +
 2 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/src/config.c b/src/config.c
index c852b01b7..bc92a423d 100644
--- a/src/config.c
+++ b/src/config.c
@@ -794,7 +794,11 @@ void loadServerConfigFromString(char *config) {
         } else if (!strcasecmp(argv[0],"user") && argc >= 2) {
             int argc_err;
             if (ACLAppendUserForLoading(argv,argc,&argc_err) == C_ERR) {
-                err = "Syntax error in user declaration";
+                char buf[1024];
+                char *errmsg = ACLSetUserStringError();
+                snprintf(buf,sizeof(buf),"Error in user declaration '%s': %s",
+                    argv[argc_err],errmsg);
+                err = buf;
                 goto loaderr;
             }
         } else if (!strcasecmp(argv[0],"loadmodule") && argc >= 2) {
diff --git a/src/server.h b/src/server.h
index cbf995d4e..a694a4dc2 100644
--- a/src/server.h
+++ b/src/server.h
@@ -1739,6 +1739,7 @@ int ACLSetUser(user *u, const char *op, ssize_t oplen);
 sds ACLDefaultUserFirstPassword(void);
 uint64_t ACLGetCommandCategoryFlagByName(const char *name);
 int ACLAppendUserForLoading(sds *argv, int argc, int *argc_err);
+char *ACLSetUserStringError(void);
 
 /* Sorted sets data type */
 

From 0082503bdf5f6919336b0261569db9ac53ffc62d Mon Sep 17 00:00:00 2001
From: antirez <antirez@gmail.com>
Date: Mon, 4 Feb 2019 16:35:15 +0100
Subject: [PATCH 26/32] ACL: implement ACLLoadConfiguredUsers().

---
 src/acl.c    | 40 ++++++++++++++++++++++++++++++++++++++++
 src/server.h |  1 +
 2 files changed, 41 insertions(+)

diff --git a/src/acl.c b/src/acl.c
index 991be210e..349f81f31 100644
--- a/src/acl.c
+++ b/src/acl.c
@@ -969,6 +969,46 @@ int ACLAppendUserForLoading(sds *argv, int argc, int *argc_err) {
     return C_OK;
 }
 
+/* This function will load the configured users appended to the server
+ * configuration via ACLAppendUserForLoading(). On loading errors it will
+ * log an error and return C_ERR, otherwise C_OK will be returned. */
+int ACLLoadConfiguredUsers(void) {
+    listIter li;
+    listNode *ln;
+    listRewind(UsersToLoad,&li);
+    while ((ln = listNext(&li)) != NULL) {
+        sds *aclrules = listNodeValue(ln);
+        user *u = ACLCreateUser(aclrules[0],sdslen(aclrules[0]));
+        if (!u) {
+            serverLog(LL_WARNING,
+                      "Error loading ACLs: user '%s' specified multiple times",
+                      aclrules[0]);
+            return C_ERR;
+        }
+
+        /* Load every rule defined for this user. */
+        for (int j = 1; aclrules[j]; j++) {
+            if (ACLSetUser(u,aclrules[j],sdslen(aclrules[j])) != C_OK) {
+                char *errmsg = ACLSetUserStringError();
+                serverLog(LL_WARNING,"Error loading ACL rule '%s' for "
+                                     "the user named '%s': %s",
+                          aclrules[0],aclrules[j],errmsg);
+                return C_ERR;
+            }
+        }
+
+        /* Having a disabled user in the configuration may be an error,
+         * warn about it without returning any error to the caller. */
+        if (u->flags & USER_FLAG_DISABLED) {
+            serverLog(LL_NOTICE, "The user '%s' is disabled (there is no "
+                                 "'on' modifier in the user description). Make "
+                                 "sure this is not a configuration error.",
+                      aclrules[0]);
+        }
+    }
+    return C_OK;
+}
+
 /* =============================================================================
  * ACL related commands
  * ==========================================================================*/
diff --git a/src/server.h b/src/server.h
index a694a4dc2..02104e6f0 100644
--- a/src/server.h
+++ b/src/server.h
@@ -1740,6 +1740,7 @@ sds ACLDefaultUserFirstPassword(void);
 uint64_t ACLGetCommandCategoryFlagByName(const char *name);
 int ACLAppendUserForLoading(sds *argv, int argc, int *argc_err);
 char *ACLSetUserStringError(void);
+int ACLLoadConfiguredUsers(void);
 
 /* Sorted sets data type */
 

From 58c070757d5dc9b7485d180aa13aa2e17f3c9d7c Mon Sep 17 00:00:00 2001
From: antirez <antirez@gmail.com>
Date: Mon, 4 Feb 2019 16:39:07 +0100
Subject: [PATCH 27/32] ACL: load the defined users at server startup.

---
 src/server.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/src/server.c b/src/server.c
index 1df63f65f..72daaedec 100644
--- a/src/server.c
+++ b/src/server.c
@@ -4907,6 +4907,11 @@ int main(int argc, char **argv) {
         linuxMemoryWarnings();
     #endif
         moduleLoadFromQueue();
+        if (ACLLoadConfiguredUsers() == C_ERR) {
+            serverLog(LL_WARNING,
+                "Critical error while loading ACLs. Exiting.");
+            exit(1);
+        }
         loadDataFromDisk();
         if (server.cluster_enabled) {
             if (verifyClusterConfigWithData() == C_ERR) {

From b74c41c8e46514c89aee5756bae4a52e621c5cbe Mon Sep 17 00:00:00 2001
From: antirez <antirez@gmail.com>
Date: Mon, 4 Feb 2019 16:58:35 +0100
Subject: [PATCH 28/32] ACL: fix user/rule inverted error message.

---
 src/acl.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/acl.c b/src/acl.c
index 349f81f31..8211dcea3 100644
--- a/src/acl.c
+++ b/src/acl.c
@@ -992,7 +992,7 @@ int ACLLoadConfiguredUsers(void) {
                 char *errmsg = ACLSetUserStringError();
                 serverLog(LL_WARNING,"Error loading ACL rule '%s' for "
                                      "the user named '%s': %s",
-                          aclrules[0],aclrules[j],errmsg);
+                          aclrules[j],aclrules[0],errmsg);
                 return C_ERR;
             }
         }

From 8ce3c1631732d1336625da213c9e7313d09469af Mon Sep 17 00:00:00 2001
From: antirez <antirez@gmail.com>
Date: Tue, 5 Feb 2019 10:48:17 +0100
Subject: [PATCH 29/32] ACL: implement rewriting of users in redis.conf.

---
 src/config.c | 33 +++++++++++++++++++++++++++++++++
 src/server.c |  1 +
 src/server.h |  5 +++++
 3 files changed, 39 insertions(+)

diff --git a/src/config.c b/src/config.c
index bc92a423d..663ac404e 100644
--- a/src/config.c
+++ b/src/config.c
@@ -1917,6 +1917,38 @@ void rewriteConfigSaveOption(struct rewriteConfigState *state) {
     rewriteConfigMarkAsProcessed(state,"save");
 }
 
+/* Rewrite the user option. */
+void rewriteConfigUserOption(struct rewriteConfigState *state) {
+    /* If there is a user file defined we just mark this configuration
+     * directive as processed, so that all the lines containing users
+     * inside the config file gets discarded. */
+    if (server.acl_filename[0] != '\0') {
+        rewriteConfigMarkAsProcessed(state,"user");
+        return;
+    }
+
+    /* Otherwise scan the list of users and rewrite every line. Note that
+     * in case the list here is empty, the effect will just be to comment
+     * all the users directive inside the config file. */
+    raxIterator ri;
+    raxStart(&ri,Users);
+    raxSeek(&ri,"^",NULL,0);
+    while(raxNext(&ri)) {
+        user *u = ri.data;
+        sds line = sdsnew("user ");
+        line = sdscatsds(line,u->name);
+        line = sdscatlen(line," ",1);
+        sds descr = ACLDescribeUser(u);
+        line = sdscatsds(line,descr);
+        sdsfree(descr);
+        rewriteConfigRewriteLine(state,"user",line,1);
+    }
+    raxStop(&ri);
+
+    /* Mark "user" as processed in case there are no defined users. */
+    rewriteConfigMarkAsProcessed(state,"user");
+}
+
 /* Rewrite the dir option, always using absolute paths.*/
 void rewriteConfigDirOption(struct rewriteConfigState *state) {
     char cwd[1024];
@@ -2186,6 +2218,7 @@ int rewriteConfig(char *path) {
     rewriteConfigStringOption(state,"syslog-ident",server.syslog_ident,CONFIG_DEFAULT_SYSLOG_IDENT);
     rewriteConfigSyslogfacilityOption(state);
     rewriteConfigSaveOption(state);
+    rewriteConfigUserOption(state);
     rewriteConfigNumericalOption(state,"databases",server.dbnum,CONFIG_DEFAULT_DBNUM);
     rewriteConfigYesNoOption(state,"stop-writes-on-bgsave-error",server.stop_writes_on_bgsave_err,CONFIG_DEFAULT_STOP_WRITES_ON_BGSAVE_ERROR);
     rewriteConfigYesNoOption(state,"rdbcompression",server.rdb_compression,CONFIG_DEFAULT_RDB_COMPRESSION);
diff --git a/src/server.c b/src/server.c
index 72daaedec..de84e430e 100644
--- a/src/server.c
+++ b/src/server.c
@@ -2267,6 +2267,7 @@ void initServerConfig(void) {
     server.pidfile = NULL;
     server.rdb_filename = zstrdup(CONFIG_DEFAULT_RDB_FILENAME);
     server.aof_filename = zstrdup(CONFIG_DEFAULT_AOF_FILENAME);
+    server.acl_filename = zstrdup(CONFIG_DEFAULT_ACL_FILENAME);
     server.rdb_compression = CONFIG_DEFAULT_RDB_COMPRESSION;
     server.rdb_checksum = CONFIG_DEFAULT_RDB_CHECKSUM;
     server.stop_writes_on_bgsave_err = CONFIG_DEFAULT_STOP_WRITES_ON_BGSAVE_ERROR;
diff --git a/src/server.h b/src/server.h
index 02104e6f0..d2c6aa1e0 100644
--- a/src/server.h
+++ b/src/server.h
@@ -148,6 +148,7 @@ typedef long long mstime_t; /* millisecond time type. */
 #define CONFIG_DEFAULT_RDB_SAVE_INCREMENTAL_FSYNC 1
 #define CONFIG_DEFAULT_MIN_SLAVES_TO_WRITE 0
 #define CONFIG_DEFAULT_MIN_SLAVES_MAX_LAG 10
+#define CONFIG_DEFAULT_ACL_FILENAME ""
 #define NET_IP_STR_LEN 46 /* INET6_ADDRSTRLEN is 46, but we need to be sure */
 #define NET_PEER_ID_LEN (NET_IP_STR_LEN+32) /* Must be enough for ip:port */
 #define CONFIG_BINDADDR_MAX 16
@@ -1337,6 +1338,8 @@ struct redisServer {
     /* Latency monitor */
     long long latency_monitor_threshold;
     dict *latency_events;
+    /* ACLs */
+    char *acl_filename;     /* ACL Users file. NULL if not configured. */
     /* Assert & bug reporting */
     const char *assert_failed;
     const char *assert_file;
@@ -1725,6 +1728,7 @@ void sendChildInfo(int process_type);
 void receiveChildInfo(void);
 
 /* acl.c -- Authentication related prototypes. */
+extern rax *Users;
 extern user *DefaultUser;
 void ACLInit(void);
 /* Return values for ACLCheckUserCredentials(). */
@@ -1741,6 +1745,7 @@ uint64_t ACLGetCommandCategoryFlagByName(const char *name);
 int ACLAppendUserForLoading(sds *argv, int argc, int *argc_err);
 char *ACLSetUserStringError(void);
 int ACLLoadConfiguredUsers(void);
+sds ACLDescribeUser(user *u);
 
 /* Sorted sets data type */
 

From 15a76e78689d6a9dcbb7ac4474ce72e96d85c018 Mon Sep 17 00:00:00 2001
From: antirez <antirez@gmail.com>
Date: Tue, 5 Feb 2019 10:52:05 +0100
Subject: [PATCH 30/32] ACL: change behavior of redefined user. Last line
 counts.

---
 src/acl.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/src/acl.c b/src/acl.c
index 8211dcea3..db2f291c6 100644
--- a/src/acl.c
+++ b/src/acl.c
@@ -978,12 +978,12 @@ int ACLLoadConfiguredUsers(void) {
     listRewind(UsersToLoad,&li);
     while ((ln = listNext(&li)) != NULL) {
         sds *aclrules = listNodeValue(ln);
-        user *u = ACLCreateUser(aclrules[0],sdslen(aclrules[0]));
+        sds username = aclrules[0];
+        user *u = ACLCreateUser(username,sdslen(username));
         if (!u) {
-            serverLog(LL_WARNING,
-                      "Error loading ACLs: user '%s' specified multiple times",
-                      aclrules[0]);
-            return C_ERR;
+            u = ACLGetUserByName(username,sdslen(username));
+            serverAssert(u != NULL);
+            ACLSetUser(u,"reset",-1);
         }
 
         /* Load every rule defined for this user. */

From 475fd7ba2e985fbaa25d559c919f7ba7c3f7ed44 Mon Sep 17 00:00:00 2001
From: antirez <antirez@gmail.com>
Date: Tue, 5 Feb 2019 17:49:52 +0100
Subject: [PATCH 31/32] ACL: ability to configure an external ACL file.

---
 src/config.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/src/config.c b/src/config.c
index 663ac404e..fc9160d26 100644
--- a/src/config.c
+++ b/src/config.c
@@ -283,6 +283,9 @@ void loadServerConfigFromString(char *config) {
                 }
                 fclose(logfp);
             }
+        } else if (!strcasecmp(argv[0],"aclfile") && argc == 2) {
+            zfree(server.acl_filename);
+            server.acl_filename = zstrdup(argv[1]);
         } else if (!strcasecmp(argv[0],"always-show-logo") && argc == 2) {
             if ((server.always_show_logo = yesnotoi(argv[1])) == -1) {
                 err = "argument must be 'yes' or 'no'"; goto loaderr;
@@ -1355,6 +1358,7 @@ void configGetCommand(client *c) {
     config_get_string_field("cluster-announce-ip",server.cluster_announce_ip);
     config_get_string_field("unixsocket",server.unixsocket);
     config_get_string_field("logfile",server.logfile);
+    config_get_string_field("aclfile",server.acl_filename);
     config_get_string_field("pidfile",server.pidfile);
     config_get_string_field("slave-announce-ip",server.slave_announce_ip);
     config_get_string_field("replica-announce-ip",server.slave_announce_ip);
@@ -2214,6 +2218,7 @@ int rewriteConfig(char *path) {
     rewriteConfigNumericalOption(state,"replica-announce-port",server.slave_announce_port,CONFIG_DEFAULT_SLAVE_ANNOUNCE_PORT);
     rewriteConfigEnumOption(state,"loglevel",server.verbosity,loglevel_enum,CONFIG_DEFAULT_VERBOSITY);
     rewriteConfigStringOption(state,"logfile",server.logfile,CONFIG_DEFAULT_LOGFILE);
+    rewriteConfigStringOption(state,"aclfile",server.acl_filename,CONFIG_DEFAULT_ACL_FILENAME);
     rewriteConfigYesNoOption(state,"syslog-enabled",server.syslog_enabled,CONFIG_DEFAULT_SYSLOG_ENABLED);
     rewriteConfigStringOption(state,"syslog-ident",server.syslog_ident,CONFIG_DEFAULT_SYSLOG_IDENT);
     rewriteConfigSyslogfacilityOption(state);

From 20ba1b7e96c6e187235676a7bbae051718f4d51e Mon Sep 17 00:00:00 2001
From: antirez <antirez@gmail.com>
Date: Tue, 5 Feb 2019 17:59:05 +0100
Subject: [PATCH 32/32] ACL: redis.conf: mark old ACL-alike stuff as
 deprecated.

---
 redis.conf | 39 +++++++++++++++++++++++++++++----------
 1 file changed, 29 insertions(+), 10 deletions(-)

diff --git a/redis.conf b/redis.conf
index 93ab9a42e..d1ced7eb3 100644
--- a/redis.conf
+++ b/redis.conf
@@ -493,20 +493,39 @@ replica-priority 100
 
 ################################## SECURITY ###################################
 
-# Require clients to issue AUTH <PASSWORD> before processing any other
-# commands.  This might be useful in environments in which you do not trust
-# others with access to the host running redis-server.
-#
-# This should stay commented out for backward compatibility and because most
-# people do not need auth (e.g. they run their own servers).
-#
 # Warning: since Redis is pretty fast an outside user can try up to
-# 150k passwords per second against a good box. This means that you should
-# use a very strong password otherwise it will be very easy to break.
+# 1 million passwords per second against a modern box. This means that you
+# should use very strong passwords, otherwise they will be very easy to break.
+# Note that because the password is really a shared secret between the client
+# and the server, and should not be memorized by any human, the password
+# can be easily a long string from /dev/urandom or whatever, so by using a
+# long and unguessable password no brute force attack will be possible.
+
+# Instead of configuring users here in this file, it is possible to use
+# a stand-alone file just listing users. The two methods cannot be mixed:
+# if you configure users here and at the same time you activate the exteranl
+# ACL file, the server will refuse to start.
+#
+# The format of the external ACL user file is exactly the same as the
+# format that is used inside redis.conf to describe users.
+#
+# aclfile /etc/redis/users.acl
+
+# IMPORTANT NOTE: starting with Redis 6 "requirepass" is just a compatiblity
+# layer on top of the new ACL system. The option effect will be just setting
+# the password for the default user. Clients will still authenticate using
+# AUTH <password> as usually, or more explicitly with AUTH default <password>
+# if they follow the new protocol: both will work.
 #
 # requirepass foobared
 
-# Command renaming.
+# Command renaming (DEPRECATED).
+#
+# ------------------------------------------------------------------------
+# WARNING: avoid using this option if possible. Instead use ACLs to remove
+# commands from the default user, and put them only in some admin user you
+# create for administrative purposes.
+# ------------------------------------------------------------------------
 #
 # It is possible to change the name of dangerous commands in a shared
 # environment. For instance the CONFIG command may be renamed into something