Обсуждение: Channel binding not supported using scram-sha-256 passwords

On Fri, Feb 15, 2019 at 03:41:37PM -0500, Hugh Ranalli wrote:
> 
> I've been trying to implement scram-sha-256 passwords on PostgreSQL 11.1.
> However, connection attempts whether through Python (psycopg2) or psql fail
> with the message: "channel binding not supported by this build." I've tried
> clearing scram_channel_binding in my global psqlrc ("\set
> scram_channel_binding"), with no success.
> 
> We are not using SSL for the connections, and the documentation labels this as
> an SASL authentication mechanism. Is SSL required for using scram-sha-256
> passwords? What am I missing?

The PG 11 release notes are clear that channel binding is not supported
in a usable way yet:

    https://www.postgresql.org/docs/11/release-11.html

    Add ability to use channel binding when using SCRAM authentication
    (Michael Paquier)
    
    Channel binding is intended to prevent man-in-the-middle attacks, but
    SCRAM cannot prevent them unless it can be forced to be active.
    Unfortunately, there is no way to do that in libpq. Support for it is
    expected in future versions of libpq and in interfaces not built using
    libpq, e.g. JDBC.

-- 
  Bruce Momjian  <bruce@momjian.us>        http://momjian.us
  EnterpriseDB                             http://enterprisedb.com

+ As you are, so once was I.  As I am, so you will be. +
+                      Ancient Roman grave inscription +

On Fri, Feb 15, 2019 at 04:18:40PM -0500, Hugh Ranalli wrote:
> I did see that. However, I'm not *trying* to use it. I set up accounts with
> scram-sha-256 passwords, and when trying to connect I get this message.
> Hence why I tried to disable it.

tls-server-end-point is implemented as channel binding type, and the
only things which got removed as the connection parameter
scram_channel_binding and the channel binding type tls-unique.  So if
you use SSL then channel binding will be used.

On my side, if I connect to a server built with SSL and SCRAM then
channel binding is used and works.

Now, the error message "channel binding not supported by this build"
would show up by either the backend or the frontend if
X509_get_signature_nid() is not present in the version of OpenSSL your
version of libpq (for the frontend) or your backend are linked to.
This function has been added in OpenSSL 1.0.2, so it seems to me that
you have an OpenSSL version mismatch between your client and the
server.  My guess is that the client uses OpenSSL 1.0.2, but the
server is linked to OpenSSL 1.0.1 or older.

(Note: I am not seeing anything bad in the code.)
--
Michael

On 2019-02-18 02:06, Michael Paquier wrote:
> Now, the error message "channel binding not supported by this build"
> would show up by either the backend or the frontend if
> X509_get_signature_nid() is not present in the version of OpenSSL your
> version of libpq (for the frontend) or your backend are linked to.
> This function has been added in OpenSSL 1.0.2, so it seems to me that
> you have an OpenSSL version mismatch between your client and the
> server.  My guess is that the client uses OpenSSL 1.0.2, but the
> server is linked to OpenSSL 1.0.1 or older.

I think there is a bug in the frontend code.  If the server offers
SCRAM-SHA-256-PLUS, the client will choose it if SSL is in use, but it
will later fail with this error message if not
HAVE_PGTLS_GET_PEER_CERTIFICATE_HASH.

The code in src/interfaces/libpq/fe-auth.c:pg_SASL_init() should take
HAVE_PGTLS_GET_PEER_CERTIFICATE_HASH into account before selecting
SCRAM-SHA-256-PLUS.

-- 
Peter Eisentraut              http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

On Wed, Feb 20, 2019 at 04:53:32PM +0100, Peter Eisentraut wrote:
> I think there is a bug in the frontend code.  If the server offers
> SCRAM-SHA-256-PLUS, the client will choose it if SSL is in use, but it
> will later fail with this error message if not
> HAVE_PGTLS_GET_PEER_CERTIFICATE_HASH.

Good catch!  Indeed that's not a good idea.  What do you think about
the attached to fix the issue?
--
Michael

On 2019-02-21 05:47, Michael Paquier wrote:
>              if (conn->ssl_in_use)
> +            {
> +                /*
> +                 * The server has offered SCRAM-SHA-256-PLUS, which is only
> +                 * supported by the client if a hash of the peer certificate
> +                 * can be created.
> +                 */
> +#ifdef HAVE_PGTLS_GET_PEER_CERTIFICATE_HASH
>                  selected_mechanism = SCRAM_SHA_256_PLUS_NAME;
> +#endif
> +            }

Is that right?  Won't we then just select nothing if the macro is not
defined?

-- 
Peter Eisentraut              http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

On Thu, Feb 21, 2019 at 08:32:01PM +0100, Peter Eisentraut wrote:
> On 2019-02-21 05:47, Michael Paquier wrote:
>>              if (conn->ssl_in_use)
>> +            {
>> +                /*
>> +                 * The server has offered SCRAM-SHA-256-PLUS, which is only
>> +                 * supported by the client if a hash of the peer certificate
>> +                 * can be created.
>> +                 */
>> +#ifdef HAVE_PGTLS_GET_PEER_CERTIFICATE_HASH
>>                  selected_mechanism = SCRAM_SHA_256_PLUS_NAME;
>> +#endif
>> +            }
>
> Is that right?  Won't we then just select nothing if the macro is not
> defined?

In the context of an SSL connection, the server would send both SCRAM
and SCRAM_PLUS as valid mechanisms if it supports channel binding
(HAVE_BE_TLS_GET_CERTIFICATE_HASH).  If the server does not support
channel binding, then only SCRAM is sent.

So you have the following possible cases for an SSL connection:
1) Server supports channel binding, sends SCRAM_PLUS and SCRAM as
allowed mechanisms.
1-1) Client supports channel binding, and it chooses SCRAM_PLUS.
1-2) Client does not support channel binding, and it chooses SCRAM.
2) Server does not support channel binding, sends SCRAM as allow
mechanism.
2-1) Client supports channel binding, still it has no choice but to
choose SCRAM.
2-2) Client does not support channel binding, it chooses SCRAM.

On HEAD, the bug you have spotted would cause case 1-2) to fail as the
client thinks that it should choose SCRAM_PLUS even if it does not
support it, causing a failure.  My patch changes things so as SCRAM
gets chosen instead of SCRAM_PLUS as the server sends both SCRAM and
SCRAM_PLUS to the client as possible choices, still the client is not
able to handle SCRAM_PLUS.

Does this explanation make sense?
--
Michael

On 2019-02-22 06:28, Michael Paquier wrote:
>> Is that right?  Won't we then just select nothing if the macro is not
>> defined?
> In the context of an SSL connection, the server would send both SCRAM
> and SCRAM_PLUS as valid mechanisms if it supports channel binding
> (HAVE_BE_TLS_GET_CERTIFICATE_HASH).  If the server does not support
> channel binding, then only SCRAM is sent.

After reading it again a few more times, I think your patch is correct.

I tried reproducing the issue locally, but the required OpenSSL version
is too old to be easily available.

-- 
Peter Eisentraut              http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

Peter Eisentraut <peter.eisentraut@2ndquadrant.com> writes:
> I tried reproducing the issue locally, but the required OpenSSL version
> is too old to be easily available.

Hm, I've got buildfarm hosts with quite old OpenSSLs handy.  What
tests do you want done, exactly?

            regards, tom lane

On Tue, Feb 26, 2019 at 10:04:35AM -0500, Tom Lane wrote:
> Peter Eisentraut <peter.eisentraut@2ndquadrant.com> writes:
>> I tried reproducing the issue locally, but the required OpenSSL version
>> is too old to be easily available.
>
> Hm, I've got buildfarm hosts with quite old OpenSSLs handy.  What
> tests do you want done, exactly?

I would think that Peter is looking for tests which use SCRAM
authentication over SSL with the following versions of OpenSSL used
by the client:
1) Server at 1.0.1, client at 1.0.2, channel binding should not be
used, authentication should succeed.
2) Server at 1.0.2, client at 1.0.1, here also channel binding should
not be used, and connection should succeed.

The patch changes the behavior of 2), which was failing as the client
would use SCRAM_PLUS as SASL mechanism over SCRAM even if the client
does not support channel binding.

What I do in such cases is to compile OpenSSL by myself and link
Postgres to it, here is a command to build shared libraries (all that
is documented in INSTALL):
./config --prefix=$INSTALLPATH shared

Another trick would be to comment out the sections in libpq where
HAVE_PGTLS_GET_PEER_CERTIFICATE_HASH is used to emulate a compilation
with OpenSSL 1.0.1 features and older, while still linking with
1.0.2.

If you want to test the patch and check by yourself, that's of course
fine by me.  Just let me know when you are done and if you think the
patch is good for commit.
--
Michael

On 2019-02-26 23:35, Michael Paquier wrote:
> What I do in such cases is to compile OpenSSL by myself and link
> Postgres to it, here is a command to build shared libraries (all that
> is documented in INSTALL):
> ./config --prefix=$INSTALLPATH shared

I did test it now using a custom-built OpenSSL, and I can confirm it works.

> Another trick would be to comment out the sections in libpq where
> HAVE_PGTLS_GET_PEER_CERTIFICATE_HASH is used to emulate a compilation
> with OpenSSL 1.0.1 features and older, while still linking with
> 1.0.2.

Yeah, that might have been easier. ;-)

-- 
Peter Eisentraut              http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

On Wed, Feb 27, 2019 at 10:21:00AM +0100, Peter Eisentraut wrote:
> On 2019-02-26 23:35, Michael Paquier wrote:
>> What I do in such cases is to compile OpenSSL by myself and link
>> Postgres to it, here is a command to build shared libraries (all that
>> is documented in INSTALL):
>> ./config --prefix=$INSTALLPATH shared
>
> I did test it now using a custom-built OpenSSL, and I can confirm it works.

Thanks for confirming, Peter!  Committed and back-patched.
--
Michael