diff options
author | YaacovHazan <31382944+YaacovHazan@users.noreply.github.com> | 2021-03-01 16:04:44 +0200 |
---|---|---|
committer | GitHub <noreply@github.com> | 2021-03-01 16:04:44 +0200 |
commit | a031d268b127d67969e9a4ec6b728dbb321ea0ce (patch) | |
tree | f1838fdf044b2301f6f91c3dd577650a69c56b58 | |
parent | 81a55d026ffe6614f78e4042921713112593457c (diff) | |
download | redis-a031d268b127d67969e9a4ec6b728dbb321ea0ce.tar.gz |
Make port, tls-port and bind configurations modifiable (#8510)
Add ability to modify port, tls-port and bind configurations by CONFIG SET command.
To simplify the code and make it cleaner, a new structure
added, socketFds, which contains the file descriptors array and its counter,
and used for TCP, TLS and Cluster sockets file descriptors.
-rw-r--r-- | src/cluster.c | 18 | ||||
-rw-r--r-- | src/config.c | 57 | ||||
-rw-r--r-- | src/server.c | 180 | ||||
-rw-r--r-- | src/server.h | 19 | ||||
-rw-r--r-- | tests/test_helper.tcl | 1 | ||||
-rw-r--r-- | tests/unit/introspection.tcl | 4 | ||||
-rw-r--r-- | tests/unit/networking.tcl | 34 | ||||
-rw-r--r-- | tests/unit/tls.tcl | 25 |
8 files changed, 282 insertions, 56 deletions
diff --git a/src/cluster.c b/src/cluster.c index efe2f652d..4605e3ea9 100644 --- a/src/cluster.c +++ b/src/cluster.c @@ -499,7 +499,7 @@ void clusterInit(void) { if (saveconf) clusterSaveConfigOrDie(1); /* We need a listening TCP port for our cluster messaging needs. */ - server.cfd_count = 0; + server.cfd.count = 0; /* Port sanity check II * The other handshake port check is triggered too late to stop @@ -512,19 +512,11 @@ void clusterInit(void) { "Your Redis port number must be 55535 or less."); exit(1); } - if (listenToPort(port+CLUSTER_PORT_INCR, - server.cfd,&server.cfd_count) == C_ERR) - { + if (listenToPort(port+CLUSTER_PORT_INCR, &server.cfd) == C_ERR) { exit(1); - } else { - int j; - - for (j = 0; j < server.cfd_count; j++) { - if (aeCreateFileEvent(server.el, server.cfd[j], AE_READABLE, - clusterAcceptHandler, NULL) == AE_ERR) - serverPanic("Unrecoverable error creating Redis Cluster " - "file event."); - } + } + if (createSocketAcceptHandler(&server.cfd, clusterAcceptHandler) != C_OK) { + serverPanic("Unrecoverable error creating Redis Cluster socket accept handler."); } /* The slots -> keys map is a radix tree. Initialize it here. */ diff --git a/src/config.c b/src/config.c index 13a6dee14..1d6ce6b8c 100644 --- a/src/config.c +++ b/src/config.c @@ -728,7 +728,23 @@ void configSetCommand(client *c) { if (0) { /* this starts the config_set macros else-if chain. */ /* Special fields that can't be handled with general macros. */ - config_set_special_field("save") { + config_set_special_field("bind") { + int vlen; + sds *v = sdssplitlen(o->ptr,sdslen(o->ptr)," ",1,&vlen); + + if (vlen < 1 || vlen > CONFIG_BINDADDR_MAX) { + addReplyError(c, "Too many bind addresses specified."); + sdsfreesplitres(v, vlen); + return; + } + + if (changeBindAddr(v, vlen) == C_ERR) { + addReplyError(c, "Failed to bind to specified addresses."); + sdsfreesplitres(v, vlen); + return; + } + sdsfreesplitres(v, vlen); + } config_set_special_field("save") { int vlen, j; sds *v = sdssplitlen(o->ptr,sdslen(o->ptr)," ",1,&vlen); @@ -2191,6 +2207,20 @@ static int updateHZ(long long val, long long prev, const char **err) { return 1; } +static int updatePort(long long val, long long prev, const char **err) { + /* Do nothing if port is unchanged */ + if (val == prev) { + return 1; + } + + if (changeListenPort(val, &server.ipfd, acceptTcpHandler) == C_ERR) { + *err = "Unable to listen on this port. Check server logs."; + return 0; + } + + return 1; +} + static int updateJemallocBgThread(int val, int prev, const char **err) { UNUSED(prev); UNUSED(err); @@ -2329,6 +2359,27 @@ static int updateTlsCfgInt(long long val, long long prev, const char **err) { UNUSED(prev); return updateTlsCfg(NULL, NULL, err); } + +static int updateTLSPort(long long val, long long prev, const char **err) { + /* Do nothing if port is unchanged */ + if (val == prev) { + return 1; + } + + /* Configure TLS if tls is enabled */ + if (prev == 0 && tlsConfigure(&server.tls_ctx_config) == C_ERR) { + *err = "Unable to update TLS configuration. Check server logs."; + return 0; + } + + if (changeListenPort(val, &server.tlsfd, acceptTLSHandler) == C_ERR) { + *err = "Unable to listen on this port. Check server logs."; + return 0; + } + + return 1; +} + #endif /* USE_OPENSSL */ standardConfig configs[] = { @@ -2409,7 +2460,7 @@ standardConfig configs[] = { /* Integer configs */ createIntConfig("databases", NULL, IMMUTABLE_CONFIG, 1, INT_MAX, server.dbnum, 16, INTEGER_CONFIG, NULL, NULL), - createIntConfig("port", NULL, IMMUTABLE_CONFIG, 0, 65535, server.port, 6379, INTEGER_CONFIG, NULL, NULL), /* TCP port. */ + createIntConfig("port", NULL, MODIFIABLE_CONFIG, 0, 65535, server.port, 6379, INTEGER_CONFIG, NULL, updatePort), /* TCP port. */ createIntConfig("io-threads", NULL, IMMUTABLE_CONFIG, 1, 128, server.io_threads_num, 1, INTEGER_CONFIG, NULL, NULL), /* Single threaded by default */ createIntConfig("auto-aof-rewrite-percentage", NULL, MODIFIABLE_CONFIG, 0, INT_MAX, server.aof_rewrite_perc, 100, INTEGER_CONFIG, NULL, NULL), createIntConfig("cluster-replica-validity-factor", "cluster-slave-validity-factor", MODIFIABLE_CONFIG, 0, INT_MAX, server.cluster_slave_validity_factor, 10, INTEGER_CONFIG, NULL, NULL), /* Slave max data age factor. */ @@ -2478,7 +2529,7 @@ standardConfig configs[] = { createOffTConfig("auto-aof-rewrite-min-size", NULL, MODIFIABLE_CONFIG, 0, LLONG_MAX, server.aof_rewrite_min_size, 64*1024*1024, MEMORY_CONFIG, NULL, NULL), #ifdef USE_OPENSSL - createIntConfig("tls-port", NULL, IMMUTABLE_CONFIG, 0, 65535, server.tls_port, 0, INTEGER_CONFIG, NULL, updateTlsCfgInt), /* TCP port. */ + createIntConfig("tls-port", NULL, MODIFIABLE_CONFIG, 0, 65535, server.tls_port, 0, INTEGER_CONFIG, NULL, updateTLSPort), /* TCP port. */ createIntConfig("tls-session-cache-size", NULL, MODIFIABLE_CONFIG, 0, INT_MAX, server.tls_ctx_config.session_cache_size, 20*1024, INTEGER_CONFIG, NULL, updateTlsCfgInt), createIntConfig("tls-session-cache-timeout", NULL, MODIFIABLE_CONFIG, 0, INT_MAX, server.tls_ctx_config.session_cache_timeout, 300, INTEGER_CONFIG, NULL, updateTlsCfgInt), createBoolConfig("tls-cluster", NULL, MODIFIABLE_CONFIG, server.tls_cluster, 0, NULL, updateTlsCfgBool), diff --git a/src/server.c b/src/server.c index 6e9dca3bc..4784431ba 100644 --- a/src/server.c +++ b/src/server.c @@ -2648,8 +2648,8 @@ void initServerConfig(void) { server.arch_bits = (sizeof(long) == 8) ? 64 : 32; server.bindaddr_count = 0; server.unixsocketperm = CONFIG_DEFAULT_UNIX_SOCKET_PERM; - server.ipfd_count = 0; - server.tlsfd_count = 0; + server.ipfd.count = 0; + server.tlsfd.count = 0; server.sofd = -1; server.active_expire_enabled = 1; server.skip_checksum_validation = 0; @@ -2988,6 +2988,34 @@ void checkTcpBacklogSettings(void) { #endif } +void closeSocketListeners(socketFds *sfd) { + int j; + + for (j = 0; j < sfd->count; j++) { + if (sfd->fd[j] == -1) continue; + + aeDeleteFileEvent(server.el, sfd->fd[j], AE_READABLE); + close(sfd->fd[j]); + } + + sfd->count = 0; +} + +/* Create an event handler for accepting new connections in TCP or TLS domain sockets. + * This works atomically for all socket fds */ +int createSocketAcceptHandler(socketFds *sfd, aeFileProc *accept_handler) { + int j; + + for (j = 0; j < sfd->count; j++) { + if (aeCreateFileEvent(server.el, sfd->fd[j], AE_READABLE, accept_handler,NULL) == AE_ERR) { + /* Rollback */ + for (j = j-1; j >= 0; j--) aeDeleteFileEvent(server.el, sfd->fd[j], AE_READABLE); + return C_ERR; + } + } + return C_OK; +} + /* Initialize a set of file descriptors to listen to the specified 'port' * binding the addresses specified in the Redis server configuration. * @@ -3006,7 +3034,7 @@ void checkTcpBacklogSettings(void) { * impossible to bind, or no bind addresses were specified in the server * configuration but the function is not able to bind * for at least * one of the IPv4 or IPv6 protocols. */ -int listenToPort(int port, int *fds, int *count) { +int listenToPort(int port, socketFds *sfd) { int j; char **bindaddr = server.bindaddr; int bindaddr_count = server.bindaddr_count; @@ -3024,12 +3052,12 @@ int listenToPort(int port, int *fds, int *count) { if (optional) addr++; if (strchr(addr,':')) { /* Bind IPv6 address. */ - fds[*count] = anetTcp6Server(server.neterr,port,addr,server.tcp_backlog); + sfd->fd[sfd->count] = anetTcp6Server(server.neterr,port,addr,server.tcp_backlog); } else { /* Bind IPv4 address. */ - fds[*count] = anetTcpServer(server.neterr,port,addr,server.tcp_backlog); + sfd->fd[sfd->count] = anetTcpServer(server.neterr,port,addr,server.tcp_backlog); } - if (fds[*count] == ANET_ERR) { + if (sfd->fd[sfd->count] == ANET_ERR) { serverLog(LL_WARNING, "Could not create server TCP listening socket %s:%d: %s", addr, port, server.neterr); @@ -3039,11 +3067,14 @@ int listenToPort(int port, int *fds, int *count) { errno == ESOCKTNOSUPPORT || errno == EPFNOSUPPORT || errno == EAFNOSUPPORT) continue; + + /* Rollback successful listens before exiting */ + closeSocketListeners(sfd); return C_ERR; } - anetNonBlock(NULL,fds[*count]); - anetCloexec(fds[*count]); - (*count)++; + anetNonBlock(NULL,sfd->fd[sfd->count]); + anetCloexec(sfd->fd[sfd->count]); + sfd->count++; } return C_OK; } @@ -3165,10 +3196,10 @@ void initServer(void) { /* Open the TCP listening socket for the user commands. */ if (server.port != 0 && - listenToPort(server.port,server.ipfd,&server.ipfd_count) == C_ERR) + listenToPort(server.port,&server.ipfd) == C_ERR) exit(1); if (server.tls_port != 0 && - listenToPort(server.tls_port,server.tlsfd,&server.tlsfd_count) == C_ERR) + listenToPort(server.tls_port,&server.tlsfd) == C_ERR) exit(1); /* Open the listening Unix domain socket. */ @@ -3185,7 +3216,7 @@ void initServer(void) { } /* Abort if there are no listening sockets at all. */ - if (server.ipfd_count == 0 && server.tlsfd_count == 0 && server.sofd < 0) { + if (server.ipfd.count == 0 && server.tlsfd.count == 0 && server.sofd < 0) { serverLog(LL_WARNING, "Configured to not listen anywhere, exiting."); exit(1); } @@ -3263,21 +3294,11 @@ void initServer(void) { /* Create an event handler for accepting new connections in TCP and Unix * domain sockets. */ - for (j = 0; j < server.ipfd_count; j++) { - if (aeCreateFileEvent(server.el, server.ipfd[j], AE_READABLE, - acceptTcpHandler,NULL) == AE_ERR) - { - serverPanic( - "Unrecoverable error creating server.ipfd file event."); - } + if (createSocketAcceptHandler(&server.ipfd, acceptTcpHandler) != C_OK) { + serverPanic("Unrecoverable error creating TCP socket accept handler."); } - for (j = 0; j < server.tlsfd_count; j++) { - if (aeCreateFileEvent(server.el, server.tlsfd[j], AE_READABLE, - acceptTLSHandler,NULL) == AE_ERR) - { - serverPanic( - "Unrecoverable error creating server.tlsfd file event."); - } + if (createSocketAcceptHandler(&server.tlsfd, acceptTLSHandler) != C_OK) { + serverPanic("Unrecoverable error creating TLS socket accept handler."); } if (server.sofd > 0 && aeCreateFileEvent(server.el,server.sofd,AE_READABLE, acceptUnixHandler,NULL) == AE_ERR) serverPanic("Unrecoverable error creating server.sofd file event."); @@ -4182,11 +4203,11 @@ void incrementErrorCount(const char *fullerr, size_t namelen) { void closeListeningSockets(int unlink_unix_socket) { int j; - for (j = 0; j < server.ipfd_count; j++) close(server.ipfd[j]); - for (j = 0; j < server.tlsfd_count; j++) close(server.tlsfd[j]); + for (j = 0; j < server.ipfd.count; j++) close(server.ipfd.fd[j]); + for (j = 0; j < server.tlsfd.count; j++) close(server.tlsfd.fd[j]); if (server.sofd != -1) close(server.sofd); if (server.cluster_enabled) - for (j = 0; j < server.cfd_count; j++) close(server.cfd[j]); + for (j = 0; j < server.cfd.count; j++) close(server.cfd.fd[j]); if (unlink_unix_socket && server.unixsocket) { serverLog(LL_NOTICE,"Removing the unix socket file."); unlink(server.unixsocket); /* don't care if this fails */ @@ -5536,6 +5557,105 @@ void redisAsciiArt(void) { zfree(buf); } +int changeBindAddr(sds *addrlist, int addrlist_len) { + int i; + int result = C_OK; + + char *prev_bindaddr[CONFIG_BINDADDR_MAX]; + int prev_bindaddr_count; + + /* Close old TCP and TLS servers */ + closeSocketListeners(&server.ipfd); + closeSocketListeners(&server.tlsfd); + + /* Keep previous settings */ + prev_bindaddr_count = server.bindaddr_count; + memcpy(prev_bindaddr, server.bindaddr, sizeof(server.bindaddr)); + + /* Copy new settings */ + memset(server.bindaddr, 0, sizeof(server.bindaddr)); + for (i = 0; i < addrlist_len; i++) { + server.bindaddr[i] = zstrdup(addrlist[i]); + } + server.bindaddr_count = addrlist_len; + + /* Bind to the new port */ + if ((server.port != 0 && listenToPort(server.port, &server.ipfd) != C_OK) || + (server.tls_port != 0 && listenToPort(server.tls_port, &server.tlsfd) != C_OK)) { + serverLog(LL_WARNING, "Failed to bind, trying to restore old listening sockets."); + + /* Restore old bind addresses */ + for (i = 0; i < addrlist_len; i++) { + zfree(server.bindaddr[i]); + } + memcpy(server.bindaddr, prev_bindaddr, sizeof(server.bindaddr)); + server.bindaddr_count = prev_bindaddr_count; + + /* Re-Listen TCP and TLS */ + server.ipfd.count = 0; + if (server.port != 0 && listenToPort(server.port, &server.ipfd) != C_OK) { + serverPanic("Failed to restore old listening sockets."); + } + + server.tlsfd.count = 0; + if (server.tls_port != 0 && listenToPort(server.tls_port, &server.tlsfd) != C_OK) { + serverPanic("Failed to restore old listening sockets."); + } + + result = C_ERR; + } else { + /* Free old bind addresses */ + for (i = 0; i < prev_bindaddr_count; i++) { + zfree(prev_bindaddr[i]); + } + } + + /* Create TCP and TLS event handlers */ + if (createSocketAcceptHandler(&server.ipfd, acceptTcpHandler) != C_OK) { + serverPanic("Unrecoverable error creating TCP socket accept handler."); + } + if (createSocketAcceptHandler(&server.tlsfd, acceptTLSHandler) != C_OK) { + serverPanic("Unrecoverable error creating TLS socket accept handler."); + } + + if (server.set_proc_title) redisSetProcTitle(NULL); + + return result; +} + +int changeListenPort(int port, socketFds *sfd, aeFileProc *accept_handler) { + socketFds new_sfd = {{0}}; + + /* Just close the server if port disabled */ + if (port == 0) { + closeSocketListeners(sfd); + if (server.set_proc_title) redisSetProcTitle(NULL); + return C_OK; + } + + /* Bind to the new port */ + if (listenToPort(port, &new_sfd) != C_OK) { + return C_ERR; + } + + /* Create event handlers */ + if (createSocketAcceptHandler(&new_sfd, accept_handler) != C_OK) { + closeSocketListeners(&new_sfd); + return C_ERR; + } + + /* Close old servers */ + closeSocketListeners(sfd); + + /* Copy new descriptors */ + sfd->count = new_sfd.count; + memcpy(sfd->fd, new_sfd.fd, sizeof(new_sfd.fd)); + + if (server.set_proc_title) redisSetProcTitle(NULL); + + return C_OK; +} + static void sigShutdownHandler(int sig) { char *msg; @@ -6124,7 +6244,7 @@ int main(int argc, char **argv) { exit(1); } } - if (server.ipfd_count > 0 || server.tlsfd_count > 0) + if (server.ipfd.count > 0 || server.tlsfd.count > 0) serverLog(LL_NOTICE,"Ready to accept connections"); if (server.sofd > 0) serverLog(LL_NOTICE,"The server is now ready to accept connections at %s", server.unixsocket); diff --git a/src/server.h b/src/server.h index f52c2ee06..e241bad70 100644 --- a/src/server.h +++ b/src/server.h @@ -1104,6 +1104,11 @@ struct malloc_stats { size_t allocator_resident; }; +typedef struct socketFds { + int fd[CONFIG_BINDADDR_MAX]; + int count; +} socketFds; + /*----------------------------------------------------------------------------- * TLS Context Configuration *----------------------------------------------------------------------------*/ @@ -1204,13 +1209,10 @@ struct redisServer { int bindaddr_count; /* Number of addresses in server.bindaddr[] */ char *unixsocket; /* UNIX socket path */ mode_t unixsocketperm; /* UNIX socket permission */ - int ipfd[CONFIG_BINDADDR_MAX]; /* TCP socket file descriptors */ - int ipfd_count; /* Used slots in ipfd[] */ - int tlsfd[CONFIG_BINDADDR_MAX]; /* TLS socket file descriptors */ - int tlsfd_count; /* Used slots in tlsfd[] */ + socketFds ipfd; /* TCP socket file descriptors */ + socketFds tlsfd; /* TLS socket file descriptors */ int sofd; /* Unix socket file descriptor */ - int cfd[CONFIG_BINDADDR_MAX];/* Cluster bus listening socket */ - int cfd_count; /* Used slots in cfd[] */ + socketFds cfd; /* Cluster bus listening socket */ list *clients; /* List of active clients */ list *clients_to_close; /* Clients to close asynchronously */ list *clients_pending_write; /* There is to write or install handler. */ @@ -1855,7 +1857,7 @@ int getClientTypeByName(char *name); char *getClientTypeName(int class); void flushSlavesOutputBuffers(void); void disconnectSlaves(void); -int listenToPort(int port, int *fds, int *count); +int listenToPort(int port, socketFds *fds); void pauseClients(mstime_t duration, pause_type type); void unpauseClients(void); int areClientsPaused(void); @@ -2184,6 +2186,9 @@ int processCommand(client *c); int processPendingCommandsAndResetClient(client *c); void setupSignalHandlers(void); void removeSignalHandlers(void); +int createSocketAcceptHandler(socketFds *sfd, aeFileProc *accept_handler); +int changeListenPort(int port, socketFds *sfd, aeFileProc *accept_handler); +int changeBindAddr(sds *addrlist, int addrlist_len); struct redisCommand *lookupCommand(sds name); struct redisCommand *lookupCommandByCString(const char *s); struct redisCommand *lookupCommandOrOriginal(sds name); diff --git a/tests/test_helper.tcl b/tests/test_helper.tcl index eb14b5ee9..3c7d46d57 100644 --- a/tests/test_helper.tcl +++ b/tests/test_helper.tcl @@ -75,6 +75,7 @@ set ::all_tests { unit/tracking unit/oom-score-adj unit/shutdown + unit/networking } # Index to the next test to run in the ::all_tests list. set ::next_test 0 diff --git a/tests/unit/introspection.tcl b/tests/unit/introspection.tcl index c1cf72f0c..698ed7789 100644 --- a/tests/unit/introspection.tcl +++ b/tests/unit/introspection.tcl @@ -100,13 +100,10 @@ start_server {tags {"introspection"}} { supervised syslog-facility databases - port - tls-port io-threads logfile unixsocketperm slaveof - bind requirepass server_cpulist bio_cpulist @@ -131,6 +128,7 @@ start_server {tags {"introspection"}} { tls-protocols tls-ciphers tls-ciphersuites + tls-port } } diff --git a/tests/unit/networking.tcl b/tests/unit/networking.tcl new file mode 100644 index 000000000..1b5f19709 --- /dev/null +++ b/tests/unit/networking.tcl @@ -0,0 +1,34 @@ +test {CONFIG SET port number} { + start_server {} { + # available port + set avail_port [find_available_port $::baseport $::portcount] + set rd [redis [srv 0 host] [srv 0 port] 0 0] + $rd CONFIG SET port $avail_port + $rd close + set rd [redis [srv 0 host] $avail_port 0 0] + $rd PING + + # already inuse port + catch {$rd CONFIG SET port $::test_server_port} e + assert_match {*Unable to listen on this port*} $e + $rd close + + # make sure server still listening on the previous port + set rd [redis [srv 0 host] $avail_port 0 0] + $rd PING + $rd close + } +} + +test {CONFIG SET bind address} { + start_server {} { + # non-valid address + catch {r CONFIG SET bind "some.wrong.bind.address"} e + assert_match {*Failed to bind to specified addresses*} $e + + # make sure server still bound to the previous address + set rd [redis [srv 0 host] [srv 0 port] 0 0] + $rd PING + $rd close + } +}
\ No newline at end of file diff --git a/tests/unit/tls.tcl b/tests/unit/tls.tcl index a6c2f3c2c..14e06fcdf 100644 --- a/tests/unit/tls.tcl +++ b/tests/unit/tls.tcl @@ -114,5 +114,30 @@ start_server {tags {"tls"}} { } } } + + test {TLS: switch between tcp and tls ports} { + set srv_port [srv 0 port] + + # TLS + set rd [redis [srv 0 host] $srv_port 0 1] + $rd PING + + # TCP + $rd CONFIG SET tls-port 0 + $rd CONFIG SET port $srv_port + $rd close + + set rd [redis [srv 0 host] $srv_port 0 0] + $rd PING + + # TLS + $rd CONFIG SET port 0 + $rd CONFIG SET tls-port $srv_port + $rd close + + set rd [redis [srv 0 host] $srv_port 0 1] + $rd PING + $rd close + } } } |