BUG #15520: PAM authentication + domain socket -> DNS query for symbolic hostname [local]

On Sat, Nov 24, 2018 at 11:46 PM PG Bug reporting form
<noreply@postgresql.org> wrote:

The following bug has been logged on the website:

Bug reference: 15520
Logged by: Albert Schabhuetl
Email address: albert_schabhuetl@yahoo.de
PostgreSQL version: 10.4
Operating system: RHEL 7.4
Description:

When PAM authentication is configured for local connections via domain
socket,
during authentication the backend process sends out a DNS query
for the following symbolic hostname: [local]

This is unnecessary as it is not intended that this hostname will ever
resolve to a valid IP address,
and depending on how DNS queries are answered in the actual system it can
cause authentication delays.

Theory why this happens:

Since this commit
https://github.com/postgres/postgres/commit/2f1d2b7a75fecad25295cb3f453503eb6a176d4f#diff-f5a9c53142c3595fe47ebf5146457c6b
during PAM authenticaion the functions
pg_getnameinfo_all
pam_set_item
are called, in order to support the PAM configuration option
"pam_use_hostname".

In the case of a TCP connection and pam_use_hostname = 0,
pg_getnameinfo_all still calls the library function
getnameinfo
but with flags = 0 to achieve "do nothing" behaviour.

In the case of a domain socket connection,
regardless of pam_use_hostname,
pg_getnameinfo_all calls
getnameinfo_unix,
which ignores the flags parameter and invariably
sets the hostname to a symbolic text - [local] in our case.

When subsequently the PAM library function
pam_set_item
is called, it gets the symbolic hostname [local]
and probably it is this function which sends the DNS request for this
hostname.

Right, with a debugger I could see Linux PAM calling libaudit.so code
that tries to resolve the hostname:

#0 __GI_getaddrinfo (name=0x55b97d269d80 "[local]", service=0x0,
hints=0x7ffee7011730, pai=0x7ffee7011728) at
../sysdeps/posix/getaddrinfo.c:2295
#1 0x00007f5f4038822a in ?? () from /lib/x86_64-linux-gnu/libaudit.so.1
#2 0x00007f5f40388bb1 in audit_log_acct_message () from
/lib/x86_64-linux-gnu/libaudit.so.1
#3 0x00007f5f41060410 in ?? () from /lib/x86_64-linux-gnu/libpam.so.0
#4 0x00007f5f41060555 in ?? () from /lib/x86_64-linux-gnu/libpam.so.0
#5 0x000055b97b07d29b in CheckPAMAuth (password=0x55b97b4ae7fe "",
user=0x55b97d1d8fd8 "munro", port=0x55b97d1ff590) at auth.c:2249

It seems we shouldn't be passing a bogus hostname in PAM_RHOST.

--
Thomas Munro
http://www.enterprisedb.com

On Sun, Nov 25, 2018 at 12:57 AM Thomas Munro
<thomas.munro@enterprisedb.com> wrote:

It seems we shouldn't be passing a bogus hostname in PAM_RHOST.

I wonder if we should simply not set PAM_RHOST for Unix sockets, since
(in the words of the Linux PAM man page) "[i]n some applications,
PAM_RHOST may be NULL", or set it to the hostname of the local
machine, since in a general sense it is "the hostname of the machine
from which the PAM_RUSER entity is requesting service" (Linux PAM) and
"[t]he name of the applicant's host" (OpenPAM). The latter doesn't
seem great because it means that a PAM module loses the ability to
distinguish this case from the IP case, so I think the former is
probably better. I haven't managed to find anything explicit about
the expected value of PAM_RHOST for Unix sockets in either the Linux
PAM or OpenPAM projects.

I wonder if anyone out there has come to rely on the value "[local]"
that PostgreSQL generates for this case (ie in a custom PAM module or
script executed with pam_exec.so), and would get upset if we changed
it. Seems pretty unlikely.

The comments for pg_getnameinfo_all() could probably do with a mention
of the special value written to "node" for Unix sockets.

Some relevant code:

