Add magic number field in the ICProxyPkt (#14926)

icproxy processes communicate with each other with the public addr:port (see gp_interconnect_proxy_addresses), but no auth control here. So, any connection may connect in and send any byte, it may cause some unexpected exceptions (e.g. OOM).

In the PR, introduces a magic number field in the ICProxyPkt and does a sanity check: drop the corresponding package if magic number doesn't match.

This method does not attempt to solve all problems, but only as a simple solution to avoid internal misaccess, looks it's good enough.

Author: interma

contrib/interconnect/proxy/ic_proxy_backend.c

@@ -182,6 +182,16 @@ ic_proxy_backend_on_read_hello_ack(uv_stream_t *stream, ssize_t nread, const uv_
 
 	/* we have received a complete HELLO ACK message */
 	pkt = (ICProxyPkt *)backend->buffer;
+
+	/* sanity check: drop the packet with incorrect magic number */
+	if (!ic_proxy_pkt_is_valid(pkt))
+	{
+		elogif(gp_log_interconnect >= GPVARS_VERBOSITY_DEBUG, DEBUG1,
+			"ic-proxy: backend %s: received %s, dropping the invalid package (magic number mismatch)",
+					ic_proxy_key_to_str(&backend->key), ic_proxy_pkt_to_str(pkt));
+		return;
+	}
+
 	Assert(ic_proxy_pkt_is(pkt, IC_PROXY_MESSAGE_HELLO_ACK));
 	uv_read_stop(stream);
 

contrib/interconnect/proxy/ic_proxy_client.c

@@ -659,6 +659,15 @@ ic_proxy_client_on_hello_pkt(void *opaque, const void *data, uint16 size)
 	ICProxyKey	key;
 	ICProxyPkt *ackpkt;
 
+	/* sanity check: drop the packet with incorrect magic number */
+	if (!ic_proxy_pkt_is_valid(pkt))
+	{
+		elogif(gp_log_interconnect >= GPVARS_VERBOSITY_DEBUG, DEBUG1,
+			"ic-proxy: %s: received %s, dropping the invalid package (magic number mismatch)",
+					ic_proxy_client_get_name(client), ic_proxy_pkt_to_str(pkt));
+		return;
+	}
+
 	/* we only expect one HELLO message */
 	uv_read_stop((uv_stream_t *) &client->pipe);
 
@@ -1176,6 +1185,15 @@ void
 ic_proxy_client_on_p2c_data(ICProxyClient *client, ICProxyPkt *pkt,
 							ic_proxy_sent_cb callback, void *opaque)
 {
+	/* sanity check: drop the packet with incorrect magic number */
+	if (!ic_proxy_pkt_is_valid(pkt))
+	{
+		elogif(gp_log_interconnect >= GPVARS_VERBOSITY_DEBUG, DEBUG1,
+			"ic-proxy: %s: received %s, dropping the invalid package (magic number mismatch)",
+					ic_proxy_client_get_name(client), ic_proxy_pkt_to_str(pkt));
+		return;
+	}
+
 	/* A placeholder does not send any packets, it always cache them */
 	if (client->state & IC_PROXY_CLIENT_STATE_PLACEHOLDER)
 	{
@@ -1256,6 +1274,8 @@ ic_proxy_client_cache_p2c_pkt(ICProxyClient *client, ICProxyPkt *pkt)
 	/* TODO: drop out-of-date pkt directly */
 	/* TODO: verify the pkt is to client */
 
+	Assert(ic_proxy_pkt_is_valid(pkt));
+
 	client->pkts = lappend(client->pkts, pkt);
 
 	elogif(gp_log_interconnect >= GPVARS_VERBOSITY_DEBUG, DEBUG3,

contrib/interconnect/proxy/ic_proxy_packet.c

@@ -82,6 +82,7 @@ void
 ic_proxy_message_init(ICProxyPkt *pkt, ICProxyMessageType type,
 					  const ICProxyKey *key)
 {
+	pkt->magicNumber = IC_PROXY_PKT_MAGIC_NUMBER;
 	pkt->type = type;
 	pkt->len = sizeof(*pkt);
 
@@ -156,13 +157,14 @@ ic_proxy_pkt_to_str(const ICProxyPkt *pkt)
 	static char	buf[256];
 
 	snprintf(buf, sizeof(buf),
-			 "%s [con%d,cmd%d,slice[%hd->%hd] %hu bytes seg%hd:dbid%hu:p%d->seg%hd:dbid%hu:p%d]",
+			 "%s [con%d,cmd%d,slice[%hd->%hd] %hu bytes seg%hd:dbid%hu:p%d->seg%hd:dbid%hu:p%d magic%u]",
 			 ic_proxy_message_type_to_str(pkt->type),
 			 pkt->sessionId, pkt->commandId,
 			 pkt->sendSliceIndex, pkt->recvSliceIndex,
 			 pkt->len,
 			 pkt->srcContentId, pkt->srcDbid, pkt->srcPid,
-			 pkt->dstContentId, pkt->dstDbid, pkt->dstPid);
+			 pkt->dstContentId, pkt->dstDbid, pkt->dstPid,
+			 pkt->magicNumber);
 
 	return buf;
 }

contrib/interconnect/proxy/ic_proxy_packet.h

@@ -22,6 +22,7 @@ typedef struct ICProxyPkt ICProxyPkt;
 #include "ic_proxy_key.h"
 
 #define IC_PROXY_MAX_PKT_SIZE (Gp_max_packet_size + sizeof(ICProxyPkt))
+#define IC_PROXY_PKT_MAGIC_NUMBER (0xBADDCAFE)
 
 typedef enum
 {
@@ -41,6 +42,9 @@ typedef enum
 
 struct ICProxyPkt
 {
+	/* for the sanity check of package */
+	uint32		magicNumber;
+
 	uint16		len;
 	uint16		type;
 
@@ -97,4 +101,12 @@ ic_proxy_pkt_is(const ICProxyPkt *pkt, ICProxyMessageType type)
 	return type == pkt->type;
 }
 
+/* check the validity of a package by magicNumber */
+static inline bool
+ic_proxy_pkt_is_valid(const ICProxyPkt *pkt)
+{
+	Assert(pkt);
+	return pkt->magicNumber == IC_PROXY_PKT_MAGIC_NUMBER;
+}
+
 #endif   /* IC_PROXY_PACKET_H */

contrib/interconnect/proxy/ic_proxy_peer.c

@@ -290,6 +290,15 @@ ic_proxy_peer_on_data_pkt(void *opaque, const void *data, uint16 size)
 	elogif(gp_log_interconnect >= GPVARS_VERBOSITY_DEBUG, DEBUG5,
 		   "ic-proxy: %s: received %s", peer->name, ic_proxy_pkt_to_str(pkt));
 
+	/* sanity check: drop the packet with incorrect magic number */
+	if (!ic_proxy_pkt_is_valid(pkt))
+	{
+		elogif(gp_log_interconnect >= GPVARS_VERBOSITY_DEBUG, DEBUG1,
+			"ic-proxy: %s: received %s, dropping the invalid package (magic number mismatch)",
+					peer->name, ic_proxy_pkt_to_str(pkt));
+		return;
+	}
+
 	if (!(peer->state & IC_PROXY_PEER_STATE_READY_FOR_DATA))
 	{
 		elog(WARNING, "ic-proxy: %s: not ready to receive DATA yet: %s",
@@ -529,6 +538,15 @@ ic_proxy_peer_on_hello_pkt(void *opaque, const void *data, uint16 size)
 	ICProxyPeer *peer = opaque;
 	ICProxyKey	key;
 
+	/* sanity check: drop the packet with incorrect magic number */
+	if (!ic_proxy_pkt_is_valid(pkt))
+	{
+		elogif(gp_log_interconnect >= GPVARS_VERBOSITY_DEBUG, DEBUG1,
+			"ic-proxy: %s: received %s, dropping the invalid package (magic number mismatch)",
+					peer->name, ic_proxy_pkt_to_str(pkt));
+		return;
+	}
+
 	/* we only expect one hello message */
 	uv_read_stop((uv_stream_t *) &peer->tcp);
 
@@ -636,6 +654,15 @@ ic_proxy_peer_on_hello_ack_pkt(void *opaque, const void *data, uint16 size)
 		elog(ERROR, "ic-proxy: %s: received incomplete HELLO ACK: size = %d",
 					 peer->name, size);
 
+	/* sanity check: drop the packet with incorrect magic number */
+	if (!ic_proxy_pkt_is_valid(pkt))
+	{
+		elogif(gp_log_interconnect >= GPVARS_VERBOSITY_DEBUG, DEBUG1,
+			"ic-proxy: %s: received %s, dropping the invalid package (magic number mismatch)",
+					peer->name, ic_proxy_pkt_to_str(pkt));
+		return;
+	}
+
 	if (peer->state & IC_PROXY_PEER_STATE_RECEIVED_HELLO_ACK)
 	{
 		/*

contrib/interconnect/proxy/ic_proxy_router.c

@@ -202,6 +202,7 @@ void
 ic_proxy_router_route(uv_loop_t *loop, ICProxyPkt *pkt,
 					  ic_proxy_sent_cb callback, void *opaque)
 {
+	Assert(ic_proxy_pkt_is_valid(pkt));
 	if (pkt->dstDbid == pkt->srcDbid)
 	{
 		/*
@@ -254,6 +255,8 @@ ic_proxy_router_on_write(uv_write_t *req, int status)
 	ICProxyWriteReq *wreq = (ICProxyWriteReq *) req;
 	ICProxyPkt *pkt = req->data;
 
+	Assert(ic_proxy_pkt_is_valid(pkt));
+
 	if (status < 0)
 	{
 		elogif(gp_log_interconnect >= GPVARS_VERBOSITY_DEBUG, DEBUG5,