Fix unportable use of getnameinfo() in pg_hba_file_rules view.

fill_hba_line() thought it could get away with passing sizeof(struct
sockaddr_storage) rather than the actual addrlen previously returned
by getaddrinfo().  While that appears to work on many platforms,
it does not work on FreeBSD 11: you get back a failure, which leads
to the view showing NULL for the address and netmask columns in all
rows.  The POSIX spec for getnameinfo() is pretty clearly on
FreeBSD's side here: you should pass the actual address length.
So it seems plausible that there are other platforms where this
coding also fails, and we just hadn't noticed.

Also, IMO the fact that getnameinfo() failure leads to a NULL output
is pretty bogus in itself.  Our pg_getnameinfo_all() wrapper is
careful to emit "???" on failure, and we should use that in such
cases.  NULL should only be emitted in rows that don't have IP
addresses.

Per bug #16695 from Peter Vandivier.  Back-patch to v10 where this
code was added.

Discussion: https://postgr.es/m/16695-a665558e2f630be7@postgresql.org

diff --git a/src/backend/libpq/hba.c b/src/backend/libpq/hba.c
index 892d16a1f9496b4aeb4fb6cb7927639cb96460d9..d7fb60fb0315a372bba539e04ce022592765bc07 100644 (file)
--- a/src/backend/libpq/hba.c
+++ b/src/backend/libpq/hba.c
@@ -1151,8 +1151,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
@@ -1201,6 +1204,7 @@ parse_hba_line(TokenizedLine *tok_line, int elevel)
                                        token->string);
                    return NULL;
                }
+               parsedline->masklen = parsedline->addrlen;
                pfree(str);
            }
            else if (!parsedline->hostname)
@@ -1251,6 +1255,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)
@@ -2420,20 +2425,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 07d92d4f9f3a975a4ed9bebb11b91fbf01c29a46..c20076bfbdae2a4ca6a787cb10b3f0b46735cd6d 100644 (file)
--- 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,
@@ -97,6 +101,8 @@ typedef struct HbaLine
    char       *radiusidentifiers_s;
    List       *radiusports;
    char       *radiusports_s;
+   int         addrlen;        /* zero if we don't have a valid addr */
+   int         masklen;        /* zero if we don't have a valid mask */
 } HbaLine;
 
 typedef struct IdentLine