Re: ecdh support causes unnecessary roundtrips

> On 17 Jun 2024, at 01:46, Andres Freund <andres@anarazel.de> wrote:

> When connecting with a libpq based client, the TLS establishment ends up like
> this in many configurations;
>
> C->S: TLSv1 393 Client Hello
> S->C: TLSv1.3 167 Hello Retry Request, Change Cipher Spec
> C->S: TLSv1.3 432 Change Cipher Spec, Client Hello
> S->C: TLSv1.3 1407 Server Hello, Application Data, Application Data, Application Data, Application Data

I wonder if this is the result of us still using TLS 1.2 as the default minimum
protocol version.  In 1.2 the ClientHello doesn't contain the extensions for
cipher and curves, so the server must reply with a HRR (Hello Retry Request)
message asking the client to set protocol, curve and cipher.  The client will
respond with a new Client Hello using 1.3 with the extensions.

Using 1.3 as the default minimum has been on my personal TODO for a while,
maybe that should be revisited for 18.

> I.e. there are two clients hellos, because the server rejects the clients
> "parameters".

Or the client didn't send any.

> This appears to be caused by ECDH support.  The difference between the two
> ClientHellos is
> -            Extension: key_share (len=38) x25519
> +            Extension: key_share (len=71) secp256r1
>
> I.e. the clients wanted to use x25519, but the server insists on secp256r1.

Somewhat related, Erica Zhang has an open patch to make the server-side curves
configuration take a list rather than a single curve [0], and modernizing the
API used as a side effect (SSL_CTX_set_tmp_ecdh is documented as obsoleted by
OpenSSL, but not deprecated with an API level).

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

> but if it is, shouldn't we at least do the same in libpq, so we don't introduce
> unnecessary roundtrips?

If we don't set the curve in the client I believe OpenSSL will pass the set of
supported curves the client has, which then should allow the server to pick the
one it wants based on ssl_ecdh_curve, so ideally we shouldn't have to I think.

> 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?  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.  If you force the
client min protocol to 1.3 in an unpatched client, do you see the same speedup?

--
Daniel Gustafsson

[0] tencent_1B368B07F4B5886F9822981189DA736CF209@qq.com