We can see that linux-audit only starts trying to resolve host if you
didn't also pass in an address:

https://github.com/linux-audit/audit-userspace/blob/e42602b7b246ae62e7a12e9cd91f0ac37b1b1968/lib/audit_logging.c#L456

We can also see that linux-pam always passes NULL as an address:

https://github.com/linux-pam/linux-pam/blob/955b3e2f100205be2db4358e9c812de2ae453b8e/libpam/pam_audit.c#L41

--
Thomas Munro
http://www.enterprisedb.com

NULL vs hostname for PAM_RHOST:
My understanding is that the purpose of the PAM configuration parameter pam_use_hostname is to avoid the adverse effects of DNS queries if set to 0. Thus if pam_use_hostname is 0, PAM_RHOST shall be NULL for domain socket connections, just like it is the case for TCP connections.

I wonder if anyone out there has come to rely on the value "[local]"I vote for changing it, and documenting it in the release notes.

regards,Albert
Am Samstag, 24. November 2018, 22:58:01 MEZ hat Thomas Munro <thomas.munro@enterprisedb.com> Folgendes geschrieben:

On Sun, Nov 25, 2018 at 12:57 AM Thomas Munro
<thomas.munro@enterprisedb.com> wrote:

It seems we shouldn't be passing a bogus hostname in PAM_RHOST.

I wonder if we should simply not set PAM_RHOST for Unix sockets, since
(in the words of the Linux PAM man page) "[i]n some applications,
PAM_RHOST may be NULL", or set it to the hostname of the local
machine, since in a general sense it is "the hostname of the machine
from which the PAM_RUSER entity is requesting service" (Linux PAM) and
"[t]he name of the applicant's host" (OpenPAM).  The latter doesn't
seem great because it means that a PAM module loses the ability to
distinguish this case from the IP case, so I think the former is
probably better.  I haven't managed to find anything explicit about
the expected value of PAM_RHOST for Unix sockets in either the Linux
PAM or OpenPAM projects.

I wonder if anyone out there has come to rely on the value "[local]"
that PostgreSQL generates for this case (ie in a custom PAM module or
script executed with pam_exec.so), and would get upset if we changed
it.  Seems pretty unlikely.

The comments for pg_getnameinfo_all() could probably do with a mention
of the special value written to "node" for Unix sockets.

Some relevant code:

We can see that linux-audit only starts trying to resolve host if you
didn't also pass in an address:

https://github.com/linux-audit/audit-userspace/blob/e42602b7b246ae62e7a12e9cd91f0ac37b1b1968/lib/audit_logging.c#L456

We can also see that linux-pam always passes NULL as an address:

https://github.com/linux-pam/linux-pam/blob/955b3e2f100205be2db4358e9c812de2ae453b8e/libpam/pam_audit.c#L41

--
Thomas Munro
http://www.enterprisedb.com

On Mon, Nov 26, 2018 at 9:10 AM Albert Schabhuetl
<albert_schabhuetl@yahoo.de> wrote:

NULL vs hostname for PAM_RHOST:

My understanding is that the purpose of the PAM configuration parameter pam_use_hostname is to avoid the adverse effects of DNS queries if set to 0. Thus if pam_use_hostname is 0, PAM_RHOST shall be NULL for domain socket connections, just like it is the case for TCP connections.

It doesn't set it to NULL for TCP connections. It tells PostgreSQL
not to bother resolving the name to an address. We can't do much
about it if your PAM implementation decides to resolve it anyway. I
suppose we could have an option not to set it at all, even for TCP.
But the main thing that I think we need to change here to address your
complaint is the Unix socket case, because we're passing a
non-hostname in a context that expects a hostname, which is silly and
generates entirely bogus DNS lookups.

Here's a test:

In pg_hba.conf I put these lines:
local all all pam
pamservice="foo"
host all all 127.0.0.1/32 pam
pamservice="foo"

In /etc/pam.d/foo I put these lines:
auth optional pam_exec.so /tmp/spy_script.sh
auth required pam_permit.so

In /tmp/spy_script.sh I put these lines, and made it executable:
#!/bin/bash
echo "PAM_USER=$PAM_USER, PAM_RHOST=$PAM_RHOST" > /tmp/spy_script.out

$ psql -h localhost postgres munro
-> PAM_USER=munro, PAM_RHOST=127.0.0.1
$ psql postgres munro
-> PAM_USER=munro, PAM_RHOST=[local]

Now in pg_hba.conf I add pam_use_hostname=1 to the end of both lines...

$ psql -h localhost postgres munro
-> PAM_USER=munro, PAM_RHOST=localhost
$ psql postgres munro
-> PAM_USER=munro, PAM_RHOST=[local]

I wonder if anyone out there has come to rely on the value "[local]"

I vote for changing it, and documenting it in the release notes.

Yeah. Here is a draft patch to change that. Test output:

$ psql -h localhost postgres munro
PAM_USER=munro, PAM_RHOST=localhost
$ psql postgres munro
PAM_USER=munro, PAM_RHOST=

--
Thomas Munro
http://www.enterprisedb.com

On 25/11/2018 23:30, Thomas Munro wrote:

I wonder if anyone out there has come to rely on the value "[local]"

I vote for changing it, and documenting it in the release notes.

Yeah. Here is a draft patch to change that. Test output:

$ psql -h localhost postgres munro
PAM_USER=munro, PAM_RHOST=localhost
$ psql postgres munro
PAM_USER=munro, PAM_RHOST=

I think this is the right thing to do.

About your patch, if we're not going to set PAM_RHOST, then we should
also avoid the call to pg_getnameinfo_all() earlier in CheckPAMAuth().
Look at the original patch linked earlier in the thread; we just need to
put if statements around both of those hunks.

--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

On Tue, Nov 27, 2018 at 3:02 AM Peter Eisentraut
<peter.eisentraut@2ndquadrant.com> wrote:

On 25/11/2018 23:30, Thomas Munro wrote:

I wonder if anyone out there has come to rely on the value "[local]"

I vote for changing it, and documenting it in the release notes.

Yeah. Here is a draft patch to change that. Test output:

$ psql -h localhost postgres munro
PAM_USER=munro, PAM_RHOST=localhost
$ psql postgres munro
PAM_USER=munro, PAM_RHOST=

I think this is the right thing to do.

About your patch, if we're not going to set PAM_RHOST, then we should
also avoid the call to pg_getnameinfo_all() earlier in CheckPAMAuth().
Look at the original patch linked earlier in the thread; we just need to
put if statements around both of those hunks.

Thanks for the review. Right. Here's a new version that moves both
things under the same if, and refactors a long line to fit in passing.

I wondered whether we could write
src/test/authentication/t/003_pam.pl, but it seems hard to do without
underhand tricks. Both open source PAM implementations really want to
read their configuration from /etc.

--
Thomas Munro
http://www.enterprisedb.com

On Tue, Nov 27, 2018 at 1:39 PM Thomas Munro
<thomas.munro@enterprisedb.com> wrote:

On Tue, Nov 27, 2018 at 3:02 AM Peter Eisentraut
<peter.eisentraut@2ndquadrant.com> wrote:

On 25/11/2018 23:30, Thomas Munro wrote:

$ psql -h localhost postgres munro
PAM_USER=munro, PAM_RHOST=localhost
$ psql postgres munro
PAM_USER=munro, PAM_RHOST=

I think this is the right thing to do.

About your patch, if we're not going to set PAM_RHOST, then we should
also avoid the call to pg_getnameinfo_all() earlier in CheckPAMAuth().
Look at the original patch linked earlier in the thread; we just need to
put if statements around both of those hunks.

Thanks for the review. Right. Here's a new version that moves both
things under the same if, and refactors a long line to fit in passing.

Pushed, and back-patched to 9.6. I wondered whether a documentation
change was warranted, but the special "[local]" value wasn't
documented in the first place, and it shouldn't really surprise anyone
that there is no remote host information for a local connection. A
release note about the change seems sufficient.

Thanks for the report.

--
Thomas Munro
http://www.enterprisedb.com