From 0a322ebab07c4b7d8283a0a546ecca99dddb4e28 Mon Sep 17 00:00:00 2001 From: Simone Basso Date: Fri, 12 Nov 2021 14:43:28 +0100 Subject: [PATCH] [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. --- .../internal/websteps/explore_test.go | 6 +-- .../internal/websteps/generate_test.go | 9 ++--- .../internal/sessionresolver/resolvermaker.go | 2 - .../engine/legacy/errorsx/integration_test.go | 14 +++---- internal/engine/netx/quicdialer/saver_test.go | 14 +++---- .../engine/netx/quicdialer/system_test.go | 5 ++- internal/netxlite/integration_test.go | 15 +++++--- internal/netxlite/quictesting/quictesting.go | 38 +++++++++++++++++++ .../netxlite/quictesting/quictesting_test.go | 20 ++++++++++ 9 files changed, 90 insertions(+), 33 deletions(-) create mode 100644 internal/netxlite/quictesting/quictesting.go create mode 100644 internal/netxlite/quictesting/quictesting_test.go diff --git a/internal/cmd/oohelperd/internal/websteps/explore_test.go b/internal/cmd/oohelperd/internal/websteps/explore_test.go index e558865..c10c09a 100644 --- a/internal/cmd/oohelperd/internal/websteps/explore_test.go +++ b/internal/cmd/oohelperd/internal/websteps/explore_test.go @@ -5,6 +5,7 @@ import ( "net/url" "testing" + "github.com/ooni/probe-cli/v3/internal/netxlite/quictesting" "github.com/ooni/probe-cli/v3/internal/runtimex" ) @@ -84,12 +85,11 @@ func TestGetFailure(t *testing.T) { } func TestGetH3Success(t *testing.T) { - u, err := url.Parse("https://www.google.com") - runtimex.PanicOnError(err, "url.Parse failed for clearly good URL") + u := &url.URL{Scheme: "https", Host: quictesting.Domain, Path: "/"} h3u := &h3URL{URL: u, proto: "h3"} resp, err := explorer.getH3(h3u, nil) if err != nil { - t.Fatal("unexpected error") + t.Fatal("unexpected error", err) } if resp == nil { t.Fatal("unexpected nil response") diff --git a/internal/cmd/oohelperd/internal/websteps/generate_test.go b/internal/cmd/oohelperd/internal/websteps/generate_test.go index 2919647..8242e13 100644 --- a/internal/cmd/oohelperd/internal/websteps/generate_test.go +++ b/internal/cmd/oohelperd/internal/websteps/generate_test.go @@ -12,6 +12,7 @@ import ( "github.com/lucas-clemente/quic-go" "github.com/ooni/probe-cli/v3/internal/netxlite" + "github.com/ooni/probe-cli/v3/internal/netxlite/quictesting" "github.com/ooni/probe-cli/v3/internal/runtimex" ) @@ -297,8 +298,7 @@ func TestGenerateHTTPSTLSFailure(t *testing.T) { } func TestGenerateH3(t *testing.T) { - u, err := url.Parse("https://www.google.com") - runtimex.PanicOnError(err, "url.Parse failed") + u := &url.URL{Scheme: "https", Host: quictesting.Domain, Path: "/"} rt := &RoundTrip{ Proto: "h3", Request: &http.Request{ @@ -309,10 +309,7 @@ func TestGenerateH3(t *testing.T) { }, SortIndex: 0, } - endpointMeasurement := generator.GenerateH3Endpoint(context.Background(), rt, "173.194.76.103:443") - if err != nil { - t.Fatal("unexpected err") - } + endpointMeasurement := generator.GenerateH3Endpoint(context.Background(), rt, quictesting.Endpoint("443")) if endpointMeasurement == nil { t.Fatal("unexpected nil urlMeasurement") } diff --git a/internal/engine/internal/sessionresolver/resolvermaker.go b/internal/engine/internal/sessionresolver/resolvermaker.go index f2e6f8d..bb15318 100644 --- a/internal/engine/internal/sessionresolver/resolvermaker.go +++ b/internal/engine/internal/sessionresolver/resolvermaker.go @@ -28,8 +28,6 @@ var allmakers = []*resolvermaker{{ url: "http3://cloudflare-dns.com/dns-query", }, { url: "https://dns.google/dns-query", -}, { - url: "http3://dns.google/dns-query", }, { url: "https://dns.quad9.net/dns-query", }, { diff --git a/internal/engine/legacy/errorsx/integration_test.go b/internal/engine/legacy/errorsx/integration_test.go index d1d5f06..3c41266 100644 --- a/internal/engine/legacy/errorsx/integration_test.go +++ b/internal/engine/legacy/errorsx/integration_test.go @@ -8,9 +8,10 @@ import ( "github.com/lucas-clemente/quic-go" errorsxlegacy "github.com/ooni/probe-cli/v3/internal/engine/legacy/errorsx" "github.com/ooni/probe-cli/v3/internal/netxlite" + "github.com/ooni/probe-cli/v3/internal/netxlite/quictesting" ) -func TestErrorWrapperQUICDialerInvalidCertificate(t *testing.T) { +func TestErrorWrapperQUICDialerFailure(t *testing.T) { nextprotos := []string{"h3"} servername := "example.com" tlsConf := &tls.Config{ @@ -21,17 +22,16 @@ func TestErrorWrapperQUICDialerInvalidCertificate(t *testing.T) { dlr := &errorsxlegacy.ErrorWrapperQUICDialer{Dialer: &netxlite.QUICDialerQUICGo{ QUICListener: &netxlite.QUICListenerStdlib{}, }} - // use Google IP sess, err := dlr.DialContext(context.Background(), "udp", - "216.58.212.164:443", tlsConf, &quic.Config{}) + quictesting.Endpoint("443"), tlsConf, &quic.Config{}) if err == nil { t.Fatal("expected an error here") } if sess != nil { t.Fatal("expected nil sess here") } - if err.Error() != netxlite.FailureSSLInvalidCertificate { - t.Fatal("unexpected failure") + if err.Error() != netxlite.FailureSSLFailedHandshake { + t.Fatal("unexpected failure", err.Error()) } } @@ -39,12 +39,12 @@ func TestErrorWrapperQUICDialerSuccess(t *testing.T) { ctx := context.Background() tlsConf := &tls.Config{ NextProtos: []string{"h3"}, - ServerName: "www.google.com", + ServerName: quictesting.Domain, } d := &errorsxlegacy.ErrorWrapperQUICDialer{Dialer: &netxlite.QUICDialerQUICGo{ QUICListener: &netxlite.QUICListenerStdlib{}, }} - sess, err := d.DialContext(ctx, "udp", "216.58.212.164:443", tlsConf, &quic.Config{}) + sess, err := d.DialContext(ctx, "udp", quictesting.Endpoint("443"), tlsConf, &quic.Config{}) if err != nil { t.Fatal(err) } diff --git a/internal/engine/netx/quicdialer/saver_test.go b/internal/engine/netx/quicdialer/saver_test.go index 89912e0..a158be5 100644 --- a/internal/engine/netx/quicdialer/saver_test.go +++ b/internal/engine/netx/quicdialer/saver_test.go @@ -12,6 +12,7 @@ import ( "github.com/ooni/probe-cli/v3/internal/engine/netx/quicdialer" "github.com/ooni/probe-cli/v3/internal/engine/netx/trace" "github.com/ooni/probe-cli/v3/internal/netxlite" + "github.com/ooni/probe-cli/v3/internal/netxlite/quictesting" ) type MockDialer struct { @@ -30,7 +31,7 @@ func (d MockDialer) DialContext(ctx context.Context, network, host string, func TestHandshakeSaverSuccess(t *testing.T) { nextprotos := []string{"h3"} - servername := "www.google.com" + servername := quictesting.Domain tlsConf := &tls.Config{ NextProtos: nextprotos, ServerName: servername, @@ -43,7 +44,7 @@ func TestHandshakeSaverSuccess(t *testing.T) { Saver: saver, } sess, err := dlr.DialContext(context.Background(), "udp", - "216.58.212.164:443", tlsConf, &quic.Config{}) + quictesting.Endpoint("443"), tlsConf, &quic.Config{}) if err != nil { t.Fatal("unexpected error", err) } @@ -57,7 +58,7 @@ func TestHandshakeSaverSuccess(t *testing.T) { if ev[0].Name != "quic_handshake_start" { t.Fatal("unexpected Name") } - if ev[0].TLSServerName != "www.google.com" { + if ev[0].TLSServerName != quictesting.Domain { t.Fatal("unexpected TLSServerName") } if !reflect.DeepEqual(ev[0].TLSNextProtos, nextprotos) { @@ -78,7 +79,7 @@ func TestHandshakeSaverSuccess(t *testing.T) { if !reflect.DeepEqual(ev[1].TLSNextProtos, nextprotos) { t.Fatal("unexpected TLSNextProtos") } - if ev[1].TLSServerName != "www.google.com" { + if ev[1].TLSServerName != quictesting.Domain { t.Fatal("unexpected TLSServerName") } if ev[1].Time.Before(ev[0].Time) { @@ -101,7 +102,7 @@ func TestHandshakeSaverHostNameError(t *testing.T) { Saver: saver, } sess, err := dlr.DialContext(context.Background(), "udp", - "216.58.212.164:443", tlsConf, &quic.Config{}) + quictesting.Endpoint("443"), tlsConf, &quic.Config{}) if err == nil { t.Fatal("expected an error here") } @@ -115,8 +116,7 @@ func TestHandshakeSaverHostNameError(t *testing.T) { if ev.NoTLSVerify == true { t.Fatal("expected NoTLSVerify to be false") } - if !strings.Contains(ev.Err.Error(), - "certificate is valid for www.google.com, not "+servername) { + if !strings.HasSuffix(ev.Err.Error(), "tls: handshake failure") { t.Fatal("unexpected error", ev.Err) } } diff --git a/internal/engine/netx/quicdialer/system_test.go b/internal/engine/netx/quicdialer/system_test.go index 91caa9a..0921628 100644 --- a/internal/engine/netx/quicdialer/system_test.go +++ b/internal/engine/netx/quicdialer/system_test.go @@ -12,6 +12,7 @@ import ( "github.com/ooni/probe-cli/v3/internal/engine/netx/trace" "github.com/ooni/probe-cli/v3/internal/netxlite" "github.com/ooni/probe-cli/v3/internal/netxlite/mocks" + "github.com/ooni/probe-cli/v3/internal/netxlite/quictesting" "github.com/ooni/probe-cli/v3/internal/netxlite/quicx" ) @@ -42,7 +43,7 @@ func TestSystemDialerSuccessWithReadWrite(t *testing.T) { // This is the most common use case for collecting reads, writes tlsConf := &tls.Config{ NextProtos: []string{"h3"}, - ServerName: "www.google.com", + ServerName: quictesting.Domain, } saver := &trace.Saver{} systemdialer := &netxlite.QUICDialerQUICGo{ @@ -52,7 +53,7 @@ func TestSystemDialerSuccessWithReadWrite(t *testing.T) { }, } _, err := systemdialer.DialContext(context.Background(), "udp", - "216.58.212.164:443", tlsConf, &quic.Config{}) + quictesting.Endpoint("443"), tlsConf, &quic.Config{}) if err != nil { t.Fatal(err) } diff --git a/internal/netxlite/integration_test.go b/internal/netxlite/integration_test.go index 8cab1a3..a1d6545 100644 --- a/internal/netxlite/integration_test.go +++ b/internal/netxlite/integration_test.go @@ -6,6 +6,7 @@ import ( "fmt" "net" "net/http" + "net/url" "testing" "time" @@ -13,6 +14,7 @@ import ( "github.com/lucas-clemente/quic-go" "github.com/ooni/probe-cli/v3/internal/netxlite" "github.com/ooni/probe-cli/v3/internal/netxlite/filtering" + "github.com/ooni/probe-cli/v3/internal/netxlite/quictesting" utls "gitlab.com/yawning/utls.git" ) @@ -435,11 +437,11 @@ func TestMeasureWithQUICDialer(t *testing.T) { defer d.CloseIdleConnections() ctx := context.Background() config := &tls.Config{ - ServerName: "dns.google", + ServerName: quictesting.Domain, NextProtos: []string{"h3"}, RootCAs: netxlite.NewDefaultCertPool(), } - sess, err := d.DialContext(ctx, "udp", "8.8.4.4:443", config, &quic.Config{}) + sess, err := d.DialContext(ctx, "udp", quictesting.Endpoint("443"), config, &quic.Config{}) if err != nil { t.Fatal(err) } @@ -455,12 +457,12 @@ func TestMeasureWithQUICDialer(t *testing.T) { defer d.CloseIdleConnections() ctx := context.Background() config := &tls.Config{ - ServerName: "dns.google", + ServerName: quictesting.Domain, NextProtos: []string{"h3"}, RootCAs: netxlite.NewDefaultCertPool(), } - // Here we assume 8.8.4.4:1 is filtered - sess, err := d.DialContext(ctx, "udp", "8.8.4.4:1", config, &quic.Config{}) + // Here we assume :1 is filtered + sess, err := d.DialContext(ctx, "udp", quictesting.Endpoint("1"), config, &quic.Config{}) if err == nil || err.Error() != netxlite.FailureGenericTimeoutError { t.Fatal("not the error we expected", err) } @@ -502,7 +504,8 @@ func TestHTTP3Transport(t *testing.T) { ) txp := netxlite.NewHTTP3Transport(log.Log, d, &tls.Config{}) client := &http.Client{Transport: txp} - resp, err := client.Get("https://www.google.com/robots.txt") + URL := (&url.URL{Scheme: "https", Host: quictesting.Domain, Path: "/"}).String() + resp, err := client.Get(URL) if err != nil { t.Fatal(err) } diff --git a/internal/netxlite/quictesting/quictesting.go b/internal/netxlite/quictesting/quictesting.go new file mode 100644 index 0000000..a2a7761 --- /dev/null +++ b/internal/netxlite/quictesting/quictesting.go @@ -0,0 +1,38 @@ +// Package quictesting contains code useful to test QUIC. +package quictesting + +import ( + "context" + "net" + "strings" + "time" + + "github.com/ooni/probe-cli/v3/internal/runtimex" +) + +// Domain is the the domain we should be testing using QUIC. +const Domain = "www.cloudflare.com" + +// Address is the address we should be testing using QUIC. +var Address string + +// Endpoint returns the endpoint to test using QUIC by combining +// the Address variable with the given port. +func Endpoint(port string) string { + return net.JoinHostPort(Address, port) +} + +func init() { + const timeout = 10 * time.Second + ctx, cancel := context.WithTimeout(context.Background(), timeout) + defer cancel() + reso := &net.Resolver{} + addrs, err := reso.LookupHost(ctx, Domain) + runtimex.PanicOnError(err, "reso.LookupHost failed") + for _, addr := range addrs { + if !strings.Contains(addr, ":") { + Address = addr + break + } + } +} diff --git a/internal/netxlite/quictesting/quictesting_test.go b/internal/netxlite/quictesting/quictesting_test.go new file mode 100644 index 0000000..0a8f99e --- /dev/null +++ b/internal/netxlite/quictesting/quictesting_test.go @@ -0,0 +1,20 @@ +package quictesting + +import ( + "net" + "testing" +) + +func TestWorksAsIntended(t *testing.T) { + epnt := Endpoint("443") + addr, port, err := net.SplitHostPort(epnt) + if err != nil { + t.Fatal(err) + } + if addr != Address { + t.Fatal("invalid addr") + } + if port != "443" { + t.Fatal("invalid port") + } +}