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 + * 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. */