Allowed passing in of password hash and fixed config rewrite

src/acl.c

@@ -94,6 +94,9 @@ void ACLResetSubcommandsForCommand(user *u, unsigned long id);
 void ACLResetSubcommands(user *u);
 void ACLAddAllowedSubcommand(user *u, unsigned long id, const char *sub);
 
+/* The length of the string representation of a hashed password. */
+#define HASH_PASSWORD_LEN SHA256_BLOCK_SIZE*2 
+
 /* =============================================================================
  * Helper functions for the rest of the ACL implementation
  * ==========================================================================*/
@@ -145,7 +148,7 @@ int time_independent_strcmp(char *a, char *b) {
 sds ACLHashPassword(unsigned char *cleartext, size_t len) {
     SHA256_CTX ctx;
     unsigned char hash[SHA256_BLOCK_SIZE];
-    char hex[SHA256_BLOCK_SIZE*2];
+    char hex[HASH_PASSWORD_LEN];
     char *cset = "0123456789abcdef";
 
     sha256_init(&ctx);
@@ -156,7 +159,7 @@ sds ACLHashPassword(unsigned char *cleartext, size_t len) {
         hex[j*2] = cset[((hash[j]&0xF0)>>4)];
         hex[j*2+1] = cset[(hash[j]&0xF)];
     }
-    return sdsnewlen(hex,SHA256_BLOCK_SIZE*2);
+    return sdsnewlen(hex,HASH_PASSWORD_LEN);
 }
 
 /* =============================================================================
@@ -522,7 +525,7 @@ sds ACLDescribeUser(user *u) {
     listRewind(u->passwords,&li);
     while((ln = listNext(&li))) {
         sds thispass = listNodeValue(ln);
-        res = sdscatlen(res,">",1);
+        res = sdscatlen(res,"#",1);
         res = sdscatsds(res,thispass);
         res = sdscatlen(res," ",1);
     }
@@ -649,6 +652,10 @@ void ACLAddAllowedSubcommand(user *u, unsigned long id, const char *sub) {
  * ><password>  Add this password to the list of valid password for the user.
  *              For example >mypass will add "mypass" to the list.
  *              This directive clears the "nopass" flag (see later).
+ * #<password hash> Add this password hash to the list of valid hashes for 
+ *                  the user. This is useful if you have previously computed
+ *                  the hash, and don't want to store it in plaintext. 
+ *                  This directive clears the "nopass" flag (see later).
  * <<password>  Remove this password from the list of valid passwords.
  * nopass       All the set passwords of the user are removed, and the user
  *              is flagged as requiring no password: it means that every
@@ -685,6 +692,7 @@ void ACLAddAllowedSubcommand(user *u, unsigned long id, const char *sub) {
  * EEXIST: You are adding a key pattern after "*" was already added. This is
  *         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. 
  */
 int ACLSetUser(user *u, const char *op, ssize_t oplen) {
     if (oplen == -1) oplen = strlen(op);
@@ -720,8 +728,30 @@ int ACLSetUser(user *u, const char *op, ssize_t oplen) {
     } else if (!strcasecmp(op,"resetpass")) {
         u->flags &= ~USER_FLAG_NOPASS;
         listEmpty(u->passwords);
-    } else if (op[0] == '>') {
+    } else if (op[0] == '>' || op[0] == '#') {
-        sds newpass = ACLHashPassword((unsigned char*)op+1,oplen-1);
+        sds newpass;
+        if (op[0] == '>') {
+            newpass = ACLHashPassword((unsigned char*)op+1,oplen-1);
+        } else {
+            if (oplen != HASH_PASSWORD_LEN + 1) {
+                errno = EBADMSG;
+                return C_ERR;      
+            }
+
+            /* Password hashes can only be characters that represent
+             * hexadecimal values, which are numbers and lowercase 
+             * characters 'a' through 'f'. 
+             */
+            for(int i = 1; i < HASH_PASSWORD_LEN + 1; i++) {
+                char c = op[i];
+                if ((c < 'a' || c > 'f') && (c < '0' || c > '9')) {
+                    errno = EBADMSG;
+                    return C_ERR;
+                }
+            }
+            newpass = sdsnewlen(op+1,oplen-1);
+        }
+
         listNode *ln = listSearchKey(u->passwords,newpass);
         /* Avoid re-adding the same password multiple times. */
         if (ln == NULL)
@@ -848,6 +878,9 @@ char *ACLSetUserStringError(void) {
     else if (errno == ENODEV)
         errmsg = "The password you are trying to remove from the user does "
                  "not exist";
+    else if (errno == EBADMSG)
+        errmsg = "The password hash must be exactly 64 characters and contain "
+                 "only lowercase hexadecimal characters";
     return errmsg;
 }
 

tests/support/test.tcl

@@ -15,6 +15,12 @@ proc assert {condition} {
     }
 }
 
+proc assert_no_match {pattern value} {
+    if {[string match $pattern $value]} {
+        error "assertion:Expected '$value' to not match '$pattern'"
+    }
+}
+
 proc assert_match {pattern value} {
     if {![string match $pattern $value]} {
         error "assertion:Expected '$value' to match '$pattern'"

tests/unit/acl.tcl

@@ -35,6 +35,26 @@ start_server {tags {"acl"}} {
         set e
     } {*WRONGPASS*}
 
+    test {Test password hashes can be added} {
+        r ACL setuser newuser #34344e4d60c2b6d639b7bd22e18f2b0b91bc34bf0ac5f9952744435093cfb4e6
+        catch {r AUTH newuser passwd4} e
+        assert {$e eq "OK"}
+    }
+
+    test {Test password hashes validate input} {
+        # Validate Length
+        catch {r ACL setuser newuser #34344e4d60c2b6d639b7bd22e18f2b0b91bc34bf0ac5f9952744435093cfb4e} e
+        # Validate character outside set
+        catch {r ACL setuser newuser #34344e4d60c2b6d639b7bd22e18f2b0b91bc34bf0ac5f9952744435093cfb4eq} e
+        set e
+    } {*Error in ACL SETUSER modifier*}
+
+    test {ACL GETUSER returns the password hash instead of the actual password} {
+        set passstr [dict get [r ACL getuser newuser] passwords]
+        assert_match {*34344e4d60c2b6d639b7bd22e18f2b0b91bc34bf0ac5f9952744435093cfb4e6*} $passstr
+        assert_no_match {*passwd4*} $passstr
+    }
+
     test {By default users are not able to access any command} {
         catch {r SET foo bar} e
         set e
@@ -67,7 +87,7 @@ start_server {tags {"acl"}} {
         set e
     } {*NOPERM*}
 
-    test {ACLs can include or excluse whole classes of commands} {
+    test {ACLs can include or exclude whole classes of commands} {
         r ACL setuser newuser -@all +@set +acl
         r SADD myset a b c; # Should not raise an error
         r ACL setuser newuser +@all -@string