c/ooni-probe-cli
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

fix(webconnectivity): allow measuring https://1.1.1.1 (#241)

Browse Source 
* fix(webconnectivity): allow measuring https://1.1.1.1

There were two issues preventing us from doing so:

1. in netx, the address resolver was too later in the resolver
chain. Therefore, its result wasn't added to the events.

2. when building the DNSCache (in httpget.go), we didn't consider
the case where the input is an address. We need to treat this
case specially to make sure there is no DNSCache.

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

* fix: add unit tests for code making the dnscache

* fix(netx): make sure all tests pass

* chore: bump webconnectivity version
This commit is contained in:
Simone Basso 2021-03-08 12:05:43 +01:00 committed by GitHub
parent 55bdebe8b2
commit 2ef5fb503a
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
8 changed files with 82 additions and 47 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
internal/engine/experiment/webconnectivity/control.go
View File

@ -57,13 +57,13 @@ func Control(
HTTPClient: sess.DefaultHTTPClient(),
Logger: sess.Logger(),
}
sess.Logger().Infof("control %s...", creq.HTTPRequest)
sess.Logger().Infof("control for %s...", creq.HTTPRequest)
// make sure error is wrapped
err = errorx.SafeErrWrapperBuilder{
Error: clnt.PostJSON(ctx, "/", creq, &out),
Operation: errorx.TopLevelOperation,
}.MaybeBuild()
sess.Logger().Infof("control %s... %+v", creq.HTTPRequest, err)
sess.Logger().Infof("control for %s... %+v", creq.HTTPRequest, err)
(&out.DNS).FillASNs(sess)
return
}

1
internal/engine/experiment/webconnectivity/dnslookup.go
View File

@ -37,6 +37,7 @@ func DNSLookup(ctx context.Context, config DNSLookupConfig) (out DNSLookupResult
}
if answer.IPv6 != "" {
out.Addrs[answer.IPv6] = answer.ASN
continue
}
}
}

19
internal/engine/experiment/webconnectivity/httpget.go
View File

@ -3,6 +3,7 @@ package webconnectivity
import (
"context"
"fmt"
"net"
"net/url"
"strings"
@ -25,6 +26,22 @@ type HTTPGetResult struct {
Failure *string
}
// TODO(bassosimone): Web Connectivity uses too much external testing
// and we should actually expose much less to the outside by using
// internal testing and by making _many_ functions private.
// HTTPGetMakeDNSCache constructs the DNSCache option for HTTPGet
// by combining domain and addresses into a single string. As a
// corner case, if the domain is an IP address, we return an empty
// string. This corner case corresponds to Web Connectivity
// inputs like https://1.1.1.1.
func HTTPGetMakeDNSCache(domain, addresses string) string {
if net.ParseIP(domain) != nil {
return ""
}
return fmt.Sprintf("%s %s", domain, addresses)
}
// HTTPGet performs the HTTP/HTTPS part of Web Connectivity.
func HTTPGet(ctx context.Context, config HTTPGetConfig) (out HTTPGetResult) {
addresses := strings.Join(config.Addresses, " ")
@ -38,7 +55,7 @@ func HTTPGet(ctx context.Context, config HTTPGetConfig) (out HTTPGetResult) {
domain := config.TargetURL.Hostname()
result, err := urlgetter.Getter{
Config: urlgetter.Config{
DNSCache: fmt.Sprintf("%s %s", domain, addresses),
DNSCache: HTTPGetMakeDNSCache(domain, addresses),
},
Session: config.Session,
Target: target,

17
internal/engine/experiment/webconnectivity/httpget_test.go
View File

@ -25,3 +25,20 @@ func TestHTTPGet(t *testing.T) {
t.Fatal(*r.Failure)
}
}
func TestHTTPGetMakeDNSCache(t *testing.T) {
// test for input being an IP
out := webconnectivity.HTTPGetMakeDNSCache(
"1.1.1.1", "1.1.1.1",
)
if out != "" {
t.Fatal("expected empty output here")
}
// test for input being a domain
out = webconnectivity.HTTPGetMakeDNSCache(
"dns.google", "8.8.8.8 8.8.4.4",
)
if out != "dns.google 8.8.8.8 8.8.4.4" {
t.Fatal("expected ordinary output here")
}
}

2
internal/engine/experiment/webconnectivity/webconnectivity.go
View File

@ -19,7 +19,7 @@ import (
const (
testName = "web_connectivity"
testVersion = "0.2.0"
testVersion = "0.3.0"
)
// Config contains the experiment config.

2
internal/engine/experiment/webconnectivity/webconnectivity_test.go
View File

@ -21,7 +21,7 @@ func TestNewExperimentMeasurer(t *testing.T) {
if measurer.ExperimentName() != "web_connectivity" {
t.Fatal("unexpected name")
}
if measurer.ExperimentVersion() != "0.2.0" {
if measurer.ExperimentVersion() != "0.3.0" {
t.Fatal("unexpected version")
}
}

2
internal/engine/netx/netx.go
View File

@ -126,6 +126,7 @@ func NewResolver(config Config) Resolver {
config.BaseResolver = resolver.SystemResolver{}
}
var r Resolver = config.BaseResolver
r = resolver.AddressResolver{Resolver: r}
if config.CacheResolutions {
r = &resolver.CacheResolver{Resolver: r}
}
@ -146,7 +147,6 @@ func NewResolver(config Config) Resolver {
if config.ResolveSaver != nil {
r = resolver.SaverResolver{Resolver: r, Saver: config.ResolveSaver}
}
r = resolver.AddressResolver{Resolver: r}
return resolver.IDNAResolver{Resolver: r}
}

82
internal/engine/netx/netx_test.go
View File

@ -23,15 +23,15 @@ func TestNewResolverVanilla(t *testing.T) {
if !ok {
t.Fatal("not the resolver we expected")
}
ar, ok := ir.Resolver.(resolver.AddressResolver)
ewr, ok := ir.Resolver.(resolver.ErrorWrapperResolver)
if !ok {
t.Fatal("not the resolver we expected")
}
ewr, ok := ar.Resolver.(resolver.ErrorWrapperResolver)
ar, ok := ewr.Resolver.(resolver.AddressResolver)
if !ok {
t.Fatal("not the resolver we expected")
}
_, ok = ewr.Resolver.(resolver.SystemResolver)
_, ok = ar.Resolver.(resolver.SystemResolver)
if !ok {
t.Fatal("not the resolver we expected")
}
@ -47,15 +47,15 @@ func TestNewResolverSpecificResolver(t *testing.T) {
if !ok {
t.Fatal("not the resolver we expected")
}
ar, ok := ir.Resolver.(resolver.AddressResolver)
ewr, ok := ir.Resolver.(resolver.ErrorWrapperResolver)
if !ok {
t.Fatal("not the resolver we expected")
}
ewr, ok := ar.Resolver.(resolver.ErrorWrapperResolver)
ar, ok := ewr.Resolver.(resolver.AddressResolver)
if !ok {
t.Fatal("not the resolver we expected")
}
_, ok = ewr.Resolver.(resolver.BogonResolver)
_, ok = ar.Resolver.(resolver.BogonResolver)
if !ok {
t.Fatal("not the resolver we expected")
}
@ -69,11 +69,7 @@ func TestNewResolverWithBogonFilter(t *testing.T) {
if !ok {
t.Fatal("not the resolver we expected")
}
ar, ok := ir.Resolver.(resolver.AddressResolver)
if !ok {
t.Fatal("not the resolver we expected")
}
ewr, ok := ar.Resolver.(resolver.ErrorWrapperResolver)
ewr, ok := ir.Resolver.(resolver.ErrorWrapperResolver)
if !ok {
t.Fatal("not the resolver we expected")
}
@ -81,7 +77,11 @@ func TestNewResolverWithBogonFilter(t *testing.T) {
if !ok {
t.Fatal("not the resolver we expected")
}
_, ok = br.Resolver.(resolver.SystemResolver)
ar, ok := br.Resolver.(resolver.AddressResolver)
if !ok {
t.Fatal("not the resolver we expected")
}
_, ok = ar.Resolver.(resolver.SystemResolver)
if !ok {
t.Fatal("not the resolver we expected")
}
@ -95,11 +95,7 @@ func TestNewResolverWithLogging(t *testing.T) {
if !ok {
t.Fatal("not the resolver we expected")
}
ar, ok := ir.Resolver.(resolver.AddressResolver)
if !ok {
t.Fatal("not the resolver we expected")
}
lr, ok := ar.Resolver.(resolver.LoggingResolver)
lr, ok := ir.Resolver.(resolver.LoggingResolver)
if !ok {
t.Fatal("not the resolver we expected")
}
@ -110,7 +106,11 @@ func TestNewResolverWithLogging(t *testing.T) {
if !ok {
t.Fatal("not the resolver we expected")
}
_, ok = ewr.Resolver.(resolver.SystemResolver)
ar, ok := ewr.Resolver.(resolver.AddressResolver)
if !ok {
t.Fatal("not the resolver we expected")
}
_, ok = ar.Resolver.(resolver.SystemResolver)
if !ok {
t.Fatal("not the resolver we expected")
}
@ -125,11 +125,7 @@ func TestNewResolverWithSaver(t *testing.T) {
if !ok {
t.Fatal("not the resolver we expected")
}
ar, ok := ir.Resolver.(resolver.AddressResolver)
if !ok {
t.Fatal("not the resolver we expected")
}
sr, ok := ar.Resolver.(resolver.SaverResolver)
sr, ok := ir.Resolver.(resolver.SaverResolver)
if !ok {
t.Fatal("not the resolver we expected")
}
@ -140,7 +136,11 @@ func TestNewResolverWithSaver(t *testing.T) {
if !ok {
t.Fatal("not the resolver we expected")
}
_, ok = ewr.Resolver.(resolver.SystemResolver)
ar, ok := ewr.Resolver.(resolver.AddressResolver)
if !ok {
t.Fatal("not the resolver we expected")
}
_, ok = ar.Resolver.(resolver.SystemResolver)
if !ok {
t.Fatal("not the resolver we expected")
}
@ -154,11 +154,7 @@ func TestNewResolverWithReadWriteCache(t *testing.T) {
if !ok {
t.Fatal("not the resolver we expected")
}
ar, ok := ir.Resolver.(resolver.AddressResolver)
if !ok {
t.Fatal("not the resolver we expected")
}
ewr, ok := ar.Resolver.(resolver.ErrorWrapperResolver)
ewr, ok := ir.Resolver.(resolver.ErrorWrapperResolver)
if !ok {
t.Fatal("not the resolver we expected")
}
@ -169,7 +165,11 @@ func TestNewResolverWithReadWriteCache(t *testing.T) {
if cr.ReadOnly != false {
t.Fatal("expected readwrite cache here")
}
_, ok = cr.Resolver.(resolver.SystemResolver)
ar, ok := cr.Resolver.(resolver.AddressResolver)
if !ok {
t.Fatal("not the resolver we expected")
}
_, ok = ar.Resolver.(resolver.SystemResolver)
if !ok {
t.Fatal("not the resolver we expected")
}
@ -185,11 +185,7 @@ func TestNewResolverWithPrefilledReadonlyCache(t *testing.T) {
if !ok {
t.Fatal("not the resolver we expected")
}
ar, ok := ir.Resolver.(resolver.AddressResolver)
if !ok {
t.Fatal("not the resolver we expected")
}
ewr, ok := ar.Resolver.(resolver.ErrorWrapperResolver)
ewr, ok := ir.Resolver.(resolver.ErrorWrapperResolver)
if !ok {
t.Fatal("not the resolver we expected")
}
@ -203,7 +199,11 @@ func TestNewResolverWithPrefilledReadonlyCache(t *testing.T) {
if cr.Get("dns.google.com")[0] != "8.8.8.8" {
t.Fatal("cache not correctly prefilled")
}
_, ok = cr.Resolver.(resolver.SystemResolver)
ar, ok := cr.Resolver.(resolver.AddressResolver)
if !ok {
t.Fatal("not the resolver we expected")
}
_, ok = ar.Resolver.(resolver.SystemResolver)
if !ok {
t.Fatal("not the resolver we expected")
}
@ -233,7 +233,7 @@ func TestNewDialerVanilla(t *testing.T) {
if !ok {
t.Fatal("not the resolver we expected")
}
if _, ok := ir.Resolver.(resolver.AddressResolver); !ok {
if _, ok := ir.Resolver.(resolver.ErrorWrapperResolver); !ok {
t.Fatal("not the resolver we expected")
}
ewd, ok := dnsd.Dialer.(dialer.ErrorWrapperDialer)
@ -315,7 +315,7 @@ func TestNewDialerWithLogger(t *testing.T) {
if !ok {
t.Fatal("not the resolver we expected")
}
if _, ok := ir.Resolver.(resolver.AddressResolver); !ok {
if _, ok := ir.Resolver.(resolver.LoggingResolver); !ok {
t.Fatal("not the resolver we expected")
}
ld, ok := dnsd.Dialer.(dialer.LoggingDialer)
@ -365,7 +365,7 @@ func TestNewDialerWithDialSaver(t *testing.T) {
if !ok {
t.Fatal("not the resolver we expected")
}
if _, ok := ir.Resolver.(resolver.AddressResolver); !ok {
if _, ok := ir.Resolver.(resolver.ErrorWrapperResolver); !ok {
t.Fatal("not the resolver we expected")
}
sad, ok := dnsd.Dialer.(dialer.SaverDialer)
@ -415,7 +415,7 @@ func TestNewDialerWithReadWriteSaver(t *testing.T) {
if !ok {
t.Fatal("not the resolver we expected")
}
if _, ok := ir.Resolver.(resolver.AddressResolver); !ok {
if _, ok := ir.Resolver.(resolver.ErrorWrapperResolver); !ok {
t.Fatal("not the resolver we expected")
}
scd, ok := dnsd.Dialer.(dialer.SaverConnDialer)
@ -468,7 +468,7 @@ func TestNewDialerWithContextByteCounting(t *testing.T) {
if !ok {
t.Fatal("not the resolver we expected")
}
if _, ok := ir.Resolver.(resolver.AddressResolver); !ok {
if _, ok := ir.Resolver.(resolver.ErrorWrapperResolver); !ok {
t.Fatal("not the resolver we expected")
}
ewd, ok := dnsd.Dialer.(dialer.ErrorWrapperDialer)