public inbox for [email protected]  
help / color / mirror / Atom feed
From: Bo Peng <[email protected]>
To: [email protected] <[email protected]>
Subject: Proposal: Restrict watchdog and heartbeat receiver to listen only on configured addresses
Date: Mon, 18 Aug 2025 00:30:00 +0000
Message-ID: <TYWP286MB2633B3C651030A0A658D2597F236A@TYWP286MB2633.JPNP286.PROD.OUTLOOK.COM> (raw)

Currently, both the watchdog and heartbeat receiver processes listen on all interfaces.

    tcp         0      0 0.0.0.0:9000            0.0.0.0:*               LISTEN      1428/pgpool: watchd 
    udp        0      0 0.0.0.0:9694            0.0.0.0:*                                1453/pgpool: heartb 
    udp        0      0 0.0.0.0:9694            0.0.0.0:*                                1444/pgpool: heartb

For security reasons, I propose to change this behavior so that they listen only on the addresses
specified by hostname and heartbeat_hostname.

    tcp         0      0 192.168.101.101:9000      0.0.0.0:*               LISTEN      727648/pgpool: watc
    udp        0      0 192.168.101.101:9694      0.0.0.0:*                                727664/pgpool: hear 
    udp        0      0 192.168.101.101:9694      0.0.0.0:*                                727660/pgpool: hear

Patch is attached.
---
Bo Peng <[email protected]>
SRA OSS K.K.
TEL: 03-5979-2701 FAX: 03-5979-2702
Mobile: 080-7752-0749
URL: https://www.sraoss.co.jp/

Attachments:

  [application/octet-stream] watchdog_lifecheck_listen_addr_v1.patch (11.1K, 2-watchdog_lifecheck_listen_addr_v1.patch)
  download | inline diff:
