Re: Add "password_protocol" connection parameter to libpq - Mailing list pgsql-hackers

From Michael Paquier
Subject Re: Add "password_protocol" connection parameter to libpq
Date
Msg-id 20190906070501.GF1608@paquier.xyz
Whole thread Raw
In response to Re: Add "password_protocol" connection parameter to libpq  (Jeff Davis <pgsql@j-davis.com>)
Responses Re: Add "password_protocol" connection parameter to libpq
List pgsql-hackers
On Wed, Sep 04, 2019 at 09:22:33PM -0700, Jeff Davis wrote:
> New patch attached.

Thanks for the new version, Jeff.

+        with <productname>PostgreSQL</productname> 11 or later servers using
+        the <literal>scram-sha-256</literal> authentication method.

Nit here: "scram-sha-256" refers to the HBA entry.  I would
just use "SCRAM" instead.

In pg_SASL_init(), if the server sends SCRAM-SHA-256-PLUS as SASL
mechanism over a non-SSL connection, should we complain even if
the "disable" mode is used?  It seems to me that there is a point to
complain in this case as a sanity check as the server should really
publicize "SCRAM-SHA-256-PLUS" only over SSL.

When the server only sends SCRAM-SHA-256 in the mechanism list and
"require" mode is used, we complain about "none of the server's SASL
authentication mechanisms are supported" which can be confusing.  Why
not generating a custom error if libpq selects SCRAM-SHA-256 when
"require" is used, say:
"SASL authentication mechanism SCRAM-SHA-256 selected but channel
binding is required"
That could be done by adding an error message when selecting
SCRAM-SHA-256 and then goto the error step.

Actually, it looks that the handling of channel_bound is incorrect.
If the server sends AUTH_REQ_SASL and libpq processes it, then the
flag gets already set.  Now let's imagine that the server is a rogue
one and sends AUTH_REQ_OK just after AUTH_REQ_SASL, then your check
would pass.  It seems to me that we should switch the flag once we are
sure that the exchange is completed, aka with AUTH_REQ_SASL_FIN when
the final message is done within pg_SASL_continue().

+# SSL not in use; channel binding still can't work
+reset_pg_hba($node, 'scram-sha-256');
+$ENV{"PGCHANNELBINDING"} = 'require';
+test_login($node, 'saslpreptest4a_role', "a", 2);
Worth testing md5 here?

PGCHANNELBINDING needs documentation.
--
Michael

Attachment

pgsql-hackers by date:

Previous
From: "Smith, Peter"
Date:
Subject: RE: [Proposal] Table-level Transparent Data Encryption (TDE) andKey Management Service (KMS)
Next
From: Michael Paquier
Date:
Subject: Re: [PATCH] vacuumlo: print the number of large objects going to beremoved