Skip to content

Commit

Permalink
fix(webconnectivitylte): don't use a DoH URL provider singleton (ooni…
Browse files Browse the repository at this point in the history 
…#1508)

Using a singleton makes tests non-deterministic. Instead use an instance
for each measurer.

Helps with ooni/probe#2677.
  • Loading branch information
@bassosimone @Murphy-OrangeMud
bassosimone authored and Murphy-OrangeMud committed Feb 13, 2024
1 parent 30d5a17 commit d0b85d0
Show file tree
Hide file tree
Showing 200 changed files with 31,756 additions and 7,729 deletions.
36 changes: 21 additions & 15 deletions internal/experiment/webconnectivitylte/cleartextflow.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import (
"github.com/ooni/probe-cli/v3/internal/model"
"github.com/ooni/probe-cli/v3/internal/netxlite"
"github.com/ooni/probe-cli/v3/internal/throttling"
"github.com/ooni/probe-cli/v3/internal/webconnectivityalgo"
)

// Measures HTTP endpoints.
Expand All @@ -34,6 +35,10 @@ type CleartextFlow struct {
// DNSCache is the MANDATORY DNS cache.
DNSCache *DNSCache

// DNSOverHTTPSURLProvider is the MANDATORY provider of DNS-over-HTTPS
// URLs that arranges for periodic measurements.
DNSOverHTTPSURLProvider *webconnectivityalgo.OpportunisticDNSOverHTTPSURLProvider

// Depth is the OPTIONAL current redirect depth.
Depth int64

Expand Down Expand Up @@ -323,21 +328,22 @@ func (t *CleartextFlow) maybeFollowRedirects(ctx context.Context, resp *http.Res
}
t.Logger.Infof("redirect to: %s", location.String())
resolvers := &DNSResolvers{
CookieJar: t.CookieJar,
Depth: t.Depth + 1,
DNSCache: t.DNSCache,
Domain: location.Hostname(),
IDGenerator: t.IDGenerator,
Logger: t.Logger,
NumRedirects: t.NumRedirects,
TestKeys: t.TestKeys,
URL: location,
ZeroTime: t.ZeroTime,
WaitGroup: t.WaitGroup,
Referer: resp.Request.URL.String(),
Session: nil, // no need to issue another control request
TestHelpers: nil, // ditto
UDPAddress: t.UDPAddress,
CookieJar: t.CookieJar,
Depth: t.Depth + 1,
DNSOverHTTPSURLProvider: t.DNSOverHTTPSURLProvider,
DNSCache: t.DNSCache,
Domain: location.Hostname(),
IDGenerator: t.IDGenerator,
Logger: t.Logger,
NumRedirects: t.NumRedirects,
TestKeys: t.TestKeys,
URL: location,
ZeroTime: t.ZeroTime,
WaitGroup: t.WaitGroup,
Referer: resp.Request.URL.String(),
Session: nil, // no need to issue another control request
TestHelpers: nil, // ditto
UDPAddress: t.UDPAddress,
}
resolvers.Start(ctx)
}
Expand Down
89 changes: 43 additions & 46 deletions internal/experiment/webconnectivitylte/dnsresolvers.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,10 @@ type DNSResolvers struct {
// DNSCache is the MANDATORY DNS cache.
DNSCache *DNSCache

// DNSOverHTTPSURLProvider is the MANDATORY provider of DNS-over-HTTPS
// URLs that arranges for periodic measurements.
DNSOverHTTPSURLProvider *webconnectivityalgo.OpportunisticDNSOverHTTPSURLProvider

// Domain is the MANDATORY domain to resolve.
Domain string

Expand Down Expand Up @@ -297,20 +301,11 @@ func (t *DNSResolvers) udpAddress() string {
return webconnectivityalgo.RandomDNSOverUDPResolverEndpointIPv4()
}

// OpportunisticDNSOverHTTPSSingleton is the singleton used to keep
// track of the opportunistic DNS-over-HTTPS measurements state.
var OpportunisticDNSOverHTTPSSingleton = webconnectivityalgo.NewOpportunisticDNSOverHTTPSURLProvider(
"https://mozilla.cloudflare-dns.com/dns-query",
"https://dns.nextdns.io/dns-query",
"https://dns.google/dns-query",
"https://dns.quad9.net/dns-query",
)

// lookupHostDNSOverHTTPS performs a DNS lookup using a DoH resolver. This function must
// always emit an ouput on the [out] channel to synchronize with the caller func.
func (t *DNSResolvers) lookupHostDNSOverHTTPS(parentCtx context.Context, out chan<- []string) {
// obtain an opportunistic DoH URL
URL, good := OpportunisticDNSOverHTTPSSingleton.MaybeNextURL()
URL, good := t.DNSOverHTTPSURLProvider.MaybeNextURL()
if !good {
// no need to perform opportunistic DoH at this time but we still
// need to fake out a lookup to please our caller
Expand Down Expand Up @@ -384,23 +379,24 @@ func (t *DNSResolvers) startCleartextFlows(
}
for _, addr := range addresses {
task := &CleartextFlow{
Address: net.JoinHostPort(addr.Addr, port),
Depth: t.Depth,
DNSCache: t.DNSCache,
IDGenerator: t.IDGenerator,
Logger: t.Logger,
NumRedirects: t.NumRedirects,
TestKeys: t.TestKeys,
ZeroTime: t.ZeroTime,
WaitGroup: t.WaitGroup,
CookieJar: t.CookieJar,
FollowRedirects: t.URL.Scheme == "http",
HostHeader: t.URL.Host,
PrioSelector: ps,
Referer: t.Referer,
UDPAddress: t.UDPAddress,
URLPath: t.URL.Path,
URLRawQuery: t.URL.RawQuery,
Address: net.JoinHostPort(addr.Addr, port),
Depth: t.Depth,
DNSCache: t.DNSCache,
DNSOverHTTPSURLProvider: t.DNSOverHTTPSURLProvider,
IDGenerator: t.IDGenerator,
Logger: t.Logger,
NumRedirects: t.NumRedirects,
TestKeys: t.TestKeys,
ZeroTime: t.ZeroTime,
WaitGroup: t.WaitGroup,
CookieJar: t.CookieJar,
FollowRedirects: t.URL.Scheme == "http",
HostHeader: t.URL.Host,
PrioSelector: ps,
Referer: t.Referer,
UDPAddress: t.UDPAddress,
URLPath: t.URL.Path,
URLRawQuery: t.URL.RawQuery,
}
task.Start(ctx)
}
Expand All @@ -427,25 +423,26 @@ func (t *DNSResolvers) startSecureFlows(
}
for _, addr := range addresses {
task := &SecureFlow{
Address: net.JoinHostPort(addr.Addr, port),
Depth: t.Depth,
DNSCache: t.DNSCache,
IDGenerator: t.IDGenerator,
Logger: t.Logger,
NumRedirects: t.NumRedirects,
TestKeys: t.TestKeys,
ZeroTime: t.ZeroTime,
WaitGroup: t.WaitGroup,
ALPN: []string{"h2", "http/1.1"},
CookieJar: t.CookieJar,
FollowRedirects: t.URL.Scheme == "https",
SNI: t.URL.Hostname(),
HostHeader: t.URL.Host,
PrioSelector: ps,
Referer: t.Referer,
UDPAddress: t.UDPAddress,
URLPath: t.URL.Path,
URLRawQuery: t.URL.RawQuery,
Address: net.JoinHostPort(addr.Addr, port),
Depth: t.Depth,
DNSCache: t.DNSCache,
DNSOverHTTPSURLProvider: t.DNSOverHTTPSURLProvider,
IDGenerator: t.IDGenerator,
Logger: t.Logger,
NumRedirects: t.NumRedirects,
TestKeys: t.TestKeys,
ZeroTime: t.ZeroTime,
WaitGroup: t.WaitGroup,
ALPN: []string{"h2", "http/1.1"},
CookieJar: t.CookieJar,
FollowRedirects: t.URL.Scheme == "https",
SNI: t.URL.Hostname(),
HostHeader: t.URL.Host,
PrioSelector: ps,
Referer: t.Referer,
UDPAddress: t.UDPAddress,
URLPath: t.URL.Path,
URLRawQuery: t.URL.RawQuery,
}
task.Start(ctx)
}
Expand Down
42 changes: 27 additions & 15 deletions internal/experiment/webconnectivitylte/measurer.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,19 +13,30 @@ import (
"github.com/ooni/probe-cli/v3/internal/experiment/webconnectivity"
"github.com/ooni/probe-cli/v3/internal/inputparser"
"github.com/ooni/probe-cli/v3/internal/model"
"github.com/ooni/probe-cli/v3/internal/webconnectivityalgo"
"golang.org/x/net/publicsuffix"
)

// Measurer for the web_connectivity experiment.
type Measurer struct {
// Contains the experiment's config.
Config *Config

// DNSOverHTTPSURLProvider is the MANDATORY provider of DNS-over-HTTPS
// URLs that arranges for periodic measurements.
DNSOverHTTPSURLProvider *webconnectivityalgo.OpportunisticDNSOverHTTPSURLProvider
}

// NewExperimentMeasurer creates a new model.ExperimentMeasurer.
func NewExperimentMeasurer(config *Config) model.ExperimentMeasurer {
return &Measurer{
Config: config,
DNSOverHTTPSURLProvider: webconnectivityalgo.NewOpportunisticDNSOverHTTPSURLProvider(
"https://mozilla.cloudflare-dns.com/dns-query",
"https://dns.nextdns.io/dns-query",
"https://dns.google/dns-query",
"https://dns.quad9.net/dns-query",
),
}
}

Expand Down Expand Up @@ -104,21 +115,22 @@ func (m *Measurer) Run(ctx context.Context, args *model.ExperimentArgs) error {

// start background tasks
resos := &DNSResolvers{
DNSCache: NewDNSCache(),
Depth: 0,
Domain: URL.Hostname(),
IDGenerator: NewIDGenerator(),
Logger: sess.Logger(),
NumRedirects: NewNumRedirects(5),
TestKeys: tk,
URL: URL,
ZeroTime: measurement.MeasurementStartTimeSaved,
WaitGroup: wg,
CookieJar: jar,
Referer: "",
Session: sess,
TestHelpers: testhelpers,
UDPAddress: m.Config.DNSOverUDPResolver,
DNSCache: NewDNSCache(),
DNSOverHTTPSURLProvider: m.DNSOverHTTPSURLProvider,
Depth: 0,
Domain: URL.Hostname(),
IDGenerator: NewIDGenerator(),
Logger: sess.Logger(),
NumRedirects: NewNumRedirects(5),
TestKeys: tk,
URL: URL,
ZeroTime: measurement.MeasurementStartTimeSaved,
WaitGroup: wg,
CookieJar: jar,
Referer: "",
Session: sess,
TestHelpers: testhelpers,
UDPAddress: m.Config.DNSOverUDPResolver,
}
resos.Start(ctx)

Expand Down
36 changes: 21 additions & 15 deletions internal/experiment/webconnectivitylte/secureflow.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import (
"github.com/ooni/probe-cli/v3/internal/model"
"github.com/ooni/probe-cli/v3/internal/netxlite"
"github.com/ooni/probe-cli/v3/internal/throttling"
"github.com/ooni/probe-cli/v3/internal/webconnectivityalgo"
)

// Measures HTTPS endpoints.
Expand All @@ -35,6 +36,10 @@ type SecureFlow struct {
// DNSCache is the MANDATORY DNS cache.
DNSCache *DNSCache

// DNSOverHTTPSURLProvider is the MANDATORY provider of DNS-over-HTTPS
// URLs that arranges for periodic measurements.
DNSOverHTTPSURLProvider *webconnectivityalgo.OpportunisticDNSOverHTTPSURLProvider

// Depth is the OPTIONAL current redirect depth.
Depth int64

Expand Down Expand Up @@ -378,21 +383,22 @@ func (t *SecureFlow) maybeFollowRedirects(ctx context.Context, resp *http.Respon
}
t.Logger.Infof("redirect to: %s", location.String())
resolvers := &DNSResolvers{
CookieJar: t.CookieJar,
Depth: t.Depth + 1,
DNSCache: t.DNSCache,
Domain: location.Hostname(),
IDGenerator: t.IDGenerator,
Logger: t.Logger,
NumRedirects: t.NumRedirects,
TestKeys: t.TestKeys,
URL: location,
ZeroTime: t.ZeroTime,
WaitGroup: t.WaitGroup,
Referer: resp.Request.URL.String(),
Session: nil, // no need to issue another control request
TestHelpers: nil, // ditto
UDPAddress: t.UDPAddress,
CookieJar: t.CookieJar,
Depth: t.Depth + 1,
DNSOverHTTPSURLProvider: t.DNSOverHTTPSURLProvider,
DNSCache: t.DNSCache,
Domain: location.Hostname(),
IDGenerator: t.IDGenerator,
Logger: t.Logger,
NumRedirects: t.NumRedirects,
TestKeys: t.TestKeys,
URL: location,
ZeroTime: t.ZeroTime,
WaitGroup: t.WaitGroup,
Referer: resp.Request.URL.String(),
Session: nil, // no need to issue another control request
TestHelpers: nil, // ditto
UDPAddress: t.UDPAddress,
}
resolvers.Start(ctx)
}
Expand Down
Loading

0 comments on commit d0b85d0

Please sign in to comment.