This is an automated email from the ASF dual-hosted git repository. maxyang pushed a commit to branch main in repository https://gitbox.apache.org/repos/asf/cloudberry.git
commit c7397dc33f3af5b07fe2aebb918d513aeb02d362 Author: Hongxu Ma <[email protected]> AuthorDate: Fri Feb 17 16:14:51 2023 +0800 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. --- contrib/interconnect/proxy/ic_proxy_backend.c | 10 ++++++++++ contrib/interconnect/proxy/ic_proxy_client.c | 20 ++++++++++++++++++++ contrib/interconnect/proxy/ic_proxy_packet.c | 6 ++++-- contrib/interconnect/proxy/ic_proxy_packet.h | 12 ++++++++++++ contrib/interconnect/proxy/ic_proxy_peer.c | 27 +++++++++++++++++++++++++++ contrib/interconnect/proxy/ic_proxy_router.c | 3 +++ 6 files changed, 76 insertions(+), 2 deletions(-) diff --git a/contrib/interconnect/proxy/ic_proxy_backend.c b/contrib/interconnect/proxy/ic_proxy_backend.c index 91131eeff2..b8a213832e 100644 --- a/contrib/interconnect/proxy/ic_proxy_backend.c +++ b/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); diff --git a/contrib/interconnect/proxy/ic_proxy_client.c b/contrib/interconnect/proxy/ic_proxy_client.c index b60c928139..5c57f5bccb 100644 --- a/contrib/interconnect/proxy/ic_proxy_client.c +++ b/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, diff --git a/contrib/interconnect/proxy/ic_proxy_packet.c b/contrib/interconnect/proxy/ic_proxy_packet.c index 8234ac8df2..56b7f1ba1a 100644 --- a/contrib/interconnect/proxy/ic_proxy_packet.c +++ b/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; } diff --git a/contrib/interconnect/proxy/ic_proxy_packet.h b/contrib/interconnect/proxy/ic_proxy_packet.h index eec86e2e6b..17caca1f8a 100644 --- a/contrib/interconnect/proxy/ic_proxy_packet.h +++ b/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 */ diff --git a/contrib/interconnect/proxy/ic_proxy_peer.c b/contrib/interconnect/proxy/ic_proxy_peer.c index b0c5a990b1..46bca00154 100644 --- a/contrib/interconnect/proxy/ic_proxy_peer.c +++ b/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) { /* diff --git a/contrib/interconnect/proxy/ic_proxy_router.c b/contrib/interconnect/proxy/ic_proxy_router.c index 77b5f36bb6..9ff19b3b47 100644 --- a/contrib/interconnect/proxy/ic_proxy_router.c +++ b/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, --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
