Re: BUG #16695: pg_hba_file_rules NULL address and netmask - Mailing list pgsql-bugs

From Tom Lane
Subject Re: BUG #16695: pg_hba_file_rules NULL address and netmask
Date
Msg-id 871849.1604366858@sss.pgh.pa.us
Whole thread Raw
In response to Re: BUG #16695: pg_hba_file_rules NULL address and netmask  (Tom Lane <tgl@sss.pgh.pa.us>)
List pgsql-bugs
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

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 = pg_getaddrinfo_all(str, NULL, &hints, &gai_result);
             if (ret == 0 && gai_result)
+            {
                 memcpy(&parsedline->addr, gai_result->ai_addr,
                        gai_result->ai_addrlen);
+                parsedline->addrlen = gai_result->ai_addrlen;
+            }
             else if (ret == EAI_NONAME)
                 parsedline->hostname = str;
             else
@@ -1238,6 +1241,7 @@ parse_hba_line(TokenizedLine *tok_line, int elevel)
                                         token->string);
                     return NULL;
                 }
+                parsedline->masklen = 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 = gai_result->ai_addrlen;
                 pg_freeaddrinfo_all(hints.ai_family, gai_result);

                 if (parsedline->addr.ss_family != parsedline->mask.ss_family)
@@ -2538,20 +2543,26 @@ fill_hba_line(Tuplestorestate *tuple_store, TupleDesc tupdesc,
                 }
                 else
                 {
-                    if (pg_getnameinfo_all(&hba->addr, sizeof(hba->addr),
-                                           buffer, sizeof(buffer),
-                                           NULL, 0,
-                                           NI_NUMERICHOST) == 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) == 0)
+                            clean_ipv6_addr(hba->addr.ss_family, buffer);
                         addrstr = pstrdup(buffer);
                     }
-                    if (pg_getnameinfo_all(&hba->mask, sizeof(hba->mask),
-                                           buffer, sizeof(buffer),
-                                           NULL, 0,
-                                           NI_NUMERICHOST) == 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) == 0)
+                            clean_ipv6_addr(hba->mask.ss_family, buffer);
                         maskstr = 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;

pgsql-bugs by date:

Previous
From: Dmitry Marakasov
Date:
Subject: Re: BUG #16696: Backend crash in llvmjit
Next
From: PG Bug reporting form
Date:
Subject: BUG #16697: timetz regression test seems to have a false failure