From: | Michael Paquier <michael(at)paquier(dot)xyz> |
---|---|
To: | Heikki Linnakangas <hlinnaka(at)iki(dot)fi> |
Cc: | Jacob Champion <jacob(dot)champion(at)enterprisedb(dot)com>, Postgres hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org> |
Subject: | Re: Direct SSL connection with ALPN and HBA rules |
Date: | 2024-04-22 07:19:53 |
Message-ID: | ZiYPmUZjcQtPz2j7@paquier.xyz |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On Sat, Apr 20, 2024 at 12:43:24AM +0300, Heikki Linnakangas wrote:
> On 19/04/2024 19:48, Jacob Champion wrote:
>> On Fri, Apr 19, 2024 at 6:56 AM Heikki Linnakangas <hlinnaka(at)iki(dot)fi> wrote:
>>> With direct SSL negotiation, we always require ALPN.
>>
>> (As an aside: I haven't gotten to test the version of the patch that
>> made it into 17 yet, but from a quick glance it looks like we're not
>> rejecting mismatched ALPN during the handshake as noted in [1].)
>
> Ah, good catch, that fell through the cracks. Agreed, the client should
> reject a direct SSL connection if the server didn't send ALPN. I'll add that
> to the Open Items so we don't forget again.
Would somebody like to write a patch for that? I'm planning to look
at this code more closely, as well.
>> You get protection against attacks that could have otherwise happened
>> during the plaintext portion of the handshake. That has architectural
>> implications for more advanced uses of SCRAM, and it should prevent
>> any repeats of CVE-2021-23222/23214. And if the peer doesn't pass the
>> TLS handshake, they can't send you anything that you might forget is
>> untrusted (like, say, an error message).
>
> Can you elaborate on the more advanced uses of SCRAM?
I'm not sure what you mean here, either, Jacob.
>>> Controlling these in HBA is a bit inconvenient, because you only find
>>> out after authentication if it's allowed or not. So if e.g. direct SSL
>>> connections are disabled for a user,
>>
>> Hopefully disabling direct SSL piecemeal is not a desired use case?
>> I'm not sure it makes sense to focus on that. Forcing it to be enabled
>> shouldn't have the same problem, should it?
I'd get behind the case where a server rejects everything except
direct SSL, yeah. Sticking that into a format similar to HBA rules
would easily give the flexibility to be able to accept or reject
direct or default SSL, though, while making it easy to parse. The
implementation is not really complicated, and not far from the
existing hostssl and nohostssl.
As a whole, I can get behind a unique GUC that forces the use of
direct. Or, we could extend the existing "ssl" GUC with a new
"direct" value to accept only direct connections and restrict the
original protocol (and a new "postgres" for the pre-16 protocol,
rejecting direct?), while "on" is able to accept both.
> Forcing it to be enabled piecemeal based on role or database has similar
> problems. Forcing it enabled for all connections seems sensible, though.
> Forcing it enabled based on the client's source IP address, but not
> user/database would be somewhat sensible too, but we don't currently have
> the HBA code to check the source IP and accept/reject SSLRequest based on
> that. The HBA rejection always happens after the client has sent the startup
> packet.
Hmm. Splitting the logic checking HBA entries (around check_hba) so
as we'd check for a portion of its contents depending on what the
server has received or not from the client would not be that
complicated. I'd question whether it makes sense to mix this
information within the same configuration files as the ones holding
the current HBA rules. If the same rules are used for the
pre-startup-packet phase and the post-startup-packet phase, we'd want
new keywords for these HBA rules, something different than the
existing sslmode and no sslmode?
--
Michael
From | Date | Subject | |
---|---|---|---|
Next Message | Ajin Cherian | 2024-04-22 07:34:58 | Re: Slow catchup of 2PC (twophase) transactions on replica in LR |
Previous Message | Michael Paquier | 2024-04-22 06:36:28 | Re: Support a wildcard in backtrace_functions |