Add bind-source-addr configuration argument. (#9142)

In the past, the first bind address that was explicitly specified was
also used to bind outgoing connections. This could result with some
problems. For example: on some systems using `bind 127.0.0.1` would
result with outgoing connections also binding to `127.0.0.1` and failing
to connect to remote addresses.

With the recent change to the way `bind` is handled, this presented
other issues:

* The default first bind address is '*' which is not a valid address.
* We make no distinction between user-supplied config that is identical
to the default, and the default config.

This commit addresses both these issues by introducing an explicit
configuration parameter to control the bind address on outgoing
connections.

8 files changed, 43 insertions, 7 deletions

diff --git a/redis.conf b/redis.conf
index 9e72d9c45..b064f8515 100644
--- a/redis.conf
+++ b/redis.conf
@@ -83,6 +83,18 @@
 # ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 bind 127.0.0.1 -::1
 
+# By default, outgoing connections (from replica to master, from Sentinel to
+# instances, cluster bus, etc.) are not bound to a specific local address. In
+# most cases, this means the operating system will handle that based on routing
+# and the interface through which the connection goes out.
+#
+# Using bind-source-addr it is possible to configure a specific address to bind
+# to, which may also affect how the connection gets routed.
+#
+# Example:
+#
+# bind-source-addr 10.0.0.1
+
 # Protected mode is a layer of security protection, in order to avoid that
 # Redis instances left open on the internet are accessed and exploited.
 #
diff --git a/src/cluster.c b/src/cluster.c
index 16c2e4b56..12ea935d5 100644
--- a/src/cluster.c
+++ b/src/cluster.c
@@ -3605,7 +3605,7 @@ void clusterCron(void) {
             clusterLink *link = createClusterLink(node);
             link->conn = server.tls_cluster ? connCreateTLS() : connCreateSocket();
             connSetPrivateData(link->conn, link);
-            if (connConnect(link->conn, node->ip, node->cport, NET_FIRST_BIND_ADDR,
+            if (connConnect(link->conn, node->ip, node->cport, server.bind_source_addr,
                         clusterLinkConnectHandler) == -1) {
                 /* We got a synchronous error from connect before
                  * clusterSendPing() had a chance to be called.
diff --git a/src/config.c b/src/config.c
index 45dc4046b..099ea0f49 100644
--- a/src/config.c
+++ b/src/config.c
@@ -2536,6 +2536,7 @@ standardConfig configs[] = {
     createStringConfig("bgsave_cpulist", NULL, IMMUTABLE_CONFIG, EMPTY_STRING_IS_NULL, server.bgsave_cpulist, NULL, NULL, NULL),
     createStringConfig("ignore-warnings", NULL, MODIFIABLE_CONFIG, ALLOW_EMPTY_STRING, server.ignore_warnings, "", NULL, NULL),
     createStringConfig("proc-title-template", NULL, MODIFIABLE_CONFIG, ALLOW_EMPTY_STRING, server.proc_title_template, CONFIG_DEFAULT_PROC_TITLE_TEMPLATE, isValidProcTitleTemplate, updateProcTitleTemplate),
+    createStringConfig("bind-source-addr", NULL, MODIFIABLE_CONFIG, EMPTY_STRING_IS_NULL, server.bind_source_addr, NULL, NULL, NULL),
 
     /* SDS Configs */
     createSDSConfig("masterauth", NULL, MODIFIABLE_CONFIG | SENSITIVE_CONFIG, EMPTY_STRING_IS_NULL, server.masterauth, NULL, NULL, NULL),
diff --git a/src/replication.c b/src/replication.c
index 7e27a9b56..b40d3f314 100644
--- a/src/replication.c
+++ b/src/replication.c
@@ -2539,7 +2539,7 @@ write_error: /* Handle sendCommand() errors. */
 int connectWithMaster(void) {
     server.repl_transfer_s = server.tls_replication ? connCreateTLS() : connCreateSocket();
     if (connConnect(server.repl_transfer_s, server.masterhost, server.masterport,
-                NET_FIRST_BIND_ADDR, syncWithMaster) == C_ERR) {
+                server.bind_source_addr, syncWithMaster) == C_ERR) {
         serverLog(LL_WARNING,"Unable to connect to MASTER: %s",
                 connGetLastError(server.repl_transfer_s));
         connClose(server.repl_transfer_s);
diff --git a/src/sentinel.c b/src/sentinel.c
index 526b2c167..038240b27 100644
--- a/src/sentinel.c
+++ b/src/sentinel.c
@@ -2403,7 +2403,7 @@ void sentinelReconnectInstance(sentinelRedisInstance *ri) {
 
     /* Commands connection. */
     if (link->cc == NULL) {
-        link->cc = redisAsyncConnectBind(ri->addr->ip,ri->addr->port,NET_FIRST_BIND_ADDR);
+        link->cc = redisAsyncConnectBind(ri->addr->ip,ri->addr->port,server.bind_source_addr);
         if (link->cc && !link->cc->err) anetCloexec(link->cc->c.fd);
         if (!link->cc) {
             sentinelEvent(LL_DEBUG,"-cmd-link-reconnection",ri,"%@ #Failed to establish connection");
@@ -2433,7 +2433,7 @@ void sentinelReconnectInstance(sentinelRedisInstance *ri) {
     }
     /* Pub / Sub */
     if ((ri->flags & (SRI_MASTER|SRI_SLAVE)) && link->pc == NULL) {
-        link->pc = redisAsyncConnectBind(ri->addr->ip,ri->addr->port,NET_FIRST_BIND_ADDR);
+        link->pc = redisAsyncConnectBind(ri->addr->ip,ri->addr->port,server.bind_source_addr);
         if (link->pc && !link->pc->err) anetCloexec(link->pc->c.fd);
         if (!link->pc) {
             sentinelEvent(LL_DEBUG,"-pubsub-link-reconnection",ri,"%@ #Failed to establish connection");
diff --git a/src/server.c b/src/server.c
index 2376c68fa..c19df1837 100644
--- a/src/server.c
+++ b/src/server.c
@@ -2640,6 +2640,7 @@ void initServerConfig(void) {
     server.bindaddr_count = CONFIG_DEFAULT_BINDADDR_COUNT;
     for (j = 0; j < CONFIG_DEFAULT_BINDADDR_COUNT; j++)
         server.bindaddr[j] = zstrdup(default_bindaddr[j]);
+    server.bind_source_addr = NULL;
     server.unixsocketperm = CONFIG_DEFAULT_UNIX_SOCKET_PERM;
     server.ipfd.count = 0;
     server.tlsfd.count = 0;
diff --git a/src/server.h b/src/server.h
index 8e5edd76a..232874a30 100644
--- a/src/server.h
+++ b/src/server.h
@@ -494,9 +494,6 @@ typedef enum {
 #define NOTIFY_MODULE (1<<13)     /* d, module key space notification */
 #define NOTIFY_ALL (NOTIFY_GENERIC | NOTIFY_STRING | NOTIFY_LIST | NOTIFY_SET | NOTIFY_HASH | NOTIFY_ZSET | NOTIFY_EXPIRED | NOTIFY_EVICTED | NOTIFY_STREAM | NOTIFY_MODULE) /* A flag */
 
-/* Get the first bind addr or NULL */
-#define NET_FIRST_BIND_ADDR (server.bindaddr_count ? server.bindaddr[0] : NULL)
-
 /* Using the following macro you can run code inside serverCron() with the
  * specified period, specified in milliseconds.
  * The actual resolution depends on server.hz. */
@@ -1265,6 +1262,7 @@ struct redisServer {
     int tcp_backlog;            /* TCP listen() backlog */
     char *bindaddr[CONFIG_BINDADDR_MAX]; /* Addresses we should bind to */
     int bindaddr_count;         /* Number of addresses in server.bindaddr[] */
+    char *bind_source_addr;     /* Source address to bind on for outgoing connections */
     char *unixsocket;           /* UNIX socket path */
     mode_t unixsocketperm;      /* UNIX socket permission */
     socketFds ipfd;             /* TCP socket file descriptors */
diff --git a/tests/unit/networking.tcl b/tests/unit/networking.tcl
index 37826d241..81e7b10e9 100644
--- a/tests/unit/networking.tcl
+++ b/tests/unit/networking.tcl
@@ -37,6 +37,30 @@ test {CONFIG SET bind address} {
     }
 } {} {external:skip}
 
+test {CONFIG SET bind-source-addr} {
+    if {[exec uname] == {Linux}} {
+        start_server {} {
+            start_server {} {
+                set replica [srv 0 client]
+                set master [srv -1 client]
+
+                $master config set protected-mode no
+
+                $replica config set bind-source-addr 127.0.0.2
+                $replica replicaof [srv -1 host] [srv -1 port]
+
+                wait_for_condition 50 100 {
+                    [s 0 master_link_status] eq {up}
+                } else {
+                    fail "Replication not started."
+                }
+
+                assert_match {*ip=127.0.0.2*} [s -1 slave0]
+            }
+        }
+    }
+} {} {external:skip}
+
 start_server {config "minimal.conf" tags {"external:skip"}} {
     test {Default bind address configuration handling} {
         # Default is explicit and sane