diff --git a/src/config/pool_config_variables.c b/src/config/pool_config_variables.c
index d7d5fe49f..5bbe46d3a 100644
--- a/src/config/pool_config_variables.c
+++ b/src/config/pool_config_variables.c
@@ -5335,14 +5335,17 @@ SetPgpoolNodeId(int elevel)
 static bool
 SetHBDestIfFunc(int elevel)
 {
-	int			idx = 0;
+	int			dest_if_idx = 0;
+	int			local_if_idx = 0;
 	char	  **addrs;
 	char	  **if_names;
 	int			i,
 				j,
+				k,
 				n_addr,
 				n_if_name;
 
+	g_pool_config.num_hb_local_if = 0;
 	g_pool_config.num_hb_dest_if = 0;
 
 	if (g_pool_config.wd_lifecheck_method != LIFECHECK_BY_HB)
@@ -5352,22 +5355,14 @@ SetHBDestIfFunc(int elevel)
 
 	/*
 	 * g_pool_config.hb_ifs is the information for sending/receiving heartbeat
-	 * for all nodes specified in pgpool.conf. If it is local pgpool node
-	 * information, set dest_port to g_pool_config.wd_heartbeat_port and
-	 * ignore addr and if_name. g_pool_config.hb_dest_if is the heartbeat
+	 * for all nodes specified in pgpool.conf. g_pool_config.hb_local_if is
+	 * the local node information. g_pool_config.hb_dest_if is the heartbeat
 	 * destination information.
 	 */
 	for (i = 0; i < WD_MAX_IF_NUM; i++)
 	{
 		if (g_pool_config.hb_ifs[i].dest_port > 0)
 		{
-			/* Ignore local pgpool node */
-			if (i == g_pool_config.pgpool_node_id)
-			{
-				g_pool_config.wd_heartbeat_port = g_pool_config.hb_ifs[i].dest_port;
-				continue;
-			}
-
 			WdHbIf	   *hbNodeInfo = &g_pool_config.hb_ifs[i];
 
 			addrs = get_list_from_string(hbNodeInfo->addr, ";", &n_addr);
@@ -5375,7 +5370,10 @@ SetHBDestIfFunc(int elevel)
 
 			if (!addrs || n_addr < 0)
 			{
-				g_pool_config.hb_dest_if[idx].addr[0] = '\0';
+				if (i == g_pool_config.pgpool_node_id)
+					g_pool_config.hb_local_if[local_if_idx].addr[0] = '\0';
+				else
+					g_pool_config.hb_dest_if[dest_if_idx].addr[0] = '\0';
 
 				if (addrs)
 					pfree(addrs);
@@ -5391,19 +5389,46 @@ SetHBDestIfFunc(int elevel)
 
 			for (j = 0; j < n_addr; j++)
 			{
-				strlcpy(g_pool_config.hb_dest_if[idx].addr, addrs[j], WD_MAX_HOST_NAMELEN - 1);
-				g_pool_config.hb_dest_if[idx].dest_port = hbNodeInfo->dest_port;
-				if (n_if_name > j)
+				/* local pgpool node */
+				if (i == g_pool_config.pgpool_node_id)
 				{
-					strlcpy(g_pool_config.hb_dest_if[idx].if_name, if_names[j], WD_MAX_IF_NAME_LEN - 1);
-					pfree(if_names[j]);
+					for (k = 0; k < g_pool_config.wd_nodes.num_wd - 1; k++)
+					{
+						strlcpy(g_pool_config.hb_local_if[local_if_idx].addr, addrs[j], WD_MAX_HOST_NAMELEN - 1);
+						g_pool_config.hb_local_if[local_if_idx].dest_port = hbNodeInfo->dest_port;
+
+						if (n_if_name > j )
+							strlcpy(g_pool_config.hb_local_if[local_if_idx].if_name, if_names[j], WD_MAX_IF_NAME_LEN - 1);
+						else
+							g_pool_config.hb_local_if[local_if_idx].if_name[0] = '\0';
+
+						g_pool_config.num_hb_local_if = local_if_idx + 1;
+						local_if_idx++;
+					}
+
+					if (n_if_name > j )
+						pfree(if_names[j]);
+
+					pfree(addrs[j]);
+
 				}
+				/* destination pgpool node */
 				else
-					g_pool_config.hb_dest_if[idx].if_name[0] = '\0';
+				{
+					strlcpy(g_pool_config.hb_dest_if[dest_if_idx].addr, addrs[j], WD_MAX_HOST_NAMELEN - 1);
+					g_pool_config.hb_dest_if[dest_if_idx].dest_port = hbNodeInfo->dest_port;
+					if (n_if_name > j)
+					{
+						strlcpy(g_pool_config.hb_dest_if[dest_if_idx].if_name, if_names[j], WD_MAX_IF_NAME_LEN - 1);
+						pfree(if_names[j]);
+					}
+					else
+						g_pool_config.hb_dest_if[dest_if_idx].if_name[0] = '\0';
 
-				g_pool_config.num_hb_dest_if = idx + 1;
-				idx++;
-				pfree(addrs[j]);
+					g_pool_config.num_hb_dest_if = dest_if_idx + 1;
+					dest_if_idx++;
+					pfree(addrs[j]);
+				}
 			}
 
 			if (addrs)
diff --git a/src/include/pool_config.h b/src/include/pool_config.h
index 3d12c0a9f..be82750e5 100644
--- a/src/include/pool_config.h
+++ b/src/include/pool_config.h
@@ -665,16 +665,17 @@ typedef struct
 										 * lifecheck */
 	char	   *wd_lifecheck_user;	/* PostgreSQL user name for watchdog */
 	char	   *wd_lifecheck_password;	/* password for watchdog user */
-	int			wd_heartbeat_port;	/* Port number for heartbeat lifecheck */
 	int			wd_heartbeat_keepalive; /* Interval time of sending heartbeat
 										 * signal (sec) */
 	int			wd_heartbeat_deadtime;	/* Deadtime interval for heartbeat
 										 * signal (sec) */
 	WdHbIf		hb_ifs[WD_MAX_IF_NUM];	/* heartbeat interfaces of all
 										 * watchdog nodes */
+	WdHbIf		hb_local_if[WD_MAX_IF_NUM];     /* local heartbeat interfaces */
+	int			num_hb_local_if;                /* number of local interface */
 	WdHbIf		hb_dest_if[WD_MAX_IF_NUM];	/* heartbeat destination
 											 * interfaces */
-	int			num_hb_dest_if; /* number of interface devices */
+	int			num_hb_dest_if; /* number of destination interface */
 	char	  **wd_monitoring_interfaces_list;	/* network interface name list
 												 * to be monitored by watchdog */
 	bool		health_check_test;	/* if on, enable health check testing */
diff --git a/src/watchdog/watchdog.c b/src/watchdog/watchdog.c
index 3913d692d..ffd1cef50 100644
--- a/src/watchdog/watchdog.c
+++ b/src/watchdog/watchdog.c
@@ -590,7 +590,7 @@ static bool get_authhash_for_node(WatchdogNode *wdNode, char *authhash);
 static bool verify_authhash_for_node(WatchdogNode *wdNode, char *authhash);
 
 static void print_watchdog_node_info(WatchdogNode *wdNode);
-static List *wd_create_recv_socket(int port);
+static List *wd_create_recv_socket(char *hostname, int port);
 static void wd_check_config(void);
 static pid_t watchdog_main(void);
 static pid_t fork_watchdog_child(void);
@@ -865,7 +865,7 @@ clear_command_node_result(WDCommandNodeResult *nodeResult)
 }
 
 static List *
-wd_create_recv_socket(int port)
+wd_create_recv_socket(char *hostname, int port)
 {
 	int			one = 1;
 	int			sock = -1;
@@ -886,7 +886,7 @@ wd_create_recv_socket(int port)
 	hints.ai_protocol = 0;
 	hints.ai_flags = AI_NUMERICSERV | AI_PASSIVE;
 
-	if ((gai_ret = getaddrinfo(NULL, portstr, &hints, &res)) != 0)
+	if ((gai_ret = getaddrinfo(!hostname ? NULL : hostname, portstr, &hints, &res)) != 0)
 	{
 		ereport(ERROR,
 				(errmsg("getaddrinfo failed with error \"%s\"", gai_strerror(gai_ret))));
@@ -1299,7 +1299,7 @@ watchdog_main(void)
 	/* initialize all the local structures for watchdog */
 	wd_cluster_initialize();
 	/* create a server socket for incoming watchdog connections */
-	g_wd_recv_socks = wd_create_recv_socket(g_cluster.localNode->wd_port);
+	g_wd_recv_socks = wd_create_recv_socket(g_cluster.localNode->hostname, g_cluster.localNode->wd_port);
 
 	/* open the command server */
 	g_cluster.command_server_sock = wd_create_command_server_socket();
diff --git a/src/watchdog/wd_heartbeat.c b/src/watchdog/wd_heartbeat.c
index a9e99dec9..acececc1a 100644
--- a/src/watchdog/wd_heartbeat.c
+++ b/src/watchdog/wd_heartbeat.c
@@ -262,7 +262,7 @@ wd_create_hb_recv_socket(WdHbIf *hb_if)
 			   *walk,
 			   *res = NULL;
 
-	portstr = psprintf("%d", pool_config->wd_heartbeat_port);
+	portstr = psprintf("%d", hb_if->dest_port);
 
 	memset(&hints, 0x00, sizeof(hints));
 	hints.ai_family = AF_UNSPEC;
@@ -270,7 +270,7 @@ wd_create_hb_recv_socket(WdHbIf *hb_if)
 	hints.ai_protocol = 0;
 	hints.ai_flags = AI_NUMERICSERV | AI_PASSIVE;
 
-	if ((gai_ret = getaddrinfo(NULL, portstr, &hints, &res)) != 0)
+	if ((gai_ret = getaddrinfo(!hb_if->addr ? NULL : hb_if->addr, portstr, &hints, &res)) != 0)
 	{
 		ereport(WARNING,
 				(errmsg("getaddrinfo() failed with error \"%s\"", gai_strerror(gai_ret))));
@@ -285,7 +285,7 @@ wd_create_hb_recv_socket(WdHbIf *hb_if)
 	if (n == 0)
 	{
 		ereport(ERROR, (errmsg("failed to create watchdog heartbeat receive socket"),
-						errdetail("getaddrinfo() result is empty: no sockets can be created because no available local address with port:%d", pool_config->wd_heartbeat_port)));
+						errdetail("getaddrinfo() result is empty: no sockets can be created because no available local address with port:%d", hb_if->dest_port)));
 		return NULL;
 	}
 	else
@@ -380,7 +380,7 @@ wd_create_hb_recv_socket(WdHbIf *hb_if)
 		}
 		ereport(LOG,
 				(errmsg("creating watchdog heartbeat receive socket."),
-				 errdetail("creating socket on %s:%d", buf, pool_config->wd_heartbeat_port)));
+				 errdetail("creating socket on %s:%d", buf, hb_if->dest_port)));
 
 		bind_is_done = 0;
 		for (bind_tries = 0; !bind_is_done && bind_tries < MAX_BIND_TRIES; bind_tries++)
diff --git a/src/watchdog/wd_lifecheck.c b/src/watchdog/wd_lifecheck.c
index 0694a2228..a2ee1143c 100644
--- a/src/watchdog/wd_lifecheck.c
+++ b/src/watchdog/wd_lifecheck.c
@@ -133,11 +133,14 @@ lifecheck_child_name(pid_t pid)
 {
 	int			i;
 
-	for (i = 0; i < pool_config->num_hb_dest_if; i++)
+	for (i = 0; i < pool_config->num_hb_local_if; i++)
 	{
 		if (g_hb_receiver_pid && pid == g_hb_receiver_pid[i])
 			return "heartBeat receiver";
-		else if (g_hb_sender_pid && pid == g_hb_sender_pid[i])
+	}
+	for (i = 0; i < pool_config->num_hb_dest_if; i++)
+	{
+		if (g_hb_sender_pid && pid == g_hb_sender_pid[i])
 			return "heartBeat sender";
 	}
 	/* Check if it was a ping to trusted server process */
@@ -211,13 +214,13 @@ wd_reaper_lifecheck(pid_t pid, int status)
 	if (g_hb_receiver_pid == NULL && g_hb_sender_pid == NULL)
 		return -1;
 
-	for (i = 0; i < pool_config->num_hb_dest_if; i++)
+	for (i = 0; i < pool_config->num_hb_local_if; i++)
 	{
 		if (g_hb_receiver_pid && pid == g_hb_receiver_pid[i])
 		{
 			if (restart_child)
 			{
-				g_hb_receiver_pid[i] = wd_hb_receiver(1, &(pool_config->hb_dest_if[i]));
+				g_hb_receiver_pid[i] = wd_hb_receiver(1, &(pool_config->hb_local_if[i]));
 				ereport(LOG,
 						(errmsg("fork a new %s process with pid: %d", proc_name, g_hb_receiver_pid[i])));
 			}
@@ -226,8 +229,11 @@ wd_reaper_lifecheck(pid_t pid, int status)
 
 			return g_hb_receiver_pid[i];
 		}
+	}
 
-		else if (g_hb_sender_pid && pid == g_hb_sender_pid[i])
+	for (i = 0; i < pool_config->num_hb_dest_if; i++)
+	{
+		if (g_hb_sender_pid && pid == g_hb_sender_pid[i])
 		{
 			if (restart_child)
 			{
@@ -253,17 +259,21 @@ lifecheck_kill_all_children(int sig)
 		&& g_hb_receiver_pid && g_hb_sender_pid)
 	{
 		int			i;
+		pid_t		pid_child;
 
-		for (i = 0; i < pool_config->num_hb_dest_if; i++)
+		for (i = 0; i < pool_config->num_hb_local_if; i++)
 		{
-			pid_t		pid_child = g_hb_receiver_pid[i];
+			pid_child = g_hb_receiver_pid[i];
 
 			if (pid_child > 0)
 			{
 				kill(pid_child, sig);
 				ret = true;
 			}
+		}
 
+		for (i = 0; i < pool_config->num_hb_dest_if; i++)
+		{
 			pid_child = g_hb_sender_pid[i];
 
 			if (pid_child > 0)
@@ -477,14 +487,17 @@ spawn_lifecheck_children(void)
 	{
 		int			i;
 
-		g_hb_receiver_pid = palloc0(sizeof(pid_t) * pool_config->num_hb_dest_if);
+		g_hb_receiver_pid = palloc0(sizeof(pid_t) * pool_config->num_hb_local_if);
 		g_hb_sender_pid = palloc0(sizeof(pid_t) * pool_config->num_hb_dest_if);
 
-		for (i = 0; i < pool_config->num_hb_dest_if; i++)
+		for (i = 0; i < pool_config->num_hb_local_if; i++)
 		{
-			/* heartbeat receiver process */
-			g_hb_receiver_pid[i] = wd_hb_receiver(1, &(pool_config->hb_dest_if[i]));
+				/* heartbeat receiver process */
+				g_hb_receiver_pid[i] = wd_hb_receiver(1, &(pool_config->hb_local_if[i]));
+		}
 
+		for (i = 0; i < pool_config->num_hb_dest_if; i++)
+		{
 			/* heartbeat sender process */
 			g_hb_sender_pid[i] = wd_hb_sender(1, &(pool_config->hb_dest_if[i]));
 		}


reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: [email protected]
  Cc: [email protected], [email protected]
  Subject: Re: Proposal: Restrict watchdog and heartbeat receiver to listen only on configured addresses
  In-Reply-To: <TYWP286MB2633B3C651030A0A658D2597F236A@TYWP286MB2633.JPNP286.PROD.OUTLOOK.COM>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox