From b43d70df568d85ba0fb79c9e0129d4654ef471ee Mon Sep 17 00:00:00 2001
From: antirez <antirez@gmail.com>
Date: Fri, 21 Dec 2018 17:16:22 +0100
Subject: [PATCH] ACL: refactoring of the original authentication code.
---
src/Makefile | 2 +-
src/acl.c | 95 ++++++++++++++++++++++++++++++++++++++++++++++++++++
src/server.c | 44 +-----------------------
src/server.h | 3 ++
4 files changed, 100 insertions(+), 44 deletions(-)
create mode 100644 src/acl.c
diff --git a/src/Makefile b/src/Makefile
index 9edbb458..adf32d55 100644
--- a/src/Makefile
+++ b/src/Makefile
@@ -164,7 +164,7 @@ endif
REDIS_SERVER_NAME=redis-server
REDIS_SENTINEL_NAME=redis-sentinel
-REDIS_SERVER_OBJ=adlist.o quicklist.o ae.o anet.o dict.o server.o sds.o zmalloc.o lzf_c.o lzf_d.o pqsort.o zipmap.o sha1.o ziplist.o release.o networking.o util.o object.o db.o replication.o rdb.o t_string.o t_list.o t_set.o t_zset.o t_hash.o config.o aof.o pubsub.o multi.o debug.o sort.o intset.o syncio.o cluster.o crc16.o endianconv.o slowlog.o scripting.o bio.o rio.o rand.o memtest.o crc64.o bitops.o sentinel.o notify.o setproctitle.o blocked.o hyperloglog.o latency.o sparkline.o redis-check-rdb.o redis-check-aof.o geo.o lazyfree.o module.o evict.o expire.o geohash.o geohash_helper.o childinfo.o defrag.o siphash.o rax.o t_stream.o listpack.o localtime.o lolwut.o lolwut5.o
+REDIS_SERVER_OBJ=adlist.o quicklist.o ae.o anet.o dict.o server.o sds.o zmalloc.o lzf_c.o lzf_d.o pqsort.o zipmap.o sha1.o ziplist.o release.o networking.o util.o object.o db.o replication.o rdb.o t_string.o t_list.o t_set.o t_zset.o t_hash.o config.o aof.o pubsub.o multi.o debug.o sort.o intset.o syncio.o cluster.o crc16.o endianconv.o slowlog.o scripting.o bio.o rio.o rand.o memtest.o crc64.o bitops.o sentinel.o notify.o setproctitle.o blocked.o hyperloglog.o latency.o sparkline.o redis-check-rdb.o redis-check-aof.o geo.o lazyfree.o module.o evict.o expire.o geohash.o geohash_helper.o childinfo.o defrag.o siphash.o rax.o t_stream.o listpack.o localtime.o lolwut.o lolwut5.o acl.o
REDIS_CLI_NAME=redis-cli
REDIS_CLI_OBJ=anet.o adlist.o dict.o redis-cli.o zmalloc.o release.o anet.o ae.o crc64.o siphash.o crc16.o
REDIS_BENCHMARK_NAME=redis-benchmark
diff --git a/src/acl.c b/src/acl.c
new file mode 100644
index 00000000..5155f947
--- /dev/null
+++ b/src/acl.c
@@ -0,0 +1,95 @@
+/*
+ * Copyright (c) 2018, Salvatore Sanfilippo <antirez at gmail dot com>
+ * All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions are met:
+ *
+ * * Redistributions of source code must retain the above copyright notice,
+ * this list of conditions and the following disclaimer.
+ * * Redistributions in binary form must reproduce the above copyright
+ * notice, this list of conditions and the following disclaimer in the
+ * documentation and/or other materials provided with the distribution.
+ * * Neither the name of Redis nor the names of its contributors may be used
+ * to endorse or promote products derived from this software without
+ * specific prior written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS"
+ * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
+ * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
+ * ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR CONTRIBUTORS BE
+ * LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
+ * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
+ * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS
+ * INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN
+ * CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
+ * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE
+ * POSSIBILITY OF SUCH DAMAGE.
+ */
+
+#include "server.h"
+
+/* Return zero if strings are the same, non-zero if they are not.
+ * The comparison is performed in a way that prevents an attacker to obtain
+ * information about the nature of the strings just monitoring the execution
+ * time of the function.
+ *
+ * Note that limiting the comparison length to strings up to 512 bytes we
+ * can avoid leaking any information about the password length and any
+ * possible branch misprediction related leak.
+ */
+int time_independent_strcmp(char *a, char *b) {
+ char bufa[CONFIG_AUTHPASS_MAX_LEN], bufb[CONFIG_AUTHPASS_MAX_LEN];
+ /* The above two strlen perform len(a) + len(b) operations where either
+ * a or b are fixed (our password) length, and the difference is only
+ * relative to the length of the user provided string, so no information
+ * leak is possible in the following two lines of code. */
+ unsigned int alen = strlen(a);
+ unsigned int blen = strlen(b);
+ unsigned int j;
+ int diff = 0;
+
+ /* We can't compare strings longer than our static buffers.
+ * Note that this will never pass the first test in practical circumstances
+ * so there is no info leak. */
+ if (alen > sizeof(bufa) || blen > sizeof(bufb)) return 1;
+
+ memset(bufa,0,sizeof(bufa)); /* Constant time. */
+ memset(bufb,0,sizeof(bufb)); /* Constant time. */
+ /* Again the time of the following two copies is proportional to
+ * len(a) + len(b) so no info is leaked. */
+ memcpy(bufa,a,alen);
+ memcpy(bufb,b,blen);
+
+ /* Always compare all the chars in the two buffers without
+ * conditional expressions. */
+ for (j = 0; j < sizeof(bufa); j++) {
+ diff |= (bufa[j] ^ bufb[j]);
+ }
+ /* Length must be equal as well. */
+ diff |= alen ^ blen;
+ return diff; /* If zero strings are the same. */
+}
+
+/* Check the username and password pair and return C_OK if they are valid,
+ * otherwise C_ERR is returned and errno is set to:
+ *
+ * EINVAL: if the username-password do not match.
+ * ENONENT: if the specified user does not exist at all.
+ */
+int ACLCheckUserCredentials(robj *username, robj *password) {
+ /* For now only the "default" user is allowed. When the RCP1 ACLs
+ * will be implemented multiple usernames will be supproted. */
+ if (username != NULL && strcmp(username->ptr,"default")) {
+ errno = ENOENT;
+ return C_ERR;
+ }
+
+ /* For now we just compare the password with the system wide one. */
+ if (!time_independent_strcmp(password->ptr, server.requirepass)) {
+ return C_OK;
+ } else {
+ errno = EINVAL;
+ return C_ERR;
+ }
+}
diff --git a/src/server.c b/src/server.c
index 6c7692ed..7a813404 100644
--- a/src/server.c
+++ b/src/server.c
@@ -2864,52 +2864,10 @@ int writeCommandsDeniedByDiskError(void) {
}
}
-/* Return zero if strings are the same, non-zero if they are not.
- * The comparison is performed in a way that prevents an attacker to obtain
- * information about the nature of the strings just monitoring the execution
- * time of the function.
- *
- * Note that limiting the comparison length to strings up to 512 bytes we
- * can avoid leaking any information about the password length and any
- * possible branch misprediction related leak.
- */
-int time_independent_strcmp(char *a, char *b) {
- char bufa[CONFIG_AUTHPASS_MAX_LEN], bufb[CONFIG_AUTHPASS_MAX_LEN];
- /* The above two strlen perform len(a) + len(b) operations where either
- * a or b are fixed (our password) length, and the difference is only
- * relative to the length of the user provided string, so no information
- * leak is possible in the following two lines of code. */
- unsigned int alen = strlen(a);
- unsigned int blen = strlen(b);
- unsigned int j;
- int diff = 0;
-
- /* We can't compare strings longer than our static buffers.
- * Note that this will never pass the first test in practical circumstances
- * so there is no info leak. */
- if (alen > sizeof(bufa) || blen > sizeof(bufb)) return 1;
-
- memset(bufa,0,sizeof(bufa)); /* Constant time. */
- memset(bufb,0,sizeof(bufb)); /* Constant time. */
- /* Again the time of the following two copies is proportional to
- * len(a) + len(b) so no info is leaked. */
- memcpy(bufa,a,alen);
- memcpy(bufb,b,blen);
-
- /* Always compare all the chars in the two buffers without
- * conditional expressions. */
- for (j = 0; j < sizeof(bufa); j++) {
- diff |= (bufa[j] ^ bufb[j]);
- }
- /* Length must be equal as well. */
- diff |= alen ^ blen;
- return diff; /* If zero strings are the same. */
-}
-
void authCommand(client *c) {
if (!server.requirepass) {
addReplyError(c,"Client sent AUTH, but no password is set");
- } else if (!time_independent_strcmp(c->argv[1]->ptr, server.requirepass)) {
+ } else if (ACLCheckUserCredentials(NULL,c->argv[1]->ptr)) {
c->authenticated = 1;
addReply(c,shared.ok);
} else {
diff --git a/src/server.h b/src/server.h
index ed6f1a7d..909e6c3d 100644
--- a/src/server.h
+++ b/src/server.h
@@ -1648,6 +1648,9 @@ void closeChildInfoPipe(void);
void sendChildInfo(int process_type);
void receiveChildInfo(void);
+/* acl.c -- Authentication related prototypes. */
+int ACLCheckUserCredentials(robj *username, robj *password);
+
/* Sorted sets data type */
/* Input flags. */