Re: [HACKERS] Server ignores contents of SASLInitialResponse

[Heikki Linnakangas]

On 05/25/2017 06:36 PM, Michael Paquier wrote:

On Thu, May 25, 2017 at 10:52 AM, Michael Paquier
<michael.paqu...@gmail.com> wrote:

Actually, I don't think that we are completely done here. Using the
patch of upthread to enforce a failure on SASLInitialResponse, I see
that connecting without SSL causes the following error:
psql: FATAL:  password authentication failed for user "mpaquier"
But connecting with SSL returns that:
psql: duplicate SASL authentication request

I have not looked at that in details yet, but it seems to me that we
should not take pg_SASL_init() twice in the scram authentication code
path in libpq for a single attempt.

Gotcha. This happens because of sslmode=prefer, on which
pqDropConnection is used to clean up the connection state. So it seems
to me that the correct fix is to move the cleanup of sasl_state to
pqDropConnection() instead of closePGconn(). Once I do so the failures
are correct, showing to the user two FATAL errors because of the two
attempts:
psql: FATAL:  password authentication failed for user "mpaquier"
FATAL:  password authentication failed for user "mpaquier"

Hmm, the SASL state cleanup is done pretty much the same way that GSS/SSPI cleanup is. Don't we have a similar problem with GSS? I tested that, and I got an error from glibc, complaining about a double-free:

*** Error in `/home/heikki/pgsql.master/bin/psql': double free or corruption 
(out): 0x000056157d13be00 ***
======= Backtrace: =========
/lib/x86_64-linux-gnu/libc.so.6(+0x70bcb)[0x7f6a460b7bcb]
/lib/x86_64-linux-gnu/libc.so.6(+0x76f96)[0x7f6a460bdf96]
/lib/x86_64-linux-gnu/libc.so.6(+0x7778e)[0x7f6a460be78e]
/home/heikki/pgsql.master/lib/libpq.so.5(+0xa1dc)[0x7f6a46d651dc]
/home/heikki/pgsql.master/lib/libpq.so.5(+0xa4b3)[0x7f6a46d654b3]
/home/heikki/pgsql.master/lib/libpq.so.5(+0xacb9)[0x7f6a46d65cb9]
/home/heikki/pgsql.master/lib/libpq.so.5(PQconnectPoll+0x14e7)[0x7f6a46d6ae81]
/home/heikki/pgsql.master/lib/libpq.so.5(+0xe895)[0x7f6a46d69895]
/home/heikki/pgsql.master/lib/libpq.so.5(PQconnectdbParams+0x4f)[0x7f6a46d675b9]
/home/heikki/pgsql.master/bin/psql(+0x3daa9)[0x56157ccafaa9]
/lib/x86_64-linux-gnu/libc.so.6(__libc_start_main+0xf1)[0x7f6a460672b1]
/home/heikki/pgsql.master/bin/psql(+0x1163a)[0x56157cc8363a]

psql: duplicate GSS authentication request



- Heikki

I bisected that; the culprit was commit 61bf96cab0, where I refactored the libpq authentication code in preparation for SCRAM. The logic around that free() was always a bit wonky, but the refactoring made it outright broken. Attached is a patch for that, see commit message for details. (Review of that would be welcome.) So, after fixing that, back to the original question; don't we have a similar "duplicate authentication request" problem with GSS? Yes, turns out that we do, even on stable branches: psql "sslmode=prefer dbname=postgres hostaddr=127.0.0.1 krbsrvname=postgres host=localhost user=krbtestuser" To fix, I suppose we can do what you did for SASL in your patch, and move the cleanup of conn->gctx from closePGconn to pgDropConnection. And I presume we need to do the same for the SSPI state too, but I don't have a Windows set up to test that at the moment.

0001-Fix-double-free-bug-in-GSS-authentication.patch
Description: invalid/octet-stream

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers