Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improved version of ECHCheck test #2840

Open
3 tasks
hellais opened this issue Jan 20, 2025 · 4 comments
Open
3 tasks

Improved version of ECHCheck test #2840

hellais opened this issue Jan 20, 2025 · 4 comments

Comments

@hellais
Copy link
Member

hellais commented Jan 20, 2025

This issue is to track the work @johnhess is doing on expanding our echcheck test to fully support the ECH handshake, see: johnhess/probe-cli#1.

What we should ensure happens as part of this new iteration is:

  • Add support for truly speaking ECH (as opposed to just greased ECH), using the tls client implementation in golang stdlib
  • Get rid of the utls dependency
  • Add support for setting the queries, tcp_connect and network_events keys

xref: #1453

@johnhess
Copy link

The first and second should be done already :-)

Re: the third item, is the intention that the probe would also populate results for the other dataformats e.g. queries? If so, I can add those. Which network events do you want to see captured?

@hellais
Copy link
Member Author

hellais commented Jan 23, 2025

is the intention that the probe would also populate results for the other dataformats e.g. queries

Yes that's correct. These should already be emitted by the measurement library functions you are using.

Which network events do you want to see captured?

Similarly also network_events are automatically collected and you just have to add them to your test_keys.

Here are some examples of how this is done in other tests

network_events: https://github.com/ooni/probe-cli/blob/ef139c525f1b1e751f21cccf842982b4a2720dbd/internal/experiment/simplequicping/simplequicping.go#L190

queries, tcp_connect: https://github.com/ooni/probe-cli/blob/ef139c525f1b1e751f21cccf842982b4a2720dbd/internal/experiment/urlgetter/getter.go#L62

@johnhess
Copy link

Working on that. Ran into a semantics question.

When we attempt to do a GREASE connection, we get an error. We expect that error; it's an EchRejectionError! We can & do use the retry_configs that come with it to attempt a real ECH connection.

Question: Do we want to log 2 ArchivalTLSOrQUICHandshakeResults with the GREASE'd one as having a Failure? A SoError? My guess is "no, that's not a failure" since it's doing what we expect (even if golang is returning a conspicuous error to the caller so they know they didn't get the ECH they wanted).

@johnhess
Copy link

For net_events: do we want to capture all net events for the DNS queries, then again for each TCP+TLS attempt? Or just a subset?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants