search

RIPE-NCC / ripe-atlas-probe-measurements

RIPE Atlas probe measurement source code
Other
41 stars 18 forks source link

sslgetcert: Add EC Point Formats extension to TLS handshake (fix connections to Vercel servers) #15

Open dkg opened 1 year ago

dkg commented 1 year ago

As described in Section 5.1.2 of RFC 8422, the EC Point Formats extension is valid for TLS 1.2 and earlier. It is deprecated in TLS 1.3 (RFC 8446), but that doesn't stop some servers from requiring it. There is no harm in supplying the extension to a TLS 1.3 server.

In particular, Vercel (https://vercel.com/) TLS terminators seem to respond with a handshake failure alert if the EC Point Format extension is not present in the Client Hello.

This can be seen with measurement 49131334: https://atlas.ripe.net/measurements/49131334/#probes

This changeset should add the EC Point Format extension to the probe, which will result in successful certificate harvesting from Vercel servers, without introducing any incompatibilities to other servers.

I have not been able to test these changes as i don't have a full build environment set up for atlas probe. I welcome feedback or edits.

robert-kisteleki commented 1 year ago

So, if I understand you correctly, this is basically a workaround for a bug in someone else's implementation. In other areas (in particular DNS nowadays) that's a hot topic, and consensus seems to be to avoid doing it, because ultimately it complicates things rather than encourage bug fixing.

I could not immediately find what "deprecated in TLS 1.3" really means, i.e. is it now a MUST NOT or a SHOULD NOT or something else. How sure are we that adding this will not actually upset other implementations, especially compliant ones?

dkg commented 1 year ago

it's not a MUST NOT or a SHOULD NOT -- and when it comes to the ClientHello flight (the initial flight) of a TLS handshake from a client willing to do TLS 1.2 or TLS 1.3, it's entirely reasonable to include the extension, knowing that a TLS 1.3-only server won't care.

At any rate, the sslgetcert probe itself can't do TLS 1.3 at the moment (it would need to be able to decrypt the incoming response since most of the server's first flight is ephemerally encrypted in TLS 1.3), so omitting that extension just because TLS 1.3 doesn't use it isn't a great reason.

I do agree with you that vercel's implementation should be more gracious in handling the lack of an extension, regardless of the client's behavior, but this is not about working around a nasty bug by being non-compliant with the standards. The sslgetcert client isn't even a standards-compliant TLS endpoint, since the only thing it does is emit the ClientHello of a TLS handshake and record the response from the server. But even sslgetcert was a full TLS client, emitting the ec_point_formats extension would be reasonable: its goal is to capture the certificate(s) from the widest range of TLS servers.

At any rate, i've opened a discussion over on vercel's community forum (i'm not a Vercel customer, and i don't know where else to do this) to encourage them to fix their server implementation as well, but i think until sslgetcert is intending to be TLS 1.3 only, it should go ahead and offer the EC point format extension just to be safe.

michel-stam commented 1 year ago

@dkg do you have the option of fixing the above comments and resubmitting the patch?

Brgrds

Michel

dkg commented 1 year ago

@michel-stam i'm not sure i understand what comments need to be fixed..

If you can point me toward anything wrong with the patch, i'd be happy to fix it.

michel-stam commented 1 year ago

@michel-stam i'm not sure i understand what comments need to be fixed..

If you can point me toward anything wrong with the patch, i'd be happy to fix it.

Hi @dkg,

Will tag you on the comments if thats ok with you. If anything is unclear, please reach out.

Cheers,

Michel

dkg commented 1 year ago

@michel-stam wrote:

Will tag you on the comments if thats ok with you. If anything is unclear, please reach out.

Sure, it is certainly OK to tag me on comments, but if you've already done that then i am not seeing them. Sorry if i'm just being confused about how github works. any explanation or pointers to specific comments would be welcome!

michel-stam commented 1 year ago

Hi @dkg . Please search this page for the text: "@dkg this comment will need fixing." You will find them. If not contact me directly via email and I'll try and send you screenshots.

Regards,

Michel

dkg commented 1 year ago

Thanks for the review, @michel-stam! I've pushed a new version that i think resolves all the issues.