Re: Add "password_protocol" connection parameter to libpq

On Tue, Aug 20, 2019 at 07:09:25PM -0700, Jeff Davis wrote:
> OK, new patch attached. Seems like everyone is in agreement that we
> need a channel_binding param.

+      <para>
+        A setting of <literal>require</literal> means that the connection must
+        employ channel binding; and that the client will not respond to
+        requests from the server for cleartext password or MD5
+        authentication. The default setting is <literal>prefer</literal>,
+        which employs channel binding if available.
+      </para>
This counts for other auth requests as well like krb5, no?  I think
that we should also add the "disable" flavor for symmetry, and
also...

+#define DefaultChannelBinding  "prefer"
If the build of Postgres does not support SSL (USE_SSL is not
defined), I think that DefaultChannelBinding should be "disable".
That would make things consistent with sslmode.

+        with <productname>PostgreSQL</productname> 11.0 or later servers using
Here I would use PostgreSQL 11, only mentioning the major version as
it was also available at beta time.

        case AUTH_REQ_OK:
+           if (conn->channel_binding[0] == 'r' && !conn->channel_bound)
+           {
+               printfPQExpBuffer(&conn->errorMessage,
+                                 libpq_gettext("Channel binding required but not offered by server\n"));
+               return STATUS_ERROR;
+           }
Doing the filtering at the time of AUTH_REQ_OK is necessary for
"trust", but shouldn't this be done as well earlier for other request
types?  This could save round-trips to the server if we know that an
exchange begins with a protocol which will never satisfy this
request, saving efforts for a client doomed to fail after the first
AUTH_REQ received.  That would be the case for all AUTH_REQ, except
the SASL ones and of course AUTH_REQ_OK.

Could you please add negative tests in src/test/authentication/?  What
could be covered there is that the case where "prefer" (and
"disable"?) is defined then the authentication is able to go through,
and that with "require" we get a proper failure as SSL is not used.
Tests in src/test/ssl/ could include:
- Make sure that "require" works properly.
- Test after "disable".

+               if (conn->channel_binding[0] == 'r')
Maybe this should comment that this means "require", in a fashion
similar to what is done when checking conn->sslmode.
--
Michael