Re: Direct SSL connection with ALPN and HBA rules

On 22/04/2024 10:19, Michael Paquier wrote:
> 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@iki.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.

I plan to write the patch later today.

>>>> 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.

I'd be OK with that, although I still don't really see the point of 
forcing this from the server side. We could also add this later.

>> 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?

Sounds complicated, and I don't really see the use case for controlling 
the direct SSL support in such a fine-grained fashion.

It would be nice if we could reject non-SSL connections before the 
client sends the startup packet, but that's not possible because in a 
plaintext connection, that's the first packet that the client sends. The 
reverse would be possible: reject SSLRequest or direct SSL connection 
immediately, if HBA doesn't allow non-SSL connections from that IP 
address. But that's not very interesting.

HBA-based control would certainly be v18 material.

-- 
Heikki Linnakangas
Neon (https://neon.tech)