From: Tom Lane <tgl@sss.pgh.pa.us>
To: petervandivier@gmail.com
cc: pgsql-bugs@lists.postgresql.org
Subject: Re: BUG #16695: pg_hba_file_rules NULL address and netmask
In-reply-to: <832503.1604361320@sss.pgh.pa.us>
References: <16695-a665558e2f630be7@postgresql.org>
 <832503.1604361320@sss.pgh.pa.us>
Comments: In-reply-to Tom Lane <tgl@sss.pgh.pa.us>
	message dated "Mon, 02 Nov 2020 18:55:20 -0500"
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="----- =_aaaaaaaaaa0"
Content-ID: <871827.1604366815.0@sss.pgh.pa.us>
Date: Mon, 02 Nov 2020 20:27:38 -0500
Message-ID: <871849.1604366858@sss.pgh.pa.us>
Precedence: bulk

------- =_aaaaaaaaaa0
Content-Type: text/plain; charset="us-ascii"
Content-ID: <871827.1604366815.1@sss.pgh.pa.us>

I wrote:
> PG Bug reporting form <noreply@postgresql.org> writes:
>> pg_hba_file_rules reports NULL address and netmask values incorrectly on
>> FreeBSD 11 for tested postgres versions 10-13 (at least). e.g. 

> So for the archives' sake: what I suppose Peter means is that the addr
> and netmask come out as NULL on every line, even where they should not.
> At least, that's what I've reproduced here on FreeBSD 11.0.

> I traced through this, and the proximate cause seems to be that
> getnameinfo(3) is failing because it is expecting the passed "salen"
> to be exactly the length it is expecting for the given sa_family.
> Which it is not, because alone among our callers of pg_getnameinfo_all(),
> fill_hba_line() thinks it can get away with passing sizeof(struct
> sockaddr_storage) rather than the actual addrlen previously returned by
> getaddrinfo().

The attached seems to fix it for me.  (This is against HEAD, but a quick
check suggests it will apply cleanly down to v10.)  As I mentioned,
I'm planning to put the new fields at the end of struct HbaLine in
the back branches.

			regards, tom lane


------- =_aaaaaaaaaa0
Content-Type: text/x-diff; name="fix-bogus-addrlen-in-hba-view.patch";
	charset="us-ascii"
Content-ID: <871827.1604366815.2@sss.pgh.pa.us>
Content-Description: fix-bogus-addrlen-in-hba-view.patch
Content-Transfer-Encoding: quoted-printable

diff --git a/src/backend/libpq/hba.c b/src/backend/libpq/hba.c
index 4c86fb6087..3a78d2043e 100644
--- a/src/backend/libpq/hba.c
+++ b/src/backend/libpq/hba.c
@@ -1188,8 +1188,11 @@ parse_hba_line(TokenizedLine *tok_line, int elevel)
 =

 			ret =3D pg_getaddrinfo_all(str, NULL, &hints, &gai_result);
 			if (ret =3D=3D 0 && gai_result)
+			{
 				memcpy(&parsedline->addr, gai_result->ai_addr,
 					   gai_result->ai_addrlen);
+				parsedline->addrlen =3D gai_result->ai_addrlen;
+			}
 			else if (ret =3D=3D EAI_NONAME)
 				parsedline->hostname =3D str;
 			else
@@ -1238,6 +1241,7 @@ parse_hba_line(TokenizedLine *tok_line, int elevel)
 										token->string);
 					return NULL;
 				}
+				parsedline->masklen =3D parsedline->addrlen;
 				pfree(str);
 			}
 			else if (!parsedline->hostname)
@@ -1288,6 +1292,7 @@ parse_hba_line(TokenizedLine *tok_line, int elevel)
 =

 				memcpy(&parsedline->mask, gai_result->ai_addr,
 					   gai_result->ai_addrlen);
+				parsedline->masklen =3D gai_result->ai_addrlen;
 				pg_freeaddrinfo_all(hints.ai_family, gai_result);
 =

 				if (parsedline->addr.ss_family !=3D parsedline->mask.ss_family)
@@ -2538,20 +2543,26 @@ fill_hba_line(Tuplestorestate *tuple_store, TupleD=
esc tupdesc,
 				}
 				else
 				{
-					if (pg_getnameinfo_all(&hba->addr, sizeof(hba->addr),
-										   buffer, sizeof(buffer),
-										   NULL, 0,
-										   NI_NUMERICHOST) =3D=3D 0)
+					/*
+					 * Note: if pg_getnameinfo_all fails, it'll set buffer to
+					 * "???", which we want to return.
+					 */
+					if (hba->addrlen > 0)
 					{
-						clean_ipv6_addr(hba->addr.ss_family, buffer);
+						if (pg_getnameinfo_all(&hba->addr, hba->addrlen,
+											   buffer, sizeof(buffer),
+											   NULL, 0,
+											   NI_NUMERICHOST) =3D=3D 0)
+							clean_ipv6_addr(hba->addr.ss_family, buffer);
 						addrstr =3D pstrdup(buffer);
 					}
-					if (pg_getnameinfo_all(&hba->mask, sizeof(hba->mask),
-										   buffer, sizeof(buffer),
-										   NULL, 0,
-										   NI_NUMERICHOST) =3D=3D 0)
+					if (hba->masklen > 0)
 					{
-						clean_ipv6_addr(hba->mask.ss_family, buffer);
+						if (pg_getnameinfo_all(&hba->mask, hba->masklen,
+											   buffer, sizeof(buffer),
+											   NULL, 0,
+											   NI_NUMERICHOST) =3D=3D 0)
+							clean_ipv6_addr(hba->mask.ss_family, buffer);
 						maskstr =3D pstrdup(buffer);
 					}
 				}
diff --git a/src/include/libpq/hba.h b/src/include/libpq/hba.h
index d638479d88..8f09b5638f 100644
--- a/src/include/libpq/hba.h
+++ b/src/include/libpq/hba.h
@@ -42,6 +42,10 @@ typedef enum UserAuth
 #define USER_AUTH_LAST uaPeer	/* Must be last value of this enum */
 } UserAuth;
 =

+/*
+ * Data structures representing pg_hba.conf entries
+ */
+
 typedef enum IPCompareMethod
 {
 	ipCmpMask,
@@ -75,11 +79,12 @@ typedef struct HbaLine
 	List	   *databases;
 	List	   *roles;
 	struct sockaddr_storage addr;
+	int			addrlen;		/* zero if we don't have a valid addr */
 	struct sockaddr_storage mask;
+	int			masklen;		/* zero if we don't have a valid mask */
 	IPCompareMethod ip_cmp_method;
 	char	   *hostname;
 	UserAuth	auth_method;
-
 	char	   *usermap;
 	char	   *pamservice;
 	bool		pam_use_hostname;

------- =_aaaaaaaaaa0--