Logging of PAM Authentication Failure

Hello,

When client authentication method is set to "pam" in pg_hba.conf,
connecting using psql results in logging of authentication failure
even before a password prompt is provided, nonetheless user is
subsequently able to connect by providing a password. Following is
what is logged:

Password: LOG: pam_authenticate failed: Conversation error
FATAL: PAM authentication failed for user "amit"

To see what's going on I debugged psql and found that without a -W
option, this is bound to happen, since psql first attempts to connect
and without a password (which it doesn't know is required for the
first time), it fails and subsequently prompts for password. Correct
password then leads to successful connection.

I tried to observe the behavior with md5 method (without -W) and
observed that no authentication failure is logged, since server
probably behaves differently in response to the psql's first
connection request in that case. But, pam method leads to it being
logged.

Is this a problem?

--

Amit Langote

On Wed, May 8, 2013 at 10:40 PM, Amit Langote <amitlangote09(at)gmail(dot)com> wrote:
> When client authentication method is set to "pam" in pg_hba.conf,
> connecting using psql results in logging of authentication failure
> even before a password prompt is provided, nonetheless user is
> subsequently able to connect by providing a password. Following is
> what is logged:
>
> Password: LOG: pam_authenticate failed: Conversation error
> FATAL: PAM authentication failed for user "amit"
>
> To see what's going on I debugged psql and found that without a -W
> option, this is bound to happen, since psql first attempts to connect
> and without a password (which it doesn't know is required for the
> first time), it fails and subsequently prompts for password. Correct
> password then leads to successful connection.
>
> I tried to observe the behavior with md5 method (without -W) and
> observed that no authentication failure is logged, since server
> probably behaves differently in response to the psql's first
> connection request in that case. But, pam method leads to it being
> logged.
>
> Is this a problem?

Not really. We could potentially fix it by extending the wire
protocol to allow the server to respond to the client's startup packet
with a further challenge, and extend libpq to report that challenge
back to the user and allow sending a response. But that would break
on-the-wire compatibility, which we haven't done in a good 10 years,
and certainly wouldn't be worthwhile just for this.

We'd also need to be careful not to create information leaks.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

Robert Haas <robertmhaas(at)gmail(dot)com> writes:
> On Wed, May 8, 2013 at 10:40 PM, Amit Langote <amitlangote09(at)gmail(dot)com> wrote:
>> I tried to observe the behavior with md5 method (without -W) and
>> observed that no authentication failure is logged, since server
>> probably behaves differently in response to the psql's first
>> connection request in that case. But, pam method leads to it being
>> logged.

auth_failed() in src/backend/libpq/auth.c intentionally logs nothing for
STATUS_EOF status (ie, client closed the connection without responding).
But it looks like the PAM code path doesn't have a way to return that
status code, even when pam_passwd_conv_proc() knows that that's what
happened, and intentionally printed no log message itself (around line
1870 in HEAD). If there's another response code we could return through
the PAM layer, this could be fixed, and I think it should be.

>> Is this a problem?

> Not really. We could potentially fix it by extending the wire
> protocol to allow the server to respond to the client's startup packet
> with a further challenge, and extend libpq to report that challenge
> back to the user and allow sending a response. But that would break
> on-the-wire compatibility, which we haven't done in a good 10 years,
> and certainly wouldn't be worthwhile just for this.

It's not the wire protocol that's the problem; it's that libpq's client
API doesn't provide a way to ask the calling application for a password
in the midst of a connection attempt.

regards, tom lane

> auth_failed() in src/backend/libpq/auth.c intentionally logs nothing for
> STATUS_EOF status (ie, client closed the connection without responding).
> But it looks like the PAM code path doesn't have a way to return that
> status code, even when pam_passwd_conv_proc() knows that that's what
> happened, and intentionally printed no log message itself (around line
> 1870 in HEAD). If there's another response code we could return through
> the PAM layer, this could be fixed, and I think it should be.

So if I get this correctly, does this mean the only thing that needs
to be fixed is unnecessary logging or is there a problem with
authentication exchange itself in case of PAM? Also, when you say PAM
layer, is that pam_passwd_conv_proc() that needs to be able to return
an alternative status code?

--
Amit Langote

Hello,

> > auth_failed() in src/backend/libpq/auth.c intentionally logs nothing for
> > STATUS_EOF status (ie, client closed the connection without responding).
> > But it looks like the PAM code path doesn't have a way to return that
> > status code, even when pam_passwd_conv_proc() knows that that's what
> > happened, and intentionally printed no log message itself (around line
> > 1870 in HEAD). If there's another response code we could return through
> > the PAM layer, this could be fixed, and I think it should be.
>
> So if I get this correctly, does this mean the only thing that needs
> to be fixed is unnecessary logging or is there a problem with
> authentication exchange itself in case of PAM? Also, when you say PAM
> layer, is that pam_passwd_conv_proc() that needs to be able to return
> an alternative status code?

Following is the point server requests psql to send password when
PAM is enabled.

backend/libpq/auth.c:1861
> if (strlen(passwd) == 0)
> {
> /*
> * Password wasn't passed to PAM the first time around -
> * let's go ask the client to send a password, which we
> * then stuff into PAM.
> */
> sendAuthRequest(pam_port_cludge, AUTH_REQ_PASSWORD);
> passwd = recv_password_packet(pam_port_cludge);
> if (passwd == NULL)
> {
> /*
> * Client didn't want to send password. We
> * intentionally do not log anything about this.
> */
> goto fail;
...
> return PAM_CONV_ERR;

This code seems to me expecting for psql to send password without
closing current connnection.On the other hand psql does as
follows.

bin/psql/startup.c: 227
> pset.db = PQconnectdbParams(keywords, values, true);
> free(keywords);
> free(values);
>
> if (PQstatus(pset.db) == CONNECTION_BAD &&
> PQconnectionNeedsPassword(pset.db) &&
> password == NULL &&
> pset.getPassword != TRI_NO)
> {
> PQfinish(pset.db);
> password = simple_prompt(password_prompt, 100, false);
> new_pass = true;
> }

psql at once disconnects the current connection and reconnects
with this new password, so pam_conv_err is observed in server.

It seems to be a kind of protocol-mimatching. Client should'nt
disconnect for password request or server should fit to what psql
does. Is this wrong?

regards,

--
Kyotaro Horiguchi
NTT Open Source Software Center

> This code seems to me expecting for psql to send password without
> closing current connnection.On the other hand psql does as
> follows.
>
> bin/psql/startup.c: 227
>> pset.db = PQconnectdbParams(keywords, values, true);
>> free(keywords);
>> free(values);
>>
>> if (PQstatus(pset.db) == CONNECTION_BAD &&
>> PQconnectionNeedsPassword(pset.db) &&
>> password == NULL &&
>> pset.getPassword != TRI_NO)
>> {
>> PQfinish(pset.db);
>> password = simple_prompt(password_prompt, 100, false);
>> new_pass = true;
>> }
>
> psql at once disconnects the current connection and reconnects
> with this new password, so pam_conv_err is observed in server.
>
> It seems to be a kind of protocol-mimatching. Client should'nt
> disconnect for password request or server should fit to what psql
> does. Is this wrong?

In fact, this is the behavior with all the authentication methods that
require a password. But, it is only in the case of PAM authentication
that auth_failed() logs error when first connection attempt is made
(without password), since the STATUS_EOF is not passed to it in that
case.
If we did not drop the connection (unlike what we do now) and
re-attempted connection with the password added to conn, would the
backend's authentication state still be waiting for the password? Can
we do away without having to create a second connection?
--
Amit Langote

> In fact, this is the behavior with all the authentication methods that
> require a password. But, it is only in the case of PAM authentication
> that auth_failed() logs error when first connection attempt is made
> (without password), since the STATUS_EOF is not passed to it in that
> case.

Well, if we are allowed to use a bit ugry way, the attached patch
seems to cope with this issue. As far as I can see there's no
problem since pg_fe_sendauth() refueses to send empty password.

Any suggestions?

> If we did not drop the connection (unlike what we do now) and
> re-attempted connection with the password added to conn, would the
> backend's authentication state still be waiting for the password? Can
> we do away without having to create a second connection?

Sorry, I've read there incorrectly. I had understood the code
after sendAuthRequest in pam_passwd_conv_proc but it is used
indeed.

regards,

--
Kyotaro Horiguchi
NTT Open Source Software Center

> Well, if we are allowed to use a bit ugry way, the attached patch
> seems to cope with this issue. As far as I can see there's no
> problem since pg_fe_sendauth() refueses to send empty password.
>
> Any suggestions?

That seems to do the trick. This probably solves the problem that I
originally posted.

> Sorry, I've read there incorrectly. I had understood the code
> after sendAuthRequest in pam_passwd_conv_proc but it is used
> indeed.

Though, I am still not sure why we drop the existing connection and
start all over again but now with the newly entered password. This
kind of seems to leave the protocol state machine (as in
PQconnectPoll() ) halfway (after pg_fe_sendauth() failed) in the first
connection attempt for the auth requests requiring the password (or
others, too?). Although, sticking to this design may have to do with
the problems of doing otherwise that I am unaware of.

--
Amit Langote

Hello,

Is it right that it is only in the case a password prompt is needed
that a new connection is created after dropping the just-failed
connection?
I created a patch which enables it to use the existing connection in
such a case (unlike what we currently do). It modifies
connectDBComplete() and PQconnectPoll() to also include states
pertaining to password being accepted from the user. That is, the
state machine in PQconnectPoll() is further extended to include a
connection state called CONNECTION_ASKING_PASSWORD which is entered
when server sends AUTH_REQ_MD5 or AUTH_REQ_PASSWORD auth requests.
These two request types require a password to be entered by the user.
There is a new PostgresPollingStatusType value called
PGRES_POLLING_WAITING_PASSWORD which is the polling status while a
password is being entered by the user.

When user enters the password the PQconnectPoll() continues forward in
CONNECTION_ASKING_PASSWORD wherein it sends the password to the server
(by calling pg_fe_sendauth() and this time with a potentially correct
password) and later goes back to CONNECTION_AWAITING_RESPONSE to read
server's response to the password just entered where it either
receives authorization OK or error response thus completing the
connection start-up process.

The backend waits for the password until authentication timeout
happens in which case the client can not send the password anymore
since the backend has exited due to authentication timeout. I wonder
if this is one of the reasons why this has not already been
implemented?

Comments?

On Tue, May 14, 2013 at 11:22 AM, Amit Langote <amitlangote09(at)gmail(dot)com> wrote:
>> Well, if we are allowed to use a bit ugry way, the attached patch
>> seems to cope with this issue. As far as I can see there's no
>> problem since pg_fe_sendauth() refueses to send empty password.
>>
>> Any suggestions?
>
> That seems to do the trick. This probably solves the problem that I
> originally posted.
>
>> Sorry, I've read there incorrectly. I had understood the code
>> after sendAuthRequest in pam_passwd_conv_proc but it is used
>> indeed.
>
> Though, I am still not sure why we drop the existing connection and
> start all over again but now with the newly entered password. This
> kind of seems to leave the protocol state machine (as in
> PQconnectPoll() ) halfway (after pg_fe_sendauth() failed) in the first
> connection attempt for the auth requests requiring the password (or
> others, too?). Although, sticking to this design may have to do with
> the problems of doing otherwise that I am unaware of.
>
>
> --
> Amit Langote

--

Amit Langote

On Tue, May 14, 2013 at 11:20 AM, Amit Langote <amitlangote09(at)gmail(dot)com> wrote:
> Hello,
>
> Is it right that it is only in the case a password prompt is needed
> that a new connection is created after dropping the just-failed
> connection?
> I created a patch which enables it to use the existing connection in
> such a case (unlike what we currently do). It modifies
> connectDBComplete() and PQconnectPoll() to also include states
> pertaining to password being accepted from the user. That is, the
> state machine in PQconnectPoll() is further extended to include a
> connection state called CONNECTION_ASKING_PASSWORD which is entered
> when server sends AUTH_REQ_MD5 or AUTH_REQ_PASSWORD auth requests.
> These two request types require a password to be entered by the user.
> There is a new PostgresPollingStatusType value called
> PGRES_POLLING_WAITING_PASSWORD which is the polling status while a
> password is being entered by the user.
>
> When user enters the password the PQconnectPoll() continues forward in
> CONNECTION_ASKING_PASSWORD wherein it sends the password to the server
> (by calling pg_fe_sendauth() and this time with a potentially correct
> password) and later goes back to CONNECTION_AWAITING_RESPONSE to read
> server's response to the password just entered where it either
> receives authorization OK or error response thus completing the
> connection start-up process.
>
> The backend waits for the password until authentication timeout
> happens in which case the client can not send the password anymore
> since the backend has exited due to authentication timeout. I wonder
> if this is one of the reasons why this has not already been
> implemented?
>
> Comments?

Please add patches here so they don't get forgotten:

https://commitfest.postgresql.org/action/commitfest_view/open

Do we really need to add *2* new libpq functions just to support this?

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

> Please add patches here so they don't get forgotten:
>
> https://commitfest.postgresql.org/action/commitfest_view/open
>
> Do we really need to add *2* new libpq functions just to support this?

I will add the patches to commitfest after reviewing it a bit to see
if we can do away without having to create more new functions than
necessary and make appropriate changes.

--
Amit Langote

> Is it right that it is only in the case a password prompt is needed
> that a new connection is created after dropping the just-failed
> connection?

It's quite doubtful.\:-p The sequense seems fragile to say the
least. Inserting password request state into the client-side state
machine looks quite reasonable.

> I created a patch which enables it to use the existing connection in
> such a case (unlike what we currently do). It modifies
> connectDBComplete() and PQconnectPoll() to also include states
> pertaining to password being accepted from the user. That is, the
> state machine in PQconnectPoll() is further extended to include a
> connection state called CONNECTION_ASKING_PASSWORD which is entered
> when server sends AUTH_REQ_MD5 or AUTH_REQ_PASSWORD auth requests.

Great! The new client state seems to be effective also for MD5. But
it seems to break existing libpq client doing the same authentication
sequence as current psql. Some means would be necessary to switch the
behavior when password is not previously provided but needed by the
server, or make the first half of the connection sequence to be
compatible to the current sequence - in other words - It should be
that when the user finds stauts is CONNECTION_BAD and
PQconnectionNeedsPassword() == true, the user can throw away the
connection and make new connection providing password, and also can
send password on existing connection.

the old style

| db = PQconnectXXXX();
| if (PQstatus(db) == CONNECTION_BAD && PQconnectionNeedsPassword(db))
| {
| PQfinish(db);
| value[..] = password = <some means to read password>;
| db = PQconnectXXXX();
| if (PQstatus(db) == CONNECTION_BAD)
| <error>

and the new style

| db = PQconnectXXXX();
| if (PQconnectionNeedsPassword(db))
| PQsendPassword(db, password);
| if (PQstatus(db) == CONNECTION_BAD)
| <error>

should be standing together.

Where, PQsendPassword is combined function of PQcopyPassword and
PQcontinuedbConnect. If the only porpose of these functions is sending
password then these functions are needed to be separately.

What do you think for the compatibility and simpler API.

> These two request types require a password to be entered by the user.
> There is a new PostgresPollingStatusType value called
> PGRES_POLLING_WAITING_PASSWORD which is the polling status while a
> password is being entered by the user.
>
> When user enters the password the PQconnectPoll() continues forward in
> CONNECTION_ASKING_PASSWORD wherein it sends the password to the server
> (by calling pg_fe_sendauth() and this time with a potentially correct
> password) and later goes back to CONNECTION_AWAITING_RESPONSE to read
> server's response to the password just entered where it either
> receives authorization OK or error response thus completing the
> connection start-up process.
>
> The backend waits for the password until authentication timeout
> happens in which case the client can not send the password anymore
> since the backend has exited due to authentication timeout. I wonder
> if this is one of the reasons why this has not already been
> implemented?
>
> Comments?

Hmmm. On current implement, server is not running while the client is
reading password so the authentication timeout is provided only for
hostile clients. Conversely, the new sequence can enforce true
authentication timeout. It results in error after leaving the password
prompt for 60 seconds. I suppose that more desirable behavior in spite of
the poor message..

| Password: <waiting over 60 seconds and ENTER RETURN>
| psql: fe_sendauth: error sending password authentication

The point at issue now seems how to inform the timeout to the client
under reading password, especially prohibiting using thread nor
SIGALRM.

Providing password input function in libpq like below might make it
viable using select(2).

PQsendPassword(prompt="Passowrd: ", in_fd = stdin)

Any thoughts?

regareds,

--
Kyotaro Horiguchi

On Wed, May 15, 2013 at 11:04 PM, Kyotaro HORIGUCHI
<kyota(dot)horiguchi(at)gmail(dot)com> wrote:
>> Is it right that it is only in the case a password prompt is needed
>> that a new connection is created after dropping the just-failed
>> connection?
>
> It's quite doubtful.\:-p The sequense seems fragile to say the
> least. Inserting password request state into the client-side state
> machine looks quite reasonable.

Looking at current code (well, pseudo-code!) :

do
{
new_pass = false;
<create a new connection>
if (CONNECTION_BAD && NEEDS_PASSWORD && password == NULL && ! FORCE_NO_PASSOWRD)
{
PQfinish(<current_connection>);
password = simple_prompt() ;
new_pass = true;
}
}while(new_pass)

So, it looks like the loop will be repeated only if an authentication
method requiring the user to enter password is encountered in the
PQconnectPoll() which are AUTH_REQ_MD5 & AUTH_REQ_PASSWORD. As you can
see in the following code fragment from pg_fe_sendauth() which
apparently sets conn->password_needed:

case AUTH_REQ_MD5:
case AUTH_REQ_PASSWORD:
conn->password_needed = true;
if (conn->pgpass == NULL || conn->pgpass[0] == '\0')
{
printfPQExpBuffer(&conn->errorMessage,
PQnoPasswordSupplied);
return STATUS_ERROR;
}
if (pg_password_sendauth(conn, conn->pgpass, areq) != STATUS_OK)
{
printfPQExpBuffer(&conn->errorMessage,
"fe_sendauth: error sending password authentication\n");
return STATUS_ERROR;
}
break;

this seems to be the only code path that causes conn->password_needed
to be set to true. So, these seem to be only cases when a prompt will
be provided and new_pass would become true causing the
drop-and-reconnect by repetition of the loop. Am I missing some other
case when this might happen?

>> I created a patch which enables it to use the existing connection in
>> such a case (unlike what we currently do). It modifies
>> connectDBComplete() and PQconnectPoll() to also include states
>> pertaining to password being accepted from the user. That is, the
>> state machine in PQconnectPoll() is further extended to include a
>> connection state called CONNECTION_ASKING_PASSWORD which is entered
>> when server sends AUTH_REQ_MD5 or AUTH_REQ_PASSWORD auth requests.
>
> Great! The new client state seems to be effective also for MD5. But
> it seems to break existing libpq client doing the same authentication
> sequence as current psql. Some means would be necessary to switch the
> behavior when password is not previously provided but needed by the
> server, or make the first half of the connection sequence to be
> compatible to the current sequence - in other words - It should be
> that when the user finds stauts is CONNECTION_BAD and
> PQconnectionNeedsPassword() == true, the user can throw away the
> connection and make new connection providing password, and also can
> send password on existing connection.

The first half of connection sequence remains same except for one
change: in PQconnectPoll(), when in case CONNECTION_AWAITING_RESPONSE,
if server sends md5/password authentication request, it returns
PGRES_POLLING_WAITING_PASSWORD, which, back in connectDBComplete()
sets conn->password = true and conn->status =
CONNECTION_ASKING_PASSWORD. Back in main(), this causes a password
prompt and then the second half of the connection sequence. Hence
pg_fe_sendauth() is not called in this first half unless it's a
different authentication method than md5 and password.

>
> the old style
>
> | db = PQconnectXXXX();
> | if (PQstatus(db) == CONNECTION_BAD && PQconnectionNeedsPassword(db))
> | {
> | PQfinish(db);
> | value[..] = password = <some means to read password>;
> | db = PQconnectXXXX();
> | if (PQstatus(db) == CONNECTION_BAD)
> | <error>
>
> and the new style
>
> | db = PQconnectXXXX();
> | if (PQconnectionNeedsPassword(db))
> | PQsendPassword(db, password);
> | if (PQstatus(db) == CONNECTION_BAD)
> | <error>
>
> should be standing together.

I see this accounts for CONNECTION_BAD (if any) in the first half. But
this CONNECTION_BAD has other reasons than conn->password_needed as
far as I can imagine since conn->password_needed would only be set in
connectDBComplete() in PGRES_POLLING_WAITING_PASSWORD. So, this
CONNECTION_BAD would require some different processing. Thoughts?

> Where, PQsendPassword is combined function of PQcopyPassword and
> PQcontinuedbConnect. If the only porpose of these functions is sending
> password then these functions are needed to be separately.
>
> What do you think for the compatibility and simpler API.

I think one function PQsendPassword(PGconn*, char *) would be
sufficient which would contain the code of both PQcopyPassword() and
PQcontinuedbConnect(). I would complete the connection sequence by
running its second half.

>> The backend waits for the password until authentication timeout
>> happens in which case the client can not send the password anymore
>> since the backend has exited due to authentication timeout. I wonder
>> if this is one of the reasons why this has not already been
>> implemented?
>>
>> Comments?
>
> Hmmm. On current implement, server is not running while the client is
> reading password so the authentication timeout is provided only for
> hostile clients. Conversely, the new sequence can enforce true
> authentication timeout. It results in error after leaving the password
> prompt for 60 seconds. I suppose that more desirable behavior in spite of
> the poor message..
>
> | Password: <waiting over 60 seconds and ENTER RETURN>
> | psql: fe_sendauth: error sending password authentication
>
> The point at issue now seems how to inform the timeout to the client
> under reading password, especially prohibiting using thread nor
> SIGALRM.
>
> Providing password input function in libpq like below might make it
> viable using select(2).
>
> PQsendPassword(prompt="Passowrd: ", in_fd = stdin)
>
> Any thoughts?

So, do you here propose to change simple_prompt() that would be able
to detect an authentication timeout on server and exit with an
appropriate message? I think that should be done.

I will try to revise the patch to incorporate these considerations and
post a revised patch.

--
Amit Langote

>>> I created a patch which enables it to use the existing connection in
>>> such a case (unlike what we currently do). It modifies
>>> connectDBComplete() and PQconnectPoll() to also include states
>>> pertaining to password being accepted from the user. That is, the
>>> state machine in PQconnectPoll() is further extended to include a
>>> connection state called CONNECTION_ASKING_PASSWORD which is entered
>>> when server sends AUTH_REQ_MD5 or AUTH_REQ_PASSWORD auth requests.
>>
>> Great! The new client state seems to be effective also for MD5. But
>> it seems to break existing libpq client doing the same authentication
>> sequence as current psql. Some means would be necessary to switch the
>> behavior when password is not previously provided but needed by the
>> server, or make the first half of the connection sequence to be
>> compatible to the current sequence - in other words - It should be
>> that when the user finds stauts is CONNECTION_BAD and
>> PQconnectionNeedsPassword() == true, the user can throw away the
>> connection and make new connection providing password, and also can
>> send password on existing connection.
>
> The first half of connection sequence remains same except for one
> change: in PQconnectPoll(), when in case CONNECTION_AWAITING_RESPONSE,
> if server sends md5/password authentication request, it returns
> PGRES_POLLING_WAITING_PASSWORD, which, back in connectDBComplete()
> sets conn->password = true and conn->status =
> CONNECTION_ASKING_PASSWORD. Back in main(), this causes a password
> prompt and then the second half of the connection sequence. Hence
> pg_fe_sendauth() is not called in this first half unless it's a
> different authentication method than md5 and password.

One more thing that I forgot to mention is that connection sequence
would enter CONNECTION_ASKING_PASSWORD in the first half, only if
password is currently not set to a non-empty value that is (
conn->pgpass ==NULL || conn->pgpass[0] = '\0' ) is true. I was
wondering what would be the case for other applications using libpq
when they return from connectionDBComplete() with conn->status set to
CONNECTION_ASKING_PASSWORD, provided they have not set conn->pgpass to
a non-empty value.If they are currently handling this based on
CONNECTION_BAD, then this change does no good to them. In fact there
needs to be a way for them to get CONNECTION_BAD.
Since, this whole patch is about not having to drop-and-reconnect *in
case of password prompt*, how it changes libpq for other applications
also needs to be addressed here. especially for md5/password
authentication cases. Currently, any attempt to connect using empty or
NULL password results in CONNECTION_BAD for all libpq based clients.
Thoughts?

Sorry that I am writing separate emails on the same topic.
I seem to have a solution that allows us to accomplish what we are
trying to without much change to the existing libpq interface
(especially what to expect about return values and connection state
that we are in when we return from connectDBComplete() and
PQconnectPoll() ).

Following are required changes roughly:

1] in src/bin/psql/startup.c, main()

if (PQstatus(pset.db) == CONNECTION_BAD &&
PQconnectionNeedsPassword(pset.db) &&
password == NULL &&
pset.getPassword != TRI_NO)
{
password = simple_prompt(password_prompt, 100, false);
/* How would this detect authentication_timeoue and exit
accordingly ?*/
PQsendPassword(pset.db, password);
}

And there is no do{...}while(new_pass); unlike current code.

2] in src/interfaces/libpq/fe-connect.c, new function: void
PQsendPassword(PGconn *conn, char *password) /*suggest better name?
*/

void PQsendPassword(PGconn *conn, char *password)
{
conn->pgpass = password;
conn->status = CONNECTION_SENDING_PASSWORD; /*suggest better
name for the status? */

(void) connectDBComplete(conn);

}

3] in src/interfaces/libpq/fe-connect.c, connectDBComplete(PGconn
*conn), No change required. :-)

4] in in src/interfaces/libpq/fe-connect.c, PQconnectPoll(PGconn *conn)

a) add a new case for both switch's (one before and after keep_going: )

/* These are writing states, so we just proceed. */
case CONNECTION_STARTED:
case CONNECTION_MADE:
case CONNECTION_SENDING_PASSWORD:
break;
...
...
keep_going:
...
...
case CONNECTION_SENDING_PASSWORD:
{
/*
** Note that conn->pghost must be
non-NULL if we are going to
** avoid the Kerberos code doing a
hostname look-up.
**/
if (pg_fe_sendauth(areq, conn) != STATUS_OK)
{
conn->errorMessage.len =
strlen(conn->errorMessage.data);
goto error_return;
}
conn->errorMessage.len =
strlen(conn->errorMessage.data);

/*
** Just make sure that any data sent
by pg_fe_sendauth is
** flushed out. Although this
theoretically could block, it
** really shouldn't since we don't
send large auth responses.
**/
if (pqFlush(conn))
goto error_return;

/*
* Now go to read the server's
response to password just sent
* */
conn->status = CONNECTION_AWAITING_RESPONSE;
return PGRES_POLLING_READING;
}

5] in src/interfaces/libpq/libpq-fe.h, add a new intermediate connection state

/*
* Although it is okay to add to these lists, values which become unused
* should never be removed, nor should constants be redefined - that would
* break compatibility with existing code.
*/

typedef enum
{
CONNECTION_OK,
CONNECTION_BAD,
/* Non-blocking mode only below here */

/*
* The existence of these should never be relied upon - they should only
* be used for user feedback or similar purposes.
*/
CONNECTION_STARTED, /* Waiting for
connection to be made. */
CONNECTION_MADE, /* Connection OK;
waiting to send. */
CONNECTION_AWAITING_RESPONSE, /* Waiting for a
response from the

* postmaster. */
CONNECTION_AUTH_OK, /* Received
authentication; waiting for
*
backend startup. */
CONNECTION_SETENV, /* Negotiating environment. */
CONNECTION_SSL_STARTUP, /* Negotiating SSL. */
CONNECTION_NEEDED, /* Internal state:
connect() needed */
CONNECTION_SENDING_PASSWORD
} ConnStatusType;

As you can probably see this requires minimum libpq changes:

1] Add one more connection state: CONNECTION_SENDING_PASSWORD
2] Add one more function: PQsendPassword(PGconn*, char*)
3] Modify PQconnectPoll() to allow to handle an intermediate
CONNECTION_SENDING_PASSWORD state for the clients which use
PQsendPassword() to send a password that user entered in between a
connection sequence over an existing connection.

Comments?

Attached herewith is a patch based on description in my previous mail.
This patch would need revision since the error situation in case of
authentication timeout on the server needs to be handled; probably in
simple_prompt()?

--
Amit Langote

On Thu, May 16, 2013 at 3:53 PM, Amit Langote <amitlangote09(at)gmail(dot)com> wrote:
> Attached herewith is a patch based on description in my previous mail.
> This patch would need revision since the error situation in case of
> authentication timeout on the server needs to be handled; probably in
> simple_prompt()?

Forgot attaching the patch in the last mail; find it with this one.

--
Amit Langote

On 2013-05-16 17:35:10 +0900, Amit Langote wrote:
> On Thu, May 16, 2013 at 3:53 PM, Amit Langote <amitlangote09(at)gmail(dot)com> wrote:
> > Attached herewith is a patch based on description in my previous mail.
> > This patch would need revision since the error situation in case of
> > authentication timeout on the server needs to be handled; probably in
> > simple_prompt()?
>
> Forgot attaching the patch in the last mail; find it with this one.

The patch seems to have windows line endings...

> --- a/src/interfaces/libpq/libpq-fe.h
> +++ b/src/interfaces/libpq/libpq-fe.h
> @@ -62,7 +62,11 @@ typedef enum
> * backend startup. */
> CONNECTION_SETENV, /* Negotiating environment. */
> CONNECTION_SSL_STARTUP, /* Negotiating SSL. */
> - CONNECTION_NEEDED /* Internal state: connect() needed */
> + CONNECTION_NEEDED, /* Internal state: connect() needed */
> + CONNECTION_SENDING_PASSWORD /* An intermediate state to help client send a password
> + * over an existing connection
> + */
> +
> } ConnStatusType;
>
> typedef enum
> @@ -258,6 +262,9 @@ extern PGconn *PQsetdbLogin(const char *pghost, const char *pgport,
> #define PQsetdb(M_PGHOST,M_PGPORT,M_PGOPT,M_PGTTY,M_DBNAME) \
> PQsetdbLogin(M_PGHOST, M_PGPORT, M_PGOPT, M_PGTTY, M_DBNAME, NULL, NULL)
>
> +/* send a password that the server asked for halfway between a connection sequence */
> +extern void PQsendPassword(PGconn *conn, char *password);
> +

I unfortunately have to say I don't really see the point of this. The
cost of the additional connection attempt is rather low and we have to
deal with the superflous attempts anyway since there will be old libpqs
around for years. Why is this worth the effort?

Greetings,

Andres Freund

--
Andres Freund http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

On Thu, May 16, 2013 at 8:01 PM, Andres Freund <andres(at)2ndquadrant(dot)com> wrote:
> On 2013-05-16 17:35:10 +0900, Amit Langote wrote:
>> On Thu, May 16, 2013 at 3:53 PM, Amit Langote <amitlangote09(at)gmail(dot)com> wrote:
>> > Attached herewith is a patch based on description in my previous mail.
>> > This patch would need revision since the error situation in case of
>> > authentication timeout on the server needs to be handled; probably in
>> > simple_prompt()?
>>
>> Forgot attaching the patch in the last mail; find it with this one.
>
> The patch seems to have windows line endings...

My bad. I will reupload the proper patch later.

>> --- a/src/interfaces/libpq/libpq-fe.h
>> +++ b/src/interfaces/libpq/libpq-fe.h
>> @@ -62,7 +62,11 @@ typedef enum
>> * backend startup. */
>> CONNECTION_SETENV, /* Negotiating environment. */
>> CONNECTION_SSL_STARTUP, /* Negotiating SSL. */
>> - CONNECTION_NEEDED /* Internal state: connect() needed */
>> + CONNECTION_NEEDED, /* Internal state: connect() needed */
>> + CONNECTION_SENDING_PASSWORD /* An intermediate state to help client send a password
>> + * over an existing connection
>> + */
>> +
>> } ConnStatusType;
>>
>> typedef enum
>> @@ -258,6 +262,9 @@ extern PGconn *PQsetdbLogin(const char *pghost, const char *pgport,
>> #define PQsetdb(M_PGHOST,M_PGPORT,M_PGOPT,M_PGTTY,M_DBNAME) \
>> PQsetdbLogin(M_PGHOST, M_PGPORT, M_PGOPT, M_PGTTY, M_DBNAME, NULL, NULL)
>>
>> +/* send a password that the server asked for halfway between a connection sequence */
>> +extern void PQsendPassword(PGconn *conn, char *password);
>> +
>
> I unfortunately have to say I don't really see the point of this. The
> cost of the additional connection attempt is rather low and we have to
> deal with the superflous attempts anyway since there will be old libpqs
> around for years. Why is this worth the effort?

While full connection sequence (with proper authentication exchanges)
appears to go smoothly for other cases (authentication methods), it
doesn't quite in this case probably because accounting for such a case
was not considered to be as important. But while investigating about
the PAM issue (original subject of this thread), it turned out that
the occurrence of that minor issue was due to this behavior in libpq.
Addition of this one more state (viz. input password in between an
ongoing connect sequence) to the possible connection states helps
account for such instances where this kind of password exchange has to
happen (as in psql for md5 and password). Also, others using libpq can
either use it if they wish to or just do away without having to worry
about this state. This patch does not introduce any change as to what
connection state applications can expect to be in after they return
from connectDBComplete() or PQconnectPoll(). On the other hand, we can
now enter these functions with one more possible connection state
which PQconnectPoll() is now able to handle. As a side effect, it also
helps avoid drop-and-reconnect occurrences at times.

Albeit, it is up to application (using libpq) whether to go via this
new alternate path or stick to drop-and-reconnect, should a need to
input password in between connect sequence arise. We can consider
having such an option, probably just for the sake of completeness
(even if to account for a possibly rare method of authentication
exchange)

--
Amit Langote

Amit Langote <amitlangote09(at)gmail(dot)com> writes:
> On Thu, May 16, 2013 at 8:01 PM, Andres Freund <andres(at)2ndquadrant(dot)com> wrote:
>> I unfortunately have to say I don't really see the point of this. The
>> cost of the additional connection attempt is rather low and we have to
>> deal with the superflous attempts anyway since there will be old libpqs
>> around for years. Why is this worth the effort?

> While full connection sequence (with proper authentication exchanges)
> appears to go smoothly for other cases (authentication methods), it
> doesn't quite in this case probably because accounting for such a case
> was not considered to be as important. But while investigating about
> the PAM issue (original subject of this thread), it turned out that
> the occurrence of that minor issue was due to this behavior in libpq.

I have to agree with Andres that it's not clear this is a reasonable
fix. To get rid of extra reconnections this way will require not merely
upgrading libpq, but upgrading every single application that uses libpq
and is capable of prompting its user for a password. The odds are
pretty good that that won't ever happen.

The real complaint here is that the server-side PAM auth code path is
losing the information that the client chose to disconnect rather than
offer a password, and thus logging a message that we could do without.
What's wrong with just fixing that?

regards, tom lane

On Fri, May 17, 2013 at 1:05 AM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> Amit Langote <amitlangote09(at)gmail(dot)com> writes:
>> On Thu, May 16, 2013 at 8:01 PM, Andres Freund <andres(at)2ndquadrant(dot)com> wrote:
>>> I unfortunately have to say I don't really see the point of this. The
>>> cost of the additional connection attempt is rather low and we have to
>>> deal with the superflous attempts anyway since there will be old libpqs
>>> around for years. Why is this worth the effort?
>
>> While full connection sequence (with proper authentication exchanges)
>> appears to go smoothly for other cases (authentication methods), it
>> doesn't quite in this case probably because accounting for such a case
>> was not considered to be as important. But while investigating about
>> the PAM issue (original subject of this thread), it turned out that
>> the occurrence of that minor issue was due to this behavior in libpq.
>
> I have to agree with Andres that it's not clear this is a reasonable
> fix. To get rid of extra reconnections this way will require not merely
> upgrading libpq, but upgrading every single application that uses libpq
> and is capable of prompting its user for a password. The odds are
> pretty good that that won't ever happen.

Can this stay in the future releases for new users of libpq to
consider using it (saving them a reconnection, however small a benefit
that is) or at least psql which is being changed to use it anyway? I
only think it makes libpq take into account a connection state that
could be used.

> The real complaint here is that the server-side PAM auth code path is
> losing the information that the client chose to disconnect rather than
> offer a password, and thus logging a message that we could do without.
> What's wrong with just fixing that?

Back in this thread, Horiguchi-san has posted a fix. It seems to fix
the original issue. Attaching his patch here again.

--
Amit Langote

On 2013-05-17 01:29:25 +0900, Amit Langote wrote:
> On Fri, May 17, 2013 at 1:05 AM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> > Amit Langote <amitlangote09(at)gmail(dot)com> writes:
> >> On Thu, May 16, 2013 at 8:01 PM, Andres Freund <andres(at)2ndquadrant(dot)com> wrote:
> >>> I unfortunately have to say I don't really see the point of this. The
> >>> cost of the additional connection attempt is rather low and we have to
> >>> deal with the superflous attempts anyway since there will be old libpqs
> >>> around for years. Why is this worth the effort?
> >
> >> While full connection sequence (with proper authentication exchanges)
> >> appears to go smoothly for other cases (authentication methods), it
> >> doesn't quite in this case probably because accounting for such a case
> >> was not considered to be as important. But while investigating about
> >> the PAM issue (original subject of this thread), it turned out that
> >> the occurrence of that minor issue was due to this behavior in libpq.
> >
> > I have to agree with Andres that it's not clear this is a reasonable
> > fix. To get rid of extra reconnections this way will require not merely
> > upgrading libpq, but upgrading every single application that uses libpq
> > and is capable of prompting its user for a password. The odds are
> > pretty good that that won't ever happen.
>
> Can this stay in the future releases for new users of libpq to
> consider using it (saving them a reconnection, however small a benefit
> that is) or at least psql which is being changed to use it anyway? I
> only think it makes libpq take into account a connection state that
> could be used.

Which basically is an API & ABI break since its not handled in existing
callers. So you would need to make it conditional. At that point the
complexity really doesn't seem warranted.

Greetings,

Andres Freund

--
Andres Freund http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

Andres Freund <andres(at)2ndquadrant(dot)com> writes:
> On 2013-05-17 01:29:25 +0900, Amit Langote wrote:
>> Can this stay in the future releases for new users of libpq to
>> consider using it (saving them a reconnection, however small a benefit
>> that is) or at least psql which is being changed to use it anyway? I
>> only think it makes libpq take into account a connection state that
>> could be used.

> Which basically is an API & ABI break since its not handled in existing
> callers. So you would need to make it conditional.

Yeah, there would need to be a way for the caller to indicate that it's
prepared to handle this new connection state; else you risk actively
breaking existing code that doesn't know it needs to do something here.

Another point worth considering is that, if you assume that what's going
to happen is manual entry of a password (probably requiring at least a
couple of seconds), the actual benefit of avoiding a second fork() is
really completely negligible. It could even be argued that the benefit
is negative, since we're tying up a postmaster child process slot that
might be better used for something else.

So, while I wouldn't have objected to this if it'd been included in the
original design for PQconnectPoll-style connections, it's really unclear
that it's worth the work to add it now.

regards, tom lane

On Fri, May 17, 2013 at 1:46 AM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> Andres Freund <andres(at)2ndquadrant(dot)com> writes:
>> On 2013-05-17 01:29:25 +0900, Amit Langote wrote:
>>> Can this stay in the future releases for new users of libpq to
>>> consider using it (saving them a reconnection, however small a benefit
>>> that is) or at least psql which is being changed to use it anyway? I
>>> only think it makes libpq take into account a connection state that
>>> could be used.
>
>> Which basically is an API & ABI break since its not handled in existing
>> callers. So you would need to make it conditional.
>
> Yeah, there would need to be a way for the caller to indicate that it's
> prepared to handle this new connection state; else you risk actively
> breaking existing code that doesn't know it needs to do something here.
>
> Another point worth considering is that, if you assume that what's going
> to happen is manual entry of a password (probably requiring at least a
> couple of seconds), the actual benefit of avoiding a second fork() is
> really completely negligible. It could even be argued that the benefit
> is negative, since we're tying up a postmaster child process slot that
> might be better used for something else.

I agree it's a pretty valid point. We'd better just fix the original
issue and leave it to that. :)

--
Amit Langote

On Thu, May 16, 2013 at 7:01 AM, Andres Freund <andres(at)2ndquadrant(dot)com> wrote:
> I unfortunately have to say I don't really see the point of this. The
> cost of the additional connection attempt is rather low and we have to
> deal with the superflous attempts anyway since there will be old libpqs
> around for years. Why is this worth the effort?

I have always considered this a wart, and I think we've had customers
complain about these kinds of things, too. So +1 from me for fixing
it. If not everyone updates their client to take advantage of the new
APIs, so be it. If psql and pgAdmin do, it'll solve 90% of the
problem.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

Hello,

The attached patch by Kyotaro Horiguchi-san fixes a PAM authentication
error logging issue which is that when using PAM authentication,
connection attempts by clients (like psql) result in an unnecessary
logging of failed authentication.

---------- Forwarded message ----------
From: Amit Langote <amitlangote09(at)gmail(dot)com>
Date: Fri, May 17, 2013 at 1:29 AM
Subject: Re: [HACKERS] Logging of PAM Authentication Failure
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: Andres Freund <andres(at)2ndquadrant(dot)com>, Kyotaro HORIGUCHI
<kyota(dot)horiguchi(at)gmail(dot)com>, Kyotaro HORIGUCHI
<horiguchi(dot)kyotaro(at)lab(dot)ntt(dot)co(dot)jp>, Robert Haas
<robertmhaas(at)gmail(dot)com>, PostgreSQL-development
<pgsql-hackers(at)postgresql(dot)org>

On Fri, May 17, 2013 at 1:05 AM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> Amit Langote <amitlangote09(at)gmail(dot)com> writes:
>> On Thu, May 16, 2013 at 8:01 PM, Andres Freund <andres(at)2ndquadrant(dot)com> wrote:
>>> I unfortunately have to say I don't really see the point of this. The
>>> cost of the additional connection attempt is rather low and we have to
>>> deal with the superflous attempts anyway since there will be old libpqs
>>> around for years. Why is this worth the effort?
>
>> While full connection sequence (with proper authentication exchanges)
>> appears to go smoothly for other cases (authentication methods), it
>> doesn't quite in this case probably because accounting for such a case
>> was not considered to be as important. But while investigating about
>> the PAM issue (original subject of this thread), it turned out that
>> the occurrence of that minor issue was due to this behavior in libpq.
>
> I have to agree with Andres that it's not clear this is a reasonable
> fix. To get rid of extra reconnections this way will require not merely
> upgrading libpq, but upgrading every single application that uses libpq
> and is capable of prompting its user for a password. The odds are
> pretty good that that won't ever happen.

Can this stay in the future releases for new users of libpq to
consider using it (saving them a reconnection, however small a benefit
that is) or at least psql which is being changed to use it anyway? I
only think it makes libpq take into account a connection state that
could be used.

> The real complaint here is that the server-side PAM auth code path is
> losing the information that the client chose to disconnect rather than
> offer a password, and thus logging a message that we could do without.
> What's wrong with just fixing that?

Back in this thread, Horiguchi-san has posted a fix. It seems to fix
the original issue. Attaching his patch here again.

--
Amit Langote

--
Amit Langote

On 05/11/2013 03:25 AM, Robert Haas wrote:
> Not really. We could potentially fix it by extending the wire
> protocol to allow the server to respond to the client's startup packet
> with a further challenge, and extend libpq to report that challenge
> back to the user and allow sending a response. But that would break
> on-the-wire compatibility, which we haven't done in a good 10 years,
> and certainly wouldn't be worthwhile just for this.
We were just talking about "things we'd like to do in wire protocol 4".

Allowing multi-stage authentication has come up repeatedly and should
perhaps go on that list. The most obvious case being "ident auth failed,
demand md5".

--
Craig Ringer http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

On 5/27/13, Craig Ringer <craig(at)2ndquadrant(dot)com> wrote:

> We were just talking about "things we'd like to do in wire protocol 4".
>
> Allowing multi-stage authentication has come up repeatedly and should
> perhaps go on that list. The most obvious case being "ident auth failed,
> demand md5".

I'd like to use LDAP with pg_ident

Cheers,

Jeff

On Tue, May 28, 2013 at 01:32:53PM +0800, Craig Ringer wrote:
> On 05/11/2013 03:25 AM, Robert Haas wrote:
> > Not really. We could potentially fix it by extending the wire
> > protocol to allow the server to respond to the client's startup packet
> > with a further challenge, and extend libpq to report that challenge
> > back to the user and allow sending a response. But that would break
> > on-the-wire compatibility, which we haven't done in a good 10 years,
> > and certainly wouldn't be worthwhile just for this.
> We were just talking about "things we'd like to do in wire protocol 4".
>
> Allowing multi-stage authentication has come up repeatedly and should
> perhaps go on that list. The most obvious case being "ident auth failed,
> demand md5".

+1

The configuration would need to be thought though, as no fixed
ordering could cover all cases.

Maybe lines like

local all postgres peer,md5

in pg_hba.conf would be the way to do this, where the list gets
evaluated in the order it's read.

Cheers,
David.
--
David Fetter <david(at)fetter(dot)org> http://fetter.org/
Phone: +1 415 235 3778 AIM: dfetter666 Yahoo!: dfetter
Skype: davidfetter XMPP: david(dot)fetter(at)gmail(dot)com
iCal: webcal://www.tripit.com/feed/ical/people/david74/tripit.ics

Remember to vote!
Consider donating to Postgres: http://www.postgresql.org/about/donate

On Tue, May 28, 2013 at 2:32 PM, Craig Ringer <craig(at)2ndquadrant(dot)com> wrote:
> On 05/11/2013 03:25 AM, Robert Haas wrote:
>> Not really. We could potentially fix it by extending the wire
>> protocol to allow the server to respond to the client's startup packet
>> with a further challenge, and extend libpq to report that challenge
>> back to the user and allow sending a response. But that would break
>> on-the-wire compatibility, which we haven't done in a good 10 years,
>> and certainly wouldn't be worthwhile just for this.
> We were just talking about "things we'd like to do in wire protocol 4".
>
> Allowing multi-stage authentication has come up repeatedly and should
> perhaps go on that list. The most obvious case being "ident auth failed,
> demand md5".
>

I wonder what you think about continuing to use the already
established connection to the server while you move onto perform
authentication using next method in the list. Earlier in this thread,
I had proposed to make changes to PGconnectPoll() to introduce an
additional connection state which is kind of an intermediate state in
the authentication sequence. For example, server might ask for a
password (md5, password methods) and client might want to send the
password over the existing connection by leveraging this new
connection state. This is unlike what we do, for example, in psql,
where we drop the connection (upon CONNECTION_BAD due to password
required), get password using a prompt and then create a new
connection with password included in the request.

--
Amit Langote

On Tue, May 28, 2013 at 5:04 PM, Amit Langote <amitlangote09(at)gmail(dot)com> wrote:
> On Tue, May 28, 2013 at 2:32 PM, Craig Ringer <craig(at)2ndquadrant(dot)com> wrote:
>> On 05/11/2013 03:25 AM, Robert Haas wrote:
>>> Not really. We could potentially fix it by extending the wire
>>> protocol to allow the server to respond to the client's startup packet
>>> with a further challenge, and extend libpq to report that challenge
>>> back to the user and allow sending a response. But that would break
>>> on-the-wire compatibility, which we haven't done in a good 10 years,
>>> and certainly wouldn't be worthwhile just for this.
>> We were just talking about "things we'd like to do in wire protocol 4".
>>
>> Allowing multi-stage authentication has come up repeatedly and should
>> perhaps go on that list. The most obvious case being "ident auth failed,
>> demand md5".
>>
>
> I wonder what you think about continuing to use the already
> established connection to the server while you move onto perform
> authentication using next method in the list. Earlier in this thread,
> I had proposed to make changes to PGconnectPoll() to introduce an
> additional connection state which is kind of an intermediate state in
> the authentication sequence. For example, server might ask for a
> password (md5, password methods) and client might want to send the
> password over the existing connection by leveraging this new
> connection state. This is unlike what we do, for example, in psql,
> where we drop the connection (upon CONNECTION_BAD due to password
> required), get password using a prompt and then create a new
> connection with password included in the request.
>
> --
> Amit Langote

Sorry, *PQconnectPoll()

--
Amit Langote

On Tue, May 28, 2013 at 01:32:53PM +0800, Craig Ringer wrote:
> On 05/11/2013 03:25 AM, Robert Haas wrote:
> > Not really. We could potentially fix it by extending the wire
> > protocol to allow the server to respond to the client's startup packet
> > with a further challenge, and extend libpq to report that challenge
> > back to the user and allow sending a response. But that would break
> > on-the-wire compatibility, which we haven't done in a good 10 years,
> > and certainly wouldn't be worthwhile just for this.
> We were just talking about "things we'd like to do in wire protocol 4".
>
> Allowing multi-stage authentication has come up repeatedly and should
> perhaps go on that list. The most obvious case being "ident auth failed,
> demand md5".

Added to TODO.

--
Bruce Momjian <bruce(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ It's impossible for everything to be true. +

On 05/28/2013 04:04 PM, Amit Langote wrote:
> On Tue, May 28, 2013 at 2:32 PM, Craig Ringer <craig(at)2ndquadrant(dot)com> wrote:
>> On 05/11/2013 03:25 AM, Robert Haas wrote:
>>> Not really. We could potentially fix it by extending the wire
>>> protocol to allow the server to respond to the client's startup packet
>>> with a further challenge, and extend libpq to report that challenge
>>> back to the user and allow sending a response. But that would break
>>> on-the-wire compatibility, which we haven't done in a good 10 years,
>>> and certainly wouldn't be worthwhile just for this.
>> We were just talking about "things we'd like to do in wire protocol 4".
>>
>> Allowing multi-stage authentication has come up repeatedly and should
>> perhaps go on that list. The most obvious case being "ident auth failed,
>> demand md5".
>>
> I wonder what you think about continuing to use the already
> established connection to the server while you move onto perform
> authentication using next method in the list.
That's precisely what I'm talking about. It'd be nice to avoid the ugly
two-connection approach for SSL too, by allowing STARTTLS or similar
within the protocol.

Being able to negotiate connections - client says "peer?", server says
"failed, peer id doesn't match postgresql username", client says "md5
<password>?" server says "yup, that's ok" - would be nice. For example,
use Kerberos or SSPI where clients are suitably enabled, then fall back
to MD5 where Kerberos or SSPI single-sign-on isn't available.

--
Craig Ringer http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services