mirrors
/
redis
mirror of https://mirror.osredm.com/root/redis.git
2
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Cluster node name sanity check (#10391) 

* Limit cluster node id length for CLUSTER commands loading
* Cluster node name sanity check for length and values

Co-authored-by: Madelyn Olson <madelyneolson@gmail.com>
This commit is contained in:
bugwz 2022-04-05 13:51:51 +08:00 committed by GitHub
parent 9578b67e0e
commit 2db0d898f8
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 57 additions and 31 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

78
src/cluster.c
View File

@ -56,7 +56,6 @@ void clusterSendFailoverAuthIfNeeded(clusterNode *node, clusterMsg *request);
void clusterUpdateState(void); void clusterUpdateState(void);
int clusterNodeGetSlotBit(clusterNode *n, int slot); int clusterNodeGetSlotBit(clusterNode *n, int slot);
sds clusterGenNodesDescription(int filter, int use_pport); sds clusterGenNodesDescription(int filter, int use_pport);
clusterNode *clusterLookupNode(const char *name);
list *clusterGetNodesServingMySlots(clusterNode *node); list *clusterGetNodesServingMySlots(clusterNode *node);
int clusterNodeAddSlave(clusterNode *master, clusterNode *slave); int clusterNodeAddSlave(clusterNode *master, clusterNode *slave);
int clusterAddSlot(clusterNode *n, int slot); int clusterAddSlot(clusterNode *n, int slot);
@ -212,7 +211,11 @@ int clusterLoadConfig(char *filename) {
} }
/* Create this node if it does not exist */ /* Create this node if it does not exist */
n = clusterLookupNode(argv[0]); if (verifyClusterNodeId(argv[0], sdslen(argv[0])) == C_ERR) {
sdsfreesplitres(argv, argc);
goto fmterr;
}
n = clusterLookupNode(argv[0], sdslen(argv[0]));
if (!n) { if (!n) {
n = createClusterNode(argv[0],0); n = createClusterNode(argv[0],0);
clusterAddNode(n); clusterAddNode(n);
@ -288,7 +291,11 @@ int clusterLoadConfig(char *filename) {
/* Get master if any. Set the master and populate master's /* Get master if any. Set the master and populate master's
* slave list. */ * slave list. */
if (argv[3][0] != '-') { if (argv[3][0] != '-') {
master = clusterLookupNode(argv[3]); if (verifyClusterNodeId(argv[3], sdslen(argv[3])) == C_ERR) {
sdsfreesplitres(argv, argc);
goto fmterr;
}
master = clusterLookupNode(argv[3], sdslen(argv[3]));
if (!master) { if (!master) {
master = createClusterNode(argv[3],0); master = createClusterNode(argv[3],0);
clusterAddNode(master); clusterAddNode(master);
@ -324,7 +331,14 @@ int clusterLoadConfig(char *filename) {
goto fmterr; goto fmterr;
} }
p += 3; p += 3;
cn = clusterLookupNode(p);
char *pr = strchr(p, ']');
size_t node_len = pr - p;
if (pr == NULL || verifyClusterNodeId(p, node_len) == C_ERR) {
sdsfreesplitres(argv, argc);
goto fmterr;
}
cn = clusterLookupNode(p, CLUSTER_NAMELEN);
if (!cn) { if (!cn) {
cn = createClusterNode(p,0); cn = createClusterNode(p,0);
clusterAddNode(cn); clusterAddNode(cn);
@ -1182,12 +1196,23 @@ void clusterDelNode(clusterNode *delnode) {
freeClusterNode(delnode); freeClusterNode(delnode);
} }
/* Node lookup by name */ /* Cluster node sanity check. Returns C_OK if the node id
clusterNode *clusterLookupNode(const char *name) { * is valid an C_ERR otherwise. */
sds s = sdsnewlen(name, CLUSTER_NAMELEN); int verifyClusterNodeId(const char *name, int length) {
dictEntry *de; if (length != CLUSTER_NAMELEN) return C_ERR;
for (int i = 0; i < length; i++) {
if (name[i] >= 'a' && name[i] <= 'z') continue;
if (name[i] >= '0' && name[i] <= '9') continue;
return C_ERR;
}
return C_OK;
}
de = dictFind(server.cluster->nodes,s); /* Node lookup by name */
clusterNode *clusterLookupNode(const char *name, int length) {
if (verifyClusterNodeId(name, length) != C_OK) return NULL;
sds s = sdsnewlen(name, length);
dictEntry *de = dictFind(server.cluster->nodes, s);
sdsfree(s); sdsfree(s);
if (de == NULL) return NULL; if (de == NULL) return NULL;
return dictGetVal(de); return dictGetVal(de);
@ -1603,7 +1628,7 @@ int clusterStartHandshake(char *ip, int port, int cport) {
void clusterProcessGossipSection(clusterMsg *hdr, clusterLink *link) { void clusterProcessGossipSection(clusterMsg *hdr, clusterLink *link) {
uint16_t count = ntohs(hdr->count); uint16_t count = ntohs(hdr->count);
clusterMsgDataGossip *g = (clusterMsgDataGossip*) hdr->data.ping.gossip; clusterMsgDataGossip *g = (clusterMsgDataGossip*) hdr->data.ping.gossip;
clusterNode *sender = link->node ? link->node : clusterLookupNode(hdr->sender); clusterNode *sender = link->node ? link->node : clusterLookupNode(hdr->sender, CLUSTER_NAMELEN);
while(count--) { while(count--) {
uint16_t flags = ntohs(g->flags); uint16_t flags = ntohs(g->flags);
@ -1622,7 +1647,7 @@ void clusterProcessGossipSection(clusterMsg *hdr, clusterLink *link) {
} }
/* Update our state accordingly to the gossip sections */ /* Update our state accordingly to the gossip sections */
node = clusterLookupNode(g->nodename); node = clusterLookupNode(g->nodename, CLUSTER_NAMELEN);
if (node) { if (node) {
/* We already know this node. /* We already know this node.
Handle failure reports, only when the sender is a master. */ Handle failure reports, only when the sender is a master. */
@ -1985,7 +2010,7 @@ int writeHostnamePingExt(clusterMsgPingExt **cursor) {
/* We previously validated the extensions, so this function just needs to /* We previously validated the extensions, so this function just needs to
* handle the extensions. */ * handle the extensions. */
void clusterProcessPingExtensions(clusterMsg *hdr, clusterLink *link) { void clusterProcessPingExtensions(clusterMsg *hdr, clusterLink *link) {
clusterNode *sender = link->node ? link->node : clusterLookupNode(hdr->sender); clusterNode *sender = link->node ? link->node : clusterLookupNode(hdr->sender, CLUSTER_NAMELEN);
char *ext_hostname = NULL; char *ext_hostname = NULL;
uint16_t extensions = ntohs(hdr->extensions); uint16_t extensions = ntohs(hdr->extensions);
/* Loop through all the extensions and process them */ /* Loop through all the extensions and process them */
@ -2018,7 +2043,7 @@ static clusterNode *getNodeFromLinkAndMsg(clusterLink *link, clusterMsg *hdr) {
sender = link->node; sender = link->node;
} else { } else {
/* Otherwise, fetch sender based on the message */ /* Otherwise, fetch sender based on the message */
sender = clusterLookupNode(hdr->sender); sender = clusterLookupNode(hdr->sender, CLUSTER_NAMELEN);
/* We know the sender node but haven't associate it with the link. This must /* We know the sender node but haven't associate it with the link. This must
* be an inbound link because only for inbound links we didn't know which node * be an inbound link because only for inbound links we didn't know which node
* to associate when they were created. */ * to associate when they were created. */
@ -2329,7 +2354,7 @@ int clusterProcessPacket(clusterLink *link) {
clusterSetNodeAsMaster(sender); clusterSetNodeAsMaster(sender);
} else { } else {
/* Node is a slave. */ /* Node is a slave. */
clusterNode *master = clusterLookupNode(hdr->slaveof); clusterNode *master = clusterLookupNode(hdr->slaveof, CLUSTER_NAMELEN);
if (nodeIsMaster(sender)) { if (nodeIsMaster(sender)) {
/* Master turned into a slave! Reconfigure the node. */ /* Master turned into a slave! Reconfigure the node. */
@ -2444,7 +2469,7 @@ int clusterProcessPacket(clusterLink *link) {
clusterNode *failing; clusterNode *failing;
if (sender) { if (sender) {
failing = clusterLookupNode(hdr->data.fail.about.nodename); failing = clusterLookupNode(hdr->data.fail.about.nodename, CLUSTER_NAMELEN);
if (failing && if (failing &&
!(failing->flags & (CLUSTER_NODE_FAIL|CLUSTER_NODE_MYSELF))) !(failing->flags & (CLUSTER_NODE_FAIL|CLUSTER_NODE_MYSELF)))
{ {
@ -2532,7 +2557,7 @@ int clusterProcessPacket(clusterLink *link) {
ntohu64(hdr->data.update.nodecfg.configEpoch); ntohu64(hdr->data.update.nodecfg.configEpoch);
if (!sender) return 1; /* We don't know the sender. */ if (!sender) return 1; /* We don't know the sender. */
n = clusterLookupNode(hdr->data.update.nodecfg.nodename); n = clusterLookupNode(hdr->data.update.nodecfg.nodename, CLUSTER_NAMELEN);
if (!n) return 1; /* We don't know the reported node. */ if (!n) return 1; /* We don't know the reported node. */
if (n->configEpoch >= reportedConfigEpoch) return 1; /* Nothing new. */ if (n->configEpoch >= reportedConfigEpoch) return 1; /* Nothing new. */
@ -3163,7 +3188,7 @@ int clusterSendModuleMessageToTarget(const char *target, uint64_t module_id, uin
clusterNode *node = NULL; clusterNode *node = NULL;
if (target != NULL) { if (target != NULL) {
node = clusterLookupNode(target); node = clusterLookupNode(target, strlen(target));
if (node == NULL || node->link == NULL) return C_ERR; if (node == NULL || node->link == NULL) return C_ERR;
} }
@ -5354,7 +5379,8 @@ NULL
addReplyErrorFormat(c,"I'm not the owner of hash slot %u",slot); addReplyErrorFormat(c,"I'm not the owner of hash slot %u",slot);
return; return;
} }
if ((n = clusterLookupNode(c->argv[4]->ptr)) == NULL) { n = clusterLookupNode(c->argv[4]->ptr, sdslen(c->argv[4]->ptr));
if (n == NULL) {
addReplyErrorFormat(c,"I don't know about node %s", addReplyErrorFormat(c,"I don't know about node %s",
(char*)c->argv[4]->ptr); (char*)c->argv[4]->ptr);
return; return;
@ -5370,7 +5396,8 @@ NULL
"I'm already the owner of hash slot %u",slot); "I'm already the owner of hash slot %u",slot);
return; return;
} }
if ((n = clusterLookupNode(c->argv[4]->ptr)) == NULL) { n = clusterLookupNode(c->argv[4]->ptr, sdslen(c->argv[4]->ptr));
if (n == NULL) {
addReplyErrorFormat(c,"I don't know about node %s", addReplyErrorFormat(c,"I don't know about node %s",
(char*)c->argv[4]->ptr); (char*)c->argv[4]->ptr);
return; return;
@ -5386,8 +5413,7 @@ NULL
server.cluster->migrating_slots_to[slot] = NULL; server.cluster->migrating_slots_to[slot] = NULL;
} else if (!strcasecmp(c->argv[3]->ptr,"node") && c->argc == 5) { } else if (!strcasecmp(c->argv[3]->ptr,"node") && c->argc == 5) {
/* CLUSTER SETSLOT <SLOT> NODE <NODE ID> */ /* CLUSTER SETSLOT <SLOT> NODE <NODE ID> */
clusterNode *n = clusterLookupNode(c->argv[4]->ptr); n = clusterLookupNode(c->argv[4]->ptr, sdslen(c->argv[4]->ptr));
if (!n) { if (!n) {
addReplyErrorFormat(c,"Unknown node %s", addReplyErrorFormat(c,"Unknown node %s",
(char*)c->argv[4]->ptr); (char*)c->argv[4]->ptr);
@ -5599,8 +5625,7 @@ NULL
} }
} else if (!strcasecmp(c->argv[1]->ptr,"forget") && c->argc == 3) { } else if (!strcasecmp(c->argv[1]->ptr,"forget") && c->argc == 3) {
/* CLUSTER FORGET <NODE ID> */ /* CLUSTER FORGET <NODE ID> */
clusterNode *n = clusterLookupNode(c->argv[2]->ptr); clusterNode *n = clusterLookupNode(c->argv[2]->ptr, sdslen(c->argv[2]->ptr));
if (!n) { if (!n) {
addReplyErrorFormat(c,"Unknown node %s", (char*)c->argv[2]->ptr); addReplyErrorFormat(c,"Unknown node %s", (char*)c->argv[2]->ptr);
return; return;
@ -5618,9 +5643,8 @@ NULL
addReply(c,shared.ok); addReply(c,shared.ok);
} else if (!strcasecmp(c->argv[1]->ptr,"replicate") && c->argc == 3) { } else if (!strcasecmp(c->argv[1]->ptr,"replicate") && c->argc == 3) {
/* CLUSTER REPLICATE <NODE ID> */ /* CLUSTER REPLICATE <NODE ID> */
clusterNode *n = clusterLookupNode(c->argv[2]->ptr);
/* Lookup the specified node in our table. */ /* Lookup the specified node in our table. */
clusterNode *n = clusterLookupNode(c->argv[2]->ptr, sdslen(c->argv[2]->ptr));
if (!n) { if (!n) {
addReplyErrorFormat(c,"Unknown node %s", (char*)c->argv[2]->ptr); addReplyErrorFormat(c,"Unknown node %s", (char*)c->argv[2]->ptr);
return; return;
@ -5656,7 +5680,7 @@ NULL
} else if ((!strcasecmp(c->argv[1]->ptr,"slaves") || } else if ((!strcasecmp(c->argv[1]->ptr,"slaves") ||
!strcasecmp(c->argv[1]->ptr,"replicas")) && c->argc == 3) { !strcasecmp(c->argv[1]->ptr,"replicas")) && c->argc == 3) {
/* CLUSTER SLAVES <NODE ID> */ /* CLUSTER SLAVES <NODE ID> */
clusterNode *n = clusterLookupNode(c->argv[2]->ptr); clusterNode *n = clusterLookupNode(c->argv[2]->ptr, sdslen(c->argv[2]->ptr));
int j; int j;
/* Lookup the specified node in our table. */ /* Lookup the specified node in our table. */
@ -5683,7 +5707,7 @@ NULL
c->argc == 3) c->argc == 3)
{ {
/* CLUSTER COUNT-FAILURE-REPORTS <NODE ID> */ /* CLUSTER COUNT-FAILURE-REPORTS <NODE ID> */
clusterNode *n = clusterLookupNode(c->argv[2]->ptr); clusterNode *n = clusterLookupNode(c->argv[2]->ptr, sdslen(c->argv[2]->ptr));
if (!n) { if (!n) {
addReplyErrorFormat(c,"Unknown node %s", (char*)c->argv[2]->ptr); addReplyErrorFormat(c,"Unknown node %s", (char*)c->argv[2]->ptr);

3
src/cluster.h
View File

@ -377,7 +377,8 @@ void clusterInit(void);
void clusterCron(void); void clusterCron(void);
void clusterBeforeSleep(void); void clusterBeforeSleep(void);
clusterNode *getNodeByQuery(client *c, struct redisCommand *cmd, robj **argv, int argc, int *hashslot, int *ask); clusterNode *getNodeByQuery(client *c, struct redisCommand *cmd, robj **argv, int argc, int *hashslot, int *ask);
clusterNode *clusterLookupNode(const char *name); int verifyClusterNodeId(const char *name, int length);
clusterNode *clusterLookupNode(const char *name, int length);
int clusterRedirectBlockedClientIfNeeded(client *c); int clusterRedirectBlockedClientIfNeeded(client *c);
void clusterRedirectClient(client *c, clusterNode *n, int hashslot, int error_code); void clusterRedirectClient(client *c, clusterNode *n, int hashslot, int error_code);
void migrateCloseTimedoutSockets(void); void migrateCloseTimedoutSockets(void);

7
src/module.c
View File

@ -7951,8 +7951,9 @@ size_t RM_GetClusterSize(void) {
} }
/* Populate the specified info for the node having as ID the specified 'id', /* Populate the specified info for the node having as ID the specified 'id',
* then returns REDISMODULE_OK. Otherwise if the node ID does not exist from * then returns REDISMODULE_OK. Otherwise if the format of node ID is invalid
* the POV of this local node, REDISMODULE_ERR is returned. * or the node ID does not exist from the POV of this local node, REDISMODULE_ERR
* is returned.
* *
* The arguments `ip`, `master_id`, `port` and `flags` can be NULL in case we don't * The arguments `ip`, `master_id`, `port` and `flags` can be NULL in case we don't
* need to populate back certain info. If an `ip` and `master_id` (only populated * need to populate back certain info. If an `ip` and `master_id` (only populated
@ -7972,7 +7973,7 @@ size_t RM_GetClusterSize(void) {
int RM_GetClusterNodeInfo(RedisModuleCtx *ctx, const char *id, char *ip, char *master_id, int *port, int *flags) { int RM_GetClusterNodeInfo(RedisModuleCtx *ctx, const char *id, char *ip, char *master_id, int *port, int *flags) {
UNUSED(ctx); UNUSED(ctx);
clusterNode *node = clusterLookupNode(id); clusterNode *node = clusterLookupNode(id, strlen(id));
if (node == NULL || if (node == NULL ||
node->flags & (CLUSTER_NODE_NOADDR|CLUSTER_NODE_HANDSHAKE)) node->flags & (CLUSTER_NODE_NOADDR|CLUSTER_NODE_HANDSHAKE))
{ {