Re: ecdh support causes unnecessary roundtrips

> On 17 Jun 2024, at 19:01, Andres Freund <andres(at)anarazel(dot)de> wrote:
> On 2024-06-17 12:00:30 +0200, Daniel Gustafsson wrote:
>>> On 17 Jun 2024, at 01:46, Andres Freund <andres(at)anarazel(dot)de> wrote:

>>> I don't know if it's good that we're calling SSL_CTX_set_tmp_ecdh at all,
>>
>> To set the specified curve in ssl_ecdh_curve we have to don't we?
>
> Sure, but it's not obvious to me why we actually want to override openssl's
> defaults here. There's not even a parameter to opt out of forcing a specific
> choice on the server side.

I agree that the GUC is a bit rough around the edges, maybe leavint it blank or
something should be defined as "OpenSSL defaults". Let's bring that to Erica's
patch for allowing a list of curves.

>>> I did confirm that doing the same thing on the client side removes the
>>> additional roundtrip.
>>
>> The roundtrip went away because the client was set to use secp256r1?
>
> Yes. Or if I change the server to not set the ecdh curve.

Configuring the server to use x25519 instead of secp256r1 should achieve the
same thing.

>> I wonder if that made OpenSSL override the min protocol version and switch
>> to a TLS1.3 ClientHello since it otherwise couldn't announce the curve.
>
> The client seems to announce the curve in the initial ClientHello even with
> 1.3 as the minimum version.

With 1.3 it should announce it in ClientHello, do you mean that it's announced
when 1.2 is the minimum version as well? It does make sense since a 1.2 server
is defined to disregard all extensions.

> What *does* make the HRR go away is setting ssl_max_protocol_version=TLSv1.2
> on the client side.

Makes sense, that would remove the curve and there is no change required.

> https://wiki.openssl.org/index.php/TLS1.3 says:
>
>> In practice most clients will use X25519 or P-256 for their initial
>> key_share. For maximum performance it is recommended that servers are
>> configured to support at least those two groups and clients use one of those
>> two for its initial key_share. This is the default case (OpenSSL clients
>> will use X25519).
>
> We're not allowing both groups and the client defaults to X25519, hence
> the HRR.

So this would be solved by the curve-list patch referenced above, especially if
allow it to have an opt-out to use OpenSSL defaults.

--
Daniel Gustafsson