This diff modifies webconnectivity@v0.5 to take decisions regarding
TLS blocking by using the response from the TH rather than using
questionable heuristics based on inspecting the TLSHandshake list
alone. This change should improve correctness _when_ we're using
the improved TH, which is currently used for 50% of the probes.
See https://github.com/ooni/probe/issues/2257
While there, modify `control.go` to specify which control is being used.
As silly as it seems, emojis help _a lot_ when eyeballing logs
to quickly identify unexpected lines.
I'm doing this work as part of https://github.com/ooni/probe/issues/2257
The residual censorship algorithm prevents dnscheck for checking the
same endpoint again in a short time frame under the assumption that a
previous measurement could have caused residual censorship.
In https://github.com/ooni/probe/issues/2234, we mentioned we probably
wanted to disable this algorithm because we didn't want to slow down
dnscheck too much and because we didn't detect this alleged source of
blocking in our previous measurements.
It turns out the algorithm was already disabled. I probably did this
after we published the paper on DNS censorship to make measurements
overall a bit faster.
So, just introduce a new extension field to the measurement telling us
that the residual censorship algorithm is disabled.
It's not super useful except as for the fact that the next time I
read the source code I notice that the algorithm is disabled.
Most of miniooni panics on errors. We should not panic on error with
--repeat-every, rather we should try the next measurement.
See https://github.com/ooni/probe/issues/2250
Part of https://github.com/ooni/probe/issues/2184, because I wanted
to allow swapping commands and options more freely.
As a side effect, this PR closes https://github.com/ooni/probe/issues/2248.
AFAICT, every usage that was legal before is still legal. What has
changed seems the freedom to swap commands and options and a much
better help that lists the available options.
This diff splits miniooni's implementation in smaller and more
easily tractable blocks ahead of future refactoring.
I'm trying to make `miniooni oonirun -i URL` as possible as
`miniooni -i URL oonirun`, because users typically expect this
kind of flexibity from modern Unix commands.
Part of https://github.com/ooni/probe/issues/2184
Closes https://github.com/ooni/probe/issues/2183
While there, avoid exposing nil values for optional fields of the
THResponse struct (i.e., "ip_info" and "tls_handshake").
While there, fix `measurexlite`'s `OperationLogger` test
and make it deterministic rather than racy.
This commit moves the TH structs and definitions to model. We don't want
oohelperd to depend on web_connectivity@v0.4.
Part of https://github.com/ooni/probe/issues/2240
A bunch of packages (including oohelperd) just need the ability to
use MaxMind-like databases. They don't need the additional functionality
implemented by the geolocate package. Such a package, in fact, is
mostly (if not only) needed by the engine package.
Therefore, move code to query MaxMind-like databases to a separate
package, and avoid depending on geolocate in all the packages for
which it's sufficient to use geoipx.
Part of https://github.com/ooni/probe/issues/2240
This diff introduces the following `oohelperd` enhancements:
1. measure both IP addresses resolved by the TH and IP addresses resolved by the probe;
2. when the URL scheme is http and there's no explicit port, measure both 80 and 443 (which will pay off big once we introduce support for optionally performing TLS handshakes);
3. include information about the probe and TH IP addresses into the results: who resolved each IP address, whether an address is a bogon, the ASN associated to an address.
This diff is part of https://github.com/ooni/probe/issues/2237
This diff refactors oohelperd to make performing additional measurements easier. We need:
1. to run the DNS task _before_ other tasks such that we can measure both IP addresses returned by the TH and the ones returned by the probe. When we'll introduce TLS measurements, this will allow us to validate probe-provided IP addresses inside the TH call. If probe-provided addresses work with TLS, they are legitimate for the domain.
2. to tie the number of TCP measurements to a list of endpoints collected by the probe _or_ the TH rather than just to the one provided by the probe. Anticipating this change, let us refactor how we read the results of the TCP task to make it independent of the number of addresses provided by the probe.
This work is part of https://github.com/ooni/probe/issues/2237
While working on https://github.com/ooni/probe/issues/2237, I noticed
there's no enforced timeout for measurement tasks.
So, this diff introduces the following timeouts:
1. use a 4 seconds timeout for the DNS lookup;
2. use a 10 seconds timeout for TCP;
3. use a 15 seconds timeout for HTTP.
They are a bit stricter than what we have on the probe because the TH
should supposedly have better bandwidth and connectivity.
See what we documented at https://github.com/ooni/spec/pull/257
Reference issue: https://github.com/ooni/probe/issues/2238
See also the related ooni/spec PR: https://github.com/ooni/spec/pull/257
See also https://github.com/ooni/probe/issues/2237
While there, bump webconnectivity@v0.5 version because this change
has an impact onto the generated data format.
The drop in coverage is unavoidable because we've written some
tests for `measurex` to ensure we deal with DNS resolvers and transport
names correctly depending on the splitting policy we use.
(However, `measurex` is only used for the `tor` experiment and, per
the step-by-step design document, new experiments should use
`measurexlite` instead, so this is hopefully fine(TM).)
While there, fix a broken integration test that does not run in `-short` mode.
In a pure step-by-step model, we don't need to trace HTTP round trips like we did before. We _may_ want in the future to also have some form of HTTP tracing (see https://github.com/ooni/probe-cli/pull/868 for a prototype) but doing that is currently not in scope for moving forward the step-by-step design. For this reason, I only added a public convenience function for formatting an OONI spec compatible request. I also added new fields, which should be documented inside the ooni/spec repository (see https://github.com/ooni/probe/issues/2238).
Required by https://github.com/ooni/probe/issues/2237
The T0 field is the moment when we started collecting data, while T
is the moment when we finished collecting data.
The TransactionID field will be repurposed for step-by-step measurements
to indicate related observations collected as part of the same flow
(e.g., TCP+TLS+HTTP).
Note that, for now, this change will only affect measurexlite and we're
not planning on changing other libraries for measuring.
Part of https://github.com/ooni/probe/issues/2137
* feat: save CNAME into archival data format
When a DNSResponse contains a non-empty CNAME, we include it
into the related list of answers.
Closes https://github.com/ooni/probe/issues/2227
* doc: add design note
While there, make code more compact and robust to a case where
we're going to extract additional answers.
* doc: document the expected growth of extraction function
Based on feedback by @DecFox
* feat(dnsovergetaddrinfo): collect the CNAME
This diff modifies how dnsovergetaddrinfo.go works such that the
returned DNSResponse includes the CNAME.
Closes https://github.com/ooni/probe/issues/2226.
While there, recognize that we can remove getaddrinfoLookupHost and
always call getaddrinfoLookupANY everywhere. (This simplification is
why we did https://github.com/ooni/probe-cli/pull/874.)
* fix: extra debugging because of failing CI
Everything is OK locally (on macOS). However, maybe things are a bit
different on GNU/Linux perhaps?
Here's the error:
```
--- FAIL: TestPass (0.11s)
resolver_test.go:113: unexpected rcode
FAIL
coverage: 95.7% of statements
FAIL github.com/ooni/probe-cli/v3/internal/cmd/jafar/resolver 0.242s
```
I'm a bit confused because jafar's resolver is _unrelated_. But actually this
error never occurred again after a committed the debugging diff.
* feat(netxlite): support extracting the CNAME
Closes https://github.com/ooni/probe/issues/2225
* fix(netxlite): attempt to increase coverage and improve tests
1. dnsovergetaddrinfo: specify the behavior of a DNSResponse returned
by this file to make it line with normal responses and write unit tests
to make sure we adhere to expectations;
2. dnsoverudp: make sure we wait to deferred responses also w/o a
custom context and post on a private channel and test that;
3. utls: recognize that we can actually write a test for NetConn and
what needs to change when we'll use go1.19 by default will just be
a cast that at that point can be removed.
* cleanup: remove UnderlyingNetworkLibrary and TProxy
While there, replace mixture of mocking and real connections inside
quicping with pure mocking of network connections.
Closes https://github.com/ooni/probe/issues/2224
* cleanup: we don't need a SimpleResolver now
This type was only used by UnderlyingNetworkLibrary and all the
rest of the code uses Resolver. So, let's avoid complexity by zapping
the SimpleResolver type and merging it inside Resolver.
* upgrade to our go.mod enabled of psiphon-tunnel-core such that
we're now using v2.0.24 of the tunnel-core;
* upgrade to the latest lucas-clemente/quic-go release;
* upgrade to the latest ooni/oohttp release (which is based on go1.19
but the diff seems good enough to continue using go1.18.x as well);
* upgrade to the latest ooni/oocrypto release (for which we can make the
same remarks regarding using go1.18.x);
* deal with changes in lucas-clemente/quic-go API as well as changes
in what a go1.19 *tls.Conn compatible type should look like.
Unfortunately, we cannot switch to go1.19 because psiphon forks quic-go
and their fork's still not building using such a version of go.
Part of ooni/probe#2211.
Skip options that begin with the `Safe` prefix from appearing in the
serialization of a Measurement that will be submitted to the OONI
backend.
Fixes https://github.com/ooni/probe/issues/2214
I made a mistake while adapting code from an experimental branch thus
breaking these two experiments because of interface conversion.
This diff fixes it.
While there, remove the panic trap for miniooni. Because miniooni is
an experimental tool, we want to see the full panic text, which definitely
leads to a more pleasant and effective debugging experience.
See https://github.com/ooni/probe/issues/2216 for context on why we
were trying to change how we register experiments.
The broken commit is 6a0ae5c70b.
* feat: add support for system resolver in measurexlite
* more tests for coverage
* Apply suggestions from code review
Co-authored-by: decfox <decfox@github.com>
Co-authored-by: Simone Basso <bassosimone@gmail.com>
Until OONI Run v2 has support for repeating the measurement with a schedule, introduce a command line flag requested by users to repeat a measurement every given number of seconds.
Part of https://github.com/ooni/probe/issues/2184
This diff adds support for running OONIRun v1 links.
Run with `miniooni` using:
```
./miniooni -i LINK oonirun
```
Part of https://github.com/ooni/probe/issues/2184
This diff refactors the ./internal/cmd/miniooni pkg and moves the code
for running experiments inside of the ./internal/oonirun pkg.
It's the first concrete step towards https://github.com/ooni/probe/issues/2184.
The integration test that was broken was:
```
--- FAIL: TestCreateInvalidExperiment (0.35s)
experiment_integration_test.go:192: expected a nil builder here
```
While there improve the documentation of the ExperimentSession
and see there's a method that we are not using.
This diff is a cleanup that I come up with while working
on https://github.com/ooni/probe/issues/2184.
This option has been disabled for a long time and we said in the
codebase we were going to remove it after 2021-11-01.
So, it feels okay to remove it.
This diff is a cleanup in preparation for https://github.com/ooni/probe/issues/2184.
This diff modifies the engine package to make Experiment and
ExperimentBuilder interfaces rather than structs.
The previosuly existing structs are now named experiment{,Builder}.
This diff helps https://github.com/ooni/probe/issues/2184
because it allows us to write unit tests more easily.
There should be no functional change.
While there, I removed a bunch of deprecated functions, which were
unnecessarily complicate the implementation and could be easily
replaced by passing them a context.Context or context.Background().
This diff refactors how we set options for experiments to accept
in input an any value or a map[string]any, depending on which method
we choose to actually set options.
There should be no functional change, except that now we're not
guessing the type and then attempting to set the value of the selected
field: now, instead, we match the provided type and the field's type
as part of the same function (i.e., SetOptionAny).
This diff is functional to https://github.com/ooni/probe/issues/2184,
because it will allow us to load options from a map[string]any,
which will be part of the OONI Run v2 JSON descriptor.
If we didn't apply this change, we would only have been to set options
from a map[string]string, which is good enough as a solution for the
CLI but is definitely clumsy when you have to write stuff like:
```JSON
{
"options": {
"HTTP3Enabled": "true"
}
}
```
when you could instead more naturally write:
```JSON
{
"options": {
"HTTP3Enabled": true
}
}
```
This diff makes the implementation of the engine package more
abstract by changing HTTPClient() to return a model.HTTPClient
as opposed to returning an *http.Client.
Part of https://github.com/ooni/probe/issues/2184
In https://github.com/ooni/probe-cli/pull/832's initial diff, I
mentioned it would be cool to flatten oohelperd's hier.
I'm doing this now, and just for the master branch.
This diff is mostly a mechanical refactoring with very light
and apparently rather safe manual changes.
This diff modifies the implementation of oohelperd in the master branch
to always use throw-away HTTPClient, Dialer, and Resolver.
The rationale of this change is to ensure we're not hitting limits of the
HTTPClient regarding the max number of connections per host.
This issue is described at https://github.com/ooni/probe/issues/2182.
While there, it feels more correct to use throw-away Dialer and Resolver.
We have a different patch for the release/3.15 branch because of
netx-related refactorings: https://github.com/ooni/probe-cli/pull/832.
## 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/2158
- [x] if you changed anything related how experiments work and you need to reflect these changes in the ooni/spec repository, please link to the related ooni/spec pull request: https://github.com/ooni/spec/pull/250
## Description
This diff refactors the codebase to reimplement tlsping and tcpping
to use the step-by-step measurements style.
See docs/design/dd-003-step-by-step.md for more information on the
step-by-step measurement style.
This pull request publishes the step-by-step design document that I have been discussing with @hellais and @DecFox recently. Compared to the document that was approved, this one has been edited for readability.
While there, I figured it was also be beneficial to publish the few ooni/probe-cli related design documents we produced in the past, because they probably help someone to get acquainted with the codebase.
Reference issue for this pull request: https://github.com/ooni/probe/issues/2148
This diff addresses the following points of https://github.com/ooni/probe/issues/2135:
- [x] the `childResolver` type is useless and we can use `model.Resolver` directly;
- [x] we should use `model/mocks` instead of custom fakes;
- [x] we should not use `log.Log` rather we should use `model.DiscardLogger`;
- [x] make `timeLimitedLookup` easier to test with a `-short` tests;
- [x] ensure `timeLimitedLookup` returns as soon as its context expires regardless of the child resolver;
Subsequent diffs will address more points mentioned in there.
The oohelperd implementation did not actually need using netx because
it was just constructing default types with logging, which is what
netxlite already does. Hence, let's avoid using netx here.
See https://github.com/ooni/probe/issues/2121
The oohelper does not need to use netx and it's enough to use
netxlite, hence let us apply this refactor.
The original code used DoT but the explanatory comment said we were
using DoT because of unclear issues inside GitHub actions.
We are now using DoH and this is fine as well. The comment implied
that any encrypted transport would do.
See https://github.com/ooni/probe/issues/2121
This diff forward ports 261d1a4cdc88522f6a8f63d6c540f51054566b28 to master
whose original commit message follows:
- - -
It's not working for me from a couple of places and also it does not
seem to be documented upstream, see:
https://docs.namebase.io/guides-1/resolving-handshake-1/hdns.io
This diff WILL need to be forwardported to master.
This diff refactors netx and netxlite to ensure we're not using
netxlite legacy names inside of netx.
To this end, we're cheating a bit. We're exposing a new factory to
get an unwrapped stdlib resolver rather than defining a legacy name
to export the private name of the same factory.
This is actually a fine place to stop, for now, the next and
netxlite refactoring at https://github.com/ooni/probe/issues/2121.
Before finishing the ongoing refactoring and leaving whatever
is left of netx in tree, I would like to restructure it so that
we'll have an easy time next time we need to modify it.
Currently, every functionality lives into the `netx.go` file and
we have a support file called `httptransport.go`.
I would like to reorganize by topic, instead. This would allow
future me to more easily perform topic-specific changes.
While there, improve `netx`'s documentation and duplicate some of
this documentation inside `internal/README.md` to provide pointers
to previous documentation, historical context, and some help to
understand the logic architecture of network extensions (aka `netx`).
Part of https://github.com/ooni/probe-cli/pull/396
Now that we have properly refactored the caching resolvers we can
move them into netxlite as optional resolvers created using the
proper abstract factories we just added.
This diff reduces the complexity and the code size of netx.
See https://github.com/ooni/probe/issues/2121.
For testability, replace most if-based construction logic with
calls to well-tested factories living in other packages.
While there, acknowledge that a bunch of types could now be private
and make them private, modifying the code to call the public
factories allowing to construct said types instead.
Part of https://github.com/ooni/probe/issues/2121
This diff modifies netx to stop using most netxlite resolver internals
but the internal function that creates a new, unwrapped system resolver,
which will be dealt with in a subsequent pull request.
See https://github.com/ooni/probe/issues/2121
1. Use the netxlite.NewHTTPTransport factory for creating a new
HTTP2 (and HTTP1) transport;
2. Recognize the netxlite.NewOOHTTPTransport has now become
an implementation detail so make it private;
3. Recognize that netxlite.NewHTTP3Transport should call
netxlite.WrapTransport so it returns the same typechain
returned by netxlite.NewHTTPTransport (modulo, of course,
the real underlying transport), so ensure that we are
calling netxlite.WrapTransport in NewHTTP3Transport;
4. Recognize that the table based constructor inside of
netx needs a logger to create HTTPTransport instances using
either netxlite.NewHTTP{,3}Transport so pass this argument
along and ensure it's not nil using a constructor inside
model that guarantees that;
5. Cleanup netx's tests to avoid type asserting on the
typechains returned by netxlite since we already test
that inside netxlite;
6. Recognize that now we can make more legacy names inside
of netxlite private because we don't need to use them
inside tests anymore (because of previous point).
Reference issue: https://github.com/ooni/probe/issues/2121
