Add support for plaintext clients in TLS cluster (#8587)

The cluster bus is established over TLS or non-TLS depending on the configuration tls-cluster. The client ports distributed in the cluster and sent to clients are assumed to be TLS or non-TLS also depending on tls-cluster.

The cluster bus is now extended to also contain the non-TLS port of clients in a TLS cluster, when available. The non-TLS port of a cluster node, when available, is sent to clients connected without TLS in responses to CLUSTER SLOTS, CLUSTER NODES, CLUSTER SLAVES and MOVED and ASK redirects, instead of the TLS port.

The user was able to override the client port by defining cluster-announce-port. Now cluster-announce-tls-port is added, so the user can define an alternative announce port for both TLS and non-TLS clients.

Fixes #8134

5 files changed, 51 insertions, 9 deletions

diff --git a/tests/cluster/tests/04-resharding.tcl b/tests/cluster/tests/04-resharding.tcl
index 1dcdb5a2c..4d31d314c 100644
--- a/tests/cluster/tests/04-resharding.tcl
+++ b/tests/cluster/tests/04-resharding.tcl
@@ -54,7 +54,17 @@ proc process_is_running {pid} {
 
 set numkeys 50000
 set numops 200000
-set cluster [redis_cluster 127.0.0.1:[get_instance_attrib redis 0 port]]
+set start_node_port [get_instance_attrib redis 0 port]
+set cluster [redis_cluster 127.0.0.1:$start_node_port]
+if {$::tls} {
+    # setup a non-TLS cluster client to the TLS cluster
+    set plaintext_port [get_instance_attrib redis 0 plaintext-port]
+    set cluster_plaintext [redis_cluster 127.0.0.1:$plaintext_port 0]
+    puts "Testing TLS cluster on start node 127.0.0.1:$start_node_port, plaintext port $plaintext_port"
+} else {
+    set cluster_plaintext $cluster
+    puts "Testing using non-TLS cluster"
+}
 catch {unset content}
 array set content {}
 set tribpid {}
@@ -94,8 +104,11 @@ test "Cluster consistency during live resharding" {
         # This way we are able to stress Lua -> Redis command invocation
         # as well, that has tests to prevent Lua to write into wrong
         # hash slots.
-        if {$listid % 2} {
+        # We also use both TLS and plaintext connections.
+        if {$listid % 3 == 0} {
             $cluster rpush $key $ele
+        } elseif {$listid % 3 == 1} {
+            $cluster_plaintext rpush $key $ele
         } else {
             $cluster eval {redis.call("rpush",KEYS[1],ARGV[1])} 1 $key $ele
         }
diff --git a/tests/cluster/tests/15-cluster-slots.tcl b/tests/cluster/tests/15-cluster-slots.tcl
index 1b33c57bd..f154b7270 100644
--- a/tests/cluster/tests/15-cluster-slots.tcl
+++ b/tests/cluster/tests/15-cluster-slots.tcl
@@ -48,3 +48,16 @@ test "client can handle keys with hash tag" {
     $cluster set foo{tag} bar
     $cluster close
 }
+
+if {$::tls} {
+    test {CLUSTER SLOTS from non-TLS client in TLS cluster} {
+        set slots_tls [R 0 cluster slots]
+        set host [get_instance_attrib redis 0 host]
+        set plaintext_port [get_instance_attrib redis 0 plaintext-port]
+        set client_plain [redis $host $plaintext_port 0 0]
+        set slots_plain [$client_plain cluster slots]
+        $client_plain close
+        # Compare the ports in the first row
+        assert_no_match [lindex $slots_tls 0 3 1] [lindex $slots_plain 0 3 1]
+    }
+}
diff --git a/tests/instances.tcl b/tests/instances.tcl
index 255d9740f..ad605c316 100644
--- a/tests/instances.tcl
+++ b/tests/instances.tcl
@@ -64,6 +64,8 @@ proc exec_instance {type dirname cfgfile} {
 proc spawn_instance {type base_port count {conf {}} {base_conf_file ""}} {
     for {set j 0} {$j < $count} {incr j} {
         set port [find_available_port $base_port $::redis_port_count]
+        # plaintext port (only used for TLS cluster)
+        set pport 0
         # Create a directory for this instance.
         set dirname "${type}_${j}"
         lappend ::dirs $dirname
@@ -83,7 +85,9 @@ proc spawn_instance {type base_port count {conf {}} {base_conf_file ""}} {
             puts $cfg "tls-port $port"
             puts $cfg "tls-replication yes"
             puts $cfg "tls-cluster yes"
-            puts $cfg "port 0"
+            # plaintext port, only used by plaintext clients in a TLS cluster
+            set pport [find_available_port $base_port $::redis_port_count]
+            puts $cfg "port $pport"
             puts $cfg [format "tls-cert-file %s/../../tls/server.crt" [pwd]]
             puts $cfg [format "tls-key-file %s/../../tls/server.key" [pwd]]
             puts $cfg [format "tls-client-cert-file %s/../../tls/client.crt" [pwd]]
@@ -118,6 +122,8 @@ proc spawn_instance {type base_port count {conf {}} {base_conf_file ""}} {
                 set cfg [open $cfgfile a+]
                 if {$::tls} {
                     puts $cfg "tls-port $port"
+                    set pport [find_available_port $base_port $::redis_port_count]
+                    puts $cfg "port $pport"
                 } else {
                     puts $cfg "port $port"
                 }
@@ -143,6 +149,7 @@ proc spawn_instance {type base_port count {conf {}} {base_conf_file ""}} {
             pid $pid \
             host $::host \
             port $port \
+            plaintext-port $pport \
             link $link \
         ]
     }
diff --git a/tests/support/cluster.tcl b/tests/support/cluster.tcl
index 97a659a1d..df4b7f3d0 100644
--- a/tests/support/cluster.tcl
+++ b/tests/support/cluster.tcl
@@ -4,7 +4,7 @@
 #
 # Example usage:
 #
-# set c [redis_cluster 127.0.0.1 6379 127.0.0.1 6380]
+# set c [redis_cluster {127.0.0.1:6379 127.0.0.1:6380}]
 # $c set foo
 # $c get foo
 # $c close
@@ -17,6 +17,7 @@ set ::redis_cluster::id 0
 array set ::redis_cluster::startup_nodes {}
 array set ::redis_cluster::nodes {}
 array set ::redis_cluster::slots {}
+array set ::redis_cluster::tls {}
 
 # List of "plain" commands, which are commands where the sole key is always
 # the first argument.
@@ -34,11 +35,14 @@ set ::redis_cluster::plain_commands {
     dump bitcount bitpos pfadd pfcount
 }
 
-proc redis_cluster {nodes} {
+# Create a cluster client. The nodes are given as a list of host:port. The TLS
+# parameter (1 or 0) is optional and defaults to the global $::tls.
+proc redis_cluster {nodes {tls -1}} {
     set id [incr ::redis_cluster::id]
     set ::redis_cluster::startup_nodes($id) $nodes
     set ::redis_cluster::nodes($id) {}
     set ::redis_cluster::slots($id) {}
+    set ::redis_cluster::tls($id) [expr $tls == -1 ? $::tls : $tls]
     set handle [interp alias {} ::redis_cluster::instance$id {} ::redis_cluster::__dispatch__ $id]
     $handle refresh_nodes_map
     return $handle
@@ -60,9 +64,10 @@ proc ::redis_cluster::__method__refresh_nodes_map {id} {
     foreach start_node $::redis_cluster::startup_nodes($id) {
         set ip_port [lindex [split $start_node @] 0]
         lassign [split $ip_port :] start_host start_port
+        set tls $::redis_cluster::tls($id)
         if {[catch {
             set r {}
-            set r [redis $start_host $start_port 0 $::tls]
+            set r [redis $start_host $start_port 0 $tls]
             set nodes_descr [$r cluster nodes]
             $r close
         } e]} {
@@ -107,7 +112,8 @@ proc ::redis_cluster::__method__refresh_nodes_map {id} {
 
         # Connect to the node
         set link {}
-        catch {set link [redis $host $port 0 $::tls]}
+        set tls $::redis_cluster::tls($id)
+        catch {set link [redis $host $port 0 $tls]}
 
         # Build this node description as an hash.
         set node [dict create \
@@ -161,6 +167,7 @@ proc ::redis_cluster::__method__close {id} {
     catch {unset ::redis_cluster::startup_nodes($id)}
     catch {unset ::redis_cluster::nodes($id)}
     catch {unset ::redis_cluster::slots($id)}
+    catch {unset ::redis_cluster::tls($id)}
     catch {interp alias {} ::redis_cluster::instance$id {}}
 }
 
diff --git a/tests/support/redis.tcl b/tests/support/redis.tcl
index 373058daf..4d321c975 100644
--- a/tests/support/redis.tcl
+++ b/tests/support/redis.tcl
@@ -35,6 +35,7 @@ array set ::redis::addr {}
 array set ::redis::blocking {}
 array set ::redis::deferred {}
 array set ::redis::reconnect {}
+array set ::redis::tls {}
 array set ::redis::callback {}
 array set ::redis::state {} ;# State in non-blocking reply reading
 array set ::redis::statestack {} ;# Stack of states, for nested mbulks
@@ -58,7 +59,7 @@ proc redis {{server 127.0.0.1} {port 6379} {defer 0} {tls 0} {tlsoptions {}}} {
     set ::redis::blocking($id) 1
     set ::redis::deferred($id) $defer
     set ::redis::reconnect($id) 0
-    set ::redis::tls $tls
+    set ::redis::tls($id) $tls
     ::redis::redis_reset_state $id
     interp alias {} ::redis::redisHandle$id {} ::redis::__dispatch__ $id
 }
@@ -83,7 +84,7 @@ proc ::redis::__dispatch__raw__ {id method argv} {
     # Reconnect the link if needed.
     if {$fd eq {}} {
         lassign $::redis::addr($id) host port
-        if {$::redis::tls} {
+        if {$::redis::tls($id)} {
             set ::redis::fd($id) [::tls::socket $host $port]
         } else {
             set ::redis::fd($id) [socket $host $port]
@@ -158,6 +159,7 @@ proc ::redis::__method__close {id fd} {
     catch {unset ::redis::blocking($id)}
     catch {unset ::redis::deferred($id)}
     catch {unset ::redis::reconnect($id)}
+    catch {unset ::redis::tls($id)}
     catch {unset ::redis::state($id)}
     catch {unset ::redis::statestack($id)}
     catch {unset ::redis::callback($id)}