search

ooni / probe

OONI Probe network measurement tool for detecting internet censorship
https://ooni.org/install
BSD 3-Clause "New" or "Revised" License
754 stars 142 forks source link

engine: adapt to use bridges #2531

Closed bassosimone closed 1 year ago

bassosimone commented 1 year ago

Update: 2023-10-06 we're now naming the functionality bridge because it looks like a better name.

This issue is about adapting the ooniprobe engine (i.e., the ./internal directory in ooni/probe-cli) to use beacons. We define as "beacon" a host that relies traffic to the OONI backend and the Web Connectivity test helper. We want to switch to use beacons for the two following reasons. First, we want to reduce the number of interactions required to run Web Connectivity. Second, we want to increase the reliability of the probes.

While working on this issue, we should also probably consider the following two side quests. First, the engine itself consists of several packages that need to interoperate and we can probably simplify the structure of the code by merging several packages together and better documenting the interaction between all the moving pieces. Second, we are currently using the same code paths for communicating with the backend and for measuring. However, the requirements are quite different. For example, using an HTTP/HTTPS proxy is more complex than it should because of this design. Additionally, the policies that we need for implementing beacons are different than we one we use for measuring. For example, the HTTP transport we are currently using has specific constraints required by the ./internal/legacy/netx package that prevent us from having parallel outstanding HTTP requests, which in turns limits our flexibility and performance. Therefore, it may be smarter to have a separate HTTP transport using a different configuration, to better meet our design goals with a conceptually simpler codebase. These two side quests are the reasons why I am tagging this issue also as "techdebt": there may be techdebt and in such case we'll pay for it.

Working on this issue proper would first entail studying the existing codebase carefully to determine the proper places where to intervene and make ameliorating changes. Once we have clarity about this, we will modify the implementation to make it (a) not more complex than it currently is and (b) suitable for using beacons in production.

We will consider this issue "fixed" when we'll have merged support for beacons into the main development branch.

Update (2023-10-02) You can read a summary of the work done this issue at https://github.com/ooni/probe/issues/2531#issuecomment-1743087781.

bassosimone commented 1 year ago

The pull requests and commits mentioned above removed some documented technical debt. Namely, we have moved the OperationLogger where it belonged and removed duplication in a bunch of netxlite factories.

bassosimone commented 1 year ago

Here's a summary of the work done to implement beacons.

As a general remark, while doing all this work, I also attempted to proactively improve and upgrade unit testing. This caused me to work on https://github.com/ooni/netem to support more network emulation.

I also tried to reduce technical debt in each piece of code I was touching. This explains many of the refactor and cleanup commits you are going to see in the list of pull requests below.

We can divide the work into the following sections.

Preliminary refactoring I started with preliminary refactoring where my objective, roughly speaking, was that of decoupling the HTTP/HTTPS stack used for measuring from the one used for communicating with backends.

The general idea here was that there are a bunch of quirks in the HTTP/HTTPS stack required to implement measurements, but those quirks complicate the job of implementing beacons.

This work consisted of the following pull requests:

Support HTTP/HTTPS proxies While refactoring the HTTP/HTTPS stack for communicating with the OONI backend and other services, I realized that we could quite easily use the Go standard library facilities to implement proxies when using the new HTTP/HTTPS stack, so I implemented this support and added tests for it.

This work consisted of the following pull requests:

Creating engine specific network with beacons support Once I have removed the peril that implementing beacons interfered with measurements, I started extending the new HTTP/HTTPS stack used for communicating with the backend to support using beacons. This work required lots of testing, trial and error. The design changed slightly from the initial pull requests that added this support.

The beacons functionality roughly works as follows:

  1. I only modified how we dial TLS connections for the new HTTP/HTTPS stack;

  2. most of the stack functionality still uses netxlite (which is OK, since I just wanted to avoid potential functionality conflict but my objective has never been that of rewriting everything from scratch, since there is no need);

  3. rather than resolving IP addresses, the beacons aware stack resolves "tactics", where each tactic specifies an IP address to use, a domain name to use as the SNI, and a domain name used to verify TLS;

  4. there is a "null" policy for creating tactics that uses the DNS to obtain IP addresses and basically behaves similarly to what we were doing before (i.e., use the domain name as both the SNI and the verification hostname);

  5. the new engine uses an "happy eyeballs" like strategy that allows us to overlap tactics (for more efficiency);

  6. there is the possibility of overriding the choises made by the beacons engine for a given domain by creating a JSON file with comments named $HOME/.ooniprobe/engine/httpsdialer.conf directory;

  7. the default policy extends the DNS results for selected domains and, for such domains, it tries a bunch of SNIs that should work and remembers what worked to use it consistently.

This work consisted of the following pull requests:

Improve HTTP headers and bodies scrubbing Because the previous chunk of work started introducing "happy eyeballs" and, in particular, the new HTTP/HTTPS stack does not have the quirk of the old one that we always try IPv4 addresses first and only in case of issues we fallback to IPv6, I started to be worried. The general future trajectory is that we want to use the new HTTP/HTTPS stack for all bootstrap related functionality including resolving the probe's IP address (which I had not time to do in the scope of this issue). So, my worry basically was that, in case we started doing that, we would have seen cases in which we were aware of, say, the probe IPv6 but then a page we measured added into its body or in some header values the probe IPv4 (or the other way around). This means that, in the future, we might find ourselves not being able to selectively scrub the IPv4 (or IPv6) address from the measurement because we only knew the IPv6 (or IPv4) address.

Facing this potential future issue, I chose to fix it proactively and rather radically. Now, the data type we use to represent HTTP bodies and header values is such that it uses regexps to automatically scrub anything that looks like IPv4 or IPv6 addresses or endpoints during its JSON serialization. I believe this change would make us robust against IPv4 vs IPv6 discovery issues.

This work was implemented in these pull requests:

bassosimone commented 1 year ago

Having documented the work done in this issue, we can now safely close it as done.