Re: BUG #17760: SCRAM authentication fails with "modern" (rsassaPss signature) server certificate - Mailing list pgsql-bugs

From Heikki Linnakangas
Subject Re: BUG #17760: SCRAM authentication fails with "modern" (rsassaPss signature) server certificate
Date
Msg-id 584842ed-e476-6bac-bb36-2aec204e9eef@iki.fi
Whole thread Raw
In response to Re: BUG #17760: SCRAM authentication fails with "modern" (rsassaPss signature) server certificate  (Heikki Linnakangas <hlinnaka@iki.fi>)
Responses Re: BUG #17760: SCRAM authentication fails with "modern" (rsassaPss signature) server certificate  (Michael Paquier <michael@paquier.xyz>)
List pgsql-bugs
cc'ing Jacob and Michael who have poked around channel binding before. 
If you have a chance to review this patch, I would appreciate that.

On 05/02/2023 03:28, Heikki Linnakangas wrote:
> On 25/01/2023 13:32, PG Bug reporting form wrote:
>> I do think however that this is an oversight on our side and has to be
>> addressed. If not in code, the docs should point out that certain server
>> certificate types (PSS) may not work with SCRAM auth (or libpq needs to be
>> compiled against a minimum version of OpenSSL, if that's the root cause).
> 
> There are a few things we change in PostgreSQL for this:
> 
> 1. If the server cannot compute a hash of the certificate, because it
> cannot unambiguously determine the hash algorithm to use, it should not
> offer the SCRAM-SHA-256-PLUS authentication method to the client. In
> other words, if the server cannot do channel binding with it's
> certificate, it should say so, instead of erroring out later.

Here's a patch for that.

> 2. Similarly in the client: if libpq cannot determinate the hash
> algorithm to use with the server's certificate, it should not attempt
> channel binding.
> 
> I believe this is OK from a security point of view. If the server is
> using a certificate that cannot be used with channel binding, and the
> client doesn't require channel binding, it's OK to not do it. A man in
> the middle can present a certificate to the client that cannot be used
> with channel binding, but if they can do that they could also just not
> offer the SCRAM-SHA-256-PLUS option to the client. So I don't see risk
> of downgrade attacks here.

On second thoughts, the above is wrong. We cannot safely downgrade to 
not using channel binding just because we don't support the 
certificate's signature algorithm. Otherwise a MITM could indeed do what 
I said above and send a certificate that uses an unsupported signature 
algorithm, to force downgrade. It cannot just modify the SASL mechanism 
negotiation by leaving out the SCRAM-SHA-256-PLUS, because the SCRAM 
handshake catches that case. The client sends a different gs2-cbind-flag 
when it chooses to not do channel binding even though the server 
supports it, and when it doesn't use channel binding because the server 
didn't support it.

I improved the error message in the client, but I'm afraid that's the 
best we can do in the client side. Fortunately, it is enough to upgrade 
the server to include this fix, to allow clients to connect. (Albeit 
without channel binding).

> 3. Add support for channel binding with RSA-PSS. The problem is that
> be_tls_get_certificate_hash() doesn't know which digest algorithm to
> use. As you noted, OBJ_find_sigid_algs() returns "undef" (NID_undef) for
> rsassaPss certificates. I did some googling: when certificate uses
> RSASSA-PSS as the signature algorithm, there is a separate field,
> RSASSA-PSS-params that specifies the hash algorithm
> (https://www.rfc-editor.org/rfc/rfc4055#section-3.1). If you ask me,
> OBJ_find_sigid_algs() should look into RSASSA-PSS-params and figure it
> out, but I'm no expert in the OpenSSL codebase so maybe that would be
> the wrong place to do it.

I didn't dare to make any such changes for now. Maybe we could do that 
in 'master', but I would be wary of backpatching.

- Heikki

Attachment

pgsql-bugs by date:

Previous
From: Tom Lane
Date:
Subject: Re: BUG #17774: Assert triggered on brin_minmax_multi.c
Next
From: Tom Lane
Date:
Subject: Re: BUG #17781: Assert in setrefs.c