Unverified Commit 22a29935 authored by Chen Tianjie's avatar Chen Tianjie Committed by GitHub
Browse files

Support TLS service when "tls-cluster" is not enabled and persist both plain...

Support TLS service when "tls-cluster" is not enabled and persist both plain and TLS port in nodes.conf (#12233)

Originally, when "tls-cluster" is enabled, `port` is set to TLS port. In order to support non-TLS clients, `pport` is used to propagate TCP port across cluster nodes. However when "tls-cluster" is disabled, `port` is set to TCP port, and `pport` is not used, which means the cluster cannot provide TLS service unless "tls-cluster" is on.
```
typedef struct {
    // ...
    uint16_t port;  /* Latest known clients port (TLS or plain). */
    uint16_t pport; /* Latest known clients plaintext port. Only used if the main clients port is for TLS. */
    // ...
} clusterNode;
```
```
typedef struct {
    // ...
    uint16_t port;   /* TCP base port number. */
    uint16_t pport;  /* Sender TCP plaintext port, if base port is TLS */
    // ...
} clusterMsg;
```
This PR renames `port` and `pport` in `clusterNode` to `tcp_port` and `tls_port`, to record both ports no matter "tls-cluster" is enabled or disabled.

This allows to provide TLS service to clients when "tls-cluster" is disabled: when displaying cluster topology, or giving `MOVED` error, server can provide TLS or TCP port according to client's connection type, no matter what type of connection cluster bus is using.

For backwards compatibility, `port` and `pport` in `clusterMsg` are preserved, when "tls-cluster" is enabled, `port` is set to TLS port and `pport` is set to TCP port, when "tls-cluster" is disabled, `port` is set to TCP port and `pport` is set to TLS port (instead of 0).

Also, in the nodes.conf file, a new aux field displaying an extra port is added to complete the persisted info. We may have `tls_port=xxxxx` or `tcp_port=xxxxx` in the aux field, to complete the cluster topology, while the other port is stored in the normal `<ip>:<port>` field. The format is shown below.
```
<node-id> <ip>:<tcp_port>@<cport>,<hostname>,shard-id=...,tls-port=6379 myself,master - 0 0 0 connected 0-1000
```
Or we can switch the position of two ports, both can be correctly resolved.
```
<node-id> <ip>:<tls_port>@<cport>,<hostname>,shard-id=...,tcp-port=6379 myself,master - 0 0 0 connected 0-1000
```
parent 9600553e
This diff is collapsed.
...@@ -142,9 +142,8 @@ typedef struct clusterNode { ...@@ -142,9 +142,8 @@ typedef struct clusterNode {
char ip[NET_IP_STR_LEN]; /* Latest known IP address of this node */ char ip[NET_IP_STR_LEN]; /* Latest known IP address of this node */
sds hostname; /* The known hostname for this node */ sds hostname; /* The known hostname for this node */
sds human_nodename; /* The known human readable nodename for this node */ sds human_nodename; /* The known human readable nodename for this node */
int port; /* Latest known clients port (TLS or plain). */ int tcp_port; /* Latest known clients TCP port. */
int pport; /* Latest known clients plaintext port. Only used int tls_port; /* Latest known clients TLS port */
if the main clients port is for TLS. */
int cport; /* Latest known cluster port of this node. */ int cport; /* Latest known cluster port of this node. */
clusterLink *link; /* TCP/IP link established toward this node */ clusterLink *link; /* TCP/IP link established toward this node */
clusterLink *inbound_link; /* TCP/IP link accepted from this node */ clusterLink *inbound_link; /* TCP/IP link accepted from this node */
...@@ -226,10 +225,10 @@ typedef struct { ...@@ -226,10 +225,10 @@ typedef struct {
uint32_t ping_sent; uint32_t ping_sent;
uint32_t pong_received; uint32_t pong_received;
char ip[NET_IP_STR_LEN]; /* IP address last time it was seen */ char ip[NET_IP_STR_LEN]; /* IP address last time it was seen */
uint16_t port; /* base port last time it was seen */ uint16_t port; /* primary port last time it was seen */
uint16_t cport; /* cluster port last time it was seen */ uint16_t cport; /* cluster port last time it was seen */
uint16_t flags; /* node->flags copy */ uint16_t flags; /* node->flags copy */
uint16_t pport; /* plaintext-port, when base port is TLS */ uint16_t pport; /* secondary port last time it was seen */
uint16_t notused1; uint16_t notused1;
} clusterMsgDataGossip; } clusterMsgDataGossip;
...@@ -338,7 +337,7 @@ typedef struct { ...@@ -338,7 +337,7 @@ typedef struct {
char sig[4]; /* Signature "RCmb" (Redis Cluster message bus). */ char sig[4]; /* Signature "RCmb" (Redis Cluster message bus). */
uint32_t totlen; /* Total length of this message */ uint32_t totlen; /* Total length of this message */
uint16_t ver; /* Protocol version, currently set to 1. */ uint16_t ver; /* Protocol version, currently set to 1. */
uint16_t port; /* TCP base port number. */ uint16_t port; /* Primary port number (TCP or TLS). */
uint16_t type; /* Message type */ uint16_t type; /* Message type */
uint16_t count; /* Only used for some kind of messages. */ uint16_t count; /* Only used for some kind of messages. */
uint64_t currentEpoch; /* The epoch accordingly to the sending node. */ uint64_t currentEpoch; /* The epoch accordingly to the sending node. */
...@@ -353,7 +352,8 @@ typedef struct { ...@@ -353,7 +352,8 @@ typedef struct {
char myip[NET_IP_STR_LEN]; /* Sender IP, if not all zeroed. */ char myip[NET_IP_STR_LEN]; /* Sender IP, if not all zeroed. */
uint16_t extensions; /* Number of extensions sent along with this packet. */ uint16_t extensions; /* Number of extensions sent along with this packet. */
char notused1[30]; /* 30 bytes reserved for future usage. */ char notused1[30]; /* 30 bytes reserved for future usage. */
uint16_t pport; /* Sender TCP plaintext port, if base port is TLS */ uint16_t pport; /* Secondary port number: if primary port is TCP port, this is
TLS port, and if primary port is TLS port, this is TCP port.*/
uint16_t cport; /* Sender TCP cluster bus port */ uint16_t cport; /* Sender TCP cluster bus port */
uint16_t flags; /* Sender node flags */ uint16_t flags; /* Sender node flags */
unsigned char state; /* Cluster state from the POV of the sender */ unsigned char state; /* Cluster state from the POV of the sender */
...@@ -429,9 +429,11 @@ void slotToChannelAdd(sds channel); ...@@ -429,9 +429,11 @@ void slotToChannelAdd(sds channel);
void slotToChannelDel(sds channel); void slotToChannelDel(sds channel);
void clusterUpdateMyselfHostname(void); void clusterUpdateMyselfHostname(void);
void clusterUpdateMyselfAnnouncedPorts(void); void clusterUpdateMyselfAnnouncedPorts(void);
sds clusterGenNodesDescription(client *c, int filter, int use_pport); sds clusterGenNodesDescription(client *c, int filter, int tls_primary);
sds genClusterInfoString(void); sds genClusterInfoString(void);
void freeClusterLink(clusterLink *link); void freeClusterLink(clusterLink *link);
void clusterUpdateMyselfHumanNodename(void); void clusterUpdateMyselfHumanNodename(void);
int isValidAuxString(char *s, unsigned int length); int isValidAuxString(char *s, unsigned int length);
int getNodeDefaultClientPort(clusterNode *n);
#endif /* __CLUSTER_H */ #endif /* __CLUSTER_H */
...@@ -446,4 +446,9 @@ int RedisRegisterConnectionTypeSocket(void); ...@@ -446,4 +446,9 @@ int RedisRegisterConnectionTypeSocket(void);
int RedisRegisterConnectionTypeUnix(void); int RedisRegisterConnectionTypeUnix(void);
int RedisRegisterConnectionTypeTLS(void); int RedisRegisterConnectionTypeTLS(void);
/* Return 1 if connection is using TLS protocol, 0 if otherwise. */
static inline int connIsTLS(connection *conn) {
return conn && conn->type == connectionTypeTls();
}
#endif /* __REDIS_CONNECTION_H */ #endif /* __REDIS_CONNECTION_H */
...@@ -8944,7 +8944,7 @@ int RM_GetClusterNodeInfo(RedisModuleCtx *ctx, const char *id, char *ip, char *m ...@@ -8944,7 +8944,7 @@ int RM_GetClusterNodeInfo(RedisModuleCtx *ctx, const char *id, char *ip, char *m
else else
memset(master_id,0,REDISMODULE_NODE_ID_LEN); memset(master_id,0,REDISMODULE_NODE_ID_LEN);
} }
if (port) *port = node->port; if (port) *port = getNodeDefaultClientPort(node);
   
/* As usually we have to remap flags for modules, in order to ensure /* As usually we have to remap flags for modules, in order to ensure
* we can provide binary compatibility. */ * we can provide binary compatibility. */
......
...@@ -70,9 +70,18 @@ proc continuous_slot_allocation {masters} { ...@@ -70,9 +70,18 @@ proc continuous_slot_allocation {masters} {
# tests run. # tests run.
proc cluster_setup {masters node_count slot_allocator code} { proc cluster_setup {masters node_count slot_allocator code} {
# Have all nodes meet # Have all nodes meet
for {set i 1} {$i < $node_count} {incr i} { if {$::tls} {
R 0 CLUSTER MEET [srv -$i host] [srv -$i port] set tls_cluster [lindex [R 0 CONFIG GET tls-cluster] 1]
} }
if {$::tls && !$tls_cluster} {
for {set i 1} {$i < $node_count} {incr i} {
R 0 CLUSTER MEET [srv -$i host] [srv -$i pport]
}
} else {
for {set i 1} {$i < $node_count} {incr i} {
R 0 CLUSTER MEET [srv -$i host] [srv -$i port]
}
}
$slot_allocator $masters $slot_allocator $masters
......
...@@ -496,7 +496,8 @@ proc start_server {options {code undefined}} { ...@@ -496,7 +496,8 @@ proc start_server {options {code undefined}} {
# start every server on a different port # start every server on a different port
set port [find_available_port $::baseport $::portcount] set port [find_available_port $::baseport $::portcount]
if {$::tls} { if {$::tls} {
dict set config "port" 0 set pport [find_available_port $::baseport $::portcount]
dict set config "port" $pport
dict set config "tls-port" $port dict set config "tls-port" $port
dict set config "tls-cluster" "yes" dict set config "tls-cluster" "yes"
dict set config "tls-replication" "yes" dict set config "tls-replication" "yes"
...@@ -567,6 +568,8 @@ proc start_server {options {code undefined}} { ...@@ -567,6 +568,8 @@ proc start_server {options {code undefined}} {
puts "Port $port was already busy, trying another port..." puts "Port $port was already busy, trying another port..."
set port [find_available_port $::baseport $::portcount] set port [find_available_port $::baseport $::portcount]
if {$::tls} { if {$::tls} {
set pport [find_available_port $::baseport $::portcount]
dict set config port $pport
dict set config "tls-port" $port dict set config "tls-port" $port
} else { } else {
dict set config port $port dict set config port $port
...@@ -615,6 +618,9 @@ proc start_server {options {code undefined}} { ...@@ -615,6 +618,9 @@ proc start_server {options {code undefined}} {
dict set srv "stdout" $stdout dict set srv "stdout" $stdout
dict set srv "stderr" $stderr dict set srv "stderr" $stderr
dict set srv "unixsocket" $unixsocket dict set srv "unixsocket" $unixsocket
if {$::tls} {
dict set srv "pport" $pport
}
# if a block of code is supplied, we wait for the server to become # if a block of code is supplied, we wait for the server to become
# available, create a client object and kill the server afterwards # available, create a client object and kill the server afterwards
......
...@@ -102,6 +102,7 @@ set ::all_tests { ...@@ -102,6 +102,7 @@ set ::all_tests {
unit/cluster/multi-slot-operations unit/cluster/multi-slot-operations
unit/cluster/slot-ownership unit/cluster/slot-ownership
unit/cluster/links unit/cluster/links
unit/cluster/cluster-response-tls
} }
# Index to the next test to run in the ::all_tests list. # Index to the next test to run in the ::all_tests list.
set ::next_test 0 set ::next_test 0
......
source tests/support/cluster.tcl
proc get_port_from_moved_error {e} {
set ip_port [lindex [split $e " "] 2]
return [lindex [split $ip_port ":"] 1]
}
proc get_pport_by_port {port} {
foreach srv $::servers {
set srv_port [dict get $srv port]
if {$port == $srv_port} {
return [dict get $srv pport]
}
}
return 0
}
proc get_port_from_node_info {line} {
set fields [split $line " "]
set addr [lindex $fields 1]
set ip_port [lindex [split $addr "@"] 0]
return [lindex [split $ip_port ":"] 1]
}
proc cluster_response_tls {tls_cluster} {
test "CLUSTER SLOTS with different connection type -- tls-cluster $tls_cluster" {
set slots1 [R 0 cluster slots]
set pport [srv 0 pport]
set cluster_client [redis_cluster 127.0.0.1:$pport 0]
set slots2 [$cluster_client cluster slots]
$cluster_client close
# Compare the ports in the first row
assert_no_match [lindex $slots1 0 2 1] [lindex $slots2 0 2 1]
}
test "CLUSTER NODES return port according to connection type -- tls-cluster $tls_cluster" {
set nodes [R 0 cluster nodes]
set port1 [get_port_from_node_info [lindex [split $nodes "\r\n"] 0]]
set pport [srv 0 pport]
set cluster_client [redis_cluster 127.0.0.1:$pport 0]
set nodes [$cluster_client cluster nodes]
set port2 [get_port_from_node_info [lindex [split $nodes "\r\n"] 0]]
$cluster_client close
assert_not_equal $port1 $port2
}
set cluster [redis_cluster 127.0.0.1:[srv 0 port]]
set cluster_pport [redis_cluster 127.0.0.1:[srv 0 pport] 0]
$cluster refresh_nodes_map
test "Set many keys in the cluster -- tls-cluster $tls_cluster" {
for {set i 0} {$i < 5000} {incr i} {
$cluster set $i $i
assert { [$cluster get $i] eq $i }
}
}
test "Test cluster responses during migration of slot x -- tls-cluster $tls_cluster" {
set slot 10
array set nodefrom [$cluster masternode_for_slot $slot]
array set nodeto [$cluster masternode_notfor_slot $slot]
$nodeto(link) cluster setslot $slot importing $nodefrom(id)
$nodefrom(link) cluster setslot $slot migrating $nodeto(id)
# Get a key from that slot
set key [$nodefrom(link) cluster GETKEYSINSLOT $slot "1"]
# MOVED REPLY
catch {$nodeto(link) set $key "newVal"} e_moved1
assert_match "*MOVED*" $e_moved1
# ASK REPLY
catch {$nodefrom(link) set "abc{$key}" "newVal"} e_ask1
assert_match "*ASK*" $e_ask1
# UNSTABLE REPLY
assert_error "*TRYAGAIN*" {$nodefrom(link) mset "a{$key}" "newVal" $key "newVal2"}
# Connecting using another protocol
array set nodefrom_pport [$cluster_pport masternode_for_slot $slot]
array set nodeto_pport [$cluster_pport masternode_notfor_slot $slot]
# MOVED REPLY
catch {$nodeto_pport(link) set $key "newVal"} e_moved2
assert_match "*MOVED*" $e_moved2
# ASK REPLY
catch {$nodefrom_pport(link) set "abc{$key}" "newVal"} e_ask2
assert_match "*ASK*" $e_ask2
# Compare MOVED error's port
set port1 [get_port_from_moved_error $e_moved1]
set port2 [get_port_from_moved_error $e_moved2]
assert_not_equal $port1 $port2
assert_equal $port1 $nodefrom(port)
assert_equal $port2 [get_pport_by_port $nodefrom(port)]
# Compare ASK error's port
set port1 [get_port_from_moved_error $e_ask1]
set port2 [get_port_from_moved_error $e_ask2]
assert_not_equal $port1 $port2
assert_equal $port1 $nodeto(port)
assert_equal $port2 [get_pport_by_port $nodeto(port)]
}
}
if {$::tls} {
start_cluster 3 3 {tags {external:skip cluster tls} overrides {tls-cluster yes tls-replication yes}} {
cluster_response_tls yes
}
start_cluster 3 3 {tags {external:skip cluster tls} overrides {tls-cluster no tls-replication no}} {
cluster_response_tls no
}
}
...@@ -404,13 +404,14 @@ start_server {tags {"other external:skip"}} { ...@@ -404,13 +404,14 @@ start_server {tags {"other external:skip"}} {
assert_match "*/redis-server" [lindex $cmdline 1] assert_match "*/redis-server" [lindex $cmdline 1]
if {$::tls} { if {$::tls} {
set expect_port 0 set expect_port [srv 0 pport]
set expect_tls_port [srv 0 port] set expect_tls_port [srv 0 port]
set port [srv 0 pport]
} else { } else {
set expect_port [srv 0 port] set expect_port [srv 0 port]
set expect_tls_port 0 set expect_tls_port 0
set port [srv 0 port]
} }
set port [srv 0 port]
assert_equal "$::host:$port" [lindex $cmdline 2] assert_equal "$::host:$port" [lindex $cmdline 2]
assert_equal $expect_port [lindex $cmdline 3] assert_equal $expect_port [lindex $cmdline 3]
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment