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.

Sign up for GitHub

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

Jump to bottom

whatsapp: better web-interface measurements #740

Closed
bassosimone opened this issue Jun 30, 2020 · 2 comments
Closed

whatsapp: better web-interface measurements #740

bassosimone opened this issue Jun 30, 2020 · 2 comments
Assignees
@bassosimone
Labels
discuss We need to have a conversation effort/M Medium effort priority/high High priority
Milestone
Sprint 17 - Θέτις

Comments

@bassosimone
Copy link
Contributor

bassosimone commented Jun 30, 2020 •
edited
Loading

We should consider redesigning these tests in light of the fact that we typically access the web interface using HTTPS also when we attempt with HTTP, because of 301 redirection. For this reason, there is opportunity to simplify the algorithms we use and make sure we reduce the rate of false positives we currently see.

This is what I am proposing:

  1. the http check is good if we get a redirection for the right domain (rationale: we don't need to care about the web interface being served over http since now we always see a redirection)

  2. the https check is good if we handshake and get a response regardless of the response code (rationale: with the correlation between the ClientHello and the User-Agent we may be flagged as MITM)

There is also issue #229 that discusses ways in which we can better avoid being flagged as MITM by using a custom ClientHello, but that work will require more time to converge and, in the meantime, I think the two above changes make our tests more robust.
@bassosimone bassosimone added this to the Sprint 17 - Θέτις milestone Jun 30, 2020
@bassosimone bassosimone self-assigned this Jun 30, 2020
bassosimone added a commit that referenced this issue Jun 30, 2020
@bassosimone
whatsapp: implement remaining checks
53bb97a 
As mentioned in the commit, we see a 400 Bad Request error from
WhatsApp when using the User-Agent we use for measurements along
with the standard Golang's ClientHello fingerprint.

This looks like MITM detection like https://mitm.watch to me.

A fix for this issue could be to find out a combination of User-Agent
and ClientHello that does not trigger 400 and keep the test as it
should according to the spec.

Yet, if there is MITM detection, it may change. This will likely
cause future false positives, and we already have a bunch of such
false positives for the IM tests.

Also, it currently seems safe to assume that, if we can perform
a TLS handshake with a certificate pool we trust, then we are
talking with WhatsApp. Therefore, the status code and the returned
web page matter much less than they did when we wrote the initial
implementation of the WhatsApp experiment.

What's more, because the HTTP request only redirects us, we should
probably also simplify that check, to avoid asserting anything on
the returned web page _if_ we're correctly redirected.

How to properly do this will be researched in the next sprint
as part of #740.

Further investigating this issue should also be fun.

This work is part of #55.
@bassosimone bassosimone mentioned this issue Jun 30, 2020
Merged
bassosimone added a commit that referenced this issue Jun 30, 2020
@bassosimone
whatsapp: implement remaining checks (#741)
a8f4faf 
* whatsapp: implement remaining checks

As mentioned in the commit, we see a 400 Bad Request error from
WhatsApp when using the User-Agent we use for measurements along
with the standard Golang's ClientHello fingerprint.

This looks like MITM detection like https://mitm.watch to me.

A fix for this issue could be to find out a combination of User-Agent
and ClientHello that does not trigger 400 and keep the test as it
should according to the spec.

Yet, if there is MITM detection, it may change. This will likely
cause future false positives, and we already have a bunch of such
false positives for the IM tests.

Also, it currently seems safe to assume that, if we can perform
a TLS handshake with a certificate pool we trust, then we are
talking with WhatsApp. Therefore, the status code and the returned
web page matter much less than they did when we wrote the initial
implementation of the WhatsApp experiment.

What's more, because the HTTP request only redirects us, we should
probably also simplify that check, to avoid asserting anything on
the returned web page _if_ we're correctly redirected.

How to properly do this will be researched in the next sprint
as part of #740.

Further investigating this issue should also be fun.

This work is part of #55.

* whatsapp.go: mention the pull request with extended rationale

Part of #55
@bassosimone bassosimone mentioned this issue Jun 30, 2020
Closed
@bassosimone bassosimone added effort/S Small effort priority/high High priority discuss We need to have a conversation effort/M Medium effort and removed effort/S Small effort labels Jul 2, 2020
@bassosimone
Copy link
Contributor Author

Discussed and approved this plan with @hellais

@bassosimone bassosimone mentioned this issue Jul 7, 2020
Closed
bassosimone added a commit that referenced this issue Jul 9, 2020
@bassosimone
whatsapp: less strict checks for the web interface
a4af440 
See #740
@bassosimone bassosimone mentioned this issue Jul 9, 2020
Merged
bassosimone added a commit that referenced this issue Jul 9, 2020
@bassosimone
whatsapp: less strict checks for the web interface (#750)
24e54f7 
See #740
bassosimone added a commit that referenced this issue Jul 9, 2020
@bassosimone
telegram: less strict checks for the web interface
c41812b 
See #740
@bassosimone bassosimone mentioned this issue Jul 9, 2020
Merged
bassosimone added a commit that referenced this issue Jul 9, 2020
@bassosimone
fix(net/archival): emit sorted headers
859eb19 
See failing test https://github.com/ooni/probe-engine/pull/751/checks?check_run_id=853562310

Spotted as part of #740
bassosimone added a commit that referenced this issue Jul 9, 2020
@bassosimone
fix(net/archival): emit sorted headers
7ba7e56 
See failing test https://github.com/ooni/probe-engine/pull/751/checks?check_run_id=853562310

Spotted as part of #740
bassosimone added a commit that referenced this issue Jul 9, 2020
@bassosimone
fix(net/archival): emit sorted headers (#751)
d99a4f0 
See failing test https://github.com/ooni/probe-engine/pull/751/checks?check_run_id=853562310

Spotted as part of #740
bassosimone added a commit to ooni/spec that referenced this issue Jul 9, 2020
@bassosimone
whatsapp: improve spec to reduce false positives
23eb619 
Specifically, say that WhatsApp web is reachable over HTTPS regardless
of the HTTP status code and that over HTTP we're happy if we get a
redirect to the correct location.

This is part of ooni/probe-engine#740.

While there, remove the all endpoints option. We should test all the
endpoints to make a better statement on whether it's blocked.

(A single endpoint down is, in fact, a false positive.)

While there, explicitly say that the CIDR check is gone for good,
since it has not been working for quite some time now.

While there, mention all the parent data formats and specify the
toplevel structure in a more strict way.
@bassosimone bassosimone mentioned this issue Jul 9, 2020
Merged
bassosimone added a commit to ooni/spec that referenced this issue Jul 9, 2020
@bassosimone
whatsapp: improve spec to reduce false positives (#197)
0c2e2d6 
Specifically, say that WhatsApp web is reachable over HTTPS regardless
of the HTTP status code and that over HTTP we're happy if we get a
redirect to the correct location.

This is part of ooni/probe-engine#740.

While there, remove the all endpoints option. We should test all the
endpoints to make a better statement on whether it's blocked.

(A single endpoint down is, in fact, a false positive.)

While there, explicitly say that the CIDR check is gone for good,
since it has not been working for quite some time now.

While there, mention all the parent data formats and specify the
toplevel structure in a more strict way.
bassosimone added a commit to ooni/spec that referenced this issue Jul 9, 2020
@bassosimone
ts-018-whatsapp.md: further clarify the text
9a74e5b 
While at it, fix JSON formatting.

Part of ooni/probe-engine#740
@bassosimone bassosimone mentioned this issue Jul 9, 2020
Merged
bassosimone added a commit to ooni/spec that referenced this issue Jul 9, 2020
@bassosimone
ts-018-whatsapp.md: further clarify the text (#198)
9a61ea8 
While at it, fix JSON formatting.

Part of ooni/probe-engine#740
@bassosimone bassosimone changed the title telegram, whatsapp: better web interface measurements whatsapp: better web-interface measurements Jul 9, 2020
@bassosimone
Copy link
Contributor Author

Turns out there is nothing to be done on the Telegram side, because we get a 200 OK for HTTP. It may be that we need to change the experiment in the future, but for now it seems we're good.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@bassosimone bassosimone

Labels
discuss We need to have a conversation effort/M Medium effort priority/high High priority
Projects
None yet
Milestone
Sprint 17 - Θέτις
Development

No branches or pull requests
1 participant
@bassosimone