From cf03a62ba78f1939b16a81d53c0de6d1aef73294 Mon Sep 17 00:00:00 2001 From: Oran Agra Date: Mon, 2 Apr 2018 18:36:17 +0300 Subject: [PATCH 01/47] Add redis-cli support for diskless replication (CAPA EOF) when setting repl-diskless-sync yes, and sending SYNC. redis-cli needs to be able to understand the EOF marker protocol in order to be able to skip or download the rdb file --- src/redis-cli.c | 112 +++++++++++++++++++++++++++++++++++++++++++----- 1 file changed, 102 insertions(+), 10 deletions(-) diff --git a/src/redis-cli.c b/src/redis-cli.c index d80973e75..6fffa9024 100644 --- a/src/redis-cli.c +++ b/src/redis-cli.c @@ -1793,9 +1793,31 @@ static void latencyDistMode(void) { * Slave mode *--------------------------------------------------------------------------- */ +#define RDB_EOF_MARK_SIZE 40 + +void sendReplconf(const char* arg1, const char* arg2) { + printf("sending REPLCONF %s %s\n", arg1, arg2); + redisReply *reply = redisCommand(context, "REPLCONF %s %s", arg1, arg2); + + /* Handle any error conditions */ + if(reply == NULL) { + fprintf(stderr, "\nI/O error\n"); + exit(1); + } else if(reply->type == REDIS_REPLY_ERROR) { + fprintf(stderr, "REPLCONF %s error: %s\n", arg1, reply->str); + /* non fatal, old versions may not support it */ + } + freeReplyObject(reply); +} + +void sendCapa() { + sendReplconf("capa", "eof"); +} + /* Sends SYNC and reads the number of bytes in the payload. Used both by - * slaveMode() and getRDB(). */ -unsigned long long sendSync(int fd) { + * slaveMode() and getRDB(). + * returns 0 in case an EOF marker is used. */ +unsigned long long sendSync(int fd, char *out_eof) { /* To start we need to send the SYNC command and return the payload. * The hiredis client lib does not understand this part of the protocol * and we don't want to mess with its buffers, so everything is performed @@ -1825,17 +1847,33 @@ unsigned long long sendSync(int fd) { printf("SYNC with master failed: %s\n", buf); exit(1); } + if (strncmp(buf+1,"EOF:",4) == 0 && strlen(buf+5) >= RDB_EOF_MARK_SIZE) { + memcpy(out_eof, buf+5, RDB_EOF_MARK_SIZE); + return 0; + } return strtoull(buf+1,NULL,10); } static void slaveMode(void) { int fd = context->fd; - unsigned long long payload = sendSync(fd); + static char eofmark[RDB_EOF_MARK_SIZE]; + static char lastbytes[RDB_EOF_MARK_SIZE]; + static int usemark = 0; + unsigned long long payload = sendSync(fd, eofmark); char buf[1024]; int original_output = config.output; - fprintf(stderr,"SYNC with master, discarding %llu " - "bytes of bulk transfer...\n", payload); + if (payload == 0) { + payload = ULLONG_MAX; + memset(lastbytes,0,RDB_EOF_MARK_SIZE); + usemark = 1; + fprintf(stderr,"SYNC with master, discarding " + "bytes of bulk transfer until EOF marker...\n"); + } else { + fprintf(stderr,"SYNC with master, discarding %llu " + "bytes of bulk transfer...\n", payload); + } + /* Discard the payload. */ while(payload) { @@ -1847,8 +1885,29 @@ static void slaveMode(void) { exit(1); } payload -= nread; + + if (usemark) { + /* Update the last bytes array, and check if it matches our delimiter.*/ + if (nread >= RDB_EOF_MARK_SIZE) { + memcpy(lastbytes,buf+nread-RDB_EOF_MARK_SIZE,RDB_EOF_MARK_SIZE); + } else { + int rem = RDB_EOF_MARK_SIZE-nread; + memmove(lastbytes,lastbytes+nread,rem); + memcpy(lastbytes+rem,buf,nread); + } + if (memcmp(lastbytes,eofmark,RDB_EOF_MARK_SIZE) == 0) + break; + } } - fprintf(stderr,"SYNC done. Logging commands from master.\n"); + + if (usemark) { + unsigned long long offset = ULLONG_MAX - payload; + fprintf(stderr,"SYNC done after %llu bytes. Logging commands from master.\n", offset); + /* put the slave online */ + sleep(1); + sendReplconf("ACK", "0"); + } else + fprintf(stderr,"SYNC done. Logging commands from master.\n"); /* Now we can use hiredis to read the incoming protocol. */ config.output = OUTPUT_CSV; @@ -1865,11 +1924,22 @@ static void slaveMode(void) { static void getRDB(void) { int s = context->fd; int fd; - unsigned long long payload = sendSync(s); + static char eofmark[RDB_EOF_MARK_SIZE]; + static char lastbytes[RDB_EOF_MARK_SIZE]; + static int usemark = 0; + unsigned long long payload = sendSync(s, eofmark); char buf[4096]; - fprintf(stderr,"SYNC sent to master, writing %llu bytes to '%s'\n", - payload, config.rdb_filename); + if (payload == 0) { + payload = ULLONG_MAX; + memset(lastbytes,0,RDB_EOF_MARK_SIZE); + usemark = 1; + fprintf(stderr,"SYNC sent to master, writing bytes of bulk transfer until EOF marker to '%s'\n", + config.rdb_filename); + } else { + fprintf(stderr,"SYNC sent to master, writing %llu bytes to '%s'\n", + payload, config.rdb_filename); + } /* Write to file. */ if (!strcmp(config.rdb_filename,"-")) { @@ -1898,11 +1968,31 @@ static void getRDB(void) { exit(1); } payload -= nread; + + if (usemark) { + /* Update the last bytes array, and check if it matches our delimiter.*/ + if (nread >= RDB_EOF_MARK_SIZE) { + memcpy(lastbytes,buf+nread-RDB_EOF_MARK_SIZE,RDB_EOF_MARK_SIZE); + } else { + int rem = RDB_EOF_MARK_SIZE-nread; + memmove(lastbytes,lastbytes+nread,rem); + memcpy(lastbytes+rem,buf,nread); + } + if (memcmp(lastbytes,eofmark,RDB_EOF_MARK_SIZE) == 0) + break; + } + } + if (usemark) { + payload = ULLONG_MAX - payload - RDB_EOF_MARK_SIZE; + if (ftruncate(fd, payload) == -1) + fprintf(stderr,"ftruncate failed: %s.\n", strerror(errno)); + fprintf(stderr,"Transfer finished with success after %llu bytes\n", payload); + } else { + fprintf(stderr,"Transfer finished with success.\n"); } close(s); /* Close the file descriptor ASAP as fsync() may take time. */ fsync(fd); close(fd); - fprintf(stderr,"Transfer finished with success.\n"); exit(0); } @@ -2893,12 +2983,14 @@ int main(int argc, char **argv) { /* Slave mode */ if (config.slave_mode) { if (cliConnect(0) == REDIS_ERR) exit(1); + sendCapa(); slaveMode(); } /* Get RDB mode. */ if (config.getrdb_mode) { if (cliConnect(0) == REDIS_ERR) exit(1); + sendCapa(); getRDB(); } From 1050774a5b4ae85cd28b84cf95fa95b7b8ae980d Mon Sep 17 00:00:00 2001 From: antirez Date: Wed, 6 Feb 2019 12:39:11 +0100 Subject: [PATCH 02/47] ACL: initial design for ACLLoadFromFile() function. --- src/acl.c | 79 +++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 79 insertions(+) diff --git a/src/acl.c b/src/acl.c index db2f291c6..1a721e628 100644 --- a/src/acl.c +++ b/src/acl.c @@ -1009,6 +1009,85 @@ int ACLLoadConfiguredUsers(void) { return C_OK; } +/* This function loads the ACL from the specified filename: every line + * is validated and shold be either empty or in the format used to specify + * users in the redis.conf configuration or in the ACL file, that is: + * + * user ... rules ... + * + * Note that this function considers comments starting with '#' as errors + * because the ACL file is meant to be rewritten, and comments would be + * lost after the rewrite. Yet empty lines are allowed to avoid being too + * strict. + * + * One important part of implementing ACL LOAD, that uses this function, is + * to avoid ending with broken rules if the ACL file is invalid for some + * reason, so the function will attempt to validate the rules before loading + * each user. For every line that will be found broken the function will + * collect an error message. All the valid lines will be correctly processed. + * + * At the end of the process, if no errors were found in the whole file then + * NULL is returned. Otherwise an SDS string describing in a single line + * a description of all the issues found is returned. */ +sds ACLLoadFromFile(const char *filename) { + FILE *fp; + char buf[1024]; + + /* Open the ACL file. */ + if ((fp = fopen(filename,"r")) == NULL) { + sds errors = sdscatprintf(sdsempty(), + "Error loading ACLs, opening file '%s': %s", + filename, strerror(errno)); + return errors; + } + + /* Load the whole file as a single string in memory. */ + sds acls = sdsempty(); + while(fgets(buf,CONFIG_MAX_LINE+1,fp) != NULL) + acls = sdscat(acls,buf); + fclose(fp); + + /* Split the file into lines and attempt to load each line. */ + int totlines; + sds *lines, errors = sdsempty(); + lines = sdssplitlen(acls,strlen(acls),"\n",1,&totlines); + + for (int i = 0; i < totlines; i++) { + sds *argv; + int argc; + int linenum = i+1; + + lines[i] = sdstrim(lines[i]," \t\r\n"); + + /* Skip blank lines */ + if (lines[i][0] == '\0') continue; + + /* Split into arguments */ + argv = sdssplitargs(lines[i],&argc); + if (argv == NULL) { + errors = sdscatprintf(errors, + "%d: unbalanced quotes in acl line.", + linenum); + continue; + } + + /* Skip this line if the resulting command vector is empty. */ + if (argc == 0) { + sdsfreesplitres(argv,argc); + continue; + } + + /* Try to process the line. */ + } + + sdsfreesplitres(lines,totlines); + if (sdslen(errors) == 0) { + sdsfree(errors); + errors = NULL; + } + return errors; +} + /* ============================================================================= * ACL related commands * ==========================================================================*/ From 1922f590d2f3132a78bf02b7fab9c484f9eca2c7 Mon Sep 17 00:00:00 2001 From: antirez Date: Wed, 6 Feb 2019 16:19:17 +0100 Subject: [PATCH 03/47] ACL: refactoring creation of unlinked users. --- src/acl.c | 23 ++++++++++++++++++----- 1 file changed, 18 insertions(+), 5 deletions(-) diff --git a/src/acl.c b/src/acl.c index 1a721e628..e19c26eb3 100644 --- a/src/acl.c +++ b/src/acl.c @@ -185,6 +185,23 @@ user *ACLCreateUser(const char *name, size_t namelen) { return u; } +/* This function should be called when we need an unlinked "fake" user + * we can use in order to validate ACL rules or for other similar reasons. + * The user will not get linked to the Users radix tree. The returned + * user should be released with ACLFreeUser() as usually. */ +user *ACLCreateUnlinkedUser(void) { + char username[64]; + for (int j = 0; ; j++) { + snprintf(username,sizeof(username),"__fakeuser:%d__",j); + user *fakeuser = ACLCreateUser(username,strlen(username)); + if (fakeuser == NULL) continue; + int retval = raxRemove(Users,(unsigned char*) username, + strlen(username),NULL); + serverAssert(retval != 0); + return fakeuser; + } +} + /* 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) { @@ -944,11 +961,7 @@ int ACLAppendUserForLoading(sds *argv, int argc, int *argc_err) { /* Try to apply the user rules in a fake user to see if they * are actually valid. */ - 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); + user *fakeuser = ACLCreateUnlinkedUser(); for (int j = 2; j < argc; j++) { if (ACLSetUser(fakeuser,argv[j],sdslen(argv[j])) == C_ERR) { From dc8605f944c49ca3aee4e8a59c967a2ab2fd4db0 Mon Sep 17 00:00:00 2001 From: antirez Date: Wed, 6 Feb 2019 16:44:55 +0100 Subject: [PATCH 04/47] ACL: now ACLLoadFromFile() validates against fake user. --- src/acl.c | 39 ++++++++++++++++++++++++++++++++++----- 1 file changed, 34 insertions(+), 5 deletions(-) diff --git a/src/acl.c b/src/acl.c index e19c26eb3..2fbc564ab 100644 --- a/src/acl.c +++ b/src/acl.c @@ -1065,6 +1065,10 @@ sds ACLLoadFromFile(const char *filename) { sds *lines, errors = sdsempty(); lines = sdssplitlen(acls,strlen(acls),"\n",1,&totlines); + /* We need a fake user to validate the rules before making changes + * to the real user mentioned in the ACL line. */ + user *fakeuser = ACLCreateUnlinkedUser(); + for (int i = 0; i < totlines; i++) { sds *argv; int argc; @@ -1079,8 +1083,8 @@ sds ACLLoadFromFile(const char *filename) { argv = sdssplitargs(lines[i],&argc); if (argv == NULL) { errors = sdscatprintf(errors, - "%d: unbalanced quotes in acl line.", - linenum); + "%d: unbalanced quotes in acl line. ", + linenum); continue; } @@ -1090,15 +1094,40 @@ sds ACLLoadFromFile(const char *filename) { continue; } - /* Try to process the line. */ + /* The line should start with the "user" keyword. */ + if (strcmp(argv[0],"user")) { + errors = sdscatprintf(errors, + "%d: line should start with user keyword. ", + linenum); + continue; + } + + /* Try to process the line using the fake user to validate iif + * the rules are able to apply cleanly. */ + ACLSetUser(fakeuser,"reset",-1); + int j; + for (j = 2; j < argc; j++) { + if (ACLSetUser(fakeuser,argv[j],sdslen(argv[j])) != C_OK) { + char *errmsg = ACLSetUserStringError(); + errors = sdscatprintf(errors, + "%d: error in ACL: %s. ", + linenum, errmsg); + continue; + } + } + if (j != argc) continue; /* Error in ACL rules, don't apply. */ + + /* We can finally lookup the user and apply the rule. */ } + ACLFreeUser(fakeuser); sdsfreesplitres(lines,totlines); if (sdslen(errors) == 0) { sdsfree(errors); - errors = NULL; + return NULL; + } else { + return sdstrim(errors," "); } - return errors; } /* ============================================================================= From a980f6168e55fbd2ddcb22c249f8e62257c4a01d Mon Sep 17 00:00:00 2001 From: antirez Date: Thu, 7 Feb 2019 12:04:25 +0100 Subject: [PATCH 05/47] ACL: fix and complete ACLLoadFromFile() loading step. --- src/acl.c | 27 +++++++++++++++++++++++---- 1 file changed, 23 insertions(+), 4 deletions(-) diff --git a/src/acl.c b/src/acl.c index 2fbc564ab..1c6e9a0c5 100644 --- a/src/acl.c +++ b/src/acl.c @@ -1095,10 +1095,12 @@ sds ACLLoadFromFile(const char *filename) { } /* The line should start with the "user" keyword. */ - if (strcmp(argv[0],"user")) { + if (strcmp(argv[0],"user") || argc < 2) { errors = sdscatprintf(errors, - "%d: line should start with user keyword. ", + "%d: line should start with user keyword followed " + "by the username. ", linenum); + sdsfreesplitres(argv,argc); continue; } @@ -1115,9 +1117,26 @@ sds ACLLoadFromFile(const char *filename) { continue; } } - if (j != argc) continue; /* Error in ACL rules, don't apply. */ + if (j != argc) { + sdsfreesplitres(argv,argc); + continue; /* Error in ACL rules, don't apply. */ + } - /* We can finally lookup the user and apply the rule. */ + /* We can finally lookup the user and apply the rule. If the + * user already exists we always reset it to start. */ + user *u = ACLCreateUser(argv[1],sdslen(argv[1])); + if (!u) { + u = ACLGetUserByName(argv[1],sdslen(argv[1])); + serverAssert(u != NULL); + ACLSetUser(u,"reset",-1); + } + + /* Note that the same rules already applied to the fake user, so + * we just assert that everything goess well: it should. */ + for (j = 2; j < argc; j++) + serverAssert(ACLSetUser(fakeuser,argv[j],sdslen(argv[j]) == C_OK); + + sdsfreesplitres(argv,argc); } ACLFreeUser(fakeuser); From dfb8b08f30bb48e97264b6cafcad12c1d136c84b Mon Sep 17 00:00:00 2001 From: antirez Date: Thu, 7 Feb 2019 12:20:30 +0100 Subject: [PATCH 06/47] ACL: implement LOAD subcommand plus some minor rafactoring. --- src/acl.c | 29 ++++++++++++++++++++++++----- 1 file changed, 24 insertions(+), 5 deletions(-) diff --git a/src/acl.c b/src/acl.c index 1c6e9a0c5..d6b22f7c7 100644 --- a/src/acl.c +++ b/src/acl.c @@ -758,10 +758,9 @@ sds ACLDefaultUserFirstPassword(void) { return listNodeValue(first); } -/* Initialization of the ACL subsystem. */ -void ACLInit(void) { - Users = raxNew(); - UsersToLoad = listCreate(); +/* Initialize the default user, that will always exist for all the process + * lifetime. */ +void ACLInitDefaultUser(void) { DefaultUser = ACLCreateUser("default",7); ACLSetUser(DefaultUser,"+@all",-1); ACLSetUser(DefaultUser,"~*",-1); @@ -769,6 +768,13 @@ void ACLInit(void) { ACLSetUser(DefaultUser,"nopass",-1); } +/* Initialization of the ACL subsystem. */ +void ACLInit(void) { + Users = raxNew(); + UsersToLoad = listCreate(); + ACLInitDefaultUser(); +} + /* Check the username and password pair and return C_OK if they are valid, * otherwise C_ERR is returned and errno is set to: * @@ -1134,7 +1140,7 @@ sds ACLLoadFromFile(const char *filename) { /* Note that the same rules already applied to the fake user, so * we just assert that everything goess well: it should. */ for (j = 2; j < argc; j++) - serverAssert(ACLSetUser(fakeuser,argv[j],sdslen(argv[j]) == C_OK); + serverAssert(ACLSetUser(fakeuser,argv[j],sdslen(argv[j])) == C_OK); sdsfreesplitres(argv,argc); } @@ -1297,6 +1303,19 @@ void aclCommand(client *c) { } else { addReplyNull(c); } + } else if (!strcasecmp(sub,"load")) { + if (server.acl_filename[0] == '\0') { + addReplyError(c,"This Redis instance is not configured to use an ACL file. You may want to specify users via the ACL SETUSER command and then issue a CONFIG REWRITE (assuming you have a Redis configuration file set) in order to store users in the Redis configuration."); + return; + } else { + sds errors = ACLLoadFromFile(server.acl_filename); + if (errors == NULL) { + addReply(c,shared.ok); + } else { + addReplyError(c,errors); + sdsfree(errors); + } + } } else if (!strcasecmp(sub,"help")) { const char *help[] = { "LIST -- Show user details in config file format.", From f7b86d2b8fa41f285f5bfeb45538bd1c7e5825ff Mon Sep 17 00:00:00 2001 From: antirez Date: Thu, 7 Feb 2019 12:57:21 +0100 Subject: [PATCH 07/47] ACL: WIP: preserve the old config on loading errors. --- src/acl.c | 68 +++++++++++++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 66 insertions(+), 2 deletions(-) diff --git a/src/acl.c b/src/acl.c index d6b22f7c7..45c875de9 100644 --- a/src/acl.c +++ b/src/acl.c @@ -90,6 +90,7 @@ struct ACLUserFlag { void ACLResetSubcommandsForCommand(user *u, unsigned long id); void ACLResetSubcommands(user *u); +void ACLAddAllowedSubcommand(user *u, unsigned long id, const char *sub); /* ============================================================================= * Helper functions for the rest of the ACL implementation @@ -163,6 +164,11 @@ void ACLListFreeSds(void *item) { sdsfree(item); } +/* Method to duplicate list elements from ACL users password/ptterns lists. */ +void *ACLListDupSds(void *item) { + return sdsdup(item); +} + /* Create a new user with the specified name, store it in the list * of users (the Users global radix tree), and returns a reference to * the structure representing the user. @@ -178,8 +184,10 @@ user *ACLCreateUser(const char *name, size_t namelen) { u->patterns = listCreate(); listSetMatchMethod(u->passwords,ACLListMatchSds); listSetFreeMethod(u->passwords,ACLListFreeSds); + listSetDupMethod(u->passwords,ACLListDupSds); listSetMatchMethod(u->patterns,ACLListMatchSds); listSetFreeMethod(u->patterns,ACLListFreeSds); + listSetDupMethod(u->patterns,ACLListDupSds); memset(u->allowed_commands,0,sizeof(u->allowed_commands)); raxInsert(Users,(unsigned char*)name,namelen,u,NULL); return u; @@ -212,6 +220,38 @@ void ACLFreeUser(user *u) { zfree(u); } +/* Copy the user ACL rules from the source user 'src' to the destination + * user 'dst' so that at the end of the process they'll have exactly the + * same rules (but the names will continue to be the original ones). */ +void ACLCopyUser(user *dst, user *src) { + listRelease(dst->passwords); + listRelease(dst->patterns); + dst->passwords = listDup(src->passwords); + dst->patterns = listDup(src->patterns); + memcpy(dst->allowed_commands,src->allowed_commands, + sizeof(dst->allowed_commands)); + dst->flags = src->flags; + ACLResetSubcommands(dst); + /* Copy the allowed subcommands array of array of SDS strings. */ + if (src->allowed_subcommands) { + for (int j = 0; j < USER_COMMAND_BITS_COUNT; j++) { + if (src->allowed_subcommands[j]) { + for (int i = 0; src->allowed_subcommands[j][i]; i++) + { + ACLAddAllowedSubcommand(dst, j, + src->allowed_subcommands[j][i]); + } + } + } + } +} + +/* Free all the users registered in the radix tree 'users' and free the + * radix tree itself. */ +void ACLFreeUsersSet(rax *users) { + /* TODO */ +} + /* 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 @@ -1043,7 +1083,10 @@ int ACLLoadConfiguredUsers(void) { * to avoid ending with broken rules if the ACL file is invalid for some * reason, so the function will attempt to validate the rules before loading * each user. For every line that will be found broken the function will - * collect an error message. All the valid lines will be correctly processed. + * collect an error message. + * + * IMPORTANT: If there is at least a single error, nothing will be loaded + * and the rules will remain exactly as they were. * * At the end of the process, if no errors were found in the whole file then * NULL is returned. Otherwise an SDS string describing in a single line @@ -1075,6 +1118,14 @@ sds ACLLoadFromFile(const char *filename) { * to the real user mentioned in the ACL line. */ user *fakeuser = ACLCreateUnlinkedUser(); + /* We do all the loading in a fresh insteance of the Users radix tree, + * so if there are errors loading the ACL file we can rollback to the + * old version. */ + rax *old_users = Users; + Users = raxNew(); + ACLInitDefaultUser(); + + /* Load each line of the file. */ for (int i = 0; i < totlines; i++) { sds *argv; int argc; @@ -1147,11 +1198,24 @@ sds ACLLoadFromFile(const char *filename) { ACLFreeUser(fakeuser); sdsfreesplitres(lines,totlines); + + /* Chec if we found errors and react accordingly. */ if (sdslen(errors) == 0) { + /* The default user pointer is referenced in different places: instead + * of replacing such occurrences it is much simpler to copy the new + * default user configuration in the old one. */ + user *new = ACLGetUserByName("default",7); + ACLCopyUser(DefaultUser,new); + ACLFreeUser(new); + raxInsert(Users,(unsigned char*)"default",7,DefaultUser,NULL); + sdsfree(errors); return NULL; } else { - return sdstrim(errors," "); + ACLFreeUsersSet(Users); + Users = old_users; + errors = sdscat(errors,"WARNING: ACL errors detected, no change to the previously active ACL rules was performed"); + return errors; } } From 61c7688b75aaf840a3273ad9685d5676bdbed52b Mon Sep 17 00:00:00 2001 From: antirez Date: Thu, 7 Feb 2019 16:20:42 +0100 Subject: [PATCH 08/47] ACL: fix a few ACLLoadFromFile() errors and finish ACLFreeUsersSet(). --- src/acl.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/acl.c b/src/acl.c index 45c875de9..3133bfecf 100644 --- a/src/acl.c +++ b/src/acl.c @@ -249,7 +249,7 @@ void ACLCopyUser(user *dst, user *src) { /* Free all the users registered in the radix tree 'users' and free the * radix tree itself. */ void ACLFreeUsersSet(rax *users) { - /* TODO */ + raxFreeWithCallback(users,(void(*)(void*))ACLFreeUser); } /* Given a command ID, this function set by reference 'word' and 'bit' @@ -1208,7 +1208,8 @@ sds ACLLoadFromFile(const char *filename) { ACLCopyUser(DefaultUser,new); ACLFreeUser(new); raxInsert(Users,(unsigned char*)"default",7,DefaultUser,NULL); - + raxRemove(old_users,(unsigned char*)"default",7,NULL); + ACLFreeUsersSet(old_users); sdsfree(errors); return NULL; } else { From 735cb69f12f720382ef695925f0083b4aa772788 Mon Sep 17 00:00:00 2001 From: antirez Date: Thu, 7 Feb 2019 16:47:14 +0100 Subject: [PATCH 09/47] ACL: add assertion and fix comment typo. --- src/acl.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/acl.c b/src/acl.c index 3133bfecf..12253f6cf 100644 --- a/src/acl.c +++ b/src/acl.c @@ -1199,12 +1199,13 @@ sds ACLLoadFromFile(const char *filename) { ACLFreeUser(fakeuser); sdsfreesplitres(lines,totlines); - /* Chec if we found errors and react accordingly. */ + /* Check if we found errors and react accordingly. */ if (sdslen(errors) == 0) { /* The default user pointer is referenced in different places: instead * of replacing such occurrences it is much simpler to copy the new * default user configuration in the old one. */ user *new = ACLGetUserByName("default",7); + serverAssert(new != NULL); ACLCopyUser(DefaultUser,new); ACLFreeUser(new); raxInsert(Users,(unsigned char*)"default",7,DefaultUser,NULL); From 68d127d7b6856d7680e04bff999453b74d69dd9b Mon Sep 17 00:00:00 2001 From: antirez Date: Thu, 7 Feb 2019 16:53:35 +0100 Subject: [PATCH 10/47] ACL: fix fgets wrong buffer size. --- src/acl.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/acl.c b/src/acl.c index 12253f6cf..b17036df7 100644 --- a/src/acl.c +++ b/src/acl.c @@ -1105,7 +1105,7 @@ sds ACLLoadFromFile(const char *filename) { /* Load the whole file as a single string in memory. */ sds acls = sdsempty(); - while(fgets(buf,CONFIG_MAX_LINE+1,fp) != NULL) + while(fgets(buf,sizeof(buf),fp) != NULL) acls = sdscat(acls,buf); fclose(fp); From 434489abb7d033f3b27ba28697402fe75ae385c0 Mon Sep 17 00:00:00 2001 From: antirez Date: Thu, 7 Feb 2019 17:00:35 +0100 Subject: [PATCH 11/47] ACL: ACLLoadFromFile(), restore DefaultUser global. --- src/acl.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/acl.c b/src/acl.c index b17036df7..4ab660e9f 100644 --- a/src/acl.c +++ b/src/acl.c @@ -1122,6 +1122,7 @@ sds ACLLoadFromFile(const char *filename) { * so if there are errors loading the ACL file we can rollback to the * old version. */ rax *old_users = Users; + user *old_default_user = DefaultUser; Users = raxNew(); ACLInitDefaultUser(); @@ -1198,6 +1199,7 @@ sds ACLLoadFromFile(const char *filename) { ACLFreeUser(fakeuser); sdsfreesplitres(lines,totlines); + DefaultUser = old_default_user; /* This pointer must never change. */ /* Check if we found errors and react accordingly. */ if (sdslen(errors) == 0) { From b1db13d8fa807b155f5bfaa25a002286af9a4ea2 Mon Sep 17 00:00:00 2001 From: antirez Date: Thu, 7 Feb 2019 17:07:35 +0100 Subject: [PATCH 12/47] ACL: ACLLoadFromFile(): several errors fixed to make it work. --- src/acl.c | 22 +++++++++++++--------- 1 file changed, 13 insertions(+), 9 deletions(-) diff --git a/src/acl.c b/src/acl.c index 4ab660e9f..fecd33e8a 100644 --- a/src/acl.c +++ b/src/acl.c @@ -1141,8 +1141,8 @@ sds ACLLoadFromFile(const char *filename) { argv = sdssplitargs(lines[i],&argc); if (argv == NULL) { errors = sdscatprintf(errors, - "%d: unbalanced quotes in acl line. ", - linenum); + "%s:%d: unbalanced quotes in acl line. ", + server.acl_filename, linenum); continue; } @@ -1155,8 +1155,8 @@ sds ACLLoadFromFile(const char *filename) { /* The line should start with the "user" keyword. */ if (strcmp(argv[0],"user") || argc < 2) { errors = sdscatprintf(errors, - "%d: line should start with user keyword followed " - "by the username. ", + "%s:%d should start with user keyword followed " + "by the username. ", server.acl_filename, linenum); sdsfreesplitres(argv,argc); continue; @@ -1170,14 +1170,18 @@ sds ACLLoadFromFile(const char *filename) { if (ACLSetUser(fakeuser,argv[j],sdslen(argv[j])) != C_OK) { char *errmsg = ACLSetUserStringError(); errors = sdscatprintf(errors, - "%d: error in ACL: %s. ", - linenum, errmsg); + "%s:%d: %s. ", + server.acl_filename, linenum, errmsg); continue; } } - if (j != argc) { + + /* Apply the rule to the new users set only if so far there + * are no errors, otherwise it's useless since we are going + * to discard the new users set anyway. */ + if (sdslen(errors) != 0) { sdsfreesplitres(argv,argc); - continue; /* Error in ACL rules, don't apply. */ + continue; } /* We can finally lookup the user and apply the rule. If the @@ -1192,7 +1196,7 @@ sds ACLLoadFromFile(const char *filename) { /* Note that the same rules already applied to the fake user, so * we just assert that everything goess well: it should. */ for (j = 2; j < argc; j++) - serverAssert(ACLSetUser(fakeuser,argv[j],sdslen(argv[j])) == C_OK); + serverAssert(ACLSetUser(u,argv[j],sdslen(argv[j])) == C_OK); sdsfreesplitres(argv,argc); } From 87ce87e68c7e307bfe4e7fb2dc3d4d5499bef4f4 Mon Sep 17 00:00:00 2001 From: antirez Date: Thu, 7 Feb 2019 17:20:03 +0100 Subject: [PATCH 13/47] ACL: load ACL file at startup. Prevent silly configurations. --- src/acl.c | 33 +++++++++++++++++++++++++++++++++ src/server.c | 6 +----- src/server.h | 1 + 3 files changed, 35 insertions(+), 5 deletions(-) diff --git a/src/acl.c b/src/acl.c index fecd33e8a..4ae5830bd 100644 --- a/src/acl.c +++ b/src/acl.c @@ -1227,6 +1227,39 @@ sds ACLLoadFromFile(const char *filename) { } } +/* This function is called once the server is already running, modules are + * loaded, and we are ready to start, in order to load the ACLs either from + * the pending list of users defined in redis.conf, or from the ACL file. + * The function will just exit with an error if the user is trying to mix + * both the loading methods. */ +void ACLLoadUsersAtStartup(void) { + if (server.acl_filename[0] != '\0' && listLength(UsersToLoad) != 0) { + serverLog(LL_WARNING, + "Configuring Redis with users defined in redis.conf and at " + "the same setting an ACL file path is invalid. This setup " + "is very likely to lead to configuration errors and security " + "holes, please define either an ACL file or declare users " + "directly in your redis.conf, but not both."); + exit(1); + } + + if (ACLLoadConfiguredUsers() == C_ERR) { + serverLog(LL_WARNING, + "Critical error while loading ACLs. Exiting."); + exit(1); + } + + if (server.acl_filename[0] != '\0') { + sds errors = ACLLoadFromFile(server.acl_filename); + if (errors) { + serverLog(LL_WARNING, + "Aborting Redis startup because of ACL errors: %s", errors); + sdsfree(errors); + exit(1); + } + } +} + /* ============================================================================= * ACL related commands * ==========================================================================*/ diff --git a/src/server.c b/src/server.c index de84e430e..c257d0573 100644 --- a/src/server.c +++ b/src/server.c @@ -4908,11 +4908,7 @@ int main(int argc, char **argv) { linuxMemoryWarnings(); #endif moduleLoadFromQueue(); - if (ACLLoadConfiguredUsers() == C_ERR) { - serverLog(LL_WARNING, - "Critical error while loading ACLs. Exiting."); - exit(1); - } + ACLLoadUsersAtStartup(); loadDataFromDisk(); if (server.cluster_enabled) { if (verifyClusterConfigWithData() == C_ERR) { diff --git a/src/server.h b/src/server.h index d2c6aa1e0..59f7cbe10 100644 --- a/src/server.h +++ b/src/server.h @@ -1746,6 +1746,7 @@ int ACLAppendUserForLoading(sds *argv, int argc, int *argc_err); char *ACLSetUserStringError(void); int ACLLoadConfiguredUsers(void); sds ACLDescribeUser(user *u); +void ACLLoadUsersAtStartup(void); /* Sorted sets data type */ From 4a7062f9bd77ca75af37454bf5a77d86b8924ff6 Mon Sep 17 00:00:00 2001 From: antirez Date: Fri, 8 Feb 2019 09:52:07 +0100 Subject: [PATCH 14/47] ACL: some documentation inside redis.conf. --- redis.conf | 88 ++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 88 insertions(+) diff --git a/redis.conf b/redis.conf index d1ced7eb3..71214b620 100644 --- a/redis.conf +++ b/redis.conf @@ -501,6 +501,94 @@ replica-priority 100 # 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. +# Redis ACL users are defined in the following format: +# +# user ... acl rules ... +# +# For example: +# +# user worker +@list +@connection ~jobs:* on >ffa9203c493aa99 +# +# The special username "default" is used for new connections. If this user +# has the "nopass" rule, then new connections will be immediately authenticated +# as the "default" user without the need of any password provided via the +# AUTH command. Otherwise if the "default" user is not flagged with "nopass" +# the connections will start in not authenticated state, and will require +# AUTH (or the HELLO command AUTH option) in order to be authenticated and +# start to work. +# +# The ACL rules that describe what an user can do are the following: +# +# on Enable the user: it is possible to authenticate as this user. +# off Disable the user: it's no longer possible to authenticate +# with this user, however the already authenticated connections +# will still work. +# + Allow the execution of that command +# - Disallow the execution of that command +# +@ Allow the execution of all the commands in such category +# with valid categories are like @admin, @set, @sortedset, ... +# and so forth, see the full list in the server.c file where +# the Redis command table is described and defined. +# The special category @all means all the commands, but currently +# present in the server, and that will be loaded in the future +# via modules. +# +|subcommand Allow a specific subcommand of an otherwise +# disabled command. Note that this form is not +# allowed as negative like -DEBUG|SEGFAULT, but +# only additive starting with "+". +# allcommands Alias for +@all. Note that it implies the ability to execute +# all the future commands loaded via the modules system. +# nocommands Alias for -@all. +# ~ Add a pattern of keys that can be mentioned as part of +# commands. For instance ~* allows all the keys. The pattern +# is a glob-style pattern like the one of KEYS. +# It is possible to specify multiple patterns. +# allkeys Alias for ~* +# resetkeys Flush the list of allowed keys patterns. +# > Add this passowrd 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). +# < 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 +# password will work against this user. If this directive is +# used for the default user, every new connection will be +# immediately authenticated with the default user without +# any explicit AUTH command required. Note that the "resetpass" +# directive will clear this condition. +# resetpass Flush the list of allowed passwords. Moreover removes the +# "nopass" status. After "resetpass" the user has no associated +# passwords and there is no way to authenticate without adding +# some password (or setting it as "nopass" later). +# reset Performs the following actions: resetpass, resetkeys, off, +# -@all. The user returns to the same state it has immediately +# after its creation. +# +# ACL rules can be specified in any order: for instance you can start with +# passwords, then flags, or key patterns. However note that the additive +# and subtractive rules will CHANGE MEANING depending on the ordering. +# For instance see the following example: +# +# user alice on +@all -DEBUG ~* >somepassword +# +# This will allow "alice" to use all the commands with the exception of the +# DEBUG command, since +@all added all the commands to the set of the commands +# alice can use, and later DEBUG was removed. However if we invert the order +# of two ACL rules the result will be different: +# +# user alice on -DEBUG +@all ~* >somepassword +# +# Now DEBUG was removed when alice had yet no commands in the set of allowed +# commands, later all the commands are added, so the user will be able to +# execute everything. +# +# Basically ACL rules are processed left-to-right. +# +# For more information about ACL configuration please refer to +# the Redis web site at https://redis.io/topics/acl + +# Using an external ACL file +# # 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 From 66fd5e058f7353cc45936957deafb37e4d4f2e60 Mon Sep 17 00:00:00 2001 From: antirez Date: Fri, 8 Feb 2019 11:50:39 +0100 Subject: [PATCH 15/47] ACL: ignore modules commands when adding categories. We can't trust modules commands flagging, so module commands must be always explicitly added, with the exception of +@all that will include everything. However something like +@readonly should not include command from modules that may be potentially dangerous: our categories must be safe and reliable and modules may not be like that. --- src/acl.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/acl.c b/src/acl.c index 4ae5830bd..159a3507a 100644 --- a/src/acl.c +++ b/src/acl.c @@ -313,6 +313,7 @@ int ACLSetUserCommandBitsForCategory(user *u, const char *category, int value) { dictEntry *de; while ((de = dictNext(di)) != NULL) { struct redisCommand *cmd = dictGetVal(de); + if (cmd->flags & CMD_MODULE) continue; /* Ignore modules commands. */ if (cmd->flags & cflag) { ACLSetUserCommandBit(u,cmd->id,value); ACLResetSubcommandsForCommand(u,cmd->id); From fcd5ff1a768b9451bac1dafd92d72086a167a410 Mon Sep 17 00:00:00 2001 From: antirez Date: Fri, 8 Feb 2019 12:38:41 +0100 Subject: [PATCH 16/47] ACL: add arity check in ACL command where missing. --- src/acl.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/acl.c b/src/acl.c index 159a3507a..2e2bd24a5 100644 --- a/src/acl.c +++ b/src/acl.c @@ -1403,13 +1403,13 @@ void aclCommand(client *c) { } } raxStop(&ri); - } else if (!strcasecmp(sub,"whoami")) { + } else if (!strcasecmp(sub,"whoami") && c->argc == 2) { if (c->user != NULL) { addReplyBulkCBuffer(c,c->user->name,sdslen(c->user->name)); } else { addReplyNull(c); } - } else if (!strcasecmp(sub,"load")) { + } else if (!strcasecmp(sub,"load") && c->argc == 2) { if (server.acl_filename[0] == '\0') { addReplyError(c,"This Redis instance is not configured to use an ACL file. You may want to specify users via the ACL SETUSER command and then issue a CONFIG REWRITE (assuming you have a Redis configuration file set) in order to store users in the Redis configuration."); return; From 48423054ead6b6f6c183efd9eac0a6e8fca3de05 Mon Sep 17 00:00:00 2001 From: antirez Date: Fri, 8 Feb 2019 12:40:42 +0100 Subject: [PATCH 17/47] ACL: add command fingerprint for CAT subcommand. --- src/acl.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/acl.c b/src/acl.c index 2e2bd24a5..3d73e0843 100644 --- a/src/acl.c +++ b/src/acl.c @@ -1268,7 +1268,9 @@ void ACLLoadUsersAtStartup(void) { /* ACL -- show and modify the configuration of ACL users. * ACL HELP * ACL LIST - * ACL SETUSER ... user attribs ... + * ACL USERS + * ACL CAT [] + * ACL SETUSER ... acl rules ... * ACL DELUSER * ACL GETUSER */ @@ -1429,6 +1431,8 @@ void aclCommand(client *c) { "SETUSER [attribs ...] -- Create or modify a user.", "GETUSER -- Get the user details.", "DELUSER -- Delete a user.", +"CAT -- List available categories.", +"CAT -- List commands inside category.", "WHOAMI -- Return the current connection username.", NULL }; From 5cfa46fd142dd445b55b8f349e0d8205ec4c0012 Mon Sep 17 00:00:00 2001 From: "zhaozhao.zz" Date: Mon, 11 Feb 2019 22:24:15 +0800 Subject: [PATCH 18/47] ACL: kill the old users clients after load aclfile --- src/acl.c | 48 ++++++++++++++++++++++++++---------------------- 1 file changed, 26 insertions(+), 22 deletions(-) diff --git a/src/acl.c b/src/acl.c index 3d73e0843..13e6ac49a 100644 --- a/src/acl.c +++ b/src/acl.c @@ -220,6 +220,30 @@ void ACLFreeUser(user *u) { zfree(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. */ +void ACLFreeUserAndKillClients(user *u) { + 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); + } + } +} + /* Copy the user ACL rules from the source user 'src' to the destination * user 'dst' so that at the end of the process they'll have exactly the * same rules (but the names will continue to be the original ones). */ @@ -249,7 +273,7 @@ void ACLCopyUser(user *dst, user *src) { /* Free all the users registered in the radix tree 'users' and free the * radix tree itself. */ void ACLFreeUsersSet(rax *users) { - raxFreeWithCallback(users,(void(*)(void*))ACLFreeUser); + raxFreeWithCallback(users,(void(*)(void*))ACLFreeUserAndKillClients); } /* Given a command ID, this function set by reference 'word' and 'bit' @@ -1304,27 +1328,7 @@ void aclCommand(client *c) { 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); - } - } + ACLFreeUserAndKillClients(u); deleted++; } } From 3822a465f291864630e8f5e2c1af5862d9b94c01 Mon Sep 17 00:00:00 2001 From: antirez Date: Mon, 11 Feb 2019 16:28:31 +0100 Subject: [PATCH 19/47] ACL: ACLFreeUserAndKillClients(): free user later. Soon or later we may have code in freeClient() that may have to deal with ACLs. Imagine for instance the command proposed multiple times (not sure if this will ever be accepted but still...): ONCLOSE DEL mykey Accumulating commands to run when a client is disconnected. Now the function is compatible with such use cases. Related to #5829. --- src/acl.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/acl.c b/src/acl.c index 13e6ac49a..7dc23f1c5 100644 --- a/src/acl.c +++ b/src/acl.c @@ -224,7 +224,6 @@ void ACLFreeUser(user *u) { * connections in order to kill all the pending ones that * are authenticated with such user. */ void ACLFreeUserAndKillClients(user *u) { - ACLFreeUser(u); listIter li; listNode *ln; listRewind(server.clients,&li); @@ -242,6 +241,7 @@ void ACLFreeUserAndKillClients(user *u) { freeClientAsync(c); } } + ACLFreeUser(u); } /* Copy the user ACL rules from the source user 'src' to the destination From 46243329d404a864787484e065cfab6232c112fc Mon Sep 17 00:00:00 2001 From: antirez Date: Mon, 11 Feb 2019 16:47:02 +0100 Subject: [PATCH 20/47] ACL: refactor+fix AUTH check in processCommand(). The part that is fixed is that now if the default user is off whatever is its configuration the user is not considered authenticated. --- src/server.c | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/src/server.c b/src/server.c index c257d0573..289b1bd23 100644 --- a/src/server.c +++ b/src/server.c @@ -3298,14 +3298,17 @@ int processCommand(client *c) { return C_OK; } - /* Check if the user is authenticated */ - if (!(DefaultUser->flags & USER_FLAG_NOPASS) && - !c->authenticated && - (c->cmd->proc != authCommand || c->cmd->proc == helloCommand)) - { - flagTransaction(c); - addReply(c,shared.noautherr); - return C_OK; + /* Check if the user is authenticated. This check is skipped in case + * the default user is flagged as "nopass" and is active. */ + int auth_required = !(DefaultUser->flags & USER_FLAG_NOPASS) && + !c->authenticated; + if (auth_required || DefaultUser->flags & USER_FLAG_DISABLED) { + /* AUTH and HELLO are valid even in non authenticated state. */ + if (c->cmd->proc != authCommand || c->cmd->proc == helloCommand) { + flagTransaction(c); + addReply(c,shared.noautherr); + return C_OK; + } } /* Check if the user can run this command according to the current From 7983f6e8840cf41ed0389396b90b99fa89ac9ca4 Mon Sep 17 00:00:00 2001 From: antirez Date: Mon, 11 Feb 2019 17:00:51 +0100 Subject: [PATCH 21/47] ACL: return error when removing a non existing password. Otherwise it's very simple for an human mistake to think a password is removed because of a typo in the ACL SETUSER myuser passwords,delpass); - if (ln) listDelNode(u->passwords,ln); sdsfree(delpass); + if (ln) { + listDelNode(u->passwords,ln); + } else { + errno = ENODEV; + return C_ERR; + } } else if (op[0] == '~') { if (u->flags & USER_FLAG_ALLKEYS) { errno = EEXIST; @@ -810,6 +816,9 @@ char *ACLSetUserStringError(void) { "'allkeys' flag) is not valid and does not have any " "effect. Try 'resetkeys' to start with an empty " "list of patterns"; + else if (errno == ENODEV) + errmsg = "The password you are trying to remove from the user does " + "not exist"; return errmsg; } From d2c38031dc9620a78d2a264eeda8d9bc448a1276 Mon Sep 17 00:00:00 2001 From: Chris Lamb Date: Mon, 11 Feb 2019 17:07:26 +0100 Subject: [PATCH 22/47] Don't assume the __x86_64__ pointer size to avoid warnings on x32. __x86_64__ is defined on the on the x32 architecture and the conditionals in debug.c therefore assume the size of (void*) etc: debug.c: In function 'getMcontextEip': debug.c:757:12: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast] return (void*) uc->uc_mcontext.gregs[16]; /* Linux 64 */ ^ debug.c: In function 'logRegisters': debug.c:920:21: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast] logStackContent((void**)uc->uc_mcontext.gregs[15]); We can remedy this by checking for __ILP32__ first. See: https://wiki.debian.org/ArchitectureSpecificsMemo ... for more info. --- src/debug.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/debug.c b/src/debug.c index a98bc61ad..0c6b5630c 100644 --- a/src/debug.c +++ b/src/debug.c @@ -803,7 +803,7 @@ static void *getMcontextEip(ucontext_t *uc) { #endif #elif defined(__linux__) /* Linux */ - #if defined(__i386__) + #if defined(__i386__) || defined(__ILP32__) return (void*) uc->uc_mcontext.gregs[14]; /* Linux 32 */ #elif defined(__X86_64__) || defined(__x86_64__) return (void*) uc->uc_mcontext.gregs[16]; /* Linux 64 */ @@ -915,7 +915,7 @@ void logRegisters(ucontext_t *uc) { /* Linux */ #elif defined(__linux__) /* Linux x86 */ - #if defined(__i386__) + #if defined(__i386__) || defined(__ILP32__) serverLog(LL_WARNING, "\n" "EAX:%08lx EBX:%08lx ECX:%08lx EDX:%08lx\n" From 9150d9246ee58b95af970ddf7040596454aeadb0 Mon Sep 17 00:00:00 2001 From: "zhaozhao.zz" Date: Tue, 12 Feb 2019 16:03:58 +0800 Subject: [PATCH 23/47] ACL: show client's user --- src/networking.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/networking.c b/src/networking.c index 9e62cb6be..621570d5f 100644 --- a/src/networking.c +++ b/src/networking.c @@ -1781,7 +1781,7 @@ sds catClientInfoString(sds s, client *client) { if (emask & AE_WRITABLE) *p++ = 'w'; *p = '\0'; return sdscatfmt(s, - "id=%U addr=%s fd=%i name=%s age=%I idle=%I flags=%s db=%i sub=%i psub=%i multi=%i qbuf=%U qbuf-free=%U obl=%U oll=%U omem=%U events=%s cmd=%s", + "id=%U addr=%s fd=%i name=%s age=%I idle=%I flags=%s db=%i sub=%i psub=%i multi=%i qbuf=%U qbuf-free=%U obl=%U oll=%U omem=%U events=%s cmd=%s user=%s", (unsigned long long) client->id, getClientPeerId(client), client->fd, @@ -1799,7 +1799,8 @@ sds catClientInfoString(sds s, client *client) { (unsigned long long) listLength(client->reply), (unsigned long long) getClientOutputBufferMemoryUsage(client), events, - client->lastcmd ? client->lastcmd->name : "NULL"); + client->lastcmd ? client->lastcmd->name : "NULL", + client->user ? client->user->name : ""); } sds getAllClientsInfoString(int type) { From 34f11c811d4f928a4dfe62ea87707bce2ebd924f Mon Sep 17 00:00:00 2001 From: antirez Date: Tue, 12 Feb 2019 09:44:25 +0100 Subject: [PATCH 24/47] ACL: when client->user is NULL the client is a superuser. Related to #5832. --- src/networking.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/networking.c b/src/networking.c index 621570d5f..23bc97eec 100644 --- a/src/networking.c +++ b/src/networking.c @@ -1800,7 +1800,7 @@ sds catClientInfoString(sds s, client *client) { (unsigned long long) getClientOutputBufferMemoryUsage(client), events, client->lastcmd ? client->lastcmd->name : "NULL", - client->user ? client->user->name : ""); + client->user ? client->user->name : "(superuser)"); } sds getAllClientsInfoString(int type) { From de0f42bff302dd3de2ed9b567d070c617b402d68 Mon Sep 17 00:00:00 2001 From: "zhaozhao.zz" Date: Tue, 12 Feb 2019 16:44:50 +0800 Subject: [PATCH 25/47] ACL: add masteruser configuration for replication In mostly production environment, normal user's behavior should be limited. Now in redis ACL mechanism we can do it like that: user default on +@all ~* -@dangerous nopass user admin on +@all ~* >someSeriousPassword Then the default normal user can not execute dangerous commands like FLUSHALL/KEYS. But some admin commands are in dangerous category too like PSYNC, and the configurations above will forbid replica from sync with master. Finally I think we could add a new configuration for replication, it is masteruser option, like this: masteruser admin masterauth someSeriousPassword Then replica will try AUTH admin someSeriousPassword and get privilege to execute PSYNC. If masteruser is NULL, replica would AUTH with only masterauth like before. --- src/config.c | 8 ++++++++ src/replication.c | 8 +++++++- src/server.h | 1 + 3 files changed, 16 insertions(+), 1 deletion(-) diff --git a/src/config.c b/src/config.c index fc9160d26..68a2ea227 100644 --- a/src/config.c +++ b/src/config.c @@ -395,6 +395,9 @@ void loadServerConfigFromString(char *config) { err = "repl-backlog-ttl can't be negative "; goto loaderr; } + } else if (!strcasecmp(argv[0],"masteruser") && argc == 2) { + zfree(server.masteruser); + server.masteruser = argv[1][0] ? zstrdup(argv[1]) : NULL; } else if (!strcasecmp(argv[0],"masterauth") && argc == 2) { zfree(server.masterauth); server.masterauth = argv[1][0] ? zstrdup(argv[1]) : NULL; @@ -943,6 +946,9 @@ void configSetCommand(client *c) { sds aclop = sdscatprintf(sdsempty(),">%s",(char*)o->ptr); ACLSetUser(DefaultUser,aclop,sdslen(aclop)); sdsfree(aclop); + } config_set_special_field("masteruser") { + zfree(server.masteruser); + server.masteruser = ((char*)o->ptr)[0] ? zstrdup(o->ptr) : NULL; } config_set_special_field("masterauth") { zfree(server.masterauth); server.masterauth = ((char*)o->ptr)[0] ? zstrdup(o->ptr) : NULL; @@ -1354,6 +1360,7 @@ void configGetCommand(client *c) { /* String values */ config_get_string_field("dbfilename",server.rdb_filename); + config_get_string_field("masteruser",server.masteruser); config_get_string_field("masterauth",server.masterauth); config_get_string_field("cluster-announce-ip",server.cluster_announce_ip); config_get_string_field("unixsocket",server.unixsocket); @@ -2232,6 +2239,7 @@ int rewriteConfig(char *path) { rewriteConfigDirOption(state); rewriteConfigSlaveofOption(state,"replicaof"); rewriteConfigStringOption(state,"replica-announce-ip",server.slave_announce_ip,CONFIG_DEFAULT_SLAVE_ANNOUNCE_IP); + rewriteConfigStringOption(state,"masteruser",server.masteruser,NULL); rewriteConfigStringOption(state,"masterauth",server.masterauth,NULL); rewriteConfigStringOption(state,"cluster-announce-ip",server.cluster_announce_ip,NULL); rewriteConfigYesNoOption(state,"replica-serve-stale-data",server.repl_serve_stale_data,CONFIG_DEFAULT_SLAVE_SERVE_STALE_DATA); diff --git a/src/replication.c b/src/replication.c index 3bc42d62a..9175bb420 100644 --- a/src/replication.c +++ b/src/replication.c @@ -1661,7 +1661,13 @@ void syncWithMaster(aeEventLoop *el, int fd, void *privdata, int mask) { /* AUTH with the master if required. */ if (server.repl_state == REPL_STATE_SEND_AUTH) { - if (server.masterauth) { + if (server.masteruser && server.masterauth) { + err = sendSynchronousCommand(SYNC_CMD_WRITE,fd,"AUTH", + server.masteruser,server.masterauth,NULL); + if (err) goto write_error; + server.repl_state = REPL_STATE_RECEIVE_AUTH; + return; + } else if (server.masterauth) { err = sendSynchronousCommand(SYNC_CMD_WRITE,fd,"AUTH",server.masterauth,NULL); if (err) goto write_error; server.repl_state = REPL_STATE_RECEIVE_AUTH; diff --git a/src/server.h b/src/server.h index 59f7cbe10..a396e1cf7 100644 --- a/src/server.h +++ b/src/server.h @@ -1217,6 +1217,7 @@ struct redisServer { int repl_diskless_sync; /* Send RDB to slaves sockets directly. */ int repl_diskless_sync_delay; /* Delay to start a diskless repl BGSAVE. */ /* Replication (slave) */ + char *masteruser; /* AUTH with this user and masterauth with master */ char *masterauth; /* AUTH with this password with master */ char *masterhost; /* Hostname of master */ int masterport; /* Port of master */ From d78a6fdcbd8c2fdb57352f3225f75a166540ef45 Mon Sep 17 00:00:00 2001 From: antirez Date: Tue, 12 Feb 2019 17:02:45 +0100 Subject: [PATCH 26/47] ACL: CAT subcommand implemented. --- src/acl.c | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/src/acl.c b/src/acl.c index f5e132727..e4122fd2c 100644 --- a/src/acl.c +++ b/src/acl.c @@ -1437,6 +1437,32 @@ void aclCommand(client *c) { sdsfree(errors); } } + } else if (!strcasecmp(sub,"cat") && c->argc == 2) { + void *dl = addReplyDeferredLen(c); + int j; + for (j = 0; ACLCommandCategories[j].flag != 0; j++) + addReplyBulkCString(c,ACLCommandCategories[j].name); + setDeferredArrayLen(c,dl,j); + } else if (!strcasecmp(sub,"cat") && c->argc == 3) { + uint64_t cflag = ACLGetCommandCategoryFlagByName(c->argv[2]->ptr); + if (cflag == 0) { + addReplyErrorFormat(c, "Unknown category '%s'", c->argv[2]->ptr); + return; + } + int arraylen = 0; + void *dl = addReplyDeferredLen(c); + dictIterator *di = dictGetIterator(server.orig_commands); + dictEntry *de; + while ((de = dictNext(di)) != NULL) { + struct redisCommand *cmd = dictGetVal(de); + if (cmd->flags & CMD_MODULE) continue; + if (cmd->flags & cflag) { + addReplyBulkCString(c,cmd->name); + arraylen++; + } + } + dictReleaseIterator(di); + setDeferredArrayLen(c,dl,arraylen); } else if (!strcasecmp(sub,"help")) { const char *help[] = { "LIST -- Show user details in config file format.", From 1c15e0ff20f9901346a3c25ec13eac7e4d9bf8f0 Mon Sep 17 00:00:00 2001 From: antirez Date: Tue, 12 Feb 2019 17:06:26 +0100 Subject: [PATCH 27/47] ACL: fix setting of FAST flag. --- src/server.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/server.c b/src/server.c index 289b1bd23..1a6c277e1 100644 --- a/src/server.c +++ b/src/server.c @@ -2914,10 +2914,10 @@ int populateCommandTableParseFlags(struct redisCommand *c, char *strflags) { return C_ERR; } } - - /* If it's not @fast is @slow in this binary world. */ - if (!(c->flags & CMD_CATEGORY_FAST)) c->flags |= CMD_CATEGORY_SLOW; } + /* If it's not @fast is @slow in this binary world. */ + if (!(c->flags & CMD_CATEGORY_FAST)) c->flags |= CMD_CATEGORY_SLOW; + sdsfreesplitres(argv,argc); return C_OK; } From 2c38dc6831e55f2060ecd34e4fbfa16ef8038667 Mon Sep 17 00:00:00 2001 From: antirez Date: Tue, 12 Feb 2019 18:23:00 +0100 Subject: [PATCH 28/47] ACL: Document masteruser option in redis.conf. --- redis.conf | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/redis.conf b/redis.conf index 71214b620..e4de9ac64 100644 --- a/redis.conf +++ b/redis.conf @@ -291,6 +291,17 @@ dir ./ # refuse the replica request. # # masterauth +# +# However this is not enough if you are using Redis ACLs (for Redis version +# 6 or greater), and the default user is not capable of running the PSYNC +# command and/or other commands needed for replication. In this case it's +# better to configure a special user to use with replication, and specify the +# masteruser configuration as such: +# +# masteruser +# +# When masteruser is specified, the replica will authenticate against its +# master using the new AUTH form: AUTH . # When a replica loses its connection with the master, or when the replication # is still in progress, the replica can act in two different ways: From 0eee72b620b05740c0cf58d9099ac0b375ae8c16 Mon Sep 17 00:00:00 2001 From: "zhaozhao.zz" Date: Wed, 13 Feb 2019 11:47:10 +0800 Subject: [PATCH 29/47] ACL: fix cat type format warning --- src/acl.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/acl.c b/src/acl.c index e4122fd2c..2a1b2cbc0 100644 --- a/src/acl.c +++ b/src/acl.c @@ -1446,7 +1446,7 @@ void aclCommand(client *c) { } else if (!strcasecmp(sub,"cat") && c->argc == 3) { uint64_t cflag = ACLGetCommandCategoryFlagByName(c->argv[2]->ptr); if (cflag == 0) { - addReplyErrorFormat(c, "Unknown category '%s'", c->argv[2]->ptr); + addReplyErrorFormat(c, "Unknown category '%s'", (char*)c->argv[2]->ptr); return; } int arraylen = 0; From ddff560cacdff92dedb888980d0a4010843273ea Mon Sep 17 00:00:00 2001 From: antirez Date: Wed, 13 Feb 2019 16:30:44 +0100 Subject: [PATCH 30/47] ACL: tag LASTSAVE as dangerous. That's not REALLY needed, but... right now with LASTSAVE being the only command tagged as "admin" but not "dangerous" what happens is that after rewrites the rewrite engine will produce from the rules: user default on +@all ~* -@dangerous nopass The rewrite: user default on nopass ~* +@all -@admin -@dangerous +lastsave Which is correct but will have users wondering about why LASTSAVE has something special. Since LASTSAVE after all also leaks information about the underlying server configuration, that may not be great for SAAS vendors, let's tag it as dangerous as well and forget about this issue :-) --- src/server.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/server.c b/src/server.c index 1a6c277e1..4adc7b678 100644 --- a/src/server.c +++ b/src/server.c @@ -658,7 +658,7 @@ struct redisCommand redisCommandTable[] = { 0,NULL,0,0,0,0,0,0}, {"lastsave",lastsaveCommand,1, - "read-only random fast @admin", + "read-only random fast @admin @dangerous", 0,NULL,0,0,0,0,0,0}, {"type",typeCommand,2, From 3eb2f4ca14dfe949e21f46060e46db0df8533981 Mon Sep 17 00:00:00 2001 From: "zhaozhao.zz" Date: Thu, 14 Feb 2019 00:12:10 +0800 Subject: [PATCH 31/47] ACL: show categories in COMMAND reply Adding another new filed categories at the end of command reply, it's easy to read and distinguish flags and categories, also compatible with old format. --- src/acl.c | 12 ++++++++++++ src/server.c | 6 ++++-- src/server.h | 1 + 3 files changed, 17 insertions(+), 2 deletions(-) diff --git a/src/acl.c b/src/acl.c index 2a1b2cbc0..407d86736 100644 --- a/src/acl.c +++ b/src/acl.c @@ -1480,3 +1480,15 @@ NULL addReplySubcommandSyntaxError(c); } } + +void addReplyCommandCategories(client *c, struct redisCommand *cmd) { + int flagcount = 0; + void *flaglen = addReplyDeferredLen(c); + for (int j = 0; ACLCommandCategories[j].flag != 0; j++) { + if (cmd->flags & ACLCommandCategories[j].flag) { + addReplyStatusFormat(c, "@%s", ACLCommandCategories[j].name); + flagcount++; + } + } + setDeferredSetLen(c, flaglen, flagcount); +} diff --git a/src/server.c b/src/server.c index 1a6c277e1..7d78efcab 100644 --- a/src/server.c +++ b/src/server.c @@ -3698,8 +3698,8 @@ void addReplyCommand(client *c, struct redisCommand *cmd) { if (!cmd) { addReplyNull(c); } else { - /* We are adding: command name, arg count, flags, first, last, offset */ - addReplyArrayLen(c, 6); + /* We are adding: command name, arg count, flags, first, last, offset, categories */ + addReplyArrayLen(c, 7); addReplyBulkCString(c, cmd->name); addReplyLongLong(c, cmd->arity); @@ -3729,6 +3729,8 @@ void addReplyCommand(client *c, struct redisCommand *cmd) { addReplyLongLong(c, cmd->firstkey); addReplyLongLong(c, cmd->lastkey); addReplyLongLong(c, cmd->keystep); + + addReplyCommandCategories(c,cmd); } } diff --git a/src/server.h b/src/server.h index a396e1cf7..994952654 100644 --- a/src/server.h +++ b/src/server.h @@ -1748,6 +1748,7 @@ char *ACLSetUserStringError(void); int ACLLoadConfiguredUsers(void); sds ACLDescribeUser(user *u); void ACLLoadUsersAtStartup(void); +void addReplyCommandCategories(client *c, struct redisCommand *cmd); /* Sorted sets data type */ From b53eec3126c1091966bc1b310f373c86b206d023 Mon Sep 17 00:00:00 2001 From: Madelyn Olson Date: Thu, 31 Jan 2019 18:44:17 +0000 Subject: [PATCH 32/47] Updated redis benchmark with us precision support --- src/redis-benchmark.c | 26 +++++++++++++++++++++++--- 1 file changed, 23 insertions(+), 3 deletions(-) diff --git a/src/redis-benchmark.c b/src/redis-benchmark.c index d30879dc4..1c369e4e0 100644 --- a/src/redis-benchmark.c +++ b/src/redis-benchmark.c @@ -39,6 +39,7 @@ #include #include #include +#include #include /* Use hiredis sds. */ #include "ae.h" @@ -48,6 +49,7 @@ #define UNUSED(V) ((void) V) #define RANDPTR_INITIAL_SIZE 8 +#define MAX_LATENCY_PRECISION 3 static struct config { aeEventLoop *el; @@ -79,6 +81,7 @@ static struct config { sds dbnumstr; char *tests; char *auth; + int precision; } config; typedef struct _client { @@ -428,6 +431,16 @@ static int compareLatency(const void *a, const void *b) { return (*(long long*)a)-(*(long long*)b); } +static int ipow(int base, int exp) { + int result = 1; + while (exp) { + if (exp & 1) result *= base; + exp /= 2; + base *= base; + } + return result; +} + static void showLatencyReport(void) { int i, curlat = 0; float perc, reqpersec; @@ -444,10 +457,10 @@ static void showLatencyReport(void) { qsort(config.latency,config.requests,sizeof(long long),compareLatency); for (i = 0; i < config.requests; i++) { - if (config.latency[i]/1000 != curlat || i == (config.requests-1)) { - curlat = config.latency[i]/1000; + if (config.latency[i]/ipow(10, MAX_LATENCY_PRECISION-config.precision) != curlat || i == (config.requests-1)) { + curlat = config.latency[i]/ipow(10, MAX_LATENCY_PRECISION-config.precision); perc = ((float)(i+1)*100)/config.requests; - printf("%.2f%% <= %d milliseconds\n", perc, curlat); + printf("%.2f%% <= %.*f milliseconds\n", perc, config.precision, curlat/pow(10.0, config.precision)); } } printf("%.2f requests per second\n\n", reqpersec); @@ -546,6 +559,11 @@ int parseOptions(int argc, const char **argv) { if (lastarg) goto invalid; config.dbnum = atoi(argv[++i]); config.dbnumstr = sdsfromlonglong(config.dbnum); + } else if (!strcmp(argv[i],"--precision")) { + if (lastarg) goto invalid; + config.precision = atoi(argv[++i]); + if (config.precision < 0) config.precision = 0; + if (config.precision > MAX_LATENCY_PRECISION) config.precision = MAX_LATENCY_PRECISION; } else if (!strcmp(argv[i],"--help")) { exit_status = 0; goto usage; @@ -585,6 +603,7 @@ usage: " -e If server replies with errors, show them on stdout.\n" " (no more than 1 error per second is displayed)\n" " -q Quiet. Just show query/sec values\n" +" --precision Number of decimal places to display in latency output (default 0)\n" " --csv Output in CSV format\n" " -l Loop. Run the tests forever\n" " -t Only run the comma separated list of tests. The test\n" @@ -679,6 +698,7 @@ int main(int argc, const char **argv) { config.tests = NULL; config.dbnum = 0; config.auth = NULL; + config.precision = 0; i = parseOptions(argc,argv); argc -= i; From 5a1f8fd6f2679d9c266f9f5eaf659030195de04a Mon Sep 17 00:00:00 2001 From: Madelyn Olson Date: Wed, 13 Feb 2019 21:03:31 +0000 Subject: [PATCH 33/47] Rename variable --- src/redis-benchmark.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/redis-benchmark.c b/src/redis-benchmark.c index 1c369e4e0..5e6f6a628 100644 --- a/src/redis-benchmark.c +++ b/src/redis-benchmark.c @@ -443,6 +443,7 @@ static int ipow(int base, int exp) { static void showLatencyReport(void) { int i, curlat = 0; + int usbetweenlat = ipow(10, MAX_LATENCY_PRECISION-config.precision); float perc, reqpersec; reqpersec = (float)config.requests_finished/((float)config.totlatency/1000); @@ -457,8 +458,8 @@ static void showLatencyReport(void) { qsort(config.latency,config.requests,sizeof(long long),compareLatency); for (i = 0; i < config.requests; i++) { - if (config.latency[i]/ipow(10, MAX_LATENCY_PRECISION-config.precision) != curlat || i == (config.requests-1)) { - curlat = config.latency[i]/ipow(10, MAX_LATENCY_PRECISION-config.precision); + if (config.latency[i]/usbetweenlat != curlat || i == (config.requests-1)) { + curlat = config.latency[i]/usbetweenlat; perc = ((float)(i+1)*100)/config.requests; printf("%.2f%% <= %.*f milliseconds\n", perc, config.precision, curlat/pow(10.0, config.precision)); } From 78a2115c4ce2b653a875b8ac5bd312fa87d54a8a Mon Sep 17 00:00:00 2001 From: antirez Date: Thu, 14 Feb 2019 13:19:45 +0100 Subject: [PATCH 34/47] redis-benchmark: default precision=1, integer ms after 2 milliseconds. Reltaed to discussion and PR #5840. --- src/redis-benchmark.c | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-) diff --git a/src/redis-benchmark.c b/src/redis-benchmark.c index 5e6f6a628..31f91eb0f 100644 --- a/src/redis-benchmark.c +++ b/src/redis-benchmark.c @@ -458,10 +458,21 @@ static void showLatencyReport(void) { qsort(config.latency,config.requests,sizeof(long long),compareLatency); for (i = 0; i < config.requests; i++) { - if (config.latency[i]/usbetweenlat != curlat || i == (config.requests-1)) { + if (config.latency[i]/usbetweenlat != curlat || + i == (config.requests-1)) + { curlat = config.latency[i]/usbetweenlat; perc = ((float)(i+1)*100)/config.requests; - printf("%.2f%% <= %.*f milliseconds\n", perc, config.precision, curlat/pow(10.0, config.precision)); + printf("%.2f%% <= %.*f milliseconds\n", perc, config.precision, + curlat/pow(10.0, config.precision)); + + /* After the 2 milliseconds latency to have percentages split + * by decimals will just add a lot of noise to the output. */ + if (config.latency[i] > 2000) { + config.precision = 0; + usbetweenlat = ipow(10, + MAX_LATENCY_PRECISION-config.precision); + } } } printf("%.2f requests per second\n\n", reqpersec); @@ -699,7 +710,7 @@ int main(int argc, const char **argv) { config.tests = NULL; config.dbnum = 0; config.auth = NULL; - config.precision = 0; + config.precision = 1; i = parseOptions(argc,argv); argc -= i; From 85cdb79d148591535493fa6fe9996831c1764517 Mon Sep 17 00:00:00 2001 From: antirez Date: Mon, 18 Feb 2019 12:39:00 +0100 Subject: [PATCH 35/47] showdist.rb utility for SRANDMEMBER analysis added. --- utils/srandmember/README.md | 2 ++ utils/srandmember/showdist.rb | 33 +++++++++++++++++++++++++++++++++ 2 files changed, 35 insertions(+) create mode 100644 utils/srandmember/README.md create mode 100644 utils/srandmember/showdist.rb diff --git a/utils/srandmember/README.md b/utils/srandmember/README.md new file mode 100644 index 000000000..e6dae5ba8 --- /dev/null +++ b/utils/srandmember/README.md @@ -0,0 +1,2 @@ +This utility plots the distribution of SRANDMEMBER to evaluate how fair it is. +See http://theshfl.com/redis_sets for more information on the topic. diff --git a/utils/srandmember/showdist.rb b/utils/srandmember/showdist.rb new file mode 100644 index 000000000..243585700 --- /dev/null +++ b/utils/srandmember/showdist.rb @@ -0,0 +1,33 @@ +require 'redis' + +r = Redis.new +r.select(9) +r.del("myset"); +r.sadd("myset",(0..999).to_a) +freq = {} +100.times { + res = r.pipelined { + 1000.times { + r.srandmember("myset") + } + } + res.each{|ele| + freq[ele] = 0 if freq[ele] == nil + freq[ele] += 1 + } +} + +# Convert into frequency distribution +dist = {} +freq.each{|item,count| + dist[count] = 0 if dist[count] == nil + dist[count] += 1 +} + +min = dist.keys.min +max = dist.keys.max +(min..max).each{|x| + count = dist[x] + count = 0 if count == nil + puts "#{x} -> #{"*"*count}" +} From aae7e1bff0cd5fb1544bf8e99024e9c24fd94a67 Mon Sep 17 00:00:00 2001 From: antirez Date: Mon, 18 Feb 2019 18:27:18 +0100 Subject: [PATCH 36/47] Better distribution for set get-random-element operations. --- src/dict.c | 24 ++++++++++++++++++++++++ src/dict.h | 1 + src/t_set.c | 2 +- 3 files changed, 26 insertions(+), 1 deletion(-) diff --git a/src/dict.c b/src/dict.c index 2cf9d4839..6844e6c8e 100644 --- a/src/dict.c +++ b/src/dict.c @@ -739,6 +739,30 @@ unsigned int dictGetSomeKeys(dict *d, dictEntry **des, unsigned int count) { return stored; } +/* This is like dictGetRandomKey() from the POV of the API, but will do more + * work to ensure a better distribution of the returned element. + * + * This function improves the distribution because the dictGetRandomKey() + * problem is that it selects a random bucket, then it selects a random + * element from the chain in the bucket. However elements being in different + * chain lengths will have different probabilities of being reported. With + * this function instead what we do is to consider a "linear" range of the table + * that may be constituted of N buckets with chains of different lengths + * appearing one after the other. Then we report a random element in the range. + * In this way we smooth away the problem of different chain lenghts. */ +#define GETFAIR_NUM_ENTRIES 20 +dictEntry *dictGetFairRandomKey(dict *d) { + dictEntry *entries[GETFAIR_NUM_ENTRIES]; + unsigned int count = dictGetSomeKeys(d,entries,GETFAIR_NUM_ENTRIES); + /* Note that dictGetSomeKeys() may return zero elements in an unlucky + * run() even if there are actually elements inside the hash table. So + * when we get zero, we call the true dictGetRandomKey() that will always + * yeld the element if the hash table has at least one. */ + if (count == 0) return dictGetRandomKey(d); + unsigned int idx = rand() % count; + return entries[idx]; +} + /* Function to reverse bits. Algorithm from: * http://graphics.stanford.edu/~seander/bithacks.html#ReverseParallel */ static unsigned long rev(unsigned long v) { diff --git a/src/dict.h b/src/dict.h index 62018cc44..dec60f637 100644 --- a/src/dict.h +++ b/src/dict.h @@ -166,6 +166,7 @@ dictIterator *dictGetSafeIterator(dict *d); dictEntry *dictNext(dictIterator *iter); void dictReleaseIterator(dictIterator *iter); dictEntry *dictGetRandomKey(dict *d); +dictEntry *dictGetFairRandomKey(dict *d); unsigned int dictGetSomeKeys(dict *d, dictEntry **des, unsigned int count); void dictGetStats(char *buf, size_t bufsize, dict *d); uint64_t dictGenHashFunction(const void *key, int len); diff --git a/src/t_set.c b/src/t_set.c index 61013dbcd..290a83e6d 100644 --- a/src/t_set.c +++ b/src/t_set.c @@ -207,7 +207,7 @@ sds setTypeNextObject(setTypeIterator *si) { * used field with values which are easy to trap if misused. */ int setTypeRandomElement(robj *setobj, sds *sdsele, int64_t *llele) { if (setobj->encoding == OBJ_ENCODING_HT) { - dictEntry *de = dictGetRandomKey(setobj->ptr); + dictEntry *de = dictGetFairRandomKey(setobj->ptr); *sdsele = dictGetKey(de); *llele = -123456789; /* Not needed. Defensive. */ } else if (setobj->encoding == OBJ_ENCODING_INTSET) { From 34982b0d0eeaf5656fcdb91d6bb56b8b6b0a642c Mon Sep 17 00:00:00 2001 From: antirez Date: Mon, 18 Feb 2019 18:38:40 +0100 Subject: [PATCH 37/47] Limit sampling size in dictGetFairRandomKey(). This way the implementation is almost as fast as the original one, but the distribution is not too bad. --- src/dict.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/dict.c b/src/dict.c index 6844e6c8e..ce48eb419 100644 --- a/src/dict.c +++ b/src/dict.c @@ -750,7 +750,7 @@ unsigned int dictGetSomeKeys(dict *d, dictEntry **des, unsigned int count) { * that may be constituted of N buckets with chains of different lengths * appearing one after the other. Then we report a random element in the range. * In this way we smooth away the problem of different chain lenghts. */ -#define GETFAIR_NUM_ENTRIES 20 +#define GETFAIR_NUM_ENTRIES 10 dictEntry *dictGetFairRandomKey(dict *d) { dictEntry *entries[GETFAIR_NUM_ENTRIES]; unsigned int count = dictGetSomeKeys(d,entries,GETFAIR_NUM_ENTRIES); From c3b4760143536fc23d93a7c8119383e6d3f05d29 Mon Sep 17 00:00:00 2001 From: antirez Date: Mon, 18 Feb 2019 18:47:49 +0100 Subject: [PATCH 38/47] Add showfreq.rb to SRANDMEMBER analysis tools. --- utils/srandmember/showfreq.rb | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) create mode 100644 utils/srandmember/showfreq.rb diff --git a/utils/srandmember/showfreq.rb b/utils/srandmember/showfreq.rb new file mode 100644 index 000000000..12f9e1857 --- /dev/null +++ b/utils/srandmember/showfreq.rb @@ -0,0 +1,23 @@ +require 'redis' + +r = Redis.new +r.select(9) +r.del("myset"); +r.sadd("myset",(0..999).to_a) +freq = {} +100.times { + res = r.pipelined { + 1000.times { + r.srandmember("myset") + } + } + res.each{|ele| + freq[ele] = 0 if freq[ele] == nil + freq[ele] += 1 + } +} + +# Print the frequency each element was yeld to process it with gnuplot +freq.each{|item,count| + puts "#{item} #{count}" +} From 438688d956403bcc2c98fb226c9e877c8f990b10 Mon Sep 17 00:00:00 2001 From: antirez Date: Tue, 19 Feb 2019 12:01:26 +0100 Subject: [PATCH 39/47] Improve README of better-random-member directory. --- utils/srandmember/README.md | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/utils/srandmember/README.md b/utils/srandmember/README.md index e6dae5ba8..d3da1e82f 100644 --- a/utils/srandmember/README.md +++ b/utils/srandmember/README.md @@ -1,2 +1,14 @@ -This utility plots the distribution of SRANDMEMBER to evaluate how fair it is. -See http://theshfl.com/redis_sets for more information on the topic. +The utilities in this directory plot the distribution of SRANDMEMBER to +evaluate how fair it is. + +See http://theshfl.com/redis_sets for more information on the topic that lead +to such investigation fix. + +showdist.rb -- shows the distribution of the frequency elements are returned. + The x axis is the number of times elements were returned, and + the y axis is how many elements were returned with such + frequency. + +showfreq.rb -- shows the frequency each element was returned. + The x axis is the element number. + The y axis is the times it was returned. From 963da462fab28b151d0d515283f1e96774e7fc12 Mon Sep 17 00:00:00 2001 From: antirez Date: Tue, 19 Feb 2019 17:25:58 +0100 Subject: [PATCH 40/47] showfreq.rb: collect more data for better graphs. --- utils/srandmember/showfreq.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/utils/srandmember/showfreq.rb b/utils/srandmember/showfreq.rb index 12f9e1857..fd47bc0ca 100644 --- a/utils/srandmember/showfreq.rb +++ b/utils/srandmember/showfreq.rb @@ -5,7 +5,7 @@ r.select(9) r.del("myset"); r.sadd("myset",(0..999).to_a) freq = {} -100.times { +500.times { res = r.pipelined { 1000.times { r.srandmember("myset") From 0ad63de59dad9dd51f8ef9bf9232e1bd16702972 Mon Sep 17 00:00:00 2001 From: antirez Date: Tue, 19 Feb 2019 17:27:42 +0100 Subject: [PATCH 41/47] Set dictGetFairRandomKey() samples to 20 for final version. Distribution improves dramatically: tests show it clearly. Better to have a slower implementation than a wrong one, because random member extraction should be correct or tends to be useless for a number of tasks. --- src/dict.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/dict.c b/src/dict.c index ce48eb419..106467ef7 100644 --- a/src/dict.c +++ b/src/dict.c @@ -750,7 +750,7 @@ unsigned int dictGetSomeKeys(dict *d, dictEntry **des, unsigned int count) { * that may be constituted of N buckets with chains of different lengths * appearing one after the other. Then we report a random element in the range. * In this way we smooth away the problem of different chain lenghts. */ -#define GETFAIR_NUM_ENTRIES 10 +#define GETFAIR_NUM_ENTRIES 15 dictEntry *dictGetFairRandomKey(dict *d) { dictEntry *entries[GETFAIR_NUM_ENTRIES]; unsigned int count = dictGetSomeKeys(d,entries,GETFAIR_NUM_ENTRIES); From 94a9e27bb84ad505efa87b94b68d9df79bedc979 Mon Sep 17 00:00:00 2001 From: antirez Date: Tue, 19 Feb 2019 17:29:51 +0100 Subject: [PATCH 42/47] Use dictGetFairRandomKey() for RANDOMKEY as well. --- src/db.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/db.c b/src/db.c index f59ae8406..7950d5074 100644 --- a/src/db.c +++ b/src/db.c @@ -241,7 +241,7 @@ robj *dbRandomKey(redisDb *db) { sds key; robj *keyobj; - de = dictGetRandomKey(db->dict); + de = dictGetFairRandomKey(db->dict); if (de == NULL) return NULL; key = dictGetKey(de); From 6f84cf009b29c672c96430246872b468c801a8aa Mon Sep 17 00:00:00 2001 From: Oran Agra Date: Thu, 21 Feb 2019 12:06:18 +0200 Subject: [PATCH 43/47] redis-cli add support for --memkeys, fix --bigkeys for module types * bigkeys used to fail on databases with module type keys * new code adds more types when it discovers them, but has no way to know element count in modules types yet * bigkeys was missing XLEN command for streams * adding --memkeys and --memkeys-samples to make use of the MEMORY USAGE command see #5167, #5175 --- src/redis-cli.c | 213 ++++++++++++++++++++++++++++++------------------ 1 file changed, 132 insertions(+), 81 deletions(-) diff --git a/src/redis-cli.c b/src/redis-cli.c index 93290e5ed..884b23e60 100644 --- a/src/redis-cli.c +++ b/src/redis-cli.c @@ -210,6 +210,8 @@ static struct config { char *pattern; char *rdb_filename; int bigkeys; + int memkeys; + unsigned memkeys_samples; int hotkeys; int stdinarg; /* get last arg from stdin. (-x option) */ char *auth; @@ -1336,6 +1338,12 @@ static int parseOptions(int argc, char **argv) { config.pipe_timeout = atoi(argv[++i]); } else if (!strcmp(argv[i],"--bigkeys")) { config.bigkeys = 1; + } else if (!strcmp(argv[i],"--memkeys")) { + config.memkeys = 1; + config.memkeys_samples = 0; /* use redis default */ + } else if (!strcmp(argv[i],"--memkeys-samples")) { + config.memkeys = 1; + config.memkeys_samples = atoi(argv[++i]); } else if (!strcmp(argv[i],"--hotkeys")) { config.hotkeys = 1; } else if (!strcmp(argv[i],"--eval") && !lastarg) { @@ -1534,7 +1542,10 @@ static void usage(void) { " --pipe-timeout In --pipe mode, abort with error if after sending all data.\n" " no reply is received within seconds.\n" " Default timeout: %d. Use 0 to wait forever.\n" -" --bigkeys Sample Redis keys looking for big keys.\n" +" --bigkeys Sample Redis keys looking for keys with many elements (complexity).\n" +" --memkeys Sample Redis keys looking for keys consuming a lot of memory.\n" +" --memkeys-samples Sample Redis keys looking for keys consuming a lot of memory.\n" +" And define number of key elements to sample\n" " --hotkeys Sample Redis keys looking for hot keys.\n" " only works when maxmemory-policy is *lfu.\n" " --scan List all keys using the SCAN command.\n" @@ -6418,15 +6429,6 @@ static void pipeMode(void) { * Find big keys *--------------------------------------------------------------------------- */ -#define TYPE_STRING 0 -#define TYPE_LIST 1 -#define TYPE_SET 2 -#define TYPE_HASH 3 -#define TYPE_ZSET 4 -#define TYPE_STREAM 5 -#define TYPE_NONE 6 -#define TYPE_COUNT 7 - static redisReply *sendScan(unsigned long long *it) { redisReply *reply = redisCommand(context, "SCAN %llu", *it); @@ -6473,28 +6475,51 @@ static int getDbSize(void) { return size; } -static int toIntType(char *key, char *type) { - if(!strcmp(type, "string")) { - return TYPE_STRING; - } else if(!strcmp(type, "list")) { - return TYPE_LIST; - } else if(!strcmp(type, "set")) { - return TYPE_SET; - } else if(!strcmp(type, "hash")) { - return TYPE_HASH; - } else if(!strcmp(type, "zset")) { - return TYPE_ZSET; - } else if(!strcmp(type, "stream")) { - return TYPE_STREAM; - } else if(!strcmp(type, "none")) { - return TYPE_NONE; - } else { - fprintf(stderr, "Unknown type '%s' for key '%s'\n", type, key); - exit(1); - } +typedef struct { + char *name; + char *sizecmd; + char *sizeunit; + unsigned long long biggest; + unsigned long long count; + unsigned long long totalsize; + sds biggest_key; +} typeinfo; + +typeinfo type_string = { "string", "STRLEN", "bytes" }; +typeinfo type_list = { "list", "LLEN", "items" }; +typeinfo type_set = { "set", "SCARD", "members" }; +typeinfo type_hash = { "hash", "HLEN", "fields" }; +typeinfo type_zset = { "zset", "ZCARD", "members" }; +typeinfo type_stream = { "stream", "XLEN", "entries" }; +typeinfo type_other = { "other", NULL, "?" }; + +static typeinfo* typeinfo_add(dict *types, char* name, typeinfo* type_template) { + typeinfo *info = zmalloc(sizeof(typeinfo)); + *info = *type_template; + info->name = sdsnew(name); + dictAdd(types, info->name, info); + return info; } -static void getKeyTypes(redisReply *keys, int *types) { +void type_free(void* priv_data, void* val) { + typeinfo *info = val; + UNUSED(priv_data); + if (info->biggest_key) + sdsfree(info->biggest_key); + sdsfree(info->name); + zfree(info); +} + +static dictType typeinfoDictType = { + dictSdsHash, /* hash function */ + NULL, /* key dup */ + NULL, /* val dup */ + dictSdsKeyCompare, /* key compare */ + NULL, /* key destructor (owned by the value)*/ + type_free /* val destructor */ +}; + +static void getKeyTypes(dict *types_dict, redisReply *keys, typeinfo **types) { redisReply *reply; unsigned int i; @@ -6520,32 +6545,47 @@ static void getKeyTypes(redisReply *keys, int *types) { exit(1); } - types[i] = toIntType(keys->element[i]->str, reply->str); + sds typereply = sdsnew(reply->str); + dictEntry *de = dictFind(types_dict, typereply); + sdsfree(typereply); + typeinfo *type = NULL; + if (de) + type = dictGetVal(de); + else if (strcmp(reply->str, "none")) /* create new types for modules, (but not for deleted keys) */ + type = typeinfo_add(types_dict, reply->str, &type_other); + types[i] = type; freeReplyObject(reply); } } -static void getKeySizes(redisReply *keys, int *types, - unsigned long long *sizes) +static void getKeySizes(redisReply *keys, typeinfo **types, + unsigned long long *sizes, int memkeys, + unsigned memkeys_samples) { redisReply *reply; - char *sizecmds[] = {"STRLEN","LLEN","SCARD","HLEN","ZCARD"}; unsigned int i; /* Pipeline size commands */ for(i=0;ielements;i++) { - /* Skip keys that were deleted */ - if(types[i]==TYPE_NONE) + /* Skip keys that disappeared between SCAN and TYPE (or unknown types when not in memkeys mode) */ + if(!types[i] || (!types[i]->sizecmd && !memkeys)) continue; - redisAppendCommand(context, "%s %s", sizecmds[types[i]], - keys->element[i]->str); + if (!memkeys) + redisAppendCommand(context, "%s %s", + types[i]->sizecmd, keys->element[i]->str); + else if (memkeys_samples==0) + redisAppendCommand(context, "%s %s %s", + "MEMORY", "USAGE", keys->element[i]->str); + else + redisAppendCommand(context, "%s %s %s SAMPLES %u", + "MEMORY", "USAGE", keys->element[i]->str, memkeys_samples); } /* Retrieve sizes */ for(i=0;ielements;i++) { - /* Skip keys that disappeared between SCAN and TYPE */ - if(types[i] == TYPE_NONE) { + /* Skip keys that disappeared between SCAN and TYPE (or unknown types when not in memkeys mode) */ + if(!types[i] || (!types[i]->sizecmd && !memkeys)) { sizes[i] = 0; continue; } @@ -6560,7 +6600,8 @@ static void getKeySizes(redisReply *keys, int *types, * added as a different type between TYPE and SIZE */ fprintf(stderr, "Warning: %s on '%s' failed (may have changed type)\n", - sizecmds[types[i]], keys->element[i]->str); + !memkeys? types[i]->sizecmd: "MEMORY USAGE", + keys->element[i]->str); sizes[i] = 0; } else { sizes[i] = reply->integer; @@ -6570,17 +6611,23 @@ static void getKeySizes(redisReply *keys, int *types, } } -static void findBigKeys(void) { - unsigned long long biggest[TYPE_COUNT] = {0}, counts[TYPE_COUNT] = {0}, totalsize[TYPE_COUNT] = {0}; +static void findBigKeys(int memkeys, unsigned memkeys_samples) { unsigned long long sampled = 0, total_keys, totlen=0, *sizes=NULL, it=0; - sds maxkeys[TYPE_COUNT] = {0}; - char *typename[] = {"string","list","set","hash","zset","stream","none"}; - char *typeunit[] = {"bytes","items","members","fields","members","entries",""}; redisReply *reply, *keys; unsigned int arrsize=0, i; - int type, *types=NULL; + dictIterator *di; + dictEntry *de; + typeinfo **types = NULL; double pct; + dict *types_dict = dictCreate(&typeinfoDictType, NULL); + typeinfo_add(types_dict, "string", &type_string); + typeinfo_add(types_dict, "list", &type_list); + typeinfo_add(types_dict, "set", &type_set); + typeinfo_add(types_dict, "hash", &type_hash); + typeinfo_add(types_dict, "zset", &type_zset); + typeinfo_add(types_dict, "stream", &type_stream); + /* Total keys pre scanning */ total_keys = getDbSize(); @@ -6589,15 +6636,6 @@ static void findBigKeys(void) { printf("# average sizes per key type. You can use -i 0.1 to sleep 0.1 sec\n"); printf("# per 100 SCAN commands (not usually needed).\n\n"); - /* New up sds strings to keep track of overall biggest per type */ - for(i=0;ielements > arrsize) { - types = zrealloc(types, sizeof(int)*keys->elements); + types = zrealloc(types, sizeof(typeinfo*)*keys->elements); sizes = zrealloc(sizes, sizeof(unsigned long long)*keys->elements); if(!types || !sizes) { @@ -6621,34 +6659,38 @@ static void findBigKeys(void) { } /* Retrieve types and then sizes */ - getKeyTypes(keys, types); - getKeySizes(keys, types, sizes); + getKeyTypes(types_dict, keys, types); + getKeySizes(keys, types, sizes, memkeys, memkeys_samples); /* Now update our stats */ for(i=0;ielements;i++) { - if((type = types[i]) == TYPE_NONE) + typeinfo *type = types[i]; + /* Skip keys that disappeared between SCAN and TYPE */ + if(!type) continue; - totalsize[type] += sizes[i]; - counts[type]++; + type->totalsize += sizes[i]; + type->count++; totlen += keys->element[i]->len; sampled++; - if(biggest[type]biggestelement[i]->str, sizes[i], - typeunit[type]); + pct, type->name, keys->element[i]->str, sizes[i], + !memkeys? type->sizeunit: "bytes"); /* Keep track of biggest key name for this type */ - maxkeys[type] = sdscpy(maxkeys[type], keys->element[i]->str); - if(!maxkeys[type]) { + if (type->biggest_key) + sdsfree(type->biggest_key); + type->biggest_key = sdsnew(keys->element[i]->str); + if(!type->biggest_key) { fprintf(stderr, "Failed to allocate memory for key!\n"); exit(1); } /* Keep track of the biggest size for this type */ - biggest[type] = sizes[i]; + type->biggest = sizes[i]; } /* Update overall progress */ @@ -6676,26 +6718,29 @@ static void findBigKeys(void) { totlen, totlen ? (double)totlen/sampled : 0); /* Output the biggest keys we found, for types we did find */ - for(i=0;i0) { - printf("Biggest %6s found '%s' has %llu %s\n", typename[i], maxkeys[i], - biggest[i], typeunit[i]); + di = dictGetIterator(types_dict); + while ((de = dictNext(di))) { + typeinfo *type = dictGetVal(de); + if(type->biggest_key) { + printf("Biggest %6s found '%s' has %llu %s\n", type->name, type->biggest_key, + type->biggest, !memkeys? type->sizeunit: "bytes"); } } + dictReleaseIterator(di); printf("\n"); - for(i=0;icount, type->name, type->totalsize, !memkeys? type->sizeunit: "bytes", + sampled ? 100 * (double)type->count/sampled : 0, + type->count ? (double)type->totalsize/type->count : 0); } + dictReleaseIterator(di); - /* Free sds strings containing max keys */ - for(i=0;i Date: Thu, 21 Feb 2019 16:31:33 +0100 Subject: [PATCH 44/47] ACL: add LOAD subcommand to ACL HELP. --- src/acl.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/acl.c b/src/acl.c index 407d86736..6ed62f6c1 100644 --- a/src/acl.c +++ b/src/acl.c @@ -1300,6 +1300,7 @@ void ACLLoadUsersAtStartup(void) { /* ACL -- show and modify the configuration of ACL users. * ACL HELP + * ACL LOAD * ACL LIST * ACL USERS * ACL CAT [] @@ -1465,6 +1466,7 @@ void aclCommand(client *c) { setDeferredArrayLen(c,dl,arraylen); } else if (!strcasecmp(sub,"help")) { const char *help[] = { +"LOAD -- Reload users from the ACL file.", "LIST -- Show user details in config file format.", "USERS -- List all the registered usernames.", "SETUSER [attribs ...] -- Create or modify a user.", From c80b647d0347c8684c0990fa9e437c208a217ba4 Mon Sep 17 00:00:00 2001 From: antirez Date: Thu, 21 Feb 2019 16:50:28 +0100 Subject: [PATCH 45/47] ACL: ACLSaveToFile() implemented. --- src/acl.c | 66 +++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 66 insertions(+) diff --git a/src/acl.c b/src/acl.c index 6ed62f6c1..f5ac01914 100644 --- a/src/acl.c +++ b/src/acl.c @@ -28,6 +28,7 @@ */ #include "server.h" +#include /* ============================================================================= * Global state for ACLs @@ -1261,6 +1262,71 @@ sds ACLLoadFromFile(const char *filename) { } } +/* Generate a copy of the ACLs currently in memory in the specified filename. + * Returns C_OK on success or C_ERR if there was an error during the I/O. + * When C_ERR is returned a log is produced with hints about the issue. */ +int ACLSaveToFile(const char *filename) { + sds acl = sdsempty(); + int fd; + + /* Let's generate an SDS string containing the new version of the + * ACL file. */ + raxIterator ri; + raxStart(&ri,Users); + raxSeek(&ri,"^",NULL,0); + while(raxNext(&ri)) { + user *u = ri.data; + /* Return information in the configuration file format. */ + sds user = sdsnew("user "); + user = sdscatsds(user,u->name); + user = sdscatlen(user," ",1); + sds descr = ACLDescribeUser(u); + user = sdscatsds(user,descr); + sdsfree(descr); + acl = sdscatsds(acl,user); + acl = sdscatlen(acl,"\n",1); + sdsfree(user); + } + raxStop(&ri); + + /* Create a temp file with the new content. */ + sds tmpfilename = sdsnew(filename); + tmpfilename = sdscatfmt(tmpfilename,".tmp-%i-%I", + (int)getpid(),(int)mstime()); + if ((fd = open(tmpfilename,O_WRONLY|O_CREAT,0644)) == -1) { + serverLog(LL_WARNING,"Opening temp ACL file for ACL SAVE: %s", + strerror(errno)); + sdsfree(tmpfilename); + sdsfree(acl); + return C_ERR; + } + + /* Write it. */ + if (write(fd,acl,sdslen(acl)) != (ssize_t)sdslen(acl)) { + serverLog(LL_WARNING,"Writing ACL file for ACL SAVE: %s", + strerror(errno)); + close(fd); + unlink(tmpfilename); + sdsfree(tmpfilename); + sdsfree(acl); + return C_ERR; + } + close(fd); + sdsfree(acl); + + /* Let's replace the new file with the old one. */ + if (rename(tmpfilename,filename) == -1) { + serverLog(LL_WARNING,"Renaming ACL file for ACL SAVE: %s", + strerror(errno)); + unlink(tmpfilename); + sdsfree(tmpfilename); + return C_ERR; + } + + sdsfree(tmpfilename); + return C_OK; +} + /* This function is called once the server is already running, modules are * loaded, and we are ready to start, in order to load the ACLs either from * the pending list of users defined in redis.conf, or from the ACL file. From c3425bc0ae47629ef34c224c6568140ca94762df Mon Sep 17 00:00:00 2001 From: antirez Date: Thu, 21 Feb 2019 17:01:08 +0100 Subject: [PATCH 46/47] ACL: implement ACL SAVE. --- src/acl.c | 28 ++++++++++++++++++---------- 1 file changed, 18 insertions(+), 10 deletions(-) diff --git a/src/acl.c b/src/acl.c index f5ac01914..e152e52af 100644 --- a/src/acl.c +++ b/src/acl.c @@ -1491,18 +1491,26 @@ void aclCommand(client *c) { } else { addReplyNull(c); } + } else if (server.acl_filename[0] == '\0' && + (!strcasecmp(sub,"load") || !strcasecmp(sub,"save"))) + { + addReplyError(c,"This Redis instance is not configured to use an ACL file. You may want to specify users via the ACL SETUSER command and then issue a CONFIG REWRITE (assuming you have a Redis configuration file set) in order to store users in the Redis configuration."); + return; } else if (!strcasecmp(sub,"load") && c->argc == 2) { - if (server.acl_filename[0] == '\0') { - addReplyError(c,"This Redis instance is not configured to use an ACL file. You may want to specify users via the ACL SETUSER command and then issue a CONFIG REWRITE (assuming you have a Redis configuration file set) in order to store users in the Redis configuration."); - return; + sds errors = ACLLoadFromFile(server.acl_filename); + if (errors == NULL) { + addReply(c,shared.ok); } else { - sds errors = ACLLoadFromFile(server.acl_filename); - if (errors == NULL) { - addReply(c,shared.ok); - } else { - addReplyError(c,errors); - sdsfree(errors); - } + addReplyError(c,errors); + sdsfree(errors); + } + } else if (!strcasecmp(sub,"save") && c->argc == 2) { + if (ACLSaveToFile(server.acl_filename) == C_OK) { + addReply(c,shared.ok); + } else { + addReplyError(c,"There was an error trying to save the ACLs. " + "Please check the server logs for more " + "information"); } } else if (!strcasecmp(sub,"cat") && c->argc == 2) { void *dl = addReplyDeferredLen(c); From c528f436e6d3451c6ad16d0ab651512e4a48cdb7 Mon Sep 17 00:00:00 2001 From: antirez Date: Thu, 21 Feb 2019 17:03:06 +0100 Subject: [PATCH 47/47] ACL: remove leak in ACLLoadFromFile(). --- src/acl.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/acl.c b/src/acl.c index e152e52af..171db43a1 100644 --- a/src/acl.c +++ b/src/acl.c @@ -1148,6 +1148,7 @@ sds ACLLoadFromFile(const char *filename) { int totlines; sds *lines, errors = sdsempty(); lines = sdssplitlen(acls,strlen(acls),"\n",1,&totlines); + sdsfree(acls); /* We need a fake user to validate the rules before making changes * to the real user mentioned in the ACL line. */