From 1c038a21d08f94735f7adb252652f25b86b06528 Mon Sep 17 00:00:00 2001 From: Simone Basso Date: Fri, 15 Sep 2023 14:26:30 +0200 Subject: [PATCH 1/5] x --- go.mod | 2 +- go.sum | 4 +- internal/enginenetx/http.go | 10 +- internal/enginenetx/http_test.go | 49 +++-- internal/experiment/dash/measurer.go | 2 +- internal/experiment/dash/runner.go | 2 +- internal/experiment/dash/runner_test.go | 2 +- internal/experiment/dnscheck/dnscheck.go | 2 +- .../fbmessenger/fbmessenger_test.go | 2 +- internal/experiment/hhfm/hhfm.go | 2 +- internal/experiment/hhfm/hhfm_test.go | 2 +- internal/experiment/hirl/hirl.go | 2 +- internal/experiment/hirl/hirl_test.go | 2 +- internal/experiment/quicping/quicping.go | 2 +- internal/experiment/riseupvpn/riseupvpn.go | 2 +- .../experiment/riseupvpn/riseupvpn_test.go | 2 +- .../stunreachability/stunreachability.go | 2 +- internal/experiment/tlsmiddlebox/trace.go | 2 +- internal/experiment/tlstool/tlstool.go | 2 +- internal/experiment/tor/tor.go | 2 +- internal/experiment/tor/tor_test.go | 2 +- internal/experiment/torsf/torsf.go | 2 +- internal/experiment/urlgetter/configurer.go | 2 +- .../experiment/urlgetter/configurer_test.go | 2 +- internal/experiment/urlgetter/getter.go | 2 +- internal/experiment/urlgetter/urlgetter.go | 2 +- internal/experiment/vanillator/vanillator.go | 2 +- .../webconnectivity/httpanalysis_test.go | 2 +- .../webconnectivity/summary_test.go | 2 +- .../webconnectivity/webconnectivity.go | 2 +- .../webconnectivity/webconnectivity_test.go | 2 +- .../experiment/webconnectivitylte/testkeys.go | 2 +- internal/legacy/measurex/dnsx.go | 2 +- internal/legacy/measurex/resolver.go | 2 +- internal/legacy/netx/config.go | 2 +- internal/legacy/netx/dnstransport_test.go | 2 +- internal/legacy/netx/http_test.go | 2 +- internal/legacy/netx/integration_test.go | 2 +- internal/legacy/netx/resolver_test.go | 2 +- internal/legacy/netx/tls_test.go | 2 +- internal/netemx/scenario.go | 2 +- internal/netxlite/httpfactory.go | 12 ++ internal/netxlite/tls.go | 6 +- internal/oohelperd/dns.go | 2 +- internal/oohelperd/http.go | 2 +- internal/testingproxy/dialer.go | 48 +++++ internal/testingproxy/doc.go | 2 + internal/testingproxy/hosthttp.go | 74 +++++++ internal/testingproxy/hosthttps.go | 83 ++++++++ internal/testingproxy/httputils.go | 53 +++++ internal/testingproxy/httputils_test.go | 122 +++++++++++ internal/testingproxy/netemhttp.go | 163 ++++++++++++++ internal/testingproxy/netemhttps.go | 164 +++++++++++++++ internal/testingproxy/qa_test.go | 19 ++ internal/testingproxy/testcase.go | 34 +++ internal/testingx/httpproxy.go | 138 ++++++++++++ internal/testingx/httpproxy_test.go | 19 ++ internal/testingx/httptestx.go | 85 ++------ internal/testingx/httptestx_test.go | 198 ------------------ internal/testingx/tlsx.go | 9 + 60 files changed, 1039 insertions(+), 333 deletions(-) create mode 100644 internal/testingproxy/dialer.go create mode 100644 internal/testingproxy/doc.go create mode 100644 internal/testingproxy/hosthttp.go create mode 100644 internal/testingproxy/hosthttps.go create mode 100644 internal/testingproxy/httputils.go create mode 100644 internal/testingproxy/httputils_test.go create mode 100644 internal/testingproxy/netemhttp.go create mode 100644 internal/testingproxy/netemhttps.go create mode 100644 internal/testingproxy/qa_test.go create mode 100644 internal/testingproxy/testcase.go create mode 100644 internal/testingx/httpproxy.go create mode 100644 internal/testingx/httpproxy_test.go diff --git a/go.mod b/go.mod index 509c8e649a..a196f65ae2 100644 --- a/go.mod +++ b/go.mod @@ -26,7 +26,7 @@ require ( github.com/mitchellh/go-wordwrap v1.0.1 github.com/montanaflynn/stats v0.7.1 github.com/ooni/go-libtor v1.1.8 - github.com/ooni/netem v0.0.0-20230906091637-85d962536ff3 + github.com/ooni/netem v0.0.0-20230915101649-ab0dc13be014 github.com/ooni/oocrypto v0.5.3 github.com/ooni/oohttp v0.6.3 github.com/ooni/probe-assets v0.18.0 diff --git a/go.sum b/go.sum index d6efac959d..cc20af397b 100644 --- a/go.sum +++ b/go.sum @@ -483,8 +483,8 @@ github.com/onsi/gomega v1.17.0/go.mod h1:HnhC7FXeEQY45zxNK3PPoIUhzk/80Xly9PcubAl github.com/onsi/gomega v1.27.7 h1:fVih9JD6ogIiHUN6ePK7HJidyEDpWGVB5mzM7cWNXoU= github.com/ooni/go-libtor v1.1.8 h1:Wo3V3DVTxl5vZdxtQakqYP+DAHx7pPtAFSl1bnAa08w= github.com/ooni/go-libtor v1.1.8/go.mod h1:q1YyLwRD9GeMyeerVvwc0vJ2YgwDLTp2bdVcrh/JXyI= -github.com/ooni/netem v0.0.0-20230906091637-85d962536ff3 h1:zpTbzNzpo00cKbjLLnWMKjZeGLdoNC81vMiBDiur7NU= -github.com/ooni/netem v0.0.0-20230906091637-85d962536ff3/go.mod h1:3LJOzTIu2O4ADDJN2ILG4ViJOqyH/u9fKY8QT2Rma8Y= +github.com/ooni/netem v0.0.0-20230915101649-ab0dc13be014 h1:4kOSV4D6mwrdoNUkAbGz1XoFUPcjsuNlLhZMc2CoHGg= +github.com/ooni/netem v0.0.0-20230915101649-ab0dc13be014/go.mod h1:3LJOzTIu2O4ADDJN2ILG4ViJOqyH/u9fKY8QT2Rma8Y= github.com/ooni/oocrypto v0.5.3 h1:CAb0Ze6q/EWD1PRGl9KqpzMfkut4O3XMaiKYsyxrWOs= github.com/ooni/oocrypto v0.5.3/go.mod h1:HjEQ5pQBl6btcWgAsKKq1tFo8CfBrZu63C/vPAUGIDk= github.com/ooni/oohttp v0.6.3 h1:MHydpeAPU/LSDSI/hIFJwZm4afBhd2Yo+rNxxFdeMCY= diff --git a/internal/enginenetx/http.go b/internal/enginenetx/http.go index e4d56df5bb..f8af2c04da 100644 --- a/internal/enginenetx/http.go +++ b/internal/enginenetx/http.go @@ -48,13 +48,13 @@ func NewHTTPTransport( resolver model.Resolver, ) *HTTPTransport { dialer := netxlite.NewDialerWithResolver(logger, resolver) - dialer = netxlite.MaybeWrapWithProxyDialer(dialer, proxyURL) handshaker := netxlite.NewTLSHandshakerStdlib(logger) tlsDialer := netxlite.NewTLSDialer(dialer, handshaker) - // TODO(https://github.com/ooni/probe/issues/2534): here we're using the QUIRKY netxlite.NewHTTPTransport - // function, but we can probably avoid using it, given that this code is - // not using tracing and does not care about those quirks. - txp := netxlite.NewHTTPTransport(logger, dialer, tlsDialer) + txp := netxlite.NewHTTPTransportWithOptions( + logger, dialer, tlsDialer, + netxlite.HTTPTransportOptionDisableCompression(false), + netxlite.HTTPTransportOptionProxyURL(proxyURL), // nil implies "no proxy" + ) txp = bytecounter.WrapHTTPTransport(txp, counter) return &HTTPTransport{txp} } diff --git a/internal/enginenetx/http_test.go b/internal/enginenetx/http_test.go index c3604cd6d0..1c5307b64a 100644 --- a/internal/enginenetx/http_test.go +++ b/internal/enginenetx/http_test.go @@ -1,33 +1,50 @@ -package enginenetx +package enginenetx_test import ( "testing" "github.com/ooni/probe-cli/v3/internal/bytecounter" + "github.com/ooni/probe-cli/v3/internal/enginenetx" "github.com/ooni/probe-cli/v3/internal/model" + "github.com/ooni/probe-cli/v3/internal/netemx" "github.com/ooni/probe-cli/v3/internal/netxlite" ) func TestHTTPTransport(t *testing.T) { - // TODO(bassosimone): we should replace this integration test with netemx - // as soon as we can sever the hard link between netxlite and this pkg - t.Run("is working as intended", func(t *testing.T) { - txp := NewHTTPTransport( - bytecounter.New(), model.DiscardLogger, nil, netxlite.NewStdlibResolver(model.DiscardLogger)) - client := txp.NewHTTPClient() - resp, err := client.Get("https://www.google.com/robots.txt") - if err != nil { - t.Fatal(err) - } - defer resp.Body.Close() - if resp.StatusCode != 200 { - t.Fatal("unexpected status code") - } + t.Run("the HTTPTransport is working as intended", func(t *testing.T) { + env := netemx.MustNewScenario(netemx.InternetScenario) + defer env.Close() + + env.Do(func() { + txp := enginenetx.NewHTTPTransport( + bytecounter.New(), model.DiscardLogger, nil, netxlite.NewStdlibResolver(model.DiscardLogger)) + client := txp.NewHTTPClient() + resp, err := client.Get("https://www.example.com/") + if err != nil { + t.Fatal(err) + } + defer resp.Body.Close() + if resp.StatusCode != 200 { + t.Fatal("unexpected status code") + } + }) + }) + + t.Run("we can use a socks5 proxy", func(t *testing.T) { + panic("not implemented") + }) + + t.Run("we can use an HTTP proxy", func(t *testing.T) { + panic("not implemented") + }) + + t.Run("we can use an HTTPS proxy", func(t *testing.T) { + panic("not implemented") }) t.Run("NewHTTPClient returns a client with a cookie jar", func(t *testing.T) { - txp := NewHTTPTransport( + txp := enginenetx.NewHTTPTransport( bytecounter.New(), model.DiscardLogger, nil, netxlite.NewStdlibResolver(model.DiscardLogger)) client := txp.NewHTTPClient() if client.Jar == nil { diff --git a/internal/experiment/dash/measurer.go b/internal/experiment/dash/measurer.go index 78904ed479..9174cb79c2 100644 --- a/internal/experiment/dash/measurer.go +++ b/internal/experiment/dash/measurer.go @@ -10,9 +10,9 @@ import ( "net/http" "github.com/ooni/probe-cli/v3/internal/legacy/netx" + "github.com/ooni/probe-cli/v3/internal/legacy/tracex" "github.com/ooni/probe-cli/v3/internal/model" "github.com/ooni/probe-cli/v3/internal/netxlite" - "github.com/ooni/probe-cli/v3/internal/legacy/tracex" ) // Config contains the experiment config. diff --git a/internal/experiment/dash/runner.go b/internal/experiment/dash/runner.go index 6467d797a1..59f1c6215e 100644 --- a/internal/experiment/dash/runner.go +++ b/internal/experiment/dash/runner.go @@ -14,9 +14,9 @@ import ( "github.com/montanaflynn/stats" "github.com/ooni/probe-cli/v3/internal/humanize" + "github.com/ooni/probe-cli/v3/internal/legacy/tracex" "github.com/ooni/probe-cli/v3/internal/model" "github.com/ooni/probe-cli/v3/internal/runtimex" - "github.com/ooni/probe-cli/v3/internal/legacy/tracex" ) // runnerConfig contains settings for running the dash experiment. This struct diff --git a/internal/experiment/dash/runner_test.go b/internal/experiment/dash/runner_test.go index 797aef76d1..1d9ce148dc 100644 --- a/internal/experiment/dash/runner_test.go +++ b/internal/experiment/dash/runner_test.go @@ -10,9 +10,9 @@ import ( "time" "github.com/apex/log" + "github.com/ooni/probe-cli/v3/internal/legacy/tracex" "github.com/ooni/probe-cli/v3/internal/mocks" "github.com/ooni/probe-cli/v3/internal/model" - "github.com/ooni/probe-cli/v3/internal/legacy/tracex" ) func TestRunnerRunAllPhasesLocateFailure(t *testing.T) { diff --git a/internal/experiment/dnscheck/dnscheck.go b/internal/experiment/dnscheck/dnscheck.go index ff4c778d32..abbe4ee33f 100644 --- a/internal/experiment/dnscheck/dnscheck.go +++ b/internal/experiment/dnscheck/dnscheck.go @@ -16,9 +16,9 @@ import ( "github.com/ooni/probe-cli/v3/internal/experiment/urlgetter" "github.com/ooni/probe-cli/v3/internal/legacy/netx" + "github.com/ooni/probe-cli/v3/internal/legacy/tracex" "github.com/ooni/probe-cli/v3/internal/model" "github.com/ooni/probe-cli/v3/internal/runtimex" - "github.com/ooni/probe-cli/v3/internal/legacy/tracex" ) const ( diff --git a/internal/experiment/fbmessenger/fbmessenger_test.go b/internal/experiment/fbmessenger/fbmessenger_test.go index 945aafe4a1..2347453f67 100644 --- a/internal/experiment/fbmessenger/fbmessenger_test.go +++ b/internal/experiment/fbmessenger/fbmessenger_test.go @@ -11,12 +11,12 @@ import ( "github.com/ooni/netem" "github.com/ooni/probe-cli/v3/internal/experiment/fbmessenger" "github.com/ooni/probe-cli/v3/internal/experiment/urlgetter" + "github.com/ooni/probe-cli/v3/internal/legacy/tracex" "github.com/ooni/probe-cli/v3/internal/mocks" "github.com/ooni/probe-cli/v3/internal/model" "github.com/ooni/probe-cli/v3/internal/netemx" "github.com/ooni/probe-cli/v3/internal/netxlite" "github.com/ooni/probe-cli/v3/internal/runtimex" - "github.com/ooni/probe-cli/v3/internal/legacy/tracex" ) // servicesAddr is the IP address implementing al fbmessenger services in netem-based tests diff --git a/internal/experiment/hhfm/hhfm.go b/internal/experiment/hhfm/hhfm.go index 500d0b8914..44d9afa4f1 100644 --- a/internal/experiment/hhfm/hhfm.go +++ b/internal/experiment/hhfm/hhfm.go @@ -15,10 +15,10 @@ import ( "time" "github.com/ooni/probe-cli/v3/internal/experiment/urlgetter" + "github.com/ooni/probe-cli/v3/internal/legacy/tracex" "github.com/ooni/probe-cli/v3/internal/model" "github.com/ooni/probe-cli/v3/internal/netxlite" "github.com/ooni/probe-cli/v3/internal/randx" - "github.com/ooni/probe-cli/v3/internal/legacy/tracex" ) const ( diff --git a/internal/experiment/hhfm/hhfm_test.go b/internal/experiment/hhfm/hhfm_test.go index 951f148f56..43897ddaae 100644 --- a/internal/experiment/hhfm/hhfm_test.go +++ b/internal/experiment/hhfm/hhfm_test.go @@ -16,9 +16,9 @@ import ( "github.com/ooni/probe-cli/v3/internal/experiment/hhfm" "github.com/ooni/probe-cli/v3/internal/experiment/urlgetter" "github.com/ooni/probe-cli/v3/internal/legacy/mockable" + "github.com/ooni/probe-cli/v3/internal/legacy/tracex" "github.com/ooni/probe-cli/v3/internal/model" "github.com/ooni/probe-cli/v3/internal/netxlite" - "github.com/ooni/probe-cli/v3/internal/legacy/tracex" ) func TestNewExperimentMeasurer(t *testing.T) { diff --git a/internal/experiment/hirl/hirl.go b/internal/experiment/hirl/hirl.go index 0c8c8a63c0..8e1938944b 100644 --- a/internal/experiment/hirl/hirl.go +++ b/internal/experiment/hirl/hirl.go @@ -12,10 +12,10 @@ import ( "time" "github.com/ooni/probe-cli/v3/internal/legacy/netx" + "github.com/ooni/probe-cli/v3/internal/legacy/tracex" "github.com/ooni/probe-cli/v3/internal/model" "github.com/ooni/probe-cli/v3/internal/netxlite" "github.com/ooni/probe-cli/v3/internal/randx" - "github.com/ooni/probe-cli/v3/internal/legacy/tracex" ) const ( diff --git a/internal/experiment/hirl/hirl_test.go b/internal/experiment/hirl/hirl_test.go index 7d9f55bd49..7685261419 100644 --- a/internal/experiment/hirl/hirl_test.go +++ b/internal/experiment/hirl/hirl_test.go @@ -10,9 +10,9 @@ import ( "github.com/ooni/probe-cli/v3/internal/experiment/hirl" "github.com/ooni/probe-cli/v3/internal/legacy/mockable" "github.com/ooni/probe-cli/v3/internal/legacy/netx" + "github.com/ooni/probe-cli/v3/internal/legacy/tracex" "github.com/ooni/probe-cli/v3/internal/model" "github.com/ooni/probe-cli/v3/internal/netxlite" - "github.com/ooni/probe-cli/v3/internal/legacy/tracex" ) func TestNewExperimentMeasurer(t *testing.T) { diff --git a/internal/experiment/quicping/quicping.go b/internal/experiment/quicping/quicping.go index 2e048324fd..4061b35e26 100644 --- a/internal/experiment/quicping/quicping.go +++ b/internal/experiment/quicping/quicping.go @@ -17,8 +17,8 @@ import ( _ "crypto/sha256" - "github.com/ooni/probe-cli/v3/internal/model" "github.com/ooni/probe-cli/v3/internal/legacy/tracex" + "github.com/ooni/probe-cli/v3/internal/model" ) // A connectionID in QUIC diff --git a/internal/experiment/riseupvpn/riseupvpn.go b/internal/experiment/riseupvpn/riseupvpn.go index 180e614061..c0447926f1 100644 --- a/internal/experiment/riseupvpn/riseupvpn.go +++ b/internal/experiment/riseupvpn/riseupvpn.go @@ -10,9 +10,9 @@ import ( "time" "github.com/ooni/probe-cli/v3/internal/experiment/urlgetter" + "github.com/ooni/probe-cli/v3/internal/legacy/tracex" "github.com/ooni/probe-cli/v3/internal/model" "github.com/ooni/probe-cli/v3/internal/netxlite" - "github.com/ooni/probe-cli/v3/internal/legacy/tracex" ) const ( diff --git a/internal/experiment/riseupvpn/riseupvpn_test.go b/internal/experiment/riseupvpn/riseupvpn_test.go index 5ef3f3c375..98dc8768c9 100644 --- a/internal/experiment/riseupvpn/riseupvpn_test.go +++ b/internal/experiment/riseupvpn/riseupvpn_test.go @@ -15,9 +15,9 @@ import ( "github.com/ooni/probe-cli/v3/internal/experiment/riseupvpn" "github.com/ooni/probe-cli/v3/internal/experiment/urlgetter" "github.com/ooni/probe-cli/v3/internal/legacy/mockable" + "github.com/ooni/probe-cli/v3/internal/legacy/tracex" "github.com/ooni/probe-cli/v3/internal/model" "github.com/ooni/probe-cli/v3/internal/netxlite" - "github.com/ooni/probe-cli/v3/internal/legacy/tracex" ) const ( diff --git a/internal/experiment/stunreachability/stunreachability.go b/internal/experiment/stunreachability/stunreachability.go index c13b6a596f..9e4e602a2f 100644 --- a/internal/experiment/stunreachability/stunreachability.go +++ b/internal/experiment/stunreachability/stunreachability.go @@ -11,9 +11,9 @@ import ( "time" "github.com/ooni/probe-cli/v3/internal/legacy/netx" + "github.com/ooni/probe-cli/v3/internal/legacy/tracex" "github.com/ooni/probe-cli/v3/internal/model" "github.com/ooni/probe-cli/v3/internal/netxlite" - "github.com/ooni/probe-cli/v3/internal/legacy/tracex" "github.com/pion/stun" ) diff --git a/internal/experiment/tlsmiddlebox/trace.go b/internal/experiment/tlsmiddlebox/trace.go index 972a8343fb..a3fbae9b26 100644 --- a/internal/experiment/tlsmiddlebox/trace.go +++ b/internal/experiment/tlsmiddlebox/trace.go @@ -3,8 +3,8 @@ package tlsmiddlebox import ( "sync" - "github.com/ooni/probe-cli/v3/internal/model" "github.com/ooni/probe-cli/v3/internal/legacy/tracex" + "github.com/ooni/probe-cli/v3/internal/model" ) // CompleteTrace records the result of the network trace diff --git a/internal/experiment/tlstool/tlstool.go b/internal/experiment/tlstool/tlstool.go index cd3cc6371b..33a8b6f11b 100644 --- a/internal/experiment/tlstool/tlstool.go +++ b/internal/experiment/tlstool/tlstool.go @@ -17,9 +17,9 @@ import ( "github.com/ooni/probe-cli/v3/internal/experiment/tlstool/internal" "github.com/ooni/probe-cli/v3/internal/legacy/netx" + "github.com/ooni/probe-cli/v3/internal/legacy/tracex" "github.com/ooni/probe-cli/v3/internal/model" "github.com/ooni/probe-cli/v3/internal/runtimex" - "github.com/ooni/probe-cli/v3/internal/legacy/tracex" ) const ( diff --git a/internal/experiment/tor/tor.go b/internal/experiment/tor/tor.go index d0040c41b1..56a429dabc 100644 --- a/internal/experiment/tor/tor.go +++ b/internal/experiment/tor/tor.go @@ -14,11 +14,11 @@ import ( "time" "github.com/ooni/probe-cli/v3/internal/legacy/measurex" + "github.com/ooni/probe-cli/v3/internal/legacy/tracex" "github.com/ooni/probe-cli/v3/internal/model" "github.com/ooni/probe-cli/v3/internal/netxlite" "github.com/ooni/probe-cli/v3/internal/runtimex" "github.com/ooni/probe-cli/v3/internal/scrubber" - "github.com/ooni/probe-cli/v3/internal/legacy/tracex" ) const ( diff --git a/internal/experiment/tor/tor_test.go b/internal/experiment/tor/tor_test.go index b817f362d5..35be66c0aa 100644 --- a/internal/experiment/tor/tor_test.go +++ b/internal/experiment/tor/tor_test.go @@ -13,8 +13,8 @@ import ( "github.com/apex/log" "github.com/google/go-cmp/cmp" - "github.com/ooni/probe-cli/v3/internal/legacy/mockable" "github.com/ooni/probe-cli/v3/internal/legacy/measurex" + "github.com/ooni/probe-cli/v3/internal/legacy/mockable" "github.com/ooni/probe-cli/v3/internal/model" "github.com/ooni/probe-cli/v3/internal/netxlite" "github.com/ooni/probe-cli/v3/internal/scrubber" diff --git a/internal/experiment/torsf/torsf.go b/internal/experiment/torsf/torsf.go index 499f1b9c3d..b04d015353 100644 --- a/internal/experiment/torsf/torsf.go +++ b/internal/experiment/torsf/torsf.go @@ -11,11 +11,11 @@ import ( "time" "github.com/ooni/probe-cli/v3/internal/bytecounter" + "github.com/ooni/probe-cli/v3/internal/legacy/tracex" "github.com/ooni/probe-cli/v3/internal/model" "github.com/ooni/probe-cli/v3/internal/ptx" "github.com/ooni/probe-cli/v3/internal/runtimex" "github.com/ooni/probe-cli/v3/internal/torlogs" - "github.com/ooni/probe-cli/v3/internal/legacy/tracex" "github.com/ooni/probe-cli/v3/internal/tunnel" ) diff --git a/internal/experiment/urlgetter/configurer.go b/internal/experiment/urlgetter/configurer.go index dd7d262d2d..ee1d510f5a 100644 --- a/internal/experiment/urlgetter/configurer.go +++ b/internal/experiment/urlgetter/configurer.go @@ -9,9 +9,9 @@ import ( "strings" "github.com/ooni/probe-cli/v3/internal/legacy/netx" + "github.com/ooni/probe-cli/v3/internal/legacy/tracex" "github.com/ooni/probe-cli/v3/internal/model" "github.com/ooni/probe-cli/v3/internal/netxlite" - "github.com/ooni/probe-cli/v3/internal/legacy/tracex" ) // The Configurer job is to construct a Configuration that can diff --git a/internal/experiment/urlgetter/configurer_test.go b/internal/experiment/urlgetter/configurer_test.go index c74c949e07..44813ec1e6 100644 --- a/internal/experiment/urlgetter/configurer_test.go +++ b/internal/experiment/urlgetter/configurer_test.go @@ -9,8 +9,8 @@ import ( "github.com/apex/log" "github.com/ooni/probe-cli/v3/internal/experiment/urlgetter" - "github.com/ooni/probe-cli/v3/internal/netxlite" "github.com/ooni/probe-cli/v3/internal/legacy/tracex" + "github.com/ooni/probe-cli/v3/internal/netxlite" ) func TestConfigurerNewConfigurationVanilla(t *testing.T) { diff --git a/internal/experiment/urlgetter/getter.go b/internal/experiment/urlgetter/getter.go index dc7a3122df..2d7931bbec 100644 --- a/internal/experiment/urlgetter/getter.go +++ b/internal/experiment/urlgetter/getter.go @@ -6,9 +6,9 @@ import ( "net/url" "time" + "github.com/ooni/probe-cli/v3/internal/legacy/tracex" "github.com/ooni/probe-cli/v3/internal/model" "github.com/ooni/probe-cli/v3/internal/netxlite" - "github.com/ooni/probe-cli/v3/internal/legacy/tracex" "github.com/ooni/probe-cli/v3/internal/tunnel" ) diff --git a/internal/experiment/urlgetter/urlgetter.go b/internal/experiment/urlgetter/urlgetter.go index 2c3bab09d8..f42a4a8ac4 100644 --- a/internal/experiment/urlgetter/urlgetter.go +++ b/internal/experiment/urlgetter/urlgetter.go @@ -13,8 +13,8 @@ import ( "crypto/x509" "time" - "github.com/ooni/probe-cli/v3/internal/model" "github.com/ooni/probe-cli/v3/internal/legacy/tracex" + "github.com/ooni/probe-cli/v3/internal/model" ) const ( diff --git a/internal/experiment/vanillator/vanillator.go b/internal/experiment/vanillator/vanillator.go index bac59671b0..8d342b1dc0 100644 --- a/internal/experiment/vanillator/vanillator.go +++ b/internal/experiment/vanillator/vanillator.go @@ -10,10 +10,10 @@ import ( "path" "time" + "github.com/ooni/probe-cli/v3/internal/legacy/tracex" "github.com/ooni/probe-cli/v3/internal/model" "github.com/ooni/probe-cli/v3/internal/runtimex" "github.com/ooni/probe-cli/v3/internal/torlogs" - "github.com/ooni/probe-cli/v3/internal/legacy/tracex" "github.com/ooni/probe-cli/v3/internal/tunnel" ) diff --git a/internal/experiment/webconnectivity/httpanalysis_test.go b/internal/experiment/webconnectivity/httpanalysis_test.go index c74a2b1c87..ddb1bb1f31 100644 --- a/internal/experiment/webconnectivity/httpanalysis_test.go +++ b/internal/experiment/webconnectivity/httpanalysis_test.go @@ -6,8 +6,8 @@ import ( "github.com/google/go-cmp/cmp" "github.com/ooni/probe-cli/v3/internal/experiment/urlgetter" "github.com/ooni/probe-cli/v3/internal/experiment/webconnectivity" - "github.com/ooni/probe-cli/v3/internal/randx" "github.com/ooni/probe-cli/v3/internal/legacy/tracex" + "github.com/ooni/probe-cli/v3/internal/randx" ) func TestHTTPBodyLengthChecks(t *testing.T) { diff --git a/internal/experiment/webconnectivity/summary_test.go b/internal/experiment/webconnectivity/summary_test.go index 8d8313cd81..de6f58794c 100644 --- a/internal/experiment/webconnectivity/summary_test.go +++ b/internal/experiment/webconnectivity/summary_test.go @@ -6,8 +6,8 @@ import ( "github.com/google/go-cmp/cmp" "github.com/ooni/probe-cli/v3/internal/experiment/webconnectivity" - "github.com/ooni/probe-cli/v3/internal/netxlite" "github.com/ooni/probe-cli/v3/internal/legacy/tracex" + "github.com/ooni/probe-cli/v3/internal/netxlite" ) func TestSummarize(t *testing.T) { diff --git a/internal/experiment/webconnectivity/webconnectivity.go b/internal/experiment/webconnectivity/webconnectivity.go index 1f66a52a0d..72b254237d 100644 --- a/internal/experiment/webconnectivity/webconnectivity.go +++ b/internal/experiment/webconnectivity/webconnectivity.go @@ -9,8 +9,8 @@ import ( "time" "github.com/ooni/probe-cli/v3/internal/experiment/webconnectivity/internal" - "github.com/ooni/probe-cli/v3/internal/model" "github.com/ooni/probe-cli/v3/internal/legacy/tracex" + "github.com/ooni/probe-cli/v3/internal/model" ) const ( diff --git a/internal/experiment/webconnectivity/webconnectivity_test.go b/internal/experiment/webconnectivity/webconnectivity_test.go index fc9aa4f4fb..69fac2cf72 100644 --- a/internal/experiment/webconnectivity/webconnectivity_test.go +++ b/internal/experiment/webconnectivity/webconnectivity_test.go @@ -11,9 +11,9 @@ import ( "github.com/google/go-cmp/cmp" "github.com/ooni/probe-cli/v3/internal/engine" "github.com/ooni/probe-cli/v3/internal/experiment/webconnectivity" + "github.com/ooni/probe-cli/v3/internal/legacy/tracex" "github.com/ooni/probe-cli/v3/internal/model" "github.com/ooni/probe-cli/v3/internal/netxlite" - "github.com/ooni/probe-cli/v3/internal/legacy/tracex" ) func TestNewExperimentMeasurer(t *testing.T) { diff --git a/internal/experiment/webconnectivitylte/testkeys.go b/internal/experiment/webconnectivitylte/testkeys.go index ec9ead7bb1..dd2e5be31a 100644 --- a/internal/experiment/webconnectivitylte/testkeys.go +++ b/internal/experiment/webconnectivitylte/testkeys.go @@ -12,8 +12,8 @@ import ( "sync" "github.com/ooni/probe-cli/v3/internal/experiment/webconnectivity" - "github.com/ooni/probe-cli/v3/internal/model" "github.com/ooni/probe-cli/v3/internal/legacy/tracex" + "github.com/ooni/probe-cli/v3/internal/model" ) // TestKeys contains the results produced by web_connectivity. diff --git a/internal/legacy/measurex/dnsx.go b/internal/legacy/measurex/dnsx.go index 6e6e45090a..2cc5b494b6 100644 --- a/internal/legacy/measurex/dnsx.go +++ b/internal/legacy/measurex/dnsx.go @@ -10,8 +10,8 @@ import ( "context" "time" - "github.com/ooni/probe-cli/v3/internal/model" "github.com/ooni/probe-cli/v3/internal/legacy/tracex" + "github.com/ooni/probe-cli/v3/internal/model" ) // WrapDNSXRoundTripper creates a new DNSXRoundTripper that diff --git a/internal/legacy/measurex/resolver.go b/internal/legacy/measurex/resolver.go index 7aa24a8792..d546980516 100644 --- a/internal/legacy/measurex/resolver.go +++ b/internal/legacy/measurex/resolver.go @@ -11,9 +11,9 @@ import ( "strings" "time" + "github.com/ooni/probe-cli/v3/internal/legacy/tracex" "github.com/ooni/probe-cli/v3/internal/model" "github.com/ooni/probe-cli/v3/internal/netxlite" - "github.com/ooni/probe-cli/v3/internal/legacy/tracex" ) // WrapResolver creates a new Resolver that saves events into the WritableDB. diff --git a/internal/legacy/netx/config.go b/internal/legacy/netx/config.go index 5f61664455..11dade3bfe 100644 --- a/internal/legacy/netx/config.go +++ b/internal/legacy/netx/config.go @@ -9,8 +9,8 @@ import ( "net/url" "github.com/ooni/probe-cli/v3/internal/bytecounter" - "github.com/ooni/probe-cli/v3/internal/model" "github.com/ooni/probe-cli/v3/internal/legacy/tracex" + "github.com/ooni/probe-cli/v3/internal/model" ) // Config contains configuration for creating new transports, dialers, etc. When diff --git a/internal/legacy/netx/dnstransport_test.go b/internal/legacy/netx/dnstransport_test.go index 15eca0aece..f419af617d 100644 --- a/internal/legacy/netx/dnstransport_test.go +++ b/internal/legacy/netx/dnstransport_test.go @@ -5,8 +5,8 @@ import ( "strings" "testing" - "github.com/ooni/probe-cli/v3/internal/netxlite" "github.com/ooni/probe-cli/v3/internal/legacy/tracex" + "github.com/ooni/probe-cli/v3/internal/netxlite" ) func TestNewDNSClientInvalidURL(t *testing.T) { diff --git a/internal/legacy/netx/http_test.go b/internal/legacy/netx/http_test.go index 9cf2d46df5..5e7d20cd86 100644 --- a/internal/legacy/netx/http_test.go +++ b/internal/legacy/netx/http_test.go @@ -7,8 +7,8 @@ import ( "net/http" "testing" - "github.com/ooni/probe-cli/v3/internal/mocks" "github.com/ooni/probe-cli/v3/internal/legacy/tracex" + "github.com/ooni/probe-cli/v3/internal/mocks" ) func TestNewHTTPTransportWithDialer(t *testing.T) { diff --git a/internal/legacy/netx/integration_test.go b/internal/legacy/netx/integration_test.go index e451b67b29..2310312ccf 100644 --- a/internal/legacy/netx/integration_test.go +++ b/internal/legacy/netx/integration_test.go @@ -7,8 +7,8 @@ import ( "github.com/apex/log" "github.com/ooni/probe-cli/v3/internal/bytecounter" - "github.com/ooni/probe-cli/v3/internal/netxlite" "github.com/ooni/probe-cli/v3/internal/legacy/tracex" + "github.com/ooni/probe-cli/v3/internal/netxlite" ) func TestHTTPTransportWorkingAsIntended(t *testing.T) { diff --git a/internal/legacy/netx/resolver_test.go b/internal/legacy/netx/resolver_test.go index 29fce8a926..6e77aabdc1 100644 --- a/internal/legacy/netx/resolver_test.go +++ b/internal/legacy/netx/resolver_test.go @@ -6,8 +6,8 @@ import ( "testing" "github.com/apex/log" - "github.com/ooni/probe-cli/v3/internal/netxlite" "github.com/ooni/probe-cli/v3/internal/legacy/tracex" + "github.com/ooni/probe-cli/v3/internal/netxlite" ) func TestNewResolverBogonResolutionNotBroken(t *testing.T) { diff --git a/internal/legacy/netx/tls_test.go b/internal/legacy/netx/tls_test.go index 831d97ac8f..188fa79dd4 100644 --- a/internal/legacy/netx/tls_test.go +++ b/internal/legacy/netx/tls_test.go @@ -5,10 +5,10 @@ import ( "crypto/tls" "testing" + "github.com/ooni/probe-cli/v3/internal/legacy/tracex" "github.com/ooni/probe-cli/v3/internal/netxlite" "github.com/ooni/probe-cli/v3/internal/runtimex" "github.com/ooni/probe-cli/v3/internal/testingx" - "github.com/ooni/probe-cli/v3/internal/legacy/tracex" ) func TestNewTLSDialer(t *testing.T) { diff --git a/internal/netemx/scenario.go b/internal/netemx/scenario.go index 17d90bc9a8..c4a8173ff7 100644 --- a/internal/netemx/scenario.go +++ b/internal/netemx/scenario.go @@ -223,7 +223,7 @@ func MustNewScenario(config []*ScenarioDomainAddresses) *QAEnv { opts = append(opts, QAEnvOptionNetStack(addr, &HTTPCleartextServerFactory{ Factory: HTTPHandlerFactoryFunc(func(env NetStackServerFactoryEnv, stack *netem.UNetStack) http.Handler { - return testingx.HTTPHandlerProxy(env.Logger(), &netxlite.Netx{ + return testingx.NewHTTPProxyHandler(env.Logger(), &netxlite.Netx{ Underlying: &netxlite.NetemUnderlyingNetworkAdapter{UNet: stack}}) }), Ports: []int{80}, diff --git a/internal/netxlite/httpfactory.go b/internal/netxlite/httpfactory.go index b39500271f..c75d06c108 100644 --- a/internal/netxlite/httpfactory.go +++ b/internal/netxlite/httpfactory.go @@ -1,6 +1,7 @@ package netxlite import ( + "crypto/tls" "net/url" oohttp "github.com/ooni/oohttp" @@ -94,3 +95,14 @@ func HTTPTransportOptionDisableCompression(value bool) HTTPTransportOption { txp.DisableCompression = value } } + +// HTTPTransportOptionTLSClientConfig configures the .TLSClientConfig field, which +// otherwise is left nil, meaning we're using the crypto/tls or ooni/ootls defaults +// including the default cert pool. Because leaving the default .TLSClientConfig +// has implications when dialing TLS connections over an HTTP proxy, be aware that +// this default value could change in a future release of ooni/probe-cli. +func HTTPTransportOptionTLSClientConfig(config *tls.Config) HTTPTransportOption { + return func(txp *oohttp.Transport) { + txp.TLSClientConfig = config + } +} diff --git a/internal/netxlite/tls.go b/internal/netxlite/tls.go index 15bf0a10be..9cace9a9cc 100644 --- a/internal/netxlite/tls.go +++ b/internal/netxlite/tls.go @@ -255,18 +255,18 @@ func (h *tlsHandshakerLogger) Handshake( ctx context.Context, conn net.Conn, config *tls.Config, ) (net.Conn, tls.ConnectionState, error) { h.DebugLogger.Debugf( - "tls {sni=%s next=%+v}...", config.ServerName, config.NextProtos) + "tls_handshake {sni=%s next=%+v}...", config.ServerName, config.NextProtos) start := time.Now() tlsconn, state, err := h.TLSHandshaker.Handshake(ctx, conn, config) elapsed := time.Since(start) if err != nil { h.DebugLogger.Debugf( - "tls {sni=%s next=%+v}... %s in %s", config.ServerName, + "tls_handshake {sni=%s next=%+v}... %s in %s", config.ServerName, config.NextProtos, err, elapsed) return nil, tls.ConnectionState{}, err } h.DebugLogger.Debugf( - "tls {sni=%s next=%+v}... ok in %s {next=%s cipher=%s v=%s}", + "tls_handshake {sni=%s next=%+v}... ok in %s {next=%s cipher=%s v=%s}", config.ServerName, config.NextProtos, elapsed, state.NegotiatedProtocol, TLSCipherSuiteString(state.CipherSuite), TLSVersionString(state.Version)) diff --git a/internal/oohelperd/dns.go b/internal/oohelperd/dns.go index 1b8cc70830..982b3241e5 100644 --- a/internal/oohelperd/dns.go +++ b/internal/oohelperd/dns.go @@ -9,10 +9,10 @@ import ( "sync" "time" + "github.com/ooni/probe-cli/v3/internal/legacy/tracex" "github.com/ooni/probe-cli/v3/internal/logx" "github.com/ooni/probe-cli/v3/internal/model" "github.com/ooni/probe-cli/v3/internal/netxlite" - "github.com/ooni/probe-cli/v3/internal/legacy/tracex" ) // newfailure is a convenience shortcut to save typing. diff --git a/internal/oohelperd/http.go b/internal/oohelperd/http.go index cdfaed1aa9..e6867187ee 100644 --- a/internal/oohelperd/http.go +++ b/internal/oohelperd/http.go @@ -13,12 +13,12 @@ import ( "sync" "time" + "github.com/ooni/probe-cli/v3/internal/legacy/tracex" "github.com/ooni/probe-cli/v3/internal/logx" "github.com/ooni/probe-cli/v3/internal/measurexlite" "github.com/ooni/probe-cli/v3/internal/model" "github.com/ooni/probe-cli/v3/internal/netxlite" "github.com/ooni/probe-cli/v3/internal/runtimex" - "github.com/ooni/probe-cli/v3/internal/legacy/tracex" ) // TODO(bassosimone): we should refactor the TH to use step-by-step such that we diff --git a/internal/testingproxy/dialer.go b/internal/testingproxy/dialer.go new file mode 100644 index 0000000000..ea87608b4c --- /dev/null +++ b/internal/testingproxy/dialer.go @@ -0,0 +1,48 @@ +package testingproxy + +import ( + "context" + "fmt" + "log" + "net" + + "github.com/ooni/probe-cli/v3/internal/model" + "github.com/ooni/probe-cli/v3/internal/runtimex" +) + +type dialerWithAssertions struct { + // ExpectAddress is the expected IP address to dial + ExpectAddress string + + // Dialer is the underlying dialer to use + Dialer model.Dialer +} + +var _ model.Dialer = &dialerWithAssertions{} + +// CloseIdleConnections implements model.Dialer. +func (d *dialerWithAssertions) CloseIdleConnections() { + d.Dialer.CloseIdleConnections() +} + +// DialContext implements model.Dialer. +func (d *dialerWithAssertions) DialContext(ctx context.Context, network string, address string) (net.Conn, error) { + // make sure the network is tcp + const expectNetwork = "tcp" + runtimex.Assert( + network == expectNetwork, + fmt.Sprintf("dialerWithAssertions: expected %s, got %s", expectNetwork, network), + ) + log.Printf("dialerWithAssertions: verified that the network is %s as expected", expectNetwork) + + // make sure the IP address is the expected one + ipAddr, _ := runtimex.Try2(net.SplitHostPort(address)) + runtimex.Assert( + ipAddr == d.ExpectAddress, + fmt.Sprintf("dialerWithAssertions: expected %s, got %s", d.ExpectAddress, ipAddr), + ) + log.Printf("dialerWithAssertions: verified that the address is %s as expected", d.ExpectAddress) + + // now that we're sure we're using the proxy, we can actually dial + return d.Dialer.DialContext(ctx, network, address) +} diff --git a/internal/testingproxy/doc.go b/internal/testingproxy/doc.go new file mode 100644 index 0000000000..b7560cc81b --- /dev/null +++ b/internal/testingproxy/doc.go @@ -0,0 +1,2 @@ +// Package testingproxy contains shared test cases for the proxies. +package testingproxy diff --git a/internal/testingproxy/hosthttp.go b/internal/testingproxy/hosthttp.go new file mode 100644 index 0000000000..013286cbfe --- /dev/null +++ b/internal/testingproxy/hosthttp.go @@ -0,0 +1,74 @@ +package testingproxy + +import ( + "fmt" + "net/http" + "net/url" + "testing" + + "github.com/apex/log" + "github.com/ooni/probe-cli/v3/internal/netxlite" + "github.com/ooni/probe-cli/v3/internal/runtimex" + "github.com/ooni/probe-cli/v3/internal/testingx" +) + +// WithHostNetworkHTTPProxyAndURL returns a [TestCase] where: +// +// - we fetch a URL; +// +// - using the host network; +// +// - and an HTTP proxy. +// +// Because this [TestCase] uses the host network, it does not run in -short mode. +func WithHostNetworkHTTPProxyAndURL(URL string) TestCase { + return &hostNetworkTestCaseWithHTTP{ + TargetURL: URL, + } +} + +type hostNetworkTestCaseWithHTTP struct { + TargetURL string +} + +var _ TestCase = &hostNetworkTestCaseWithHTTP{} + +// Name implements TestCase. +func (tc *hostNetworkTestCaseWithHTTP) Name() string { + return fmt.Sprintf("fetching %s using the host network and an HTTP proxy", tc.TargetURL) +} + +// Run implements TestCase. +func (tc *hostNetworkTestCaseWithHTTP) Run(t *testing.T) { + // create an instance of Netx where the underlying network is nil, + // which means we're using the host's network + netx := &netxlite.Netx{Underlying: nil} + + // create the proxy server using the host network + proxyServer := testingx.MustNewHTTPServer(testingx.NewHTTPProxyHandler(log.Log, netx)) + defer proxyServer.Close() + + log.SetLevel(log.DebugLevel) + + // create an HTTP client configured to use the given proxy + // + // note how we use a dialer that asserts that we're using the proxy IP address + // rather than the host address, so we're sure that we're using the proxy + dialer := &dialerWithAssertions{ + ExpectAddress: "127.0.0.1", + Dialer: netx.NewDialerWithResolver(log.Log, netx.NewStdlibResolver(log.Log)), + } + tlsDialer := netxlite.NewTLSDialer(dialer, netxlite.NewTLSHandshakerStdlib(log.Log)) + txp := netxlite.NewHTTPTransportWithOptions(log.Log, dialer, tlsDialer, + netxlite.HTTPTransportOptionProxyURL(runtimex.Try1(url.Parse(proxyServer.URL)))) + client := &http.Client{Transport: txp} + defer client.CloseIdleConnections() + + // get the homepage and assert we're getting a succesful response + httpCheckResponse(t, client, tc.TargetURL) +} + +// Short implements TestCase. +func (tc *hostNetworkTestCaseWithHTTP) Short() bool { + return false +} diff --git a/internal/testingproxy/hosthttps.go b/internal/testingproxy/hosthttps.go new file mode 100644 index 0000000000..67be7cb417 --- /dev/null +++ b/internal/testingproxy/hosthttps.go @@ -0,0 +1,83 @@ +package testingproxy + +import ( + "crypto/tls" + "fmt" + "net/http" + "net/url" + "testing" + + "github.com/apex/log" + "github.com/ooni/probe-cli/v3/internal/netxlite" + "github.com/ooni/probe-cli/v3/internal/runtimex" + "github.com/ooni/probe-cli/v3/internal/testingx" +) + +// WithHostNetworkHTTPWithTLSProxyAndURL returns a [TestCase] where: +// +// - we fetch a URL; +// +// - using the host network; +// +// - and an HTTPS proxy. +// +// Because this [TestCase] uses the host network, it does not run in -short mode. +func WithHostNetworkHTTPWithTLSProxyAndURL(URL string) TestCase { + return &hostNetworkTestCaseWithHTTPWithTLS{ + TargetURL: URL, + } +} + +type hostNetworkTestCaseWithHTTPWithTLS struct { + TargetURL string +} + +var _ TestCase = &hostNetworkTestCaseWithHTTPWithTLS{} + +// Name implements TestCase. +func (tc *hostNetworkTestCaseWithHTTPWithTLS) Name() string { + return fmt.Sprintf("fetching %s using the host network and an HTTPS proxy", tc.TargetURL) +} + +// Run implements TestCase. +func (tc *hostNetworkTestCaseWithHTTPWithTLS) Run(t *testing.T) { + // create an instance of Netx where the underlying network is nil, + // which means we're using the host's network + netx := &netxlite.Netx{Underlying: nil} + + // create the proxy server using the host network + proxyServer := testingx.MustNewHTTPServerTLS(testingx.NewHTTPProxyHandler(log.Log, netx)) + defer proxyServer.Close() + + //log.SetLevel(log.DebugLevel) + + // extend the default cert pool with the proxy's own CA + pool := netxlite.NewMozillaCertPool() + pool.AddCert(proxyServer.CACert) + tlsConfig := &tls.Config{RootCAs: pool} + + // create an HTTP client configured to use the given proxy + // + // note how we use a dialer that asserts that we're using the proxy IP address + // rather than the host address, so we're sure that we're using the proxy + dialer := &dialerWithAssertions{ + ExpectAddress: "127.0.0.1", + Dialer: netx.NewDialerWithResolver(log.Log, netx.NewStdlibResolver(log.Log)), + } + tlsDialer := netxlite.NewTLSDialerWithConfig( + dialer, netxlite.NewTLSHandshakerStdlib(log.Log), + tlsConfig, + ) + txp := netxlite.NewHTTPTransportWithOptions(log.Log, dialer, tlsDialer, + netxlite.HTTPTransportOptionProxyURL(runtimex.Try1(url.Parse(proxyServer.URL)))) + client := &http.Client{Transport: txp} + defer client.CloseIdleConnections() + + // get the homepage and assert we're getting a succesful response + httpCheckResponse(t, client, tc.TargetURL) +} + +// Short implements TestCase. +func (tc *hostNetworkTestCaseWithHTTPWithTLS) Short() bool { + return false +} diff --git a/internal/testingproxy/httputils.go b/internal/testingproxy/httputils.go new file mode 100644 index 0000000000..4fb9b732c6 --- /dev/null +++ b/internal/testingproxy/httputils.go @@ -0,0 +1,53 @@ +package testingproxy + +import "net/http" + +type httpClient interface { + Get(URL string) (*http.Response, error) +} + +type httpClientMock struct { + MockGet func(URL string) (*http.Response, error) +} + +var _ httpClient = &httpClientMock{} + +// Get implements httpClient. +func (c *httpClientMock) Get(URL string) (*http.Response, error) { + return c.MockGet(URL) +} + +type httpTestingT interface { + Logf(format string, v ...any) + Fatal(v ...any) +} + +type httpTestingTMock struct { + MockLogf func(format string, v ...any) + MockFatal func(v ...any) +} + +var _ httpTestingT = &httpTestingTMock{} + +// Fatal implements httpTestingT. +func (t *httpTestingTMock) Fatal(v ...any) { + t.MockFatal(v...) +} + +// Logf implements httpTestingT. +func (t *httpTestingTMock) Logf(format string, v ...any) { + t.MockLogf(format, v...) +} + +func httpCheckResponse(t httpTestingT, client httpClient, targetURL string) { + // get the homepage and assert we're getting a succesful response + resp, err := client.Get(targetURL) + if err != nil { + t.Fatal(err) + } + defer resp.Body.Close() + t.Logf("%+v", resp) + if resp.StatusCode != 200 { + t.Fatal("invalid status code") + } +} diff --git a/internal/testingproxy/httputils_test.go b/internal/testingproxy/httputils_test.go new file mode 100644 index 0000000000..a01c34cac9 --- /dev/null +++ b/internal/testingproxy/httputils_test.go @@ -0,0 +1,122 @@ +package testingproxy + +import ( + "bytes" + "errors" + "io" + "net/http" + "testing" +) + +func TestHTTPClientMock(t *testing.T) { + t.Run("for Get", func(t *testing.T) { + expected := errors.New("mocked error") + c := &httpClientMock{ + MockGet: func(URL string) (*http.Response, error) { + return nil, expected + }, + } + resp, err := c.Get("https://www.google.com/") + if !errors.Is(err, expected) { + t.Fatal("unexpected error") + } + if resp != nil { + t.Fatal("expected nil response") + } + }) +} + +func TestHTTPTestingTMock(t *testing.T) { + t.Run("for Fatal", func(t *testing.T) { + var called bool + mt := &httpTestingTMock{ + MockFatal: func(v ...any) { + called = true + }, + } + mt.Fatal("antani") + if !called { + t.Fatal("not called") + } + }) + + t.Run("for Logf", func(t *testing.T) { + var called bool + mt := &httpTestingTMock{ + MockLogf: func(format string, v ...any) { + called = true + }, + } + mt.Logf("antani %v", "mascetti") + if !called { + t.Fatal("not called") + } + }) +} + +func TestHTTPCheckResponseHandlesFailures(t *testing.T) { + type testcase struct { + name string + mclient httpClient + expectLog bool + } + + testcases := []testcase{{ + name: "when HTTP round trip fails", + mclient: &httpClientMock{ + MockGet: func(URL string) (*http.Response, error) { + return nil, io.EOF + }, + }, + expectLog: false, + }, { + name: "with unexpected status code", + mclient: &httpClientMock{ + MockGet: func(URL string) (*http.Response, error) { + resp := &http.Response{ + StatusCode: 404, + Body: io.NopCloser(bytes.NewReader(nil)), + } + return resp, nil + }, + }, + expectLog: true, + }} + + for _, tc := range testcases { + t.Run(tc.name, func(t *testing.T) { + + // prepare for capturing what happened + var ( + calledLogf bool + calledFatal bool + ) + mt := &httpTestingTMock{ + MockLogf: func(format string, v ...any) { + calledLogf = true + }, + MockFatal: func(v ...any) { + calledFatal = true + panic(v) + }, + } + + // make sure we handle the panic and check what happened + defer func() { + result := recover() + if result == nil { + t.Fatal("did not panic") + } + if !calledFatal { + t.Fatal("did not actually call t.Fatal") + } + if tc.expectLog != calledLogf { + t.Fatal("tc.expectLog is", tc.expectLog, "but calledLogf is", calledLogf) + } + }() + + // invoke the function we're testing + httpCheckResponse(mt, tc.mclient, "https://www.google.com/") + }) + } +} diff --git a/internal/testingproxy/netemhttp.go b/internal/testingproxy/netemhttp.go new file mode 100644 index 0000000000..0caf7d5906 --- /dev/null +++ b/internal/testingproxy/netemhttp.go @@ -0,0 +1,163 @@ +package testingproxy + +import ( + "crypto/tls" + "fmt" + "net" + "net/http" + "net/url" + "testing" + + "github.com/apex/log" + "github.com/ooni/netem" + "github.com/ooni/probe-cli/v3/internal/netxlite" + "github.com/ooni/probe-cli/v3/internal/runtimex" + "github.com/ooni/probe-cli/v3/internal/testingx" +) + +// WithNetemHTTPProxyAndURL returns a [TestCase] where: +// +// - we fetch a URL; +// +// - using the github.com/ooni.netem; +// +// - and an HTTP proxy. +// +// Because this [TestCase] uses netem, it also runs in -short mode. +func WithNetemHTTPProxyAndURL(URL string) TestCase { + return &netemTestCaseWithHTTP{ + TargetURL: URL, + } +} + +type netemTestCaseWithHTTP struct { + TargetURL string +} + +var _ TestCase = &netemTestCaseWithHTTP{} + +// Name implements TestCase. +func (tc *netemTestCaseWithHTTP) Name() string { + return fmt.Sprintf("fetching %s using netem and an HTTP proxy", tc.TargetURL) +} + +// Run implements TestCase. +func (tc *netemTestCaseWithHTTP) Run(t *testing.T) { + topology := runtimex.Try1(netem.NewStarTopology(log.Log)) + defer topology.Close() + + const ( + wwwIPAddr = "93.184.216.34" + proxyIPAddr = "10.0.0.1" + clientIPAddr = "10.0.0.2" + ) + + // create: + // + // - a www stack modeling www.example.com + // + // - a proxy stack + // + // - a client stack + // + // Note that www.example.com's IP address is also the resolver used by everyone + wwwStack := runtimex.Try1(topology.AddHost(wwwIPAddr, wwwIPAddr, &netem.LinkConfig{})) + proxyStack := runtimex.Try1(topology.AddHost(proxyIPAddr, wwwIPAddr, &netem.LinkConfig{})) + clientStack := runtimex.Try1(topology.AddHost(clientIPAddr, wwwIPAddr, &netem.LinkConfig{})) + + // configure the wwwStack as the DNS resolver with proper configuration + dnsConfig := netem.NewDNSConfig() + dnsConfig.AddRecord("www.example.com.", "", wwwIPAddr) + dnsServer := runtimex.Try1(netem.NewDNSServer(log.Log, wwwStack, wwwIPAddr, dnsConfig)) + defer dnsServer.Close() + + // configure the wwwStack to respond to HTTP requests on port 80 + wwwServer80 := testingx.MustNewHTTPServerEx( + &net.TCPAddr{IP: net.ParseIP(wwwIPAddr), Port: 80}, + wwwStack, + http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Write([]byte("Bonsoir, Elliot!\r\n")) + }), + ) + defer wwwServer80.Close() + + // configure the wwwStack to respond to HTTPS requests on port 443 + wwwServer443 := testingx.MustNewHTTPServerTLSEx( + &net.TCPAddr{IP: net.ParseIP(wwwIPAddr), Port: 443}, + wwwStack, + http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Write([]byte("Bonsoir, Elliot!\r\n")) + }), + wwwStack, + ) + defer wwwServer443.Close() + + // configure the proxyStack to implement the HTTP proxy on port 8080 + proxyServer := testingx.MustNewHTTPServerEx( + &net.TCPAddr{IP: net.ParseIP(proxyIPAddr), Port: 8080}, + proxyStack, + testingx.NewHTTPProxyHandler(log.Log, &netxlite.Netx{ + Underlying: &netxlite.NetemUnderlyingNetworkAdapter{UNet: proxyStack}}), + ) + defer proxyServer.Close() + + // crete the netx instance for the client + netx := &netxlite.Netx{Underlying: &netxlite.NetemUnderlyingNetworkAdapter{UNet: clientStack}} + + log.SetLevel(log.DebugLevel) + + /* + ,ggg, gg ,gg,ggggggggggggggg ,gggggggggggggg + dP""Y8a 88 ,8PdP""""""88"""""""dP""""""88"""""" + Yb, `88 88 d8'Yb,_ 88 Yb,_ 88 + `" 88 88 88 `"" 88 `"" 88 + 88 88 88 88 ggg88gggg + 88 88 88 88 88 8 + 88 88 88 88 88 + Y8 ,88, 8P gg, 88 gg, 88 + Yb,,d8""8b,,dP "Yb,,8P "Yb,,8P + "88" "88" "Y8P' "Y8P' + + + Not necessarily wrong, but certainly I did not expect this! When we are + using an HTTP proxy, the stdlib/oohttp *Transport uses the DialContext for + dialing with the proxy but then uses its own TLS for handshaking over + the TCP connection with the proxy. + + So, the naive implementation of this test case fails with an X.509 + certificate error when we're using netem, because we're not using the + overriden DialTLSContext anymore, and the *Transport does not + otherwise know about the root CA used by netem. + + The current fix is to use netxlite.HTTPTransportOptionTLSClientConfig + below. However, I'm now wondering if we're using the right default. + */ + + // create an HTTP client configured to use the given proxy + // + // note how we use a dialer that asserts that we're using the proxy IP address + // rather than the host address, so we're sure that we're using the proxy + dialer := &dialerWithAssertions{ + ExpectAddress: proxyIPAddr, + Dialer: netx.NewDialerWithResolver(log.Log, netx.NewStdlibResolver(log.Log)), + } + tlsDialer := netxlite.NewTLSDialer(dialer, netx.NewTLSHandshakerStdlib(log.Log)) + txp := netxlite.NewHTTPTransportWithOptions(log.Log, dialer, tlsDialer, + netxlite.HTTPTransportOptionProxyURL(runtimex.Try1(url.Parse(proxyServer.URL))), + + // see above WTF comment + netxlite.HTTPTransportOptionTLSClientConfig(&tls.Config{ + RootCAs: runtimex.Try1(clientStack.DefaultCertPool()), + }), + ) + client := &http.Client{Transport: txp} + defer client.CloseIdleConnections() + + // get the homepage and assert we're getting a succesful response + httpCheckResponse(t, client, tc.TargetURL) +} + +// Short implements TestCase. +func (tc *netemTestCaseWithHTTP) Short() bool { + return true +} diff --git a/internal/testingproxy/netemhttps.go b/internal/testingproxy/netemhttps.go new file mode 100644 index 0000000000..8abdfd4d56 --- /dev/null +++ b/internal/testingproxy/netemhttps.go @@ -0,0 +1,164 @@ +package testingproxy + +import ( + "crypto/tls" + "fmt" + "net" + "net/http" + "net/url" + "testing" + + "github.com/apex/log" + "github.com/ooni/netem" + "github.com/ooni/probe-cli/v3/internal/netxlite" + "github.com/ooni/probe-cli/v3/internal/runtimex" + "github.com/ooni/probe-cli/v3/internal/testingx" +) + +// WithNetemHTTPWithTLSProxyAndURL returns a [TestCase] where: +// +// - we fetch a URL; +// +// - using the github.com/ooni.netem; +// +// - and an HTTPS proxy. +// +// Because this [TestCase] uses netem, it also runs in -short mode. +func WithNetemHTTPWithTLSProxyAndURL(URL string) TestCase { + return &netemTestCaseWithHTTPWithTLS{ + TargetURL: URL, + } +} + +type netemTestCaseWithHTTPWithTLS struct { + TargetURL string +} + +var _ TestCase = &netemTestCaseWithHTTPWithTLS{} + +// Name implements TestCase. +func (tc *netemTestCaseWithHTTPWithTLS) Name() string { + return fmt.Sprintf("fetching %s using netem and an HTTPS proxy", tc.TargetURL) +} + +// Run implements TestCase. +func (tc *netemTestCaseWithHTTPWithTLS) Run(t *testing.T) { + topology := runtimex.Try1(netem.NewStarTopology(log.Log)) + defer topology.Close() + + const ( + wwwIPAddr = "93.184.216.34" + proxyIPAddr = "10.0.0.1" + clientIPAddr = "10.0.0.2" + ) + + // create: + // + // - a www stack modeling www.example.com + // + // - a proxy stack + // + // - a client stack + // + // Note that www.example.com's IP address is also the resolver used by everyone + wwwStack := runtimex.Try1(topology.AddHost(wwwIPAddr, wwwIPAddr, &netem.LinkConfig{})) + proxyStack := runtimex.Try1(topology.AddHost(proxyIPAddr, wwwIPAddr, &netem.LinkConfig{})) + clientStack := runtimex.Try1(topology.AddHost(clientIPAddr, wwwIPAddr, &netem.LinkConfig{})) + + // configure the wwwStack as the DNS resolver with proper configuration + dnsConfig := netem.NewDNSConfig() + dnsConfig.AddRecord("www.example.com.", "", wwwIPAddr) + dnsServer := runtimex.Try1(netem.NewDNSServer(log.Log, wwwStack, wwwIPAddr, dnsConfig)) + defer dnsServer.Close() + + // configure the wwwStack to respond to HTTP requests on port 80 + wwwServer80 := testingx.MustNewHTTPServerEx( + &net.TCPAddr{IP: net.ParseIP(wwwIPAddr), Port: 80}, + wwwStack, + http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Write([]byte("Bonsoir, Elliot!\r\n")) + }), + ) + defer wwwServer80.Close() + + // configure the wwwStack to respond to HTTPS requests on port 443 + wwwServer443 := testingx.MustNewHTTPServerTLSEx( + &net.TCPAddr{IP: net.ParseIP(wwwIPAddr), Port: 443}, + wwwStack, + http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Write([]byte("Bonsoir, Elliot!\r\n")) + }), + wwwStack, + ) + defer wwwServer443.Close() + + // configure the proxyStack to implement the HTTP proxy on port 8080 + proxyServer := testingx.MustNewHTTPServerTLSEx( + &net.TCPAddr{IP: net.ParseIP(proxyIPAddr), Port: 80443}, + proxyStack, + testingx.NewHTTPProxyHandler(log.Log, &netxlite.Netx{ + Underlying: &netxlite.NetemUnderlyingNetworkAdapter{UNet: proxyStack}}), + proxyStack, + ) + defer proxyServer.Close() + + // crete the netx instance for the client + netx := &netxlite.Netx{Underlying: &netxlite.NetemUnderlyingNetworkAdapter{UNet: clientStack}} + + log.SetLevel(log.DebugLevel) + + /* + ,ggg, gg ,gg,ggggggggggggggg ,gggggggggggggg + dP""Y8a 88 ,8PdP""""""88"""""""dP""""""88"""""" + Yb, `88 88 d8'Yb,_ 88 Yb,_ 88 + `" 88 88 88 `"" 88 `"" 88 + 88 88 88 88 ggg88gggg + 88 88 88 88 88 8 + 88 88 88 88 88 + Y8 ,88, 8P gg, 88 gg, 88 + Yb,,d8""8b,,dP "Yb,,8P "Yb,,8P + "88" "88" "Y8P' "Y8P' + + + Not necessarily wrong, but certainly I did not expect this! When we are + using an HTTPS proxy, the stdlib/oohttp *Transport uses the DialTLSContext for + dialing with the proxy but then uses its own TLS for handshaking over + the TLS connection with the proxy. + + So, the naive implementation of this test case fails with an X.509 + certificate error when we're using netem, because we're not using the + overriden DialTLSContext anymore, and the *Transport does not + otherwise know about the root CA used by netem. + + The current fix is to use netxlite.HTTPTransportOptionTLSClientConfig + below. However, I'm now wondering if we're using the right default. + */ + + // create an HTTP client configured to use the given proxy + // + // note how we use a dialer that asserts that we're using the proxy IP address + // rather than the host address, so we're sure that we're using the proxy + dialer := &dialerWithAssertions{ + ExpectAddress: proxyIPAddr, + Dialer: netx.NewDialerWithResolver(log.Log, netx.NewStdlibResolver(log.Log)), + } + tlsDialer := netxlite.NewTLSDialer(dialer, netx.NewTLSHandshakerStdlib(log.Log)) + txp := netxlite.NewHTTPTransportWithOptions(log.Log, dialer, tlsDialer, + netxlite.HTTPTransportOptionProxyURL(runtimex.Try1(url.Parse(proxyServer.URL))), + + // see above WTF comment + netxlite.HTTPTransportOptionTLSClientConfig(&tls.Config{ + RootCAs: runtimex.Try1(clientStack.DefaultCertPool()), + }), + ) + client := &http.Client{Transport: txp} + defer client.CloseIdleConnections() + + // get the homepage and assert we're getting a succesful response + httpCheckResponse(t, client, tc.TargetURL) +} + +// Short implements TestCase. +func (tc *netemTestCaseWithHTTPWithTLS) Short() bool { + return true +} diff --git a/internal/testingproxy/qa_test.go b/internal/testingproxy/qa_test.go new file mode 100644 index 0000000000..3e4019068a --- /dev/null +++ b/internal/testingproxy/qa_test.go @@ -0,0 +1,19 @@ +package testingproxy_test + +import ( + "testing" + + "github.com/ooni/probe-cli/v3/internal/testingproxy" +) + +func TestWorkingAsIntended(t *testing.T) { + for _, testCase := range testingproxy.AllTestCases { + short := testCase.Short() + if !short && testing.Short() { + t.Skip("skip test in short mode") + } + t.Run(testCase.Name(), func(t *testing.T) { + testCase.Run(t) + }) + } +} diff --git a/internal/testingproxy/testcase.go b/internal/testingproxy/testcase.go new file mode 100644 index 0000000000..4ad2d864ff --- /dev/null +++ b/internal/testingproxy/testcase.go @@ -0,0 +1,34 @@ +package testingproxy + +import "testing" + +// TestCase is a test case implemented by this package. +type TestCase interface { + // Name returns the test case name. + Name() string + + // Run runs the test case. + Run(t *testing.T) + + // Short returns whether this is a short test. + Short() bool +} + +// AllTestCases contains all the test cases. +var AllTestCases = []TestCase{ + // host network and HTTP proxy + WithHostNetworkHTTPProxyAndURL("http://www.example.com/"), + WithHostNetworkHTTPProxyAndURL("https://www.example.com/"), + + // host network and HTTPS proxy + WithHostNetworkHTTPWithTLSProxyAndURL("http://www.example.com/"), + WithHostNetworkHTTPWithTLSProxyAndURL("https://www.example.com/"), + + // netem and HTTP proxy + WithNetemHTTPProxyAndURL("http://www.example.com/"), + WithNetemHTTPProxyAndURL("https://www.example.com/"), + + // netem and HTTPS proxy + WithNetemHTTPWithTLSProxyAndURL("http://www.example.com/"), + WithNetemHTTPWithTLSProxyAndURL("https://www.example.com/"), +} diff --git a/internal/testingx/httpproxy.go b/internal/testingx/httpproxy.go new file mode 100644 index 0000000000..a66c060d75 --- /dev/null +++ b/internal/testingx/httpproxy.go @@ -0,0 +1,138 @@ +package testingx + +import ( + "io" + "net/http" + "sync" + + "github.com/ooni/probe-cli/v3/internal/logx" + "github.com/ooni/probe-cli/v3/internal/model" + "github.com/ooni/probe-cli/v3/internal/runtimex" +) + +// HTTPProxyHandlerNetx abstracts [*netxlite.Netx] for the [*HTTPProxyHandler]. +type HTTPProxyHandlerNetx interface { + // NewDialerWithResolver creates a new dialer using the given resolver and logger. + NewDialerWithResolver(dl model.DebugLogger, r model.Resolver, w ...model.DialerWrapper) model.Dialer + + // NewHTTPTransportStdlib creates a new HTTP transport using the stdlib. + NewHTTPTransportStdlib(dl model.DebugLogger) model.HTTPTransport + + // NewStdlibResolver creates a new resolver that tries to use the getaddrinfo libc call. + NewStdlibResolver(logger model.DebugLogger) model.Resolver +} + +// httpProxyHandler is an HTTP/HTTPS proxy. +type httpProxyHandler struct { + // Logger is the logger to use. + Logger model.Logger + + // Netx is the network to use. + Netx HTTPProxyHandlerNetx +} + +// NewHTTPProxyHandler constructs a new [*HTTPProxyHandler]. +func NewHTTPProxyHandler(logger model.Logger, netx HTTPProxyHandlerNetx) http.Handler { + return &httpProxyHandler{ + Logger: &logx.PrefixLogger{ + Prefix: "PROXY: ", + Logger: logger, + }, + Netx: netx, + } +} + +// ServeHTTP implements http.Handler. +func (ph *httpProxyHandler) ServeHTTP(rw http.ResponseWriter, req *http.Request) { + ph.Logger.Infof("request: %+v", req) + + switch req.Method { + case http.MethodConnect: + ph.connect(rw, req) + + case http.MethodGet: + ph.get(rw, req) + + default: + rw.WriteHeader(http.StatusNotImplemented) + } +} + +func (ph *httpProxyHandler) connect(rw http.ResponseWriter, req *http.Request) { + resolver := ph.Netx.NewStdlibResolver(ph.Logger) + dialer := ph.Netx.NewDialerWithResolver(ph.Logger, resolver) + + sconn, err := dialer.DialContext(req.Context(), "tcp", req.Host) + if err != nil { + rw.WriteHeader(http.StatusBadGateway) + return + } + + hijacker := rw.(http.Hijacker) + cconn, buffered := runtimex.Try2(hijacker.Hijack()) + runtimex.Assert(buffered.Reader.Buffered() <= 0, "data before finishing HTTP handshake") + + _, _ = cconn.Write([]byte("HTTP/1.1 200 Ok\r\n\r\n")) + + wg := &sync.WaitGroup{} + wg.Add(1) + go func() { + defer wg.Done() + _, _ = io.Copy(sconn, cconn) + }() + + wg.Add(1) + go func() { + defer wg.Done() + _, _ = io.Copy(cconn, sconn) + }() + + wg.Wait() +} + +func (ph *httpProxyHandler) get(rw http.ResponseWriter, req *http.Request) { + // reject requests that already visited the proxy and requests we cannot route + if req.Host == "" || req.Header.Get("Via") != "" { + rw.WriteHeader(http.StatusBadRequest) + return + } + + // clone the request before modifying it + req = req.Clone(req.Context()) + + // include proxy header to prevent sending requests to ourself + req.Header.Add("Via", "testingx/0.1.0") + + // fix: "http: Request.RequestURI can't be set in client requests" + req.RequestURI = "" + + // fix: `http: unsupported protocol scheme ""` + req.URL.Host = req.Host + + // fix: "http: no Host in request URL" + req.URL.Scheme = "http" + + ph.Logger.Debugf("sending request: %s", req) + + // create HTTP client using netx + txp := ph.Netx.NewHTTPTransportStdlib(ph.Logger) + + // obtain response + resp, err := txp.RoundTrip(req) + if err != nil { + ph.Logger.Warnf("request failed: %s", err.Error()) + rw.WriteHeader(http.StatusBadGateway) + return + } + + // write response + rw.WriteHeader(resp.StatusCode) + for key, values := range resp.Header { + for _, value := range values { + rw.Header().Add(key, value) + } + } + + // write response body + _, _ = io.Copy(rw, resp.Body) +} diff --git a/internal/testingx/httpproxy_test.go b/internal/testingx/httpproxy_test.go new file mode 100644 index 0000000000..56fb6c5058 --- /dev/null +++ b/internal/testingx/httpproxy_test.go @@ -0,0 +1,19 @@ +package testingx_test + +import ( + "testing" + + "github.com/ooni/probe-cli/v3/internal/testingproxy" +) + +func TestHTTPProxyHandler(t *testing.T) { + for _, testCase := range testingproxy.AllTestCases { + short := testCase.Short() + if !short && testing.Short() { + t.Skip("skip test in short mode") + } + t.Run(testCase.Name(), func(t *testing.T) { + testCase.Run(t) + }) + } +} diff --git a/internal/testingx/httptestx.go b/internal/testingx/httptestx.go index 61e4dd21d0..488fe1d186 100644 --- a/internal/testingx/httptestx.go +++ b/internal/testingx/httptestx.go @@ -3,12 +3,10 @@ package testingx import ( "crypto/tls" "crypto/x509" - "io" "net" "net/http" "net/url" - "github.com/ooni/probe-cli/v3/internal/model" "github.com/ooni/probe-cli/v3/internal/optional" "github.com/ooni/probe-cli/v3/internal/runtimex" ) @@ -21,20 +19,35 @@ import ( // transitioning the code from that struct to this one. type HTTPServer struct { // Config contains the server started by the constructor. + // + // This field also exists in the [*net/http/httptest.Server] struct. Config *http.Server // Listener is the underlying [net.Listener]. + // + // This field also exists in the [*net/http/httptest.Server] struct. Listener net.Listener // TLS contains the TLS configuration used by the constructor, or nil // if you constructed a server that does not use TLS. + // + // This field also exists in the [*net/http/httptest.Server] struct. TLS *tls.Config // URL is the base URL used by the server. + // + // This field also exists in the [*net/http/httptest.Server] struct. URL string // X509CertPool is the X.509 cert pool we're using or nil. + // + // This field is an extension that is not present in the httptest package. X509CertPool *x509.CertPool + + // CACert is the CA used by this server. + // + // This field is an extension that is not present in the httptest package. + CACert *x509.Certificate } // MustNewHTTPServer is morally equivalent to [httptest.NewHTTPServer]. @@ -79,6 +92,7 @@ func mustNewHTTPServer( switch !tlsConfig.IsNone() { case true: baseURL.Scheme = "https" + srv.CACert = tlsConfig.Unwrap().CACert() srv.TLS = tlsConfig.Unwrap().ServerTLSConfig() srv.Config.TLSConfig = srv.TLS srv.X509CertPool = runtimex.Try1(tlsConfig.Unwrap().DefaultCertPool()) @@ -160,70 +174,3 @@ func httpHandlerHijack(w http.ResponseWriter, r *http.Request, policy string) { // nothing } } - -// TODO(bassosimone): eventually we may want to have a model type -// that models the equivalent of [netxlite.Netx]. - -// HTTPHandlerProxyNetx is [netxlite.Netx] as seen by [HTTPHandlerProxy]. -type HTTPHandlerProxyNetx interface { - NewHTTPTransportStdlib(logger model.DebugLogger) model.HTTPTransport -} - -// HTTPHandlerProxy is a handler implementing an HTTP proxy using the host header -// to determine who to connect to. We additionally use the via header to avoid sending -// requests to ourself. Please, note that we designed this proxy ONLY to be used for -// testing purposes and that it's rather simplistic. -func HTTPHandlerProxy(logger model.Logger, netx HTTPHandlerProxyNetx) http.Handler { - return http.HandlerFunc(func(rw http.ResponseWriter, req *http.Request) { - // reject requests that already visited the proxy and requests we cannot route - if req.Host == "" || req.Header.Get("Via") != "" { - rw.WriteHeader(http.StatusBadRequest) - return - } - - // be explicit about not supporting request bodies - if req.Method != http.MethodGet { - rw.WriteHeader(http.StatusNotImplemented) - return - } - - // clone the request before modifying it - req = req.Clone(req.Context()) - - // include proxy header to prevent sending requests to ourself - req.Header.Add("Via", "testingx/0.1.0") - - // fix: "http: Request.RequestURI can't be set in client requests" - req.RequestURI = "" - - // fix: `http: unsupported protocol scheme ""` - req.URL.Host = req.Host - - // fix: "http: no Host in request URL" - req.URL.Scheme = "http" - - logger.Debugf("PROXY: sending request: %s", req) - - // create HTTP client using netx - txp := netx.NewHTTPTransportStdlib(logger) - - // obtain response - resp, err := txp.RoundTrip(req) - if err != nil { - logger.Warnf("PROXY: request failed: %s", err.Error()) - rw.WriteHeader(http.StatusBadGateway) - return - } - - // write response - rw.WriteHeader(resp.StatusCode) - for key, values := range resp.Header { - for _, value := range values { - rw.Header().Add(key, value) - } - } - - // write response body - _, _ = io.Copy(rw, resp.Body) - }) -} diff --git a/internal/testingx/httptestx_test.go b/internal/testingx/httptestx_test.go index c34e6676a0..9c00133fa6 100644 --- a/internal/testingx/httptestx_test.go +++ b/internal/testingx/httptestx_test.go @@ -10,15 +10,12 @@ import ( "io" "net" "net/http" - "net/http/httptest" - "net/url" "testing" "time" "github.com/apex/log" "github.com/google/go-cmp/cmp" "github.com/ooni/netem" - "github.com/ooni/probe-cli/v3/internal/mocks" "github.com/ooni/probe-cli/v3/internal/netxlite" "github.com/ooni/probe-cli/v3/internal/runtimex" "github.com/ooni/probe-cli/v3/internal/testingx" @@ -476,198 +473,3 @@ func TestHTTPTestxWithNetem(t *testing.T) { }) } } - -func TestHTTPHandlerProxy(t *testing.T) { - expectedBody := []byte("Google is built by a large team of engineers, designers, researchers, robots, and others in many different sites across the globe. It is updated continuously, and built with more tools and technologies than we can shake a stick at. If you'd like to help us out, see careers.google.com.\n") - - type testcase struct { - name string - construct func() (*netxlite.Netx, string, []io.Closer) - short bool - } - - testcases := []testcase{ - { - name: "using the real network", - construct: func() (*netxlite.Netx, string, []io.Closer) { - var closers []io.Closer - - netx := &netxlite.Netx{ - Underlying: nil, // so we're using the real network - } - - proxyServer := testingx.MustNewHTTPServer(testingx.HTTPHandlerProxy(log.Log, netx)) - closers = append(closers, proxyServer) - - return netx, proxyServer.URL, closers - }, - short: false, - }, - - { - name: "using netem", - construct: func() (*netxlite.Netx, string, []io.Closer) { - var closers []io.Closer - - topology := runtimex.Try1(netem.NewStarTopology(log.Log)) - closers = append(closers, topology) - - wwwStack := runtimex.Try1(topology.AddHost("142.251.209.14", "142.251.209.14", &netem.LinkConfig{})) - proxyStack := runtimex.Try1(topology.AddHost("10.0.0.1", "142.251.209.14", &netem.LinkConfig{})) - clientStack := runtimex.Try1(topology.AddHost("10.0.0.2", "142.251.209.14", &netem.LinkConfig{})) - - dnsConfig := netem.NewDNSConfig() - dnsConfig.AddRecord("www.google.com", "", "142.251.209.14") - dnsServer := runtimex.Try1(netem.NewDNSServer(log.Log, wwwStack, "142.251.209.14", dnsConfig)) - closers = append(closers, dnsServer) - - wwwServer := testingx.MustNewHTTPServerEx( - &net.TCPAddr{IP: net.IPv4(142, 251, 209, 14), Port: 80}, - wwwStack, - http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - w.Write(expectedBody) - }), - ) - closers = append(closers, wwwServer) - - proxyServer := testingx.MustNewHTTPServerEx( - &net.TCPAddr{IP: net.IPv4(10, 0, 0, 1), Port: 80}, - proxyStack, - testingx.HTTPHandlerProxy(log.Log, &netxlite.Netx{ - Underlying: &netxlite.NetemUnderlyingNetworkAdapter{UNet: proxyStack}, - }), - ) - closers = append(closers, proxyServer) - - clientNet := &netxlite.Netx{Underlying: &netxlite.NetemUnderlyingNetworkAdapter{UNet: clientStack}} - return clientNet, proxyServer.URL, closers - }, - short: true, - }} - - for _, tc := range testcases { - t.Run(tc.name, func(t *testing.T) { - if !tc.short && testing.Short() { - t.Skip("skip test in short mode") - } - - netx, proxyURL, closers := tc.construct() - defer func() { - for _, closer := range closers { - closer.Close() - } - }() - - URL := runtimex.Try1(url.Parse(proxyURL)) - URL.Path = "/humans.txt" - - req := runtimex.Try1(http.NewRequest("GET", URL.String(), nil)) - req.Host = "www.google.com" - - //log.SetLevel(log.DebugLevel) - - txp := netx.NewHTTPTransportStdlib(log.Log) - client := netxlite.NewHTTPClient(txp) - - resp, err := client.Do(req) - if err != nil { - t.Fatal(err) - } - defer resp.Body.Close() - - if resp.StatusCode != 200 { - t.Fatal("expected to see 200, got", resp.StatusCode) - } - - t.Logf("%+v", resp) - - body, err := netxlite.ReadAllContext(req.Context(), resp.Body) - if err != nil { - t.Fatal(err) - } - - t.Logf("%s", string(body)) - - if diff := cmp.Diff(expectedBody, body); diff != "" { - t.Fatal(diff) - } - }) - } - - t.Run("rejects requests without a host header", func(t *testing.T) { - rr := httptest.NewRecorder() - netx := &netxlite.Netx{Underlying: &mocks.UnderlyingNetwork{ - // all nil: panic if we hit the network - }} - handler := testingx.HTTPHandlerProxy(log.Log, netx) - req := &http.Request{ - Host: "", // explicitly empty - } - handler.ServeHTTP(rr, req) - res := rr.Result() - if res.StatusCode != http.StatusBadRequest { - t.Fatal("unexpected status code", res.StatusCode) - } - }) - - t.Run("rejects requests with a via header", func(t *testing.T) { - rr := httptest.NewRecorder() - netx := &netxlite.Netx{Underlying: &mocks.UnderlyingNetwork{ - // all nil: panic if we hit the network - }} - handler := testingx.HTTPHandlerProxy(log.Log, netx) - req := &http.Request{ - Host: "www.example.com", - Header: http.Header{ - "Via": {"antani/0.1.0"}, - }, - } - handler.ServeHTTP(rr, req) - res := rr.Result() - if res.StatusCode != http.StatusBadRequest { - t.Fatal("unexpected status code", res.StatusCode) - } - }) - - t.Run("rejects requests with a POST method", func(t *testing.T) { - rr := httptest.NewRecorder() - netx := &netxlite.Netx{Underlying: &mocks.UnderlyingNetwork{ - // all nil: panic if we hit the network - }} - handler := testingx.HTTPHandlerProxy(log.Log, netx) - req := &http.Request{ - Host: "www.example.com", - Header: http.Header{}, - Method: http.MethodPost, - } - handler.ServeHTTP(rr, req) - res := rr.Result() - if res.StatusCode != http.StatusNotImplemented { - t.Fatal("unexpected status code", res.StatusCode) - } - }) - - t.Run("returns 502 when the round trip fails", func(t *testing.T) { - rr := httptest.NewRecorder() - netx := &netxlite.Netx{Underlying: &mocks.UnderlyingNetwork{ - MockGetaddrinfoLookupANY: func(ctx context.Context, domain string) ([]string, string, error) { - return nil, "", errors.New("mocked error") - }, - MockGetaddrinfoResolverNetwork: func() string { - return "antani" - }, - }} - handler := testingx.HTTPHandlerProxy(log.Log, netx) - req := &http.Request{ - Host: "www.example.com", - Header: http.Header{}, - Method: http.MethodGet, - URL: &url.URL{}, - } - handler.ServeHTTP(rr, req) - res := rr.Result() - if res.StatusCode != http.StatusBadGateway { - t.Fatal("unexpected status code", res.StatusCode) - } - }) -} diff --git a/internal/testingx/tlsx.go b/internal/testingx/tlsx.go index efcdc676ba..296493bfdb 100644 --- a/internal/testingx/tlsx.go +++ b/internal/testingx/tlsx.go @@ -25,6 +25,10 @@ import ( // // Use the former when you're using netem; the latter when using the stdlib. type TLSMITMProvider interface { + // CACert returns the CA certificate used by the server, which + // allows you to add to an existing [*x509.CertPool]. + CACert() *x509.Certificate + // DefaultCertPool returns the default cert pool to use. DefaultCertPool() (*x509.CertPool, error) @@ -43,6 +47,11 @@ type netemTLSMITMProvider struct { cfg *netem.TLSMITMConfig } +// CACert implements TLSMITMProvider. +func (p *netemTLSMITMProvider) CACert() *x509.Certificate { + return p.cfg.Cert +} + // DefaultCertPool implements TLSMITMProvider. func (p *netemTLSMITMProvider) DefaultCertPool() (*x509.CertPool, error) { return p.cfg.CertPool() From 1224c402edc9f0512121f5a237fb95f7a79b7f3d Mon Sep 17 00:00:00 2001 From: Simone Basso Date: Fri, 15 Sep 2023 15:59:40 +0200 Subject: [PATCH 2/5] x --- internal/enginenetx/http.go | 10 +- internal/enginenetx/http_test.go | 49 +++---- internal/netemx/scenario.go | 2 +- internal/netxlite/tls.go | 6 +- internal/testingx/httptestx.go | 69 ++++++++++ internal/testingx/httptestx_test.go | 198 ++++++++++++++++++++++++++++ 6 files changed, 292 insertions(+), 42 deletions(-) diff --git a/internal/enginenetx/http.go b/internal/enginenetx/http.go index f8af2c04da..e4d56df5bb 100644 --- a/internal/enginenetx/http.go +++ b/internal/enginenetx/http.go @@ -48,13 +48,13 @@ func NewHTTPTransport( resolver model.Resolver, ) *HTTPTransport { dialer := netxlite.NewDialerWithResolver(logger, resolver) + dialer = netxlite.MaybeWrapWithProxyDialer(dialer, proxyURL) handshaker := netxlite.NewTLSHandshakerStdlib(logger) tlsDialer := netxlite.NewTLSDialer(dialer, handshaker) - txp := netxlite.NewHTTPTransportWithOptions( - logger, dialer, tlsDialer, - netxlite.HTTPTransportOptionDisableCompression(false), - netxlite.HTTPTransportOptionProxyURL(proxyURL), // nil implies "no proxy" - ) + // TODO(https://github.com/ooni/probe/issues/2534): here we're using the QUIRKY netxlite.NewHTTPTransport + // function, but we can probably avoid using it, given that this code is + // not using tracing and does not care about those quirks. + txp := netxlite.NewHTTPTransport(logger, dialer, tlsDialer) txp = bytecounter.WrapHTTPTransport(txp, counter) return &HTTPTransport{txp} } diff --git a/internal/enginenetx/http_test.go b/internal/enginenetx/http_test.go index 1c5307b64a..c3604cd6d0 100644 --- a/internal/enginenetx/http_test.go +++ b/internal/enginenetx/http_test.go @@ -1,50 +1,33 @@ -package enginenetx_test +package enginenetx import ( "testing" "github.com/ooni/probe-cli/v3/internal/bytecounter" - "github.com/ooni/probe-cli/v3/internal/enginenetx" "github.com/ooni/probe-cli/v3/internal/model" - "github.com/ooni/probe-cli/v3/internal/netemx" "github.com/ooni/probe-cli/v3/internal/netxlite" ) func TestHTTPTransport(t *testing.T) { - t.Run("the HTTPTransport is working as intended", func(t *testing.T) { - env := netemx.MustNewScenario(netemx.InternetScenario) - defer env.Close() - - env.Do(func() { - txp := enginenetx.NewHTTPTransport( - bytecounter.New(), model.DiscardLogger, nil, netxlite.NewStdlibResolver(model.DiscardLogger)) - client := txp.NewHTTPClient() - resp, err := client.Get("https://www.example.com/") - if err != nil { - t.Fatal(err) - } - defer resp.Body.Close() - if resp.StatusCode != 200 { - t.Fatal("unexpected status code") - } - }) - }) - - t.Run("we can use a socks5 proxy", func(t *testing.T) { - panic("not implemented") - }) - - t.Run("we can use an HTTP proxy", func(t *testing.T) { - panic("not implemented") - }) - - t.Run("we can use an HTTPS proxy", func(t *testing.T) { - panic("not implemented") + // TODO(bassosimone): we should replace this integration test with netemx + // as soon as we can sever the hard link between netxlite and this pkg + t.Run("is working as intended", func(t *testing.T) { + txp := NewHTTPTransport( + bytecounter.New(), model.DiscardLogger, nil, netxlite.NewStdlibResolver(model.DiscardLogger)) + client := txp.NewHTTPClient() + resp, err := client.Get("https://www.google.com/robots.txt") + if err != nil { + t.Fatal(err) + } + defer resp.Body.Close() + if resp.StatusCode != 200 { + t.Fatal("unexpected status code") + } }) t.Run("NewHTTPClient returns a client with a cookie jar", func(t *testing.T) { - txp := enginenetx.NewHTTPTransport( + txp := NewHTTPTransport( bytecounter.New(), model.DiscardLogger, nil, netxlite.NewStdlibResolver(model.DiscardLogger)) client := txp.NewHTTPClient() if client.Jar == nil { diff --git a/internal/netemx/scenario.go b/internal/netemx/scenario.go index c4a8173ff7..17d90bc9a8 100644 --- a/internal/netemx/scenario.go +++ b/internal/netemx/scenario.go @@ -223,7 +223,7 @@ func MustNewScenario(config []*ScenarioDomainAddresses) *QAEnv { opts = append(opts, QAEnvOptionNetStack(addr, &HTTPCleartextServerFactory{ Factory: HTTPHandlerFactoryFunc(func(env NetStackServerFactoryEnv, stack *netem.UNetStack) http.Handler { - return testingx.NewHTTPProxyHandler(env.Logger(), &netxlite.Netx{ + return testingx.HTTPHandlerProxy(env.Logger(), &netxlite.Netx{ Underlying: &netxlite.NetemUnderlyingNetworkAdapter{UNet: stack}}) }), Ports: []int{80}, diff --git a/internal/netxlite/tls.go b/internal/netxlite/tls.go index 9cace9a9cc..15bf0a10be 100644 --- a/internal/netxlite/tls.go +++ b/internal/netxlite/tls.go @@ -255,18 +255,18 @@ func (h *tlsHandshakerLogger) Handshake( ctx context.Context, conn net.Conn, config *tls.Config, ) (net.Conn, tls.ConnectionState, error) { h.DebugLogger.Debugf( - "tls_handshake {sni=%s next=%+v}...", config.ServerName, config.NextProtos) + "tls {sni=%s next=%+v}...", config.ServerName, config.NextProtos) start := time.Now() tlsconn, state, err := h.TLSHandshaker.Handshake(ctx, conn, config) elapsed := time.Since(start) if err != nil { h.DebugLogger.Debugf( - "tls_handshake {sni=%s next=%+v}... %s in %s", config.ServerName, + "tls {sni=%s next=%+v}... %s in %s", config.ServerName, config.NextProtos, err, elapsed) return nil, tls.ConnectionState{}, err } h.DebugLogger.Debugf( - "tls_handshake {sni=%s next=%+v}... ok in %s {next=%s cipher=%s v=%s}", + "tls {sni=%s next=%+v}... ok in %s {next=%s cipher=%s v=%s}", config.ServerName, config.NextProtos, elapsed, state.NegotiatedProtocol, TLSCipherSuiteString(state.CipherSuite), TLSVersionString(state.Version)) diff --git a/internal/testingx/httptestx.go b/internal/testingx/httptestx.go index 488fe1d186..fb7480d4c7 100644 --- a/internal/testingx/httptestx.go +++ b/internal/testingx/httptestx.go @@ -3,10 +3,12 @@ package testingx import ( "crypto/tls" "crypto/x509" + "io" "net" "net/http" "net/url" + "github.com/ooni/probe-cli/v3/internal/model" "github.com/ooni/probe-cli/v3/internal/optional" "github.com/ooni/probe-cli/v3/internal/runtimex" ) @@ -174,3 +176,70 @@ func httpHandlerHijack(w http.ResponseWriter, r *http.Request, policy string) { // nothing } } + +// TODO(bassosimone): eventually we may want to have a model type +// that models the equivalent of [netxlite.Netx]. + +// HTTPHandlerProxyNetx is [netxlite.Netx] as seen by [HTTPHandlerProxy]. +type HTTPHandlerProxyNetx interface { + NewHTTPTransportStdlib(logger model.DebugLogger) model.HTTPTransport +} + +// HTTPHandlerProxy is a handler implementing an HTTP proxy using the host header +// to determine who to connect to. We additionally use the via header to avoid sending +// requests to ourself. Please, note that we designed this proxy ONLY to be used for +// testing purposes and that it's rather simplistic. +func HTTPHandlerProxy(logger model.Logger, netx HTTPHandlerProxyNetx) http.Handler { + return http.HandlerFunc(func(rw http.ResponseWriter, req *http.Request) { + // reject requests that already visited the proxy and requests we cannot route + if req.Host == "" || req.Header.Get("Via") != "" { + rw.WriteHeader(http.StatusBadRequest) + return + } + + // be explicit about not supporting request bodies + if req.Method != http.MethodGet { + rw.WriteHeader(http.StatusNotImplemented) + return + } + + // clone the request before modifying it + req = req.Clone(req.Context()) + + // include proxy header to prevent sending requests to ourself + req.Header.Add("Via", "testingx/0.1.0") + + // fix: "http: Request.RequestURI can't be set in client requests" + req.RequestURI = "" + + // fix: `http: unsupported protocol scheme ""` + req.URL.Host = req.Host + + // fix: "http: no Host in request URL" + req.URL.Scheme = "http" + + logger.Debugf("PROXY: sending request: %s", req) + + // create HTTP client using netx + txp := netx.NewHTTPTransportStdlib(logger) + + // obtain response + resp, err := txp.RoundTrip(req) + if err != nil { + logger.Warnf("PROXY: request failed: %s", err.Error()) + rw.WriteHeader(http.StatusBadGateway) + return + } + + // write response + rw.WriteHeader(resp.StatusCode) + for key, values := range resp.Header { + for _, value := range values { + rw.Header().Add(key, value) + } + } + + // write response body + _, _ = io.Copy(rw, resp.Body) + }) +} diff --git a/internal/testingx/httptestx_test.go b/internal/testingx/httptestx_test.go index 9c00133fa6..c34e6676a0 100644 --- a/internal/testingx/httptestx_test.go +++ b/internal/testingx/httptestx_test.go @@ -10,12 +10,15 @@ import ( "io" "net" "net/http" + "net/http/httptest" + "net/url" "testing" "time" "github.com/apex/log" "github.com/google/go-cmp/cmp" "github.com/ooni/netem" + "github.com/ooni/probe-cli/v3/internal/mocks" "github.com/ooni/probe-cli/v3/internal/netxlite" "github.com/ooni/probe-cli/v3/internal/runtimex" "github.com/ooni/probe-cli/v3/internal/testingx" @@ -473,3 +476,198 @@ func TestHTTPTestxWithNetem(t *testing.T) { }) } } + +func TestHTTPHandlerProxy(t *testing.T) { + expectedBody := []byte("Google is built by a large team of engineers, designers, researchers, robots, and others in many different sites across the globe. It is updated continuously, and built with more tools and technologies than we can shake a stick at. If you'd like to help us out, see careers.google.com.\n") + + type testcase struct { + name string + construct func() (*netxlite.Netx, string, []io.Closer) + short bool + } + + testcases := []testcase{ + { + name: "using the real network", + construct: func() (*netxlite.Netx, string, []io.Closer) { + var closers []io.Closer + + netx := &netxlite.Netx{ + Underlying: nil, // so we're using the real network + } + + proxyServer := testingx.MustNewHTTPServer(testingx.HTTPHandlerProxy(log.Log, netx)) + closers = append(closers, proxyServer) + + return netx, proxyServer.URL, closers + }, + short: false, + }, + + { + name: "using netem", + construct: func() (*netxlite.Netx, string, []io.Closer) { + var closers []io.Closer + + topology := runtimex.Try1(netem.NewStarTopology(log.Log)) + closers = append(closers, topology) + + wwwStack := runtimex.Try1(topology.AddHost("142.251.209.14", "142.251.209.14", &netem.LinkConfig{})) + proxyStack := runtimex.Try1(topology.AddHost("10.0.0.1", "142.251.209.14", &netem.LinkConfig{})) + clientStack := runtimex.Try1(topology.AddHost("10.0.0.2", "142.251.209.14", &netem.LinkConfig{})) + + dnsConfig := netem.NewDNSConfig() + dnsConfig.AddRecord("www.google.com", "", "142.251.209.14") + dnsServer := runtimex.Try1(netem.NewDNSServer(log.Log, wwwStack, "142.251.209.14", dnsConfig)) + closers = append(closers, dnsServer) + + wwwServer := testingx.MustNewHTTPServerEx( + &net.TCPAddr{IP: net.IPv4(142, 251, 209, 14), Port: 80}, + wwwStack, + http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Write(expectedBody) + }), + ) + closers = append(closers, wwwServer) + + proxyServer := testingx.MustNewHTTPServerEx( + &net.TCPAddr{IP: net.IPv4(10, 0, 0, 1), Port: 80}, + proxyStack, + testingx.HTTPHandlerProxy(log.Log, &netxlite.Netx{ + Underlying: &netxlite.NetemUnderlyingNetworkAdapter{UNet: proxyStack}, + }), + ) + closers = append(closers, proxyServer) + + clientNet := &netxlite.Netx{Underlying: &netxlite.NetemUnderlyingNetworkAdapter{UNet: clientStack}} + return clientNet, proxyServer.URL, closers + }, + short: true, + }} + + for _, tc := range testcases { + t.Run(tc.name, func(t *testing.T) { + if !tc.short && testing.Short() { + t.Skip("skip test in short mode") + } + + netx, proxyURL, closers := tc.construct() + defer func() { + for _, closer := range closers { + closer.Close() + } + }() + + URL := runtimex.Try1(url.Parse(proxyURL)) + URL.Path = "/humans.txt" + + req := runtimex.Try1(http.NewRequest("GET", URL.String(), nil)) + req.Host = "www.google.com" + + //log.SetLevel(log.DebugLevel) + + txp := netx.NewHTTPTransportStdlib(log.Log) + client := netxlite.NewHTTPClient(txp) + + resp, err := client.Do(req) + if err != nil { + t.Fatal(err) + } + defer resp.Body.Close() + + if resp.StatusCode != 200 { + t.Fatal("expected to see 200, got", resp.StatusCode) + } + + t.Logf("%+v", resp) + + body, err := netxlite.ReadAllContext(req.Context(), resp.Body) + if err != nil { + t.Fatal(err) + } + + t.Logf("%s", string(body)) + + if diff := cmp.Diff(expectedBody, body); diff != "" { + t.Fatal(diff) + } + }) + } + + t.Run("rejects requests without a host header", func(t *testing.T) { + rr := httptest.NewRecorder() + netx := &netxlite.Netx{Underlying: &mocks.UnderlyingNetwork{ + // all nil: panic if we hit the network + }} + handler := testingx.HTTPHandlerProxy(log.Log, netx) + req := &http.Request{ + Host: "", // explicitly empty + } + handler.ServeHTTP(rr, req) + res := rr.Result() + if res.StatusCode != http.StatusBadRequest { + t.Fatal("unexpected status code", res.StatusCode) + } + }) + + t.Run("rejects requests with a via header", func(t *testing.T) { + rr := httptest.NewRecorder() + netx := &netxlite.Netx{Underlying: &mocks.UnderlyingNetwork{ + // all nil: panic if we hit the network + }} + handler := testingx.HTTPHandlerProxy(log.Log, netx) + req := &http.Request{ + Host: "www.example.com", + Header: http.Header{ + "Via": {"antani/0.1.0"}, + }, + } + handler.ServeHTTP(rr, req) + res := rr.Result() + if res.StatusCode != http.StatusBadRequest { + t.Fatal("unexpected status code", res.StatusCode) + } + }) + + t.Run("rejects requests with a POST method", func(t *testing.T) { + rr := httptest.NewRecorder() + netx := &netxlite.Netx{Underlying: &mocks.UnderlyingNetwork{ + // all nil: panic if we hit the network + }} + handler := testingx.HTTPHandlerProxy(log.Log, netx) + req := &http.Request{ + Host: "www.example.com", + Header: http.Header{}, + Method: http.MethodPost, + } + handler.ServeHTTP(rr, req) + res := rr.Result() + if res.StatusCode != http.StatusNotImplemented { + t.Fatal("unexpected status code", res.StatusCode) + } + }) + + t.Run("returns 502 when the round trip fails", func(t *testing.T) { + rr := httptest.NewRecorder() + netx := &netxlite.Netx{Underlying: &mocks.UnderlyingNetwork{ + MockGetaddrinfoLookupANY: func(ctx context.Context, domain string) ([]string, string, error) { + return nil, "", errors.New("mocked error") + }, + MockGetaddrinfoResolverNetwork: func() string { + return "antani" + }, + }} + handler := testingx.HTTPHandlerProxy(log.Log, netx) + req := &http.Request{ + Host: "www.example.com", + Header: http.Header{}, + Method: http.MethodGet, + URL: &url.URL{}, + } + handler.ServeHTTP(rr, req) + res := rr.Result() + if res.StatusCode != http.StatusBadGateway { + t.Fatal("unexpected status code", res.StatusCode) + } + }) +} From 39ede6c767cb4bbec3a3e19cca074eb3e85a46f2 Mon Sep 17 00:00:00 2001 From: Simone Basso Date: Fri, 15 Sep 2023 16:32:40 +0200 Subject: [PATCH 3/5] x --- internal/testingproxy/netemhttp.go | 33 +++-------------------------- internal/testingproxy/netemhttps.go | 31 ++------------------------- internal/testingproxy/testcase.go | 8 +++++++ internal/testingx/httpproxy.go | 3 +++ 4 files changed, 16 insertions(+), 59 deletions(-) diff --git a/internal/testingproxy/netemhttp.go b/internal/testingproxy/netemhttp.go index 0caf7d5906..c478ad4178 100644 --- a/internal/testingproxy/netemhttp.go +++ b/internal/testingproxy/netemhttp.go @@ -101,37 +101,10 @@ func (tc *netemTestCaseWithHTTP) Run(t *testing.T) { ) defer proxyServer.Close() - // crete the netx instance for the client + // create the netx instance for the client netx := &netxlite.Netx{Underlying: &netxlite.NetemUnderlyingNetworkAdapter{UNet: clientStack}} - log.SetLevel(log.DebugLevel) - - /* - ,ggg, gg ,gg,ggggggggggggggg ,gggggggggggggg - dP""Y8a 88 ,8PdP""""""88"""""""dP""""""88"""""" - Yb, `88 88 d8'Yb,_ 88 Yb,_ 88 - `" 88 88 88 `"" 88 `"" 88 - 88 88 88 88 ggg88gggg - 88 88 88 88 88 8 - 88 88 88 88 88 - Y8 ,88, 8P gg, 88 gg, 88 - Yb,,d8""8b,,dP "Yb,,8P "Yb,,8P - "88" "88" "Y8P' "Y8P' - - - Not necessarily wrong, but certainly I did not expect this! When we are - using an HTTP proxy, the stdlib/oohttp *Transport uses the DialContext for - dialing with the proxy but then uses its own TLS for handshaking over - the TCP connection with the proxy. - - So, the naive implementation of this test case fails with an X.509 - certificate error when we're using netem, because we're not using the - overriden DialTLSContext anymore, and the *Transport does not - otherwise know about the root CA used by netem. - - The current fix is to use netxlite.HTTPTransportOptionTLSClientConfig - below. However, I'm now wondering if we're using the right default. - */ + //log.SetLevel(log.DebugLevel) // create an HTTP client configured to use the given proxy // @@ -145,7 +118,7 @@ func (tc *netemTestCaseWithHTTP) Run(t *testing.T) { txp := netxlite.NewHTTPTransportWithOptions(log.Log, dialer, tlsDialer, netxlite.HTTPTransportOptionProxyURL(runtimex.Try1(url.Parse(proxyServer.URL))), - // see above WTF comment + // TODO(https://github.com/ooni/probe/issues/2536) netxlite.HTTPTransportOptionTLSClientConfig(&tls.Config{ RootCAs: runtimex.Try1(clientStack.DefaultCertPool()), }), diff --git a/internal/testingproxy/netemhttps.go b/internal/testingproxy/netemhttps.go index 8abdfd4d56..6a0a5f7b9c 100644 --- a/internal/testingproxy/netemhttps.go +++ b/internal/testingproxy/netemhttps.go @@ -105,34 +105,7 @@ func (tc *netemTestCaseWithHTTPWithTLS) Run(t *testing.T) { // crete the netx instance for the client netx := &netxlite.Netx{Underlying: &netxlite.NetemUnderlyingNetworkAdapter{UNet: clientStack}} - log.SetLevel(log.DebugLevel) - - /* - ,ggg, gg ,gg,ggggggggggggggg ,gggggggggggggg - dP""Y8a 88 ,8PdP""""""88"""""""dP""""""88"""""" - Yb, `88 88 d8'Yb,_ 88 Yb,_ 88 - `" 88 88 88 `"" 88 `"" 88 - 88 88 88 88 ggg88gggg - 88 88 88 88 88 8 - 88 88 88 88 88 - Y8 ,88, 8P gg, 88 gg, 88 - Yb,,d8""8b,,dP "Yb,,8P "Yb,,8P - "88" "88" "Y8P' "Y8P' - - - Not necessarily wrong, but certainly I did not expect this! When we are - using an HTTPS proxy, the stdlib/oohttp *Transport uses the DialTLSContext for - dialing with the proxy but then uses its own TLS for handshaking over - the TLS connection with the proxy. - - So, the naive implementation of this test case fails with an X.509 - certificate error when we're using netem, because we're not using the - overriden DialTLSContext anymore, and the *Transport does not - otherwise know about the root CA used by netem. - - The current fix is to use netxlite.HTTPTransportOptionTLSClientConfig - below. However, I'm now wondering if we're using the right default. - */ + //log.SetLevel(log.DebugLevel) // create an HTTP client configured to use the given proxy // @@ -146,7 +119,7 @@ func (tc *netemTestCaseWithHTTPWithTLS) Run(t *testing.T) { txp := netxlite.NewHTTPTransportWithOptions(log.Log, dialer, tlsDialer, netxlite.HTTPTransportOptionProxyURL(runtimex.Try1(url.Parse(proxyServer.URL))), - // see above WTF comment + // TODO(https://github.com/ooni/probe/issues/2536) netxlite.HTTPTransportOptionTLSClientConfig(&tls.Config{ RootCAs: runtimex.Try1(clientStack.DefaultCertPool()), }), diff --git a/internal/testingproxy/testcase.go b/internal/testingproxy/testcase.go index 8892795a67..f2fab7e6b9 100644 --- a/internal/testingproxy/testcase.go +++ b/internal/testingproxy/testcase.go @@ -23,4 +23,12 @@ var AllTestCases = []TestCase{ // host network and HTTPS proxy WithHostNetworkHTTPWithTLSProxyAndURL("http://www.example.com/"), WithHostNetworkHTTPWithTLSProxyAndURL("https://www.example.com/"), + + // with netem and HTTP proxy + WithNetemHTTPProxyAndURL("http://www.example.com/"), + WithNetemHTTPProxyAndURL("https://www.example.com/"), + + // with netem and HTTPS proxy + WithNetemHTTPWithTLSProxyAndURL("http://www.example.com/"), + WithNetemHTTPWithTLSProxyAndURL("https://www.example.com/"), } diff --git a/internal/testingx/httpproxy.go b/internal/testingx/httpproxy.go index a66c060d75..f5adfac6d7 100644 --- a/internal/testingx/httpproxy.go +++ b/internal/testingx/httpproxy.go @@ -67,10 +67,12 @@ func (ph *httpProxyHandler) connect(rw http.ResponseWriter, req *http.Request) { rw.WriteHeader(http.StatusBadGateway) return } + defer sconn.Close() hijacker := rw.(http.Hijacker) cconn, buffered := runtimex.Try2(hijacker.Hijack()) runtimex.Assert(buffered.Reader.Buffered() <= 0, "data before finishing HTTP handshake") + defer cconn.Close() _, _ = cconn.Write([]byte("HTTP/1.1 200 Ok\r\n\r\n")) @@ -116,6 +118,7 @@ func (ph *httpProxyHandler) get(rw http.ResponseWriter, req *http.Request) { // create HTTP client using netx txp := ph.Netx.NewHTTPTransportStdlib(ph.Logger) + defer txp.CloseIdleConnections() // obtain response resp, err := txp.RoundTrip(req) From 6a4b1c02e97d41c0b44fc6300761562609405b51 Mon Sep 17 00:00:00 2001 From: Simone Basso Date: Fri, 15 Sep 2023 16:37:08 +0200 Subject: [PATCH 4/5] x --- internal/netxlite/httpfactory.go | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/internal/netxlite/httpfactory.go b/internal/netxlite/httpfactory.go index c75d06c108..0e57f249fe 100644 --- a/internal/netxlite/httpfactory.go +++ b/internal/netxlite/httpfactory.go @@ -96,11 +96,13 @@ func HTTPTransportOptionDisableCompression(value bool) HTTPTransportOption { } } -// HTTPTransportOptionTLSClientConfig configures the .TLSClientConfig field, which -// otherwise is left nil, meaning we're using the crypto/tls or ooni/ootls defaults -// including the default cert pool. Because leaving the default .TLSClientConfig -// has implications when dialing TLS connections over an HTTP proxy, be aware that -// this default value could change in a future release of ooni/probe-cli. +// HTTPTransportOptionTLSClientConfig configures the .TLSClientConfig field, +// which otherwise is nil, to imply using the default config. +// +// TODO(https://github.com/ooni/probe/issues/2536): using the default config breaks +// tests using netem and this option is the workaround we're using to address +// this limitation. Future releases MIGHT use a different technique and, as such, +// we MAY remove this option when we don't need it anymore. func HTTPTransportOptionTLSClientConfig(config *tls.Config) HTTPTransportOption { return func(txp *oohttp.Transport) { txp.TLSClientConfig = config From 0dc51f4ad71391f2f089b557ba2756098564b334 Mon Sep 17 00:00:00 2001 From: Simone Basso Date: Fri, 15 Sep 2023 16:38:48 +0200 Subject: [PATCH 5/5] Update internal/testingproxy/netemhttps.go --- internal/testingproxy/netemhttps.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/testingproxy/netemhttps.go b/internal/testingproxy/netemhttps.go index 6a0a5f7b9c..c44e836c00 100644 --- a/internal/testingproxy/netemhttps.go +++ b/internal/testingproxy/netemhttps.go @@ -102,7 +102,7 @@ func (tc *netemTestCaseWithHTTPWithTLS) Run(t *testing.T) { ) defer proxyServer.Close() - // crete the netx instance for the client + // create the netx instance for the client netx := &netxlite.Netx{Underlying: &netxlite.NetemUnderlyingNetworkAdapter{UNet: clientStack}} //log.SetLevel(log.DebugLevel)