Re: [JDBC] [HACKERS] Channel binding support for SCRAM-SHA-256

From: Michael Paquier <michael(dot)paquier(at)gmail(dot)com>
To: Peter Eisentraut <peter(dot)eisentraut(at)2ndquadrant(dot)com>
Cc: Álvaro Hernández Tortosa <aht(at)8kdata(dot)com>, "pgsql-hackers(at)postgresql(dot)org" <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: [JDBC] [HACKERS] Channel binding support for SCRAM-SHA-256
Date: 2017-11-21 04:36:08
Message-ID: CAB7nPqQZZeyRjGGR1oK+hdFnoOWZ6VdSeCteqXvRAUnDe1uEXg@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers pgsql-jdbc

On Sun, Nov 19, 2017 at 8:13 AM, Michael Paquier
<michael(dot)paquier(at)gmail(dot)com> wrote:
> On Sun, Nov 19, 2017 at 12:56 AM, Peter Eisentraut
> <peter(dot)eisentraut(at)2ndquadrant(dot)com> wrote:
>> I'll be off for a week, so perhaps by that time you could make a rebased
>> version of the rest? I'm not sure how much more time I'll have, so
>> maybe it will end up being moved to the next CF.
>
> OK, let's see then. That's not an issue for me if this gets bumped.

I have been considering the set of connection parameters introduced
previously, and I am thinking that the one to enforce the SASL
mechanism name shold be dropped. It becomes useful if willing to test
the code path where an incorrect mechanism name is used, or to enforce
the client to not ask for channel binding. Still I think that we
should keep the code simple and always assume that channel binding is
supported if the SSL implementation allows it to not give users the
option to use something less secure. Perhaps this could be replaced
with a on/off switch to disable channel binding, but I am not much
into the idea either. This also keeps libpq slightly cleaner.

So attached are rebased patches:
- 0001 to introduce the connection parameter saslchannelbinding, which
allows libpq to enforce the type of channel binding used during an
exchange.
- 0002 to add tls-endpoint as channel binding type, which is where 0001 shines.

I have created a different thread with a patch about the bug of v10
servers not able to accept "y,," as valid input for channel binding as
this needs its own discussion:
https://www.postgresql.org/message-id/CAB7nPqSFcNsuQcWcqhX8QSz0R8oKz8ZM4Yw4ky=cfO9rpVdTUA@mail.gmail.com

Peter, also in CheckSCRAMAuth() the server publishes
SCRAM-SHA-256-PLUS is SSL is being used. However, seeing recent
patches to add more SSL implementations (gnuTLS, macos and windows
channel), it seems to me that the choice of publishing
SCRAM-SHA-256-PLUS should be done depending on if there is any binding
data available instead, either TLS finish message data or certificate
hash. On the client-side, it seems also to me that the "y" flag should
not be sent if there is no binding data available because the client
may not support it. All things stand now because OpenSSL is the only
implementation in core and can support both tls-unique and
tls-server-end-point, this may not stand true should any other
implementations get added. For example SSL implementation of MacOS has
no documented API to get a hash of the server or the TLS-finished
message bytes.
--
Michael

Attachment Content-Type Size
0002-Implement-channel-binding-tls-server-end-point-for-S.patch text/x-patch 17.3 KB
0001-Add-connection-parameter-saslchannelbinding.patch text/x-patch 6.5 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Michael Paquier 2017-11-21 06:44:48 Re: [HACKERS] subscription worker signalling wal writer too much
Previous Message Andres Freund 2017-11-21 03:36:42 Combine function returning NULL unhandled?

Browse pgsql-jdbc by date

  From Date Subject
Next Message Thomas Kellerer 2017-11-21 07:01:39 Retrieve the server's time zone
Previous Message Jorge Solórzano 2017-11-20 21:42:21 Re: Abort() doesn't abort backends which are in WAITING state