Re: Add "password_protocol" connection parameter to libpq

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