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/testingproxy/dialer.go b/internal/testingproxy/dialer.go deleted file mode 100644 index ea87608b4c..0000000000 --- a/internal/testingproxy/dialer.go +++ /dev/null @@ -1,48 +0,0 @@ -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 deleted file mode 100644 index b7560cc81b..0000000000 --- a/internal/testingproxy/doc.go +++ /dev/null @@ -1,2 +0,0 @@ -// Package testingproxy contains shared test cases for the proxies. -package testingproxy diff --git a/internal/testingproxy/hosthttp.go b/internal/testingproxy/hosthttp.go deleted file mode 100644 index 013286cbfe..0000000000 --- a/internal/testingproxy/hosthttp.go +++ /dev/null @@ -1,74 +0,0 @@ -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 deleted file mode 100644 index 67be7cb417..0000000000 --- a/internal/testingproxy/hosthttps.go +++ /dev/null @@ -1,83 +0,0 @@ -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 deleted file mode 100644 index 4fb9b732c6..0000000000 --- a/internal/testingproxy/httputils.go +++ /dev/null @@ -1,53 +0,0 @@ -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 deleted file mode 100644 index a01c34cac9..0000000000 --- a/internal/testingproxy/httputils_test.go +++ /dev/null @@ -1,122 +0,0 @@ -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 deleted file mode 100644 index 0caf7d5906..0000000000 --- a/internal/testingproxy/netemhttp.go +++ /dev/null @@ -1,163 +0,0 @@ -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 deleted file mode 100644 index 8abdfd4d56..0000000000 --- a/internal/testingproxy/netemhttps.go +++ /dev/null @@ -1,164 +0,0 @@ -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 deleted file mode 100644 index 3e4019068a..0000000000 --- a/internal/testingproxy/qa_test.go +++ /dev/null @@ -1,19 +0,0 @@ -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 deleted file mode 100644 index 4ad2d864ff..0000000000 --- a/internal/testingproxy/testcase.go +++ /dev/null @@ -1,34 +0,0 @@ -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 deleted file mode 100644 index a66c060d75..0000000000 --- a/internal/testingx/httpproxy.go +++ /dev/null @@ -1,138 +0,0 @@ -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 deleted file mode 100644 index 56fb6c5058..0000000000 --- a/internal/testingx/httpproxy_test.go +++ /dev/null @@ -1,19 +0,0 @@ -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 488fe1d186..61e4dd21d0 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" ) @@ -19,35 +21,20 @@ 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]. @@ -92,7 +79,6 @@ 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()) @@ -174,3 +160,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) + } + }) +}