Commit Graph

210 Commits

Author SHA1 Message Date
Srijan Srivastava
f7fd29b246
geolocate: add cloudflare-based IP lookup (#676)
Cloudflare hosted services provide a certain service of `/cdn-cgi/trace` with their base url (for example, `www.cloudflare.com` or `www.nginx.com`), which can be used to obtain `ip` in the probe's `geolocate` feature.

The same feature was added in this pr, hence, increasing the number of `baseURL`s in `geolocate`.

Co-authored-by: Simone Basso <bassosimone@gmail.com>
2022-02-09 11:54:19 +01:00
Simone Basso
85664f1e31
feat(torsf): collect tor logs, select rendezvous method, count bytes (#683)
This diff contains significant improvements over the previous
implementation of the torsf experiment.

We add support for configuring different rendezvous methods after
the convo at https://github.com/ooni/probe/issues/2004. In doing
that, I've tried to use a terminology that is consistent with the
names being actually used by tor developers.

In terms of what to do next, this diff basically instruments
torsf to always rendezvous using domain fronting. Yet, it's also
possible to change the rendezvous method from the command line,
when using miniooni, which allows to experiment a bit more. In the
same vein, by default we use a persistent tor datadir, but it's
also possible to use a temporary datadir using the cmdline.

Here's how a generic invocation of `torsf` looks like:

```bash
./miniooni -O DisablePersistentDatadir=true \
           -O RendezvousMethod=amp \
           -O DisableProgress=true \
           torsf
```

(The default is `DisablePersistentDatadir=false` and
`RendezvousMethod=domain_fronting`.)

With this implementation, we can start measuring whether snowflake
and tor together can boostrap, which seems the most important thing
to focus on at the beginning. Understanding why the bootstrap most
often does not converge with a temporary datadir on Android devices
remains instead an open problem for now. (I'll also update the
relevant issues or create new issues after commit this.)

We also address some methodology improvements that were proposed
in https://github.com/ooni/probe/issues/1686. Namely:

1. we record the tor version;

2. we include the bootstrap percentage by reading the logs;

3. we set the anomaly key correctly;

4. we measure the bytes send and received (by `tor` not by `snowflake`, since
doing it for snowflake seems more complex at this stage).

What remains to be done is the possibility of including Snowflake
events into the measurement, which is not possible until the new
improvements at common/event in snowflake.git are included into a
tagged version of snowflake itself. (I'll make sure to mention
this aspect to @cohosh in https://github.com/ooni/probe/issues/2004.)
2022-02-07 17:05:36 +01:00
Yeganathan S
502ce1267a
fix(resolvermake): re-enable dns.google DoH HTTP3 resolutions (#680) 2022-01-31 19:12:43 +01:00
Simone Basso
2a566f2046
feat: start preparing for a cli release (#672)
This diff includes some final changes to be ready for blessing
a cli release. These changes are:

1. run `go generate ./...` to update the bundled CA

2. update the header we use for measuring

3. ensure `mk` uses the latest version of several tools

Reference issue: https://github.com/ooni/probe/issues/1845
2022-01-24 14:56:51 +01:00
Simone Basso
d92c1641ac
feat: start adding torsf to desktop and mobile (#671)
This commit message is the same across probe-cli, probe-desktop,
and probe-android. With the changes contained in the enclosed
diff, I'm starting to add support for torsf for android and for
desktop.

When smoke testing that torsf was WAI, I also noticed that its
progress messages in output are too frequent. We may want to do
better in a future version when we'll be able to read `tor`'s
output. In the meanwhile, make the progress messages less
frequent and indicated the maximum runtime inside of the messages
themselves. This improved message, albeit not so nice from the
UX PoV, should at least provide a clue that we're not stuck.

Reference issue: https://github.com/ooni/probe/issues/1917
2022-01-24 12:39:27 +01:00
Simone Basso
a01f901e13
feat(ooniprobe): add torsf to experimental group (#670)
Reference issue: https://github.com/ooni/probe/issues/1917.

I needed to change the summary key type returned by `torsf` to be a value. It seems the DB layer assumes that. If we pass it a pointer, it panics because it's experiment a value rather than a pointer 🤷.
2022-01-21 12:32:08 +01:00
Simone Basso
cfb054efd4
feat(snowflake): upgrade to v2 (+ small tweaks) (#667)
This diff contains the following changes and enhancements:

1. upgrade snowflake to v2

2. observe that we were not changing defaults from outside of snowflake.go, so remove code allowing to do that;

3. bump the timeout to 600 seconds (it seems 300 was not always enough based on my testing);

4. add useful knob to disable `torsf` progress (it's really annoying on console, we should do something about this);

5. ptx.go: avoid printing an error when the connection has just been closed;

6. snowflake: test AMP cache, see that it's not working currently, so leave it disabled.

Related issues: https://github.com/ooni/probe/issues/1845, https://github.com/ooni/probe/issues/1894, and https://github.com/ooni/probe/issues/1917.
2022-01-19 17:23:27 +01:00
Simone Basso
e904b90006
feature: merge measurex and netx archival layer (1/N) (#663)
This diff introduces a new package called `./internal/archival`. This package collects data from `./internal/model` network interfaces (e.g., `Dialer`, `QUICDialer`, `HTTPTransport`), saves such data into an internal tabular data format suitable for on-line processing and analysis, and allows exporting data into the OONI data format.

The code for collecting and the internal tabular data formats are adapted from `measurex`. The code for formatting and exporting OONI data-format-compliant structures is adapted from `netx/archival`.

My original objective was to _also_ (1) fully replace `netx/archival` with this package and (2) adapt `measurex` to use this package rather than its own code. Both operations seem easily feasible because: (a) this code is `measurex` code without extensions that are `measurex` related, which will need to be added back as part of the process; (b) the API provided by this code allows for trivially converting from using `netx/archival` to using this code.

Yet, both changes should not be taken lightly. After implementing them, there's need to spend some time doing QA and ensuring all nettests work as intended. However, I am planning a release in the next two weeks, and this QA task is likely going to defer the release. For this reason, I have chosen to commit the work done so far into the tree and defer the second part of this refactoring for a later moment in time. (This explains why the title mentions "1/N").

On a more high-level perspective, it would also be beneficial, I guess, to explain _why_ I am doing these changes. There are two intertwined reasons. The first reason is that `netx/archival` has shortcomings deriving from its original https://github.com/ooni/netx legacy. The most relevant shortcoming is that it saves all kind of data into the same tabular structure named `Event`. This design choice is unfortunate because it does not allow one to apply data-type specific logic when processing the results. In turn, this choice results in complex processing code. Therefore, I believe that replacing the code with event-specific data structures is clearly an improvement in terms of code maintainability and would quite likely lead us to more confidently change and evolve the codebase.

The second reason why I would like to move forward these changes is to unify the codepaths used for measuring. At this point in time, we basically have two codepaths: `./internal/engine/netx` and `./internal/measurex`. They both have pros and cons and I don't think we want to rewrite whole experiments using `netx`. Rather, what we probably want is to gradually merge these two codepaths such that `netx` is a set of abstractions on top of `measurex` (which is more low-level and has a more-easily-testable design). Because saving events and generating an archival data format out of them consists of at least 50% of the complexity of both `netx` and `measurex`, it seems reasonable to unify this archival-related part of the two codebases as the first step.

At the highest level of abstraction, these changes are part of the train of changes which will eventually lead us to bless `websteps` as a first class citizen in OONI land. Because `websteps` requires different underlying primitives, I chose to develop these primitives from scratch rather than wrestling with `netx`, which used another model. The model used by `websteps` is that we perform each operation in isolation and immediately we save the results, while `netx` creates whole data structures and collects all the events happening via tracing. We believe the model used by `websteps` to be better because it does not require your code to figure out everything that happened after the measurement, which is a source of subtle bugs in the current implementation. So, when I started implementing websteps I extracted the bits of `netx` that could also be beneficial to `websteps` into a separate library, thus `netxlite` was born.

The reference issue describing merging the archival of `netx` and `measurex` is https://github.com/ooni/probe/issues/1957. As of this writing the issue still references the original plan, which I could not complete by the end of this Sprint, so I am going to adapt the text of the issue to only refer to what was done in here next. Of course, I also need follow-up issues.
2022-01-14 12:13:10 +01:00
Simone Basso
b5da8be183
fix(netxlite): robust {ReadAll,Copy}Context with wrapped io.EOF (#661)
* chore(netxlite): add currently failing test case

This diff introduces a test cases that will fail because of the reason
explained in https://github.com/ooni/probe/issues/1965.

* chore(netxlite/iox_test.go): add failing unit tests

These tests directly show how the Go implementation of ReadAll
and Copy has the issue of checking for io.EOF equality.

* fix(netxlite): make {ReadAll,Copy}Context robust to wrapped io.EOF

The fix is simple: we just need to check for `errors.Is(err, io.EOF)`
after either io.ReadAll or io.Copy has returned. When this condition is
true, we need to convert the error back to `nil` as it ought to be.

While there, observe that the unit tests I committed in the previous
commit are wrongly asserting that the error must be wrapped. This
assertion is not correct, because in both cases we have just ensured
that the returned error is `nil` (i.e., success).

See https://github.com/ooni/probe/issues/1965.

* cleanup: remove previous workaround for wrapped io.EOF

These workarounds were partial, meaning that they would cover some
cases in which the issue occurred but not all of them.

Handling the problem in `netxlite.{ReadAll,Copy}Context` is the
right thing to do _as long as_ we always use these functions instead
of `io.{ReadAll,Copy}`.

This is why it's now important to ensure we clearly mention that
inside of the `CONTRIBUTING.md` guide and to also ensure that we're
not using these functions in the code base.

* fix(urlgetter): repair tests who assumed to see EOF error

Now that we have established that we should normalize EOF when
reading bodies like the stdlib does and now that it's clear why
our behavior diverged from the stdlib, we also need to repair
all the tests that assumed this incorrect behavior.

* fix(all): don't use io{,util}.{Copy,ReadAll}

* feat: add checks to ensure we don't use io.{Copy,ReadAll}

* doc(netxlite): document we know how to deal w/ wrapped io.EOF

* fix(nocopyreadall.bash): add exception for i/n/iox.go
2022-01-12 14:26:10 +01:00
Simone Basso
d3c6c11e48
cleanup(netx): remove the DNSClient type (#660)
The DNSClient type existed because the Resolver type did not
include CloseIdleConnections in its signature.

Now that Resolver includes CloseIdleConnections, the DNSClient
type has become unnecessary and can be safely removed.

See https://github.com/ooni/probe/issues/1956.
2022-01-10 11:53:06 +01:00
Simone Basso
730373cc75
refactor: move i/netx/archival structs to i/model (#659)
We recently started moving core data structures inside of the
internal/model package as detailed in https://github.com/ooni/probe/issues/1885.

The chief reason to do that is to have a set of fundamental
shared data types to help us rationalize the codebase.

This specific diff moves internal/netx/archival's core data types
inside the internal/model package. While there, it also refactors the
existing tests to improve their quality. Additionally, we also added
an extra test to ensure `ArchivalHTTPBody` is an alias for
`ArchivalMaybeBinaryData`, which is required to ensure the
custom JSON serialization process works for it.

We're doing that because both internal/netx/archival and
internal/measurex define their own archival data structures.

We developed measurex using its own structures because it
allowed to iterate more quickly. Now that we have sketched
out measurex, the time has come to consolidate.

My overall aim is to spend a few more hours this week on
engineering measurex. This work is preliminary work before
we finish up both measurex and websteps.

We described this cleanup in https://github.com/ooni/probe/issues/1957.
2022-01-10 11:25:52 +01:00
Simone Basso
554ae47c5a
cleanup(netx): remove more legacy names and functions (#658)
This diff addresses two items of https://github.com/ooni/probe/issues/1956:

> - [ ] we can remove legacy names from `./internal/engine/netx/resolver/legacy.go`
>
> - [ ] we can remove `DialTLSContext` from `./internal/engine/netx/resolver/tls_test.go`

More cleanups may follow.
2022-01-07 20:02:19 +01:00
Simone Basso
423a3feacc
cleanup(netx): remove unused ChainResolver (#657)
This is another cleanup point mentioned by https://github.com/ooni/probe/issues/1956.

While there, fix a bunch of comments in jafar that were incorrectly
referring to the netx package name.
2022-01-07 19:18:33 +01:00
Simone Basso
566c6b246a
cleanup: remove unnecessary legacy interfaces (#656)
This diff addresses another point of https://github.com/ooni/probe/issues/1956:

> - [ ] observe that we're still using a bunch of private interfaces for common interfaces such as the `Dialer`, so we can get rid of these private interfaces and always use the ones in `model`, which allows us to remove a bunch of legacy wrappers

Additional cleanups may still be possible. The more I cleanup, the more I see
there's extra legacy code we can dispose of (which seems good?).
2022-01-07 18:33:37 +01:00
Simone Basso
1c057d322d
cleanup: merge legacy errorsx in netxlite and hide classifiers (#655)
This diff implements the first two cleanups defined at https://github.com/ooni/probe/issues/1956:

> - [ ] observe that `netxlite` and `netx` differ in error wrapping only in the way in which we set `ErrWrapper.Operation`. Observe that the code using `netxlite` does not care about such a field. Therefore, we can modify `netxlite` to set such a field using the code of `netx` and we can remove `netx` specific code for errors (which currently lives inside of the `./internal/engine/legacy/errorsx` package
>
> - [ ] after we've done the previous cleanup, we can make all the classifiers code private, since there's no code outside `netxlite` that needs them

A subsequent diff will address the remaining cleanup.

While there, notice that there are failing, unrelated obfs4 tests, so disable them in short mode. (I am confident these tests are unrelated because they fail for me when running test locally from the `master` branch.)
2022-01-07 17:31:21 +01:00
Simone Basso
99ec7ffca9
fix: ensure experiments return nil when we want to submit (#654)
Since https://github.com/ooni/probe-cli/pull/527, if an experiment
returns an error, the corresponding measurement is not submitted since
the semantics of returning an error is that something fundamental
went wrong (e.g., we could not parse the input URL).

This diff ensures that all experiments only return and error when
something fundamental was wrong and return nil otherwise.

Reference issue: https://github.com/ooni/probe/issues/1808.
2022-01-07 13:17:20 +01:00
Simone Basso
60a3c372f5
cleanup: remove the original netx implementation (#653)
This commit completely removes the original netx implementation,
which was only used by `tor`, since this has changed in
https://github.com/ooni/probe-cli/pull/652.

The original netx implementation was my first attempt at performing
network measurements using Go. It started its life inside of the
https://github.com/ooni/netx repository. It was later merged into
the https://github.com/ooni/probe-engine repository. It finally
ended up into this repository when we merged probe-engine with it.

The main issue with the original implementation is that it was
a bit too complex and used channels where they were probably not
necessary. Because of that, later I introduced a second netx
implementation, which currently lives in ./internal/engine/netx.

The current netx implementation, the third one, lives in the
./internal/netxlite package. We are currently working to replace
the second implementation with the third one, but this is happening
at a slow pace. Also, the second implementation does not have big
maintenance concerns but it's just a bit too bureaucratic to use
since it involves creating lots of `Config` structures.

The reference issue is probably https://github.com/ooni/probe/issues/1688,
since this diff has been enabled by rewriting Tor to use `measurex`
(a library living on top of `netxlite`).
2022-01-05 19:00:50 +01:00
Simone Basso
dfa5e708fe
refactor(tor): rewrite using measurex (#652)
This diff rewrites the tor experiment to use measurex "easy" API.

To this end, we need to introduce an "easy" measurex API, which basically
performs easy measurements returning two pieces of data:

1. the resulting measurement, which is already using the OONI
archival data format and is always non-nil

2. a failure (i.e., the pointer to an error string), which
is nil on success and points to a string on failure

With this change, we should now be able to completely dispose of
the original netx API, which was only used by tor.

Reference issue: https://github.com/ooni/probe/issues/1688.
2022-01-05 18:41:11 +01:00
Simone Basso
f0181c432f
refactor: move httpx into the internal package (#646)
This concludes the TODO list at https://github.com/ooni/probe/issues/1951
2022-01-05 17:17:20 +01:00
Simone Basso
dba861d262
feat(httpx): implement optional body logging also on http error (#651)
1. we want optionally to log the body (we don't want to log the body
when we're fetching psiphon secrets or tor targets)

2. we want body logging to _also_ happen on error since this is quite
useful to debug possible errors when accessing the API

This diff adds the above functionality, which were previously
described in https://github.com/ooni/probe/issues/1951.

This diff also adds comprehensive testing.
2022-01-05 16:26:51 +01:00
Simone Basso
ee0aa18616
refactor(httpx): use mocks to implement tests (#650)
* refactor(httpx): use mocks to implement tests

While there, make sure no test depends on external services by
replacing such tests with httptest.

See https://github.com/ooni/probe/issues/1951.

* fix(httpx): ensure we honour the context
2022-01-05 16:13:42 +01:00
Simone Basso
93f084598e
feat(httpx): improve testing using the fakefiller (#649)
This diff extracts the fakefiller inside of internal/ooapi (a
currently unused package) into its own package.

The fakefiller knows how to fill many fields that are typically
shared as data structures across processes.

It is not perfect in that it cannot fill logger or http client
fields, but still helps with better filling and testing.

So, here we're using the fakefiller to improve testing of httpx
and, nicely enough, we've already catched a bug in the way in
which APIClientTemplate.Build misses to forward Authorization from
the original template. Yay!

Work part of https://github.com/ooni/probe/issues/1951
2022-01-05 14:49:31 +01:00
Simone Basso
eed51978ca
refactor(httpx): hide the real APIClient (#648)
As mentioned in https://github.com/ooni/probe/issues/1951, one of
the main issues I did see with httpx.APIClient is that in some cases
it's used in a very fragile way by probeservices.Client.

This happens in psiphon.go and tor.go, where we create a copy of
the APIClient and then modify it's Authorization field.

If we ever refactor probeservices.Client to take a pointer to
httpx.Client, we are now mutating the httpx.Client.

Of course, we don't want that to happen.

This diff attempts to address such a problem as follows:

1. we create a new APIClientTemplate type that holds the same
fields of an APIClient and allows to build an APIClient

2. we modify every user of APIClient to use APIClientTemplate

3. when we need an APIClient, we build it from the corresponding
template and, when we need to use a specific Authorization, we
use a build factory that sets APIClient.Authorization

4. we hide APIClient by renaming it apiClient and by defining
an interface called APIClient that allows to use it

So, now the codebase always uses the opaque APIClient interface to
issue API calls and always uses the APIClientTemplate to build an
opaque APIClient.

Boom! We have separated construction from usage and we are not
mutating in weird ways the APIClient anymore.
2022-01-05 14:15:42 +01:00
Simone Basso
7b7df2c6af
refactor(httpx): improve and modernize (1/n) (#647)
This PR starts to implement the refactoring described at https://github.com/ooni/probe/issues/1951. I originally wrote more patches than the ones in this PR, but overall they were not readable. Since I want to squash and merge, here's a reasonable subset of the original patches that will still be readable and understandable in the future.
2022-01-05 12:48:32 +01:00
Simone Basso
43161a8138
cleanup: remove redundant HTTPClient definition (#643)
This counts as a follow-up cleanup as part of doing
https://github.com/ooni/probe/issues/1885.
2022-01-03 16:47:54 +01:00
Simone Basso
273b70bacc
refactor: interfaces and data types into the model package (#642)
## Checklist

- [x] I have read the [contribution guidelines](https://github.com/ooni/probe-cli/blob/master/CONTRIBUTING.md)
- [x] reference issue for this pull request: https://github.com/ooni/probe/issues/1885
- [x] related ooni/spec pull request: N/A

Location of the issue tracker: https://github.com/ooni/probe

## Description

This PR contains a set of changes to move important interfaces and data types into the `./internal/model` package.

The criteria for including an interface or data type in here is roughly that the type should be important and used by several packages. We are especially interested to move more interfaces here to increase modularity.

An additional side effect is that, by reading this package, one should be able to understand more quickly how different parts of the codebase interact with each other.

This is what I want to move in `internal/model`:

- [x] most important interfaces from `internal/netxlite`
- [x] everything that was previously part of `internal/engine/model`
- [x] mocks from `internal/netxlite/mocks` should also be moved in here as a subpackage
2022-01-03 13:53:23 +01:00
Simone Basso
611fed05f4
[forwardport] release 3.11: update all the dependencies (#636) (#637)
* [forwardport] release 3.11: update all the dependencies (#636)

This diff forward ports e291e436b3c332300f5567796f9c48bb9bc1e652.

* chore: use go1.17.4 everywhere

Part of https://github.com/ooni/probe/issues/1843

* chore: update to the latest ooni/oohttp

Part of https://github.com/ooni/probe/issues/1843

* chore: update the dependencies

Note: I did an update and not an upgrade (i.e., I didn't check
whether we have next-major-versions of dependencies).

Part of https://github.com/ooni/probe/issues/1843

* chore: update the user-agent we use

Part of https://github.com/ooni/probe/issues/1843

* chore: update ooni/oohttp and ooni/probe-assets

Part of https://github.com/ooni/probe/issues/1843

* chore: run go generate again

Part of https://github.com/ooni/probe/issues/1843

* fix(jafar): skip currently broken test

Created issue for it here: https://github.com/ooni/probe/issues/1913
2021-12-06 17:46:13 +01:00
Simone Basso
dc9fbe9c64
fix(oonimkall): run tests with InputOrStaticDefault policy (#634)
Previous work to make https://github.com/ooni/probe/issues/1814
possible has broken running stunreachability on mobile.

This diff repairs the blunder and allows to run any experiment
using InputOrStaticDefault with oonimkall.

Diff extracted from https://github.com/ooni/probe-cli/pull/539.
2021-12-03 17:43:09 +01:00
Simone Basso
2044b78a5a
refactor: introduce and use InputOrStaticDefault (#632)
This commit introduces a new `InputLoader` policy by which, if no
input is provided, we use a static default input list.

We also modify the code to use this policy for dnscheck and
stunreachability, with proper input.

We also modify `miniooni` to pass the new `ExperimentName` field to
the `InputLoader` to indicate which default input list to use.

This diff is part of a set of diffs aiming at fixing
https://github.com/ooni/probe/issues/1814 and has been
extracted from https://github.com/ooni/probe-cli/pull/539.

What remains to be done, after this diff has landed is to ensure
things also work for ooniprobe and oonimkall.
2021-12-03 15:30:56 +01:00
Simone Basso
cba72d1ca3
refactor(stunreachability): input required and must be an URL (#630)
Here we're refactoring stunreachability to not provide internally a
default input and to take in input an URL rather than a string.

The related ooni/spec change is https://github.com/ooni/spec/pull/227.

This diff has been extracted from https://github.com/ooni/probe-cli/pull/539.

Because the original diff was large, I'm splitting it in a set of
more easily manageable diffs.

The reference issue is https://github.com/ooni/probe/issues/1814, which
is complex enough to require us to proceed incrementally.

This diff WILL need to be backported to release/3.11.
2021-12-03 14:27:04 +01:00
Simone Basso
9cdca4137d
forwardport: pull the patches mentioned in ooni/probe#1908 (#629)
* [forwardport] fix(oonimkall): make logger used by tasks unit testable (#623)

This diff forward ports e4b04642c51e7461728b25941624e1b97ef0ec83.

Reference issue: https://github.com/ooni/probe/issues/1903

* [forwardport] feat(oonimkall): improve taskEmitter testability (#624)

This diff forward ports 3e0f01a389c1f4cdd7878ec151aff91870a0bdff.

1. rename eventemitter{,_test}.go => taskemitter{,_test}.go because
the new name is more proper after we merged the internal/task package
inside of the oonimkall package;

2. rename runner.go's `run` function to `runTask`;

3. modify `runTask` to use the new `taskEmitterUsingChan` abstraction
on which we will spend more works in a later point of this list;

4. introduce `runTaskWithEmitter` factory that is called by `runTask`
and allows us to more easily write unit tests;

5. acknowledge that `runner` was not using its `out` field;

6. use the new `taskEmitterWrapper` in `newRunner`;

7. acknowledge that `runnerCallbacks` could use a generic
`taskEmitter` as field type rather than a specific type;

8. rewrite tests to use `runTaskWithEmitter` which leads to
simpler code that does not require a goroutine;

9. acknowledge that the code has been ignoring the `DisabledEvents`
settings for quite some time, so stop supporting it;

10. refactor the `taskEmitter` implementation to be like:

    1. we still have the `taskEmitter` interface;

    2. `taskEmitterUsingChan` wraps the channel and allows for
    emitting events using the channel;

    3. `taskEmitterUsingChan` owns an `eof` channel that is
    closed by `Close` (which is idempotent) and signals we
    should be stop emitting;

    4. make sure `runTask` creates a `taskEmitterUsingChan`
    and calls its `Close` method when done;

    5. completely remove the code for disabling events
    since the code was actually ignoring the stting;

    6. add a `taskEmitterWrapper` that adds common functions
    for emitting events to _any_ `taskWrapper`;

    7. write unit tests for `taskEmitterUsingChan` and
    for `taskEmitterWrapper`;

11. acknowledge that the abstraction we need for testing is
actually a thread-safe thing that collects events into a
vector containing events and refactor all tests accordingly.

See https://github.com/ooni/probe/issues/1903

* [forwardport] refactor(oonimkall): make the runner unit-testable (#625)

This diff forward ports 9423947faf6980d92d2fe67efe3829e8fef76586.

See https://github.com/ooni/probe/issues/1903

* [forwardport] feat(oonimkall): write unit tests for the runner component (#626)

This diff forward ports 35dd0e3788b8fa99c541452bbb5e0ae4871239e1.

Forward porting note: compared to 35dd0e3788b8fa99c541452bbb5e0ae4871239e1,
the diff I'm committing here is slightly different. In `master` we do not
have the case where a measurement fails and a measurement is returned, thus
I needed to adapt the test to become like this:

```diff
diff --git a/pkg/oonimkall/runner_internal_test.go b/pkg/oonimkall/runner_internal_test.go
index 334b574..84c7436 100644
--- a/pkg/oonimkall/runner_internal_test.go
+++ b/pkg/oonimkall/runner_internal_test.go
@@ -568,15 +568,6 @@ func TestTaskRunnerRun(t *testing.T) {
                }, {
                        Key:   failureMeasurement,
                        Count: 1,
-               }, {
-                       Key:   measurement,
-                       Count: 1,
-               }, {
-                       Key:   statusMeasurementSubmission,
-                       Count: 1,
-               }, {
-                       Key:   statusMeasurementDone,
-                       Count: 1,
                }, {
                        Key:   statusEnd,
                        Count: 1,
```

I still need to write more assertions for each emitted event
but the code we've here is already a great starting point.

See https://github.com/ooni/probe/issues/1903

* [forwardport] refactor(oonimkall): merge files, use proper names, zap unneeded integration tests (#627)

This diff forward ports f894427d24edc9a03fc78306d0093e7b51c46c25.

Forward porting note: this diff is slightly different from the original
mentioned above because it carries forward changes mentioned in the
previous diff caused by a different way of handling a failed measurement
in the master branch compared to the release/3.11 branch.

Move everything that looked like "task's model" inside of the
taskmodel.go file, for consistency.

Make sure it's clear some variables are event types.

Rename the concrete `runner` as `runnerForTask`.

Also, remove now-unnecessary (and flaky!) integration tests
for the `runnerForTask` type.

While there, notice there were wrong URLs that were generated
during the probe-engine => probe-cli move and fix them.

See https://github.com/ooni/probe/issues/1903

* [forwardport] refactor(oonimkall): we can simplify StartTask tests (#628)

This diff forward ports dcf2986c2032d8185d58d24130a7f2c2d61ef2fb.

* refactor(oonimkall): we can simplify StartTask tests

We have enough checks for runnerForTask. So we do not need to
duplicate them when checking for StartTask.

While there, refactor how we start tasks to remove the need for
extra runner functions.

This is the objective I wanted to achieve for oonimkall:

1. less duplicate tests, and

2. more unit tests (which are less flaky)

At this point, we're basically done (pending forwardporting to
master) with https://github.com/ooni/probe/issues/1903.

* fix(oonimkall): TestStartTaskGood shouldn't cancel the test

This creates a race condition where the test may fail if we cannot
complete the whole "Example" test in less than one second.

This should explain the build failures I've seen so far and why
I didn't see those failures when running locally.
2021-12-02 12:47:07 +01:00
Simone Basso
ece6f3d48d
fix(websteps, webconnectivity): send the correct user agent (#616)
* [forwardport] fix(webconnectivity): send specific user agent (#615)

This forward ports b8c530388e66b2cc86abad26d077202782e4a823 to `master`.

See https://github.com/ooni/probe/issues/1902

* fix(websteps): send the correct user agent

Also related to https://github.com/ooni/probe/issues/1902: let's just
ensure that also websteps behaves in the correct way.
2021-11-26 19:20:24 +01:00
Simone Basso
264e30f016
[forwardport] fix(mk): do nothing if we already have psiphon config (#605) (#606)
This diff forward ports ea44e99451f345474738b9010ff791759a1f1367.

Original commit message:

- - -

This change allows for producing cloud builds using the psiphon
config files. We will add those files as build secrets. Only people
in the organization and collaborators with at least "write"
access could trigger builds containing such secrets.

Before this change, `./mk` unconditionally attempted to clone
github.com/ooni/probe-private. Now, it only checks whether
we need to clone _if_ files are not already there.

This allows us to use GitHub actions and secrets to copy the
files in there _without_ needing to clone a private repo.

Cloning a private repo would require us to include as repository
secret an access token with full `repo` scope, which is a very
broad scope. Instead, by using secrets to include psiphon config,
we are narrowing down the secrets required to make a release build.

See https://github.com/ooni/probe/issues/1878

This diff WILL require forward porting to the master branch.
2021-11-19 12:40:10 +01:00
Simone Basso
0cb535b022
[forwardport] fix(signal): use the correct string to score a measurement (#602) (#603)
This diff forwardports 3b1cc1b6afbc29fbc9ff8221d2cd84f34032df60.

Original commit message:

- - -

See https://github.com/ooni/probe/issues/1858#issuecomment-970322363

This diff will need forward porting to master.
2021-11-16 15:57:16 +01:00
Simone Basso
c527ca1f61
fix(psiphon): increase the maximum runtime to 300s (#596)
See: https://github.com/ooni/probe/issues/1856.

This diff will need to be backported to release/3.11.
2021-11-15 12:43:19 +01:00
Simone Basso
5a481b395a
fix(geolocate): use powerdns.org whoami service (#592)
This diff needs to be backported to the release/3.11 branch.

Reference issue https://github.com/ooni/probe/issues/1865.
2021-11-12 14:53:15 +01:00
Simone Basso
0a322ebab0
[forwardport] fix: avoid http3 for dns.google and www.google.com (#593) (#594)
This commit forward ports dedd84fa7ecb09f718f6b1a9c83999cb37b34dfa.

Original commit message:

- - -

This diff changes code the release/3.11 branch to ensure we're not using dns.google and www.google.com over HTTP3. As documented in https://github.com/ooni/probe/issues/1873, since this morning (approx) these services do not support HTTP3 anymore. (I didn't bother with checking whether this issue affects _other_ Google services; I just limited my analysis to the services that we were using as part of testing.)

This patch WILL require forward porting to the master branch.
2021-11-12 14:43:28 +01:00
Simone Basso
ba7b981fcb
fix(measurex): allow API user to choose parallelism (#581)
Closes https://github.com/ooni/probe/issues/1818
2021-11-05 14:37:03 +01:00
Simone Basso
3b27780836
fix(webconnectivity): ignore any status code <= 0 (#579)
This diff changes the algorithm used by webconnectivity's
httpanalysis.go to ignore any status code <= 0 rather
than just ignoring the == 0 case.

Make sure we add test cases for when the control's status
code is negative rather than being zero.

While there, simplify code where boolean checks could be
more compact according to staticcheck.

Closes https://github.com/ooni/probe/issues/1825
2021-11-05 13:51:22 +01:00
Simone Basso
1f348803dc
fix(websteps): use a single Do53 resolver (#575)
Closes https://github.com/ooni/probe/issues/1807

While there, bump the experiment version number, because we have
applied changes since the last alpha release.
2021-11-05 11:40:58 +01:00
Simone Basso
dad8db4a36
fix(httpheader): use same Accept-Language as Brave (#574)
Closes https://github.com/ooni/probe/issues/1792
2021-11-05 11:31:10 +01:00
Simone Basso
aa27bbe33f
fix(measurex): use same keys of the OONI data format (#572)
This change should simplify the pipeline's job.

Reference issue: https://github.com/ooni/probe/issues/1817.

I previously dismissed this possibility, but now it seems clear it
is simpler to have a very tabular data format internally and to
convert such a format to OONI's data format when serializing.

The OONI data format is what the pipeline expects, but processing
is easier with a more linear/tabular format.
2021-11-05 10:46:45 +01:00
Simone Basso
f91de2ecd6
cleanup: move bogon checking code in netxlite (#562)
I develop this diff while working on https://github.com/ooni/probe/issues/1803#issuecomment-957323297.

While there, make sure we don't have duplicate bogon code
and always use the code inside netxlite.
2021-11-02 12:20:04 +01:00
Simone Basso
d44970a43f
master: update deps and set version to 3.12.0-alpha (#548)
* chore: update all workflows to use go1.17.2

See https://github.com/ooni/probe/issues/1815

* chore: update all dependencies

See https://github.com/ooni/probe/issues/1815

* chore: run `go generate`

See https://github.com/ooni/probe/issues/1815

* chore: update the user-agent

Part of https://github.com/ooni/probe/issues/1815

* Set version to 3.12.0-alpha

Part of https://github.com/ooni/probe/issues/1815

* fix: update to ooni/probe-assets@v0.5.0

This overcomes https://github.com/ooni/probe/issues/1836 in the CLI and,
while there, let us also make maxminddb tests stricter.

* fix(QA/Dockerfile): build using go1.17

See https://github.com/ooni/probe-cli/pull/547#issuecomment-947760839

* chore(mk): use go1.17.2

Part of https://github.com/ooni/probe/issues/1815

* fix(codeql): always run for master

Otherwise we see a warning that there is no CodeQL information
available for the base branch and this is sub-optimal.

Part of https://github.com/ooni/probe/issues/1815
2021-10-20 18:16:40 +02:00
Simone Basso
299834174a
fix(webconnectivity): gather longer HTML titles (#542)
Allows us to get http://www.isa.gov.il/Pages/default.aspx's one.

Discovered when working on https://github.com/ooni/probe/issues/1707.
2021-10-13 13:50:22 +02:00
Simone Basso
43c349fe38
feat: annotate measurements with their architecture (#540)
Closes https://github.com/ooni/probe/issues/1772
2021-10-13 10:31:46 +02:00
Simone Basso
a6318d6de9
feat(webconnectivity): collect timing information (#537)
Work related to https://github.com/ooni/probe/issues/1797
2021-10-05 12:52:19 +02:00
Simone Basso
3f511d100b
fix(websteps): use correct input for each URL measurement (#536)
Closes https://github.com/ooni/probe/issues/1798
2021-10-05 12:29:00 +02:00
Simone Basso
ba9151d4fa
feat(webstepsx): websteps using measurex (#530)
This diff adds the prototype websteps implementation that used
to live at https://github.com/ooni/probe-cli/pull/506.

The code is reasonably good already and it's pointing to a roaming
test helper that I've properly configured.

You can run websteps with:

```
./miniooni -n websteps
```

This will go over the test list for your country.

At this stage the mechanics of the experiment is set, but we
still need to have a conversation on the following topics:

1. whether we're okay with reusing the data format used by other
OONI experiments, or we would like to use a more compact data
format (which may either be a more compact JSON or we can choose
to always submit compressed measurements for websteps);

2. the extent to which we would like to keep the measurement as
a collection of "the experiment saw this" and "the test helper
saw that" and let the pipeline choose an overall score: this is
clearly an option, but there is also the opposite option to
build a summary of the measurement on the probe.

Compared to the previous prototype of websteps, the main
architectural change we have here is that we are following
the point of view of the probe and the test helper is
much more dumb. Basically, the probe will choose which
redirection to follow and ask the test helper every time
it discovers a new URL to measure it w/o redirections.

Reference issue: https://github.com/ooni/probe/issues/1733
2021-09-30 02:06:27 +02:00
Simone Basso
ff1c170562
feat(engine): allow runner to return many measurements (#527)
This is required to implement websteps, which is currently tracked
by https://github.com/ooni/probe/issues/1733.

We introduce the concept of async runner. An async runner will
post measurements on a channel until it is done. When it is done,
it will close the channel to notify the reader about that.

This change causes sync experiments now to strictly return either
a non-nil measurement or a non-nil error.

While this is a pretty much obvious situation in golang, we had
some parts of the codebase that were not robust to this assumption
and attempted to submit a measurement after the measure call
returned an error.

Luckily, we had enough tests to catch this change in our assumption
and this is why there are extra docs and tests changes.
2021-09-30 00:54:52 +02:00