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: rewrite in Go (1/2) #55

Closed
bassosimone opened this issue Sep 10, 2019 · 2 comments
Closed

whatsapp: rewrite in Go (1/2) #55

bassosimone opened this issue Sep 10, 2019 · 2 comments
Assignees
@bassosimone
Labels
effort/L Large effort enhancement New feature or request ooni/probe-engine Issues related to github.com/ooni/probe-engine priority/high High priority
Milestone
Sprint 16 - Neon

Comments

@bassosimone
Copy link
Contributor

No description provided.

@bassosimone bassosimone added the P1 label Sep 10, 2019
@bassosimone bassosimone added help wanted enhancement New feature or request labels Sep 12, 2019
@bassosimone bassosimone modified the milestones: new-nettests, new-experiments-otfy2 Oct 2, 2019
@bassosimone bassosimone added effort/M Medium effort priority/low Low priority labels Dec 10, 2019
@bassosimone bassosimone added priority/medium Medium priority and removed help wanted priority/low Low priority labels Jan 12, 2020
@hellais hellais added the ooni/probe-engine Issues related to github.com/ooni/probe-engine label Jan 16, 2020
@bassosimone bassosimone self-assigned this Feb 17, 2020
@bassosimone bassosimone added priority/high High priority and removed priority/medium Medium priority labels Jun 8, 2020
@bassosimone bassosimone added effort/L Large effort and removed effort/M Medium effort labels Jun 22, 2020
@bassosimone bassosimone added this to the Sprint 16 - Neon milestone Jun 22, 2020
bassosimone added a commit that referenced this issue Jun 29, 2020
@bassosimone
urlgetter/multi: allow setting common beginning of time
df37567 
This way, measurements run by, e.g., Telegram will use the same
base of time for each input being measured.

Part of #55.
bassosimone added a commit that referenced this issue Jun 29, 2020
@bassosimone
telegram: use common begin time, bump timeout
02aac97 
We want all network events and other events used by telegram to use the
same common begin of time, so we can correlate events.

While there, bump timeout to 60 seconds.

Part of #55.
@bassosimone bassosimone mentioned this issue Jun 29, 2020
Merged
bassosimone added a commit that referenced this issue Jun 29, 2020
@bassosimone
experiment/whatsapp: rewrite in Go
a295e5c 
Part of #55
bassosimone added a commit that referenced this issue Jun 29, 2020
@bassosimone
experiment/whatsapp: compute high level keys
ed5e315 
Part of #55
bassosimone added a commit that referenced this issue Jun 29, 2020
@bassosimone
netx/errorx: map "operation was canceled error" to "interrupted"
fcc8e80 
Part of #55
bassosimone added a commit that referenced this issue Jun 29, 2020
@bassosimone
whatsapp: document limitations and next steps
7e2caf6 
Part of #55
@bassosimone
Copy link
Contributor Author

bassosimone commented Jun 29, 2020 •
edited
Loading

We're mostly done, what is missing TODO is:

After all of this has been done, we can finally call this work complete.

bassosimone added a commit that referenced this issue Jun 29, 2020
@bassosimone
whatsapp: Go rewrite (#735)
89f3bf0 
* urlgetter/multi: allow setting common beginning of time

This way, measurements run by, e.g., Telegram will use the same
base of time for each input being measured.

Part of #55.

* telegram: use common begin time, bump timeout

We want all network events and other events used by telegram to use the
same common begin of time, so we can correlate events.

While there, bump timeout to 60 seconds.

Part of #55.

* experiment/urlgetter: increase robustness of tests

Part of #55

* experiment/whatsapp: rewrite in Go

Part of #55

* experiment/whatsapp: compute high level keys

Part of #55

* netx/errorx: map "operation was canceled error" to "interrupted"

Part of #55

* whatsapp: document limitations and next steps

Part of #55

* Repair tests

* Write mote tests

* Document some more TODOs
@bassosimone bassosimone mentioned this issue Jun 30, 2020
Merged
bassosimone added a commit that referenced this issue Jun 30, 2020
@bassosimone
urlgetter: implement FailOnHTTPError
b98559c 
In some cases, we want to fail if we see an HTTP status code >= 400.

This is certainly the case when we're doing plain text HTTP.

Both the Telegram implementation and WhatsApp can benefit from that
functionality, when connecting to the web interface.

Thus, I've factored the code out of the Telegram implementation
and I have moved it inside the urlgetter package.

Part of #55
@bassosimone bassosimone mentioned this issue Jun 30, 2020
Merged
bassosimone added a commit that referenced this issue Jun 30, 2020
@bassosimone
urlgetter: implement FailOnHTTPError (#739)
cef51e8 
In some cases, we want to fail if we see an HTTP status code >= 400.

This is certainly the case when we're doing plain text HTTP.

Both the Telegram implementation and WhatsApp can benefit from that
functionality, when connecting to the web interface.

Thus, I've factored the code out of the Telegram implementation
and I have moved it inside the urlgetter package.

Part of #55
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.go: mention the pull request with extended rationale
4ae07df 
Part of #55
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 added a commit that referenced this issue Jun 30, 2020
@bassosimone
urlgetter: allow to force a User-Agent
f6d6894 
This is for investigating what seems MITM detection implemented
by WhatsApp spotted during #55.
@bassosimone bassosimone mentioned this issue Jun 30, 2020
Merged
@bassosimone bassosimone changed the title whatsapp: rewrite in Go whatsapp: rewrite in Go (1/2) Jun 30, 2020
@bassosimone bassosimone mentioned this issue Jun 30, 2020
Closed
@bassosimone
Copy link
Contributor Author

Done enough for this sprint, will continue with #743 in the next sprint

bassosimone added a commit that referenced this issue Jun 30, 2020
@bassosimone
whatsapp: instruments to investigate suspect MITM detection (#742)
ceda856 
* urlgetter: allow to force a User-Agent

This is for investigating what seems MITM detection implemented
by WhatsApp spotted during #55.

* experiment.go: self-identify as ooniprobe-engine

Closes #737
@bassosimone bassosimone mentioned this issue Jul 2, 2020
Closed
1 task
This was referenced Aug 25, 2020
Closed
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@bassosimone bassosimone

Labels
effort/L Large effort enhancement New feature or request ooni/probe-engine Issues related to github.com/ooni/probe-engine priority/high High priority
Projects
None yet
Milestone
Sprint 16 - Neon
Development

No branches or pull requests
2 participants
@bassosimone @hellais