Re: Logging of PAM Authentication Failure

On Thu, May 16, 2013 at 8:01 PM, Andres Freund <andres@2ndquadrant.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@gmail.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