Possible PostgreSQL 8.3beta4 bug with MD5 authentication in psql?

Hi everyone,

I think that I may have found a minor bug with PostgreSQL 8.3beta4 with
respect to md5 authentication. I actually discovered this on Win32, but
it appears that the behaviour is the same under Linux too.

As part of the PostGIS install under Win32, I have a few scripts that
check for the existence of a particular database by doing the following:

psql -d adatabase -h localhost -c "SELECT version();"

By checking the psql exit code, it is fairly easy to see whether this
failed, and if so display the contents of stdout for the user. The
problem I have is that under PostgreSQL 8.3beta4, if the database
doesn't exist then I get an extra password prompt which breaks the
install scripts as they run in the background :(

To recreate this is fairly easy:

1. Temporarily rename any .pgpass files so they aren't found by libpq
2. Stop the PostgreSQL 8.3 server
3. Change pg_hba.conf so that local connections are disabled, but
connections to 127.0.0.1 are allowed with md5 authentication
4. Restart the PostgreSQL server
5. Open up a shell and do the following:

pg83(at)mca-desktop:~$ export PGPASSWORD=mypass
pg83(at)mca-desktop:~$ psql -h localhost -d postgres -c "SELECT version();"
version
---------------------------------------------------------------------------------------------------
PostgreSQL 8.3beta2 on i686-pc-linux-gnu, compiled by GCC gcc (GCC)
4.0.3 (Ubuntu 4.0.3-1ubuntu5)
(1 row)

So far so good. But now try with a database that doesn't exist:

pg83(at)mca-desktop:~$ psql -h localhost -d doesntexist -c "SELECT
version();"
Password:
psql: FATAL: database "doesntexist" does not exist

Hmmmm. So even though PGPASSWORD is set (and the command works if the
database exists within the cluster), if I specify a non-existent
database then I still get prompted for a password.

I've run the same test against PostgreSQL 8.2.5 and the test works in
that I don't get prompted for a password the second time. So the
behaviour has changed between versions, but I wanted to check that it
wasn't a deliberate change before looking deeper.

Many thanks,

Mark.

--
ILande - Open Source Consultancy
http://www.ilande.co.uk

Mark Cave-Ayland wrote:
> Hmmmm. So even though PGPASSWORD is set (and the command works if the
> database exists within the cluster), if I specify a non-existent
> database then I still get prompted for a password.

Just to add a note to that - when running it in the same shell from
which I started the server with messages going to stdout, it seemed
clear that it trys to connect once using PGPASSWORD, then when that
fails, it prompts for the password instead, and then tries to connect
with that and fails a second time.

/D

Dave Page <dpage(at)postgresql(dot)org> writes:
> Just to add a note to that - when running it in the same shell from
> which I started the server with messages going to stdout, it seemed
> clear that it trys to connect once using PGPASSWORD, then when that
> fails, it prompts for the password instead, and then tries to connect
> with that and fails a second time.

Hmmm ... it seems the problem is that we've defined
PQconnectionUsedPassword in such a way that it returns true (causing a
prompt) regardless of whether the reason for the connection failure was
a bad password or not. We might need to reconsider that API.

regards, tom lane

On Fri, 2007-12-07 at 11:03 -0500, Tom Lane wrote:

> Hmmm ... it seems the problem is that we've defined
> PQconnectionUsedPassword in such a way that it returns true (causing a
> prompt) regardless of whether the reason for the connection failure was
> a bad password or not. We might need to reconsider that API.

Right. I think it depends on the interpretation of the
PQconnectionUsedPassword function. If it should simply return whether or
not the connection used a password or not (as it does now), then you
could argue that it should be psql which should incorporate an
additional check to determine whether the attempt was cancelled due to
an invalid database name.

On first glance, libpq appears to return just CONNECTION_BAD and an
error message, so I'm not sure whether we can easily detect the
difference between an incorrect password and an invalid database name :(
Is there an additional status code in PGconn that can be used to
determine the exact cause of connection failure?

ATB,

Mark.

--
ILande - Open Source Consultancy
http://www.ilande.co.uk

Mark Cave-Ayland <mark(dot)cave-ayland(at)ilande(dot)co(dot)uk> writes:
> On Fri, 2007-12-07 at 11:03 -0500, Tom Lane wrote:
>> Hmmm ... it seems the problem is that we've defined
>> PQconnectionUsedPassword in such a way that it returns true (causing a
>> prompt) regardless of whether the reason for the connection failure was
>> a bad password or not. We might need to reconsider that API.

> Right. I think it depends on the interpretation of the
> PQconnectionUsedPassword function. If it should simply return whether or
> not the connection used a password or not (as it does now), then you
> could argue that it should be psql which should incorporate an
> additional check to determine whether the attempt was cancelled due to
> an invalid database name.

I think the problem is that we've tried to make PQconnectionUsedPassword
serve two masters. What it was originally designed for was to provide
a way for dblink to determine whether the user had provided an
authentication token, or was trying to impersonate the postgres user
on a remote connection. For that purpose its behavior is fine.
However, we (I think this is my fault :-() also tried to use it to
replace the ugly technique of checking for a specific error message
string to decide whether to prompt for a password. This example shows
that that doesn't work.

I can see three ways to proceed:

1. Revert the changes that removed dependencies on PQnoPasswordSupplied.
This is ugly but might be the safest solution for 8.3 --- we can always
revisit the issue later.

2. Try to adjust PQconnectionUsedPassword so that what it reports after
successful connection (which is what dblink cares about) isn't
necessarily defined the same as what it says after a failed connection.
This seems pretty ugly and nonintuitive.

3. Invent another libpq function, maybe PQconnectionNeedsPassword,
that does the right thing for the password-checking tests.

I don't think that leaving it as-is is acceptable --- if we do that,
we'll be encouraging client apps to adopt a broken API.

Thoughts?

regards, tom lane

Tom Lane wrote:

> 1. Revert the changes that removed dependencies on PQnoPasswordSupplied.
> This is ugly but might be the safest solution for 8.3 --- we can always
> revisit the issue later.

> 3. Invent another libpq function, maybe PQconnectionNeedsPassword,
> that does the right thing for the password-checking tests.

My vote goes to (3), if the work can be done quickly, or (1) if it
can't.

--
Alvaro Herrera http://www.PlanetPostgreSQL.org/
"When the proper man does nothing (wu-wei),
his thought is felt ten thousand miles." (Lao Tse)

Alvaro Herrera <alvherre(at)alvh(dot)no-ip(dot)org> writes:
> Tom Lane wrote:
>> 3. Invent another libpq function, maybe PQconnectionNeedsPassword,
>> that does the right thing for the password-checking tests.

> My vote goes to (3), if the work can be done quickly, or (1) if it
> can't.

I don't think it's a big problem to do, as long as we are agreed on
the behavior we want. In particular, consider:

1. If libpq obtains a password internally (ie, from PGPASSWORD or
~/.pgpass), and it's wrong, do we want a user password prompt?

2. If we prompt the user for a password, and it's wrong, do we
want to try again?

Our historical behavior on both points was "no", but at least
the second one seems a bit questionable.

regards, tom lane

On Sat, 2007-12-08 at 10:09 -0500, Tom Lane wrote:

> > My vote goes to (3), if the work can be done quickly, or (1) if it
> > can't.

Ditto.

> I don't think it's a big problem to do, as long as we are agreed on
> the behavior we want. In particular, consider:
>
> 1. If libpq obtains a password internally (ie, from PGPASSWORD or
> ~/.pgpass), and it's wrong, do we want a user password prompt?
>
> 2. If we prompt the user for a password, and it's wrong, do we
> want to try again?
>
> Our historical behavior on both points was "no", but at least
> the second one seems a bit questionable.

I'd say that you definitely don't want a user password prompt if libpq's
password is wrong, since I can see this would break a lot of scripts
that weren't launched directly from the shell - the switch from
non-interactive to interactive would cause the script to hang in the
background rather than return immediately with an error (indeed it was
this confusing symptom in the PostGIS installer that flagged the change
in behaviour).

As for the second point, I'm not too worried about how many times you
are asked for the password - I personally am happy with the one attempt
as it stands at the moment. My main concern is the switch from a
non-interactive mode to an interactive mode.

Kind regards,

Mark.

--
ILande - Open Source Consultancy
http://www.ilande.co.uk

Mark Cave-Ayland <mark(dot)cave-ayland(at)ilande(dot)co(dot)uk> writes:
> I'd say that you definitely don't want a user password prompt if libpq's
> password is wrong, since I can see this would break a lot of scripts
> that weren't launched directly from the shell

Agreed, we don't want the behavior to suddenly turn interactive when
the user expected it not to be.

> As for the second point, I'm not too worried about how many times you
> are asked for the password - I personally am happy with the one attempt
> as it stands at the moment.

On reflection, this ought to be an application decision anyway; libpq
isn't going to be a good place to do it since it can have no memory
of how many times the "same" connection was tried.

Also, on further reflection I realize that PQconnectionUsedPassword
is broken for dblink's usage too! It's currently defined, in essence,
as "the connection used a password-based auth method". But that fails
to distinguish where it got the password from. In particular, as of
CVS HEAD it's still possible for a non-superuser to impersonate
postgres on a dblink connection, if the DBA has taken the
not-unreasonable step of setting up a ~/.pgpass in postgres' home
directory.

Fortunately, existing release branches don't use that technique,
or we'd have a live security problem here. But anyway, this seemingly
trivial patch has turned out to be pretty snake-bit :-(

So what I think we must do is split the function into two:

PQconnectionNeedsPassword: true if server demanded a password and there
was none to send (hence, can only be true for a failed connection)

PQconnectionUsedPassword: true if server demanded a password and it
was supplied from the connection function's argument list, *not*
from environment or a password file.

Offhand it looks like only dblink needs the second version ---
all of our client-side code wants the first one.

Barring objections I'll go code this up ...

regards, tom lane

On Sat, 2007-12-08 at 17:09 -0500, Tom Lane wrote:

> So what I think we must do is split the function into two:
>
> PQconnectionNeedsPassword: true if server demanded a password and there
> was none to send (hence, can only be true for a failed connection)
>
> PQconnectionUsedPassword: true if server demanded a password and it
> was supplied from the connection function's argument list, *not*
> from environment or a password file.
>
> Offhand it looks like only dblink needs the second version ---
> all of our client-side code wants the first one.
>
> Barring objections I'll go code this up ...

Yup, that looks good to me. My only criticism would be that the name
PQconnectionUsedPassword could be a little misleading, in that based
upon the name I would expect it to return true if a password was
specified regardless of whether the method used was interactive, .pgpass
or PGPASSWORD.

The only other suggestion I can think of at the moment would be
PQconnectionUsedConnectionPassword which seems to better sum up its
purpose to me...

Kind regards,

Mark.

--
ILande - Open Source Consultancy
http://www.ilande.co.uk