refactor(netx): reorganize by topic (#800)

Before finishing the ongoing refactoring and leaving whatever
is left of netx in tree, I would like to restructure it so that
we'll have an easy time next time we need to modify it.

Currently, every functionality lives into the `netx.go` file and
we have a support file called `httptransport.go`.

I would like to reorganize by topic, instead. This would allow
future me to more easily perform topic-specific changes.

While there, improve `netx`'s documentation and duplicate some of
this documentation inside `internal/README.md` to provide pointers
to previous documentation, historical context, and some help to
understand the logic architecture of network extensions (aka `netx`).

Part of https://github.com/ooni/probe-cli/pull/396

internal/README.md

@@ -11,12 +11,86 @@ go doc -all ./internal/$package
 
 where `$package` is the name of the package.
 
-Some notable packages:
+## Tutorials
 
-- [model](model) contains the interfaces and data model shared
+The [tutorial](tutorial) package contains tutorials on writing new experiments,
-by most packages inside this directory;
-
-- [netxlite](netxlite) is the underlying networking library;
-
-- [tutorial](tutorial) contains tutorials on writing new experiments,
 using measurements libraries, and networking code.
+
+## Network extensions
+
+This section briefly describes the overall design of the network
+extensions (aka `netx`) inside `ooni/probe-cli`. In OONI, we have
+two distinct but complementary needs:
+
+1. speaking with our backends or accessing other services useful
+to bootstrap OONI probe and perform measurements;
+
+2. implementing network experiments.
+
+We originally implemented these functionality into a separate
+repository: [ooni/netx](https://github.com/ooni/netx). The
+original [design document](https://github.com/ooni/netx/blob/master/DESIGN.md)
+still provides a good overview of the problems we wanted to solve.
+
+The general idea was to provide interfaces replacing standard library
+objects that we could further wrap to perform network measurements without
+deviating from the normal APIs expected by Go programmers. For example,
+
+```Go
+type Dialer interface {
+	DialContext(ctx context.Context, network, address string) (net.Conn, error)
+}
+```
+
+is a generic dialer that could be a `&net.Dialer{}` but could also be a
+*saving* dialer that saves the results of dial events. So, you could write
+something like:
+
+```Go
+saver := &Saver{}
+var dialer Dialer = NewDialer()
+dialer = saver.WrapDialer(dialer)
+conn, err := dialer.DialContext(ctx, network, address)
+events := saver.ExtractEvents()
+```
+
+In short, with the original `netx` you could write measurement code
+resembling ordinary Go code but you could also save network events
+from which to derive whether there was censorship.
+
+Since then, the architecture itself has evolved and `netx` has been
+merged into `ooni/probe-engine` and later `ooni/probe-cli`. As of
+2022-06-06, these are the fundamental `netx` packages:
+
+- [model/netx.go](model/netx.go): contains the interfaces and structs
+patterned after the Go standard library used by `netx`;
+
+- [netxlite](netxlite): implements error wrapping (i.e., mapping
+Go errors to OONI errors), enforces timeouts, and generally ensures
+that we're using a stdlib-like network API that meet all our
+constraints and requirements (e.g., logging);
+
+- [bytecounter](bytecounter): provides support for counting the
+number of bytes consumed by network interactions;
+
+- [multierror](multierror): defines an `error` type that contains
+a list of errors for representing the results of operations where
+multiple sub-operations may fail (e.g., TCP connect fails for
+all the IP addresses associated with a domain name);
+
+- [tracex](tracex): support for collecting events during operations
+such as TCP connect, QUIC handshake, HTTP round trip. Collecting
+events allows us to analyze such events and determine whether there
+was blocking. This measurement strategy is called tracing because
+we wrap fundamental types (e.g., a dialer or an HTTP transport) to
+save the result of each operation into a "list of events" type
+called `Saver;
+
+- [engine/netx](engine/netx): code surviving from the original `netx`
+implementation that we're still using for measuring. Issue
+[ooni/probe#2121](https://github.com/ooni/probe/issues/2121) describes
+a slow refactoring process where we'll move code outside of `netx`
+and inside `netxlite` or other packages. We are currently experimenting
+with step-by-step measurements, an alternative measurement
+approach where we break down operations in simpler building blocks. This
+alternative approach may eventually make `netx` obsolete.

internal/engine/netx/README.md

@@ -1,23 +0,0 @@
-# Package github.com/ooni/probe-engine/netx
-
-OONI extensions to the `net` and `net/http` packages. This code is
-used by `ooni/probe-engine` as a low level library to collect
-network, DNS, and HTTP events occurring during OONI measurements.
-
-This library contains replacements for commonly used standard library
-interfaces that facilitate seamless network measurements. By using
-such replacements, as opposed to standard library interfaces, we can:
-
-* save the timing of HTTP events (e.g. received response headers)
-* save the timing and result of every Connect, Read, Write, Close operation
-* save the timing and result of the TLS handshake (including certificates)
-
-By default, this library uses the system resolver. In addition, it
-is possible to configure alternative DNS transports and remote
-servers. We support DNS over UDP, DNS over TCP, DNS over TLS (DoT),
-and DNS over HTTPS (DoH). When using an alternative transport, we
-are also able to intercept and save DNS messages, as well as any
-other interaction with the remote server (e.g., the result of the
-TLS handshake for DoT and DoH).
-
-This package is a fork of [github.com/ooni/netx](https://github.com/ooni/netx).

internal/engine/netx/config.go

@@ -0,0 +1,35 @@
+package netx
+
+//
+// Config struct.
+//
+
+import (
+	"crypto/tls"
+	"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/tracex"
+)
+
+// Config contains configuration for creating new transports, dialers, etc. When
+// any field of Config is nil/empty, we will use a suitable default.
+type Config struct {
+	BaseResolver        model.Resolver       // default: system resolver
+	BogonIsError        bool                 // default: bogon is not error
+	ByteCounter         *bytecounter.Counter // default: no explicit byte counting
+	CacheResolutions    bool                 // default: no caching
+	ContextByteCounting bool                 // default: no implicit byte counting
+	DNSCache            map[string][]string  // default: cache is empty
+	Dialer              model.Dialer         // default: dialer.DNSDialer
+	FullResolver        model.Resolver       // default: base resolver + goodies
+	QUICDialer          model.QUICDialer     // default: quicdialer.DNSDialer
+	HTTP3Enabled        bool                 // default: disabled
+	Logger              model.Logger         // default: no logging
+	ProxyURL            *url.URL             // default: no proxy
+	ReadWriteSaver      *tracex.Saver        // default: not saving I/O events
+	Saver               *tracex.Saver        // default: not saving non-I/O events
+	TLSConfig           *tls.Config          // default: attempt using h2
+	TLSDialer           model.TLSDialer      // default: dialer.TLSDialer
+}

internal/engine/netx/dialer.go

@@ -0,0 +1,26 @@
+package netx
+
+//
+// Dialer from Config.
+//
+
+import (
+	"github.com/ooni/probe-cli/v3/internal/bytecounter"
+	"github.com/ooni/probe-cli/v3/internal/model"
+	"github.com/ooni/probe-cli/v3/internal/netxlite"
+)
+
+// NewDialer creates a new Dialer from the specified config.
+func NewDialer(config Config) model.Dialer {
+	if config.FullResolver == nil {
+		config.FullResolver = NewResolver(config)
+	}
+	logger := model.ValidLoggerOrDefault(config.Logger)
+	d := netxlite.NewDialerWithResolver(
+		logger, config.FullResolver, config.Saver.NewConnectObserver(),
+		config.ReadWriteSaver.NewReadWriteObserver(),
+	)
+	d = netxlite.NewMaybeProxyDialer(d, config.ProxyURL)
+	d = bytecounter.MaybeWrapWithContextAwareDialer(config.ContextByteCounting, d)
+	return d
+}

internal/engine/netx/dnstransport.go

@@ -0,0 +1,160 @@
+package netx
+
+//
+// DNSTransport from Config.
+//
+// TODO(bassosimone): this code should be refactored to return
+// a DNSTransport rather than a model.Resolver. With this in mind,
+// I've named this file dnstransport.go.
+//
+
+import (
+	"crypto/tls"
+	"errors"
+	"net"
+	"net/http"
+	"net/url"
+
+	"github.com/ooni/probe-cli/v3/internal/model"
+	"github.com/ooni/probe-cli/v3/internal/netxlite"
+)
+
+// NewDNSClient creates a new DNS client. The config argument is used to
+// create the underlying Dialer and/or HTTP transport, if needed. The URL
+// argument describes the kind of client that we want to make:
+//
+// - if the URL is `doh://powerdns`, `doh://google` or `doh://cloudflare` or the URL
+// starts with `https://`, then we create a DoH client.
+//
+// - if the URL is `` or `system:///`, then we create a system client,
+// i.e. a client using the system resolver.
+//
+// - if the URL starts with `udp://`, then we create a client using
+// a resolver that uses the specified UDP endpoint.
+//
+// We return error if the URL does not parse or the URL scheme does not
+// fall into one of the cases described above.
+//
+// If config.ResolveSaver is not nil and we're creating an underlying
+// resolver where this is possible, we will also save events.
+func NewDNSClient(config Config, URL string) (model.Resolver, error) {
+	return NewDNSClientWithOverrides(config, URL, "", "", "")
+}
+
+// NewDNSClientWithOverrides creates a new DNS client, similar to NewDNSClient,
+// with the option to override the default Hostname and SNI.
+func NewDNSClientWithOverrides(config Config, URL, hostOverride, SNIOverride,
+	TLSVersion string) (model.Resolver, error) {
+	switch URL {
+	case "doh://powerdns":
+		URL = "https://doh.powerdns.org/"
+	case "doh://google":
+		URL = "https://dns.google/dns-query"
+	case "doh://cloudflare":
+		URL = "https://cloudflare-dns.com/dns-query"
+	case "":
+		URL = "system:///"
+	}
+	resolverURL, err := url.Parse(URL)
+	if err != nil {
+		return nil, err
+	}
+	config.TLSConfig = &tls.Config{ServerName: SNIOverride}
+	if err := netxlite.ConfigureTLSVersion(config.TLSConfig, TLSVersion); err != nil {
+		return nil, err
+	}
+	switch resolverURL.Scheme {
+	case "system":
+		return netxlite.NewResolverSystem(), nil
+	case "https":
+		config.TLSConfig.NextProtos = []string{"h2", "http/1.1"}
+		httpClient := &http.Client{Transport: NewHTTPTransport(config)}
+		var txp model.DNSTransport = netxlite.NewUnwrappedDNSOverHTTPSTransportWithHostOverride(
+			httpClient, URL, hostOverride)
+		txp = config.Saver.WrapDNSTransport(txp) // safe when config.Saver == nil
+		return netxlite.NewUnwrappedSerialResolver(txp), nil
+	case "udp":
+		dialer := NewDialer(config)
+		endpoint, err := makeValidEndpoint(resolverURL)
+		if err != nil {
+			return nil, err
+		}
+		var txp model.DNSTransport = netxlite.NewUnwrappedDNSOverUDPTransport(
+			dialer, endpoint)
+		txp = config.Saver.WrapDNSTransport(txp) // safe when config.Saver == nil
+		return netxlite.NewUnwrappedSerialResolver(txp), nil
+	case "dot":
+		config.TLSConfig.NextProtos = []string{"dot"}
+		tlsDialer := NewTLSDialer(config)
+		endpoint, err := makeValidEndpoint(resolverURL)
+		if err != nil {
+			return nil, err
+		}
+		var txp model.DNSTransport = netxlite.NewUnwrappedDNSOverTLSTransport(
+			tlsDialer.DialTLSContext, endpoint)
+		txp = config.Saver.WrapDNSTransport(txp) // safe when config.Saver == nil
+		return netxlite.NewUnwrappedSerialResolver(txp), nil
+	case "tcp":
+		dialer := NewDialer(config)
+		endpoint, err := makeValidEndpoint(resolverURL)
+		if err != nil {
+			return nil, err
+		}
+		var txp model.DNSTransport = netxlite.NewUnwrappedDNSOverTCPTransport(
+			dialer.DialContext, endpoint)
+		txp = config.Saver.WrapDNSTransport(txp) // safe when config.Saver == nil
+		return netxlite.NewUnwrappedSerialResolver(txp), nil
+	default:
+		return nil, errors.New("unsupported resolver scheme")
+	}
+}
+
+// makeValidEndpoint makes a valid endpoint for DoT and Do53 given the
+// input URL representing such endpoint. Specifically, we are
+// concerned with the case where the port is missing. In such a
+// case, we ensure that we are using the default port 853 for DoT
+// and default port 53 for TCP and UDP.
+func makeValidEndpoint(URL *url.URL) (string, error) {
+	// Implementation note: when we're using a quoted IPv6
+	// address, URL.Host contains the quotes but instead the
+	// return value from URL.Hostname() does not.
+	//
+	// For example:
+	//
+	// - Host: [2620:fe::9]
+	// - Hostname(): 2620:fe::9
+	//
+	// We need to keep this in mind when trying to determine
+	// whether there is also a port or not.
+	//
+	// So the first step is to check whether URL.Host is already
+	// a whatever valid TCP/UDP endpoint and, if so, use it.
+	if _, _, err := net.SplitHostPort(URL.Host); err == nil {
+		return URL.Host, nil
+	}
+	// The second step is to assume that appending the default port
+	// to a host parsed by url.Parse should be giving us a valid
+	// endpoint. The possibilities in fact are:
+	//
+	// 1. domain w/o port
+	// 2. IPv4 w/o port
+	// 3. square bracket quoted IPv6 w/o port
+	// 4. other
+	//
+	// In the first three cases, appending a port leads us to a
+	// good endpoint. The fourth case does not.
+	//
+	// For this reason we check again whether we can split it using
+	// net.SplitHostPort. If we cannot, we were in case four.
+	host := URL.Host
+	if URL.Scheme == "dot" {
+		host += ":853"
+	} else {
+		host += ":53"
+	}
+	if _, _, err := net.SplitHostPort(host); err != nil {
+		return "", err
+	}
+	// Otherwise it's one of the three valid cases above.
+	return host, nil
+}

internal/engine/netx/dnstransport_test.go

@@ -1,123 +1,14 @@
 package netx
 
 import (
-	"context"
-	"crypto/tls"
 	"errors"
-	"net"
-	"net/http"
 	"strings"
 	"testing"
 
-	"github.com/apex/log"
-	"github.com/ooni/probe-cli/v3/internal/bytecounter"
-	"github.com/ooni/probe-cli/v3/internal/model/mocks"
 	"github.com/ooni/probe-cli/v3/internal/netxlite"
-	"github.com/ooni/probe-cli/v3/internal/netxlite/filtering"
 	"github.com/ooni/probe-cli/v3/internal/tracex"
 )
 
-func TestNewTLSDialer(t *testing.T) {
-	t.Run("we always have error wrapping", func(t *testing.T) {
-		server := filtering.NewTLSServer(filtering.TLSActionReset)
-		defer server.Close()
-		tdx := NewTLSDialer(Config{})
-		conn, err := tdx.DialTLSContext(context.Background(), "tcp", server.Endpoint())
-		if err == nil || err.Error() != netxlite.FailureConnectionReset {
-			t.Fatal("unexpected err", err)
-		}
-		if conn != nil {
-			t.Fatal("expected nil conn")
-		}
-	})
-
-	t.Run("we can collect measurements", func(t *testing.T) {
-		server := filtering.NewTLSServer(filtering.TLSActionReset)
-		defer server.Close()
-		saver := &tracex.Saver{}
-		tdx := NewTLSDialer(Config{
-			Saver: saver,
-		})
-		conn, err := tdx.DialTLSContext(context.Background(), "tcp", server.Endpoint())
-		if err == nil || err.Error() != netxlite.FailureConnectionReset {
-			t.Fatal("unexpected err", err)
-		}
-		if conn != nil {
-			t.Fatal("expected nil conn")
-		}
-		if len(saver.Read()) <= 0 {
-			t.Fatal("did not read any event")
-		}
-	})
-
-	t.Run("we can skip TLS verification", func(t *testing.T) {
-		server := filtering.NewTLSServer(filtering.TLSActionBlockText)
-		defer server.Close()
-		tdx := NewTLSDialer(Config{TLSConfig: &tls.Config{
-			InsecureSkipVerify: true,
-		}})
-		conn, err := tdx.DialTLSContext(context.Background(), "tcp", server.Endpoint())
-		if err != nil {
-			t.Fatal(err.(*netxlite.ErrWrapper).WrappedErr)
-		}
-		conn.Close()
-	})
-
-	t.Run("we can set the cert pool", func(t *testing.T) {
-		server := filtering.NewTLSServer(filtering.TLSActionBlockText)
-		defer server.Close()
-		tdx := NewTLSDialer(Config{
-			TLSConfig: &tls.Config{
-				RootCAs:    server.CertPool(),
-				ServerName: "dns.google",
-			},
-		})
-		conn, err := tdx.DialTLSContext(context.Background(), "tcp", server.Endpoint())
-		if err != nil {
-			t.Fatal(err)
-		}
-		conn.Close()
-	})
-}
-
-func TestNewWithDialer(t *testing.T) {
-	expected := errors.New("mocked error")
-	dialer := &mocks.Dialer{
-		MockDialContext: func(ctx context.Context, network, address string) (net.Conn, error) {
-			return nil, expected
-		},
-	}
-	txp := NewHTTPTransport(Config{
-		Dialer: dialer,
-	})
-	client := &http.Client{Transport: txp}
-	resp, err := client.Get("http://www.google.com")
-	if !errors.Is(err, expected) {
-		t.Fatal("not the error we expected")
-	}
-	if resp != nil {
-		t.Fatal("not the response we expected")
-	}
-}
-
-func TestNewWithSaver(t *testing.T) {
-	saver := new(tracex.Saver)
-	txp := NewHTTPTransport(Config{
-		Saver: saver,
-	})
-	stxptxp, ok := txp.(*tracex.HTTPTransportSaver)
-	if !ok {
-		t.Fatal("not the transport we expected")
-	}
-	if stxptxp.Saver != saver {
-		t.Fatal("not the logger we expected")
-	}
-	if stxptxp.Saver != saver {
-		t.Fatal("not the logger we expected")
-	}
-	// We are going to trust the underlying type returned by netxlite
-}
-
 func TestNewDNSClientInvalidURL(t *testing.T) {
 	dnsclient, err := NewDNSClient(Config{}, "\t\t\t")
 	if err == nil || !strings.HasSuffix(err.Error(), "invalid control character in URL") {
@@ -398,66 +289,3 @@ func TestNewDNSCLientWithInvalidTLSVersion(t *testing.T) {
 		t.Fatalf("not the error we expected: %+v", err)
 	}
 }
-
-func TestSuccess(t *testing.T) {
-	if testing.Short() {
-		t.Skip("skip test in short mode")
-	}
-	log.SetLevel(log.DebugLevel)
-	counter := bytecounter.New()
-	config := Config{
-		BogonIsError:        true,
-		ByteCounter:         counter,
-		CacheResolutions:    true,
-		ContextByteCounting: true,
-		Logger:              log.Log,
-		ReadWriteSaver:      &tracex.Saver{},
-		Saver:               &tracex.Saver{},
-	}
-	txp := NewHTTPTransport(config)
-	client := &http.Client{Transport: txp}
-	resp, err := client.Get("https://www.google.com")
-	if err != nil {
-		t.Fatal(err)
-	}
-	if _, err = netxlite.ReadAllContext(context.Background(), resp.Body); err != nil {
-		t.Fatal(err)
-	}
-	if err = resp.Body.Close(); err != nil {
-		t.Fatal(err)
-	}
-	if counter.Sent.Load() <= 0 {
-		t.Fatal("no bytes sent?!")
-	}
-	if counter.Received.Load() <= 0 {
-		t.Fatal("no bytes received?!")
-	}
-	if ev := config.ReadWriteSaver.Read(); len(ev) <= 0 {
-		t.Fatal("no R/W events?!")
-	}
-	if ev := config.Saver.Read(); len(ev) <= 0 {
-		t.Fatal("no non-I/O events?!")
-	}
-}
-
-func TestBogonResolutionNotBroken(t *testing.T) {
-	saver := new(tracex.Saver)
-	r := NewResolver(Config{
-		BogonIsError: true,
-		DNSCache: map[string][]string{
-			"www.google.com": {"127.0.0.1"},
-		},
-		Saver:  saver,
-		Logger: log.Log,
-	})
-	addrs, err := r.LookupHost(context.Background(), "www.google.com")
-	if !errors.Is(err, netxlite.ErrDNSBogon) {
-		t.Fatal("not the error we expected")
-	}
-	if err.Error() != netxlite.FailureDNSBogonError {
-		t.Fatal("error not correctly wrapped")
-	}
-	if len(addrs) > 0 {
-		t.Fatal("expected no addresses here")
-	}
-}

internal/engine/netx/doc.go

@@ -0,0 +1,46 @@
+// Package netx contains code to perform network measurements.
+//
+// This library derives from https://github.com/ooni/netx and contains
+// the original code we wrote for performing measurements in Go. Over
+// time, most of the original code has been refactored away inside:
+//
+// * model/netx.go: definition of interfaces and structs
+//
+// * netxlite: low-level network library
+//
+// * bytecounter: support for counting bytes sent and received
+//
+// * multierror: representing multiple errors using a single error
+//
+// * tracex: support for measuring using tracing
+//
+// This refactoring of netx (called "the netx pivot") has been described
+// in https://github.com/ooni/probe-cli/pull/396. We described the
+// design, implementation, and pain points of the pre-pivot netx library
+// in https://github.com/ooni/probe-engine/issues/359. In turn,
+// https://github.com/ooni/netx/blob/master/DESIGN.md contains the
+// original design document for the netx library.
+//
+// Measuring using tracing means that we use ordinary stdlib-like
+// objects such as model.Dialer and model.HTTPTransport. Then, we'll
+// extract results from a tracex.Saver to determine the result of
+// the measurement. The most notable user of this library is
+// experiment/urlgetter, which implements a flexible URL-getting library.
+//
+// Tracing has its own set of limitations, so while we're still using
+// it for implementing many experiments, we're also tinkering with
+// step-by-step approaches where we break down operations in more basic
+// building blocks, e.g., DNS resolution and fetching URL given an
+// hostname, a protocol (e.g., QUIC or HTTPS), and an endpoint.
+//
+// While we're experimenting with alternative approaches, we also want
+// to keep this library running and stable. New code will probably
+// not be implemented here rather in step-by-step libraries.
+//
+// New experiments that can be written in terms of netxlite and tracex
+// SHOULD NOT use netx. Existing experiment using netx MAY be rewritten
+// using just netxlite and tracex when feasible.
+//
+// Additionally, new code that does not need to perform measurements
+// SHOULD NOT use netx and SHOULD instead use netxlite.
+package netx

internal/engine/netx/http.go

@@ -0,0 +1,78 @@
+package netx
+
+//
+// HTTPTransport from Config.
+//
+
+import (
+	"crypto/tls"
+
+	"github.com/ooni/probe-cli/v3/internal/model"
+	"github.com/ooni/probe-cli/v3/internal/netxlite"
+)
+
+// NewHTTPTransport creates a new HTTPRoundTripper from the given Config.
+func NewHTTPTransport(config Config) model.HTTPTransport {
+	if config.Dialer == nil {
+		config.Dialer = NewDialer(config)
+	}
+	if config.TLSDialer == nil {
+		config.TLSDialer = NewTLSDialer(config)
+	}
+	if config.QUICDialer == nil {
+		config.QUICDialer = NewQUICDialer(config)
+	}
+	tInfo := allTransportsInfo[config.HTTP3Enabled]
+	txp := tInfo.Factory(httpTransportConfig{
+		Dialer:     config.Dialer,
+		Logger:     model.ValidLoggerOrDefault(config.Logger),
+		QUICDialer: config.QUICDialer,
+		TLSDialer:  config.TLSDialer,
+		TLSConfig:  config.TLSConfig,
+	})
+	// TODO(bassosimone): I am not super convinced by this code because it
+	// seems we're currently counting bytes twice in some cases. I think we
+	// should review how we're counting bytes and using netx currently.
+	txp = config.ByteCounter.MaybeWrapHTTPTransport(txp)                 // WAI with ByteCounter == nil
+	const defaultSnapshotSize = 0                                        // means: use the default snapsize
+	return config.Saver.MaybeWrapHTTPTransport(txp, defaultSnapshotSize) // WAI with Saver == nil
+}
+
+// httpTransportInfo contains the constructing function as well as the transport name
+type httpTransportInfo struct {
+	Factory       func(httpTransportConfig) model.HTTPTransport
+	TransportName string
+}
+
+var allTransportsInfo = map[bool]httpTransportInfo{
+	false: {
+		Factory:       newSystemTransport,
+		TransportName: "tcp",
+	},
+	true: {
+		Factory:       newHTTP3Transport,
+		TransportName: "quic",
+	},
+}
+
+// httpTransportConfig contains configuration for constructing an HTTPTransport.
+type httpTransportConfig struct {
+	Dialer     model.Dialer
+	Logger     model.Logger
+	QUICDialer model.QUICDialer
+	TLSDialer  model.TLSDialer
+	TLSConfig  *tls.Config
+}
+
+// newHTTP3Transport creates a new HTTP3Transport instance.
+func newHTTP3Transport(config httpTransportConfig) model.HTTPTransport {
+	// Rationale for using NoLogger here: previously this code did
+	// not use a logger as well, so it's fine to keep it as is.
+	return netxlite.NewHTTP3Transport(config.Logger, config.QUICDialer, config.TLSConfig)
+}
+
+// newSystemTransport creates a new "system" HTTP transport. That is a transport
+// using the Go standard library with custom dialer and TLS dialer.
+func newSystemTransport(config httpTransportConfig) model.HTTPTransport {
+	return netxlite.NewHTTPTransport(config.Logger, config.Dialer, config.TLSDialer)
+}

internal/engine/netx/http_test.go

@@ -0,0 +1,50 @@
+package netx
+
+import (
+	"context"
+	"errors"
+	"net"
+	"net/http"
+	"testing"
+
+	"github.com/ooni/probe-cli/v3/internal/model/mocks"
+	"github.com/ooni/probe-cli/v3/internal/tracex"
+)
+
+func TestNewHTTPTransportWithDialer(t *testing.T) {
+	expected := errors.New("mocked error")
+	dialer := &mocks.Dialer{
+		MockDialContext: func(ctx context.Context, network, address string) (net.Conn, error) {
+			return nil, expected
+		},
+	}
+	txp := NewHTTPTransport(Config{
+		Dialer: dialer,
+	})
+	client := &http.Client{Transport: txp}
+	resp, err := client.Get("http://www.google.com")
+	if !errors.Is(err, expected) {
+		t.Fatal("not the error we expected")
+	}
+	if resp != nil {
+		t.Fatal("not the response we expected")
+	}
+}
+
+func TestNewHTTPTransportWithSaver(t *testing.T) {
+	saver := new(tracex.Saver)
+	txp := NewHTTPTransport(Config{
+		Saver: saver,
+	})
+	stxptxp, ok := txp.(*tracex.HTTPTransportSaver)
+	if !ok {
+		t.Fatal("not the transport we expected")
+	}
+	if stxptxp.Saver != saver {
+		t.Fatal("not the logger we expected")
+	}
+	if stxptxp.Saver != saver {
+		t.Fatal("not the logger we expected")
+	}
+	// We are going to trust the underlying type returned by netxlite
+}

internal/engine/netx/httptransport.go

@@ -1,30 +0,0 @@
-package netx
-
-import (
-	"crypto/tls"
-
-	"github.com/ooni/probe-cli/v3/internal/model"
-	"github.com/ooni/probe-cli/v3/internal/netxlite"
-)
-
-// httpTransportConfig contains the configuration required for constructing an HTTP transport
-type httpTransportConfig struct {
-	Dialer     model.Dialer
-	Logger     model.Logger
-	QUICDialer model.QUICDialer
-	TLSDialer  model.TLSDialer
-	TLSConfig  *tls.Config
-}
-
-// newHTTP3Transport creates a new HTTP3Transport instance.
-func newHTTP3Transport(config httpTransportConfig) model.HTTPTransport {
-	// Rationale for using NoLogger here: previously this code did
-	// not use a logger as well, so it's fine to keep it as is.
-	return netxlite.NewHTTP3Transport(config.Logger, config.QUICDialer, config.TLSConfig)
-}
-
-// newSystemTransport creates a new "system" HTTP transport. That is a transport
-// using the Go standard library with custom dialer and TLS dialer.
-func newSystemTransport(config httpTransportConfig) model.HTTPTransport {
-	return netxlite.NewHTTPTransport(config.Logger, config.Dialer, config.TLSDialer)
-}

internal/engine/netx/integration_test.go

@@ -0,0 +1,53 @@
+package netx
+
+import (
+	"context"
+	"net/http"
+	"testing"
+
+	"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/tracex"
+)
+
+func TestHTTPTransportWorkingAsIntended(t *testing.T) {
+	if testing.Short() {
+		t.Skip("skip test in short mode")
+	}
+	log.SetLevel(log.DebugLevel)
+	counter := bytecounter.New()
+	config := Config{
+		BogonIsError:        true,
+		ByteCounter:         counter,
+		CacheResolutions:    true,
+		ContextByteCounting: true,
+		Logger:              log.Log,
+		ReadWriteSaver:      &tracex.Saver{},
+		Saver:               &tracex.Saver{},
+	}
+	txp := NewHTTPTransport(config)
+	client := &http.Client{Transport: txp}
+	resp, err := client.Get("https://www.google.com")
+	if err != nil {
+		t.Fatal(err)
+	}
+	if _, err = netxlite.ReadAllContext(context.Background(), resp.Body); err != nil {
+		t.Fatal(err)
+	}
+	if err = resp.Body.Close(); err != nil {
+		t.Fatal(err)
+	}
+	if counter.Sent.Load() <= 0 {
+		t.Fatal("no bytes sent?!")
+	}
+	if counter.Received.Load() <= 0 {
+		t.Fatal("no bytes received?!")
+	}
+	if ev := config.ReadWriteSaver.Read(); len(ev) <= 0 {
+		t.Fatal("no R/W events?!")
+	}
+	if ev := config.Saver.Read(); len(ev) <= 0 {
+		t.Fatal("no non-I/O events?!")
+	}
+}

internal/engine/netx/netx.go

@@ -1,297 +0,0 @@
-// Package netx contains code to perform network measurements.
-//
-// This library contains replacements for commonly used standard library
-// interfaces that facilitate seamless network measurements. By using
-// such replacements, as opposed to standard library interfaces, we can:
-//
-// * save the timing of HTTP events (e.g. received response headers)
-// * save the timing and result of every Connect, Read, Write, Close operation
-// * save the timing and result of the TLS handshake (including certificates)
-//
-// By default, this library uses the system resolver. In addition, it
-// is possible to configure alternative DNS transports and remote
-// servers. We support DNS over UDP, DNS over TCP, DNS over TLS (DoT),
-// and DNS over HTTPS (DoH). When using an alternative transport, we
-// are also able to intercept and save DNS messages, as well as any
-// other interaction with the remote server (e.g., the result of the
-// TLS handshake for DoT and DoH).
-//
-// We described the design and implementation of the most recent version of
-// this package at <https://github.com/ooni/probe-engine/issues/359>. Such
-// issue also links to a previous design document.
-package netx
-
-import (
-	"crypto/tls"
-	"errors"
-	"net"
-	"net/http"
-	"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/netxlite"
-	"github.com/ooni/probe-cli/v3/internal/tracex"
-)
-
-// Config contains configuration for creating a new transport. When any
-// field of Config is nil/empty, we will use a suitable default.
-//
-// We use different savers for different kind of events such that the
-// user of this library can choose what to save.
-type Config struct {
-	BaseResolver        model.Resolver       // default: system resolver
-	BogonIsError        bool                 // default: bogon is not error
-	ByteCounter         *bytecounter.Counter // default: no explicit byte counting
-	CacheResolutions    bool                 // default: no caching
-	ContextByteCounting bool                 // default: no implicit byte counting
-	DNSCache            map[string][]string  // default: cache is empty
-	Dialer              model.Dialer         // default: dialer.DNSDialer
-	FullResolver        model.Resolver       // default: base resolver + goodies
-	QUICDialer          model.QUICDialer     // default: quicdialer.DNSDialer
-	HTTP3Enabled        bool                 // default: disabled
-	Logger              model.Logger         // default: no logging
-	ProxyURL            *url.URL             // default: no proxy
-	ReadWriteSaver      *tracex.Saver        // default: not saving I/O events
-	Saver               *tracex.Saver        // default: not saving non-I/O events
-	TLSConfig           *tls.Config          // default: attempt using h2
-	TLSDialer           model.TLSDialer      // default: dialer.TLSDialer
-}
-
-// NewResolver creates a new resolver from the specified config
-func NewResolver(config Config) model.Resolver {
-	if config.BaseResolver == nil {
-		config.BaseResolver = netxlite.NewResolverSystem()
-	}
-	r := netxlite.WrapResolver(
-		model.ValidLoggerOrDefault(config.Logger),
-		config.BaseResolver,
-	)
-	r = netxlite.MaybeWrapWithCachingResolver(config.CacheResolutions, r)
-	r = netxlite.MaybeWrapWithStaticDNSCache(config.DNSCache, r)
-	r = netxlite.MaybeWrapWithBogonResolver(config.BogonIsError, r)
-	return config.Saver.WrapResolver(r) // WAI when config.Saver==nil
-}
-
-// NewDialer creates a new Dialer from the specified config
-func NewDialer(config Config) model.Dialer {
-	if config.FullResolver == nil {
-		config.FullResolver = NewResolver(config)
-	}
-	logger := model.ValidLoggerOrDefault(config.Logger)
-	d := netxlite.NewDialerWithResolver(
-		logger, config.FullResolver, config.Saver.NewConnectObserver(),
-		config.ReadWriteSaver.NewReadWriteObserver(),
-	)
-	d = netxlite.NewMaybeProxyDialer(d, config.ProxyURL)
-	d = bytecounter.MaybeWrapWithContextAwareDialer(config.ContextByteCounting, d)
-	return d
-}
-
-// NewQUICDialer creates a new DNS Dialer for QUIC, with the resolver from the specified config
-func NewQUICDialer(config Config) model.QUICDialer {
-	if config.FullResolver == nil {
-		config.FullResolver = NewResolver(config)
-	}
-	// TODO(bassosimone): we should count the bytes consumed by this QUIC dialer
-	ql := config.ReadWriteSaver.WrapQUICListener(netxlite.NewQUICListener())
-	logger := model.ValidLoggerOrDefault(config.Logger)
-	return netxlite.NewQUICDialerWithResolver(ql, logger, config.FullResolver, config.Saver)
-}
-
-// NewTLSDialer creates a new TLSDialer from the specified config
-func NewTLSDialer(config Config) model.TLSDialer {
-	if config.Dialer == nil {
-		config.Dialer = NewDialer(config)
-	}
-	logger := model.ValidLoggerOrDefault(config.Logger)
-	thx := netxlite.NewTLSHandshakerStdlib(logger)
-	thx = config.Saver.WrapTLSHandshaker(thx) // WAI even when config.Saver is nil
-	tlsConfig := netxlite.ClonedTLSConfigOrNewEmptyConfig(config.TLSConfig)
-	return netxlite.NewTLSDialerWithConfig(config.Dialer, thx, tlsConfig)
-}
-
-// NewHTTPTransport creates a new HTTPRoundTripper. You can further extend the returned
-// HTTPRoundTripper before wrapping it into an http.Client.
-func NewHTTPTransport(config Config) model.HTTPTransport {
-	if config.Dialer == nil {
-		config.Dialer = NewDialer(config)
-	}
-	if config.TLSDialer == nil {
-		config.TLSDialer = NewTLSDialer(config)
-	}
-	if config.QUICDialer == nil {
-		config.QUICDialer = NewQUICDialer(config)
-	}
-	tInfo := allTransportsInfo[config.HTTP3Enabled]
-	txp := tInfo.Factory(httpTransportConfig{
-		Dialer:     config.Dialer,
-		Logger:     model.ValidLoggerOrDefault(config.Logger),
-		QUICDialer: config.QUICDialer,
-		TLSDialer:  config.TLSDialer,
-		TLSConfig:  config.TLSConfig,
-	})
-	// TODO(bassosimone): I am not super convinced by this code because it
-	// seems we're currently counting bytes twice in some cases. I think we
-	// should review how we're counting bytes and using netx currently.
-	txp = config.ByteCounter.MaybeWrapHTTPTransport(txp)                 // WAI with ByteCounter == nil
-	const defaultSnapshotSize = 0                                        // means: use the default snapsize
-	return config.Saver.MaybeWrapHTTPTransport(txp, defaultSnapshotSize) // WAI with Saver == nil
-}
-
-// httpTransportInfo contains the constructing function as well as the transport name
-type httpTransportInfo struct {
-	Factory       func(httpTransportConfig) model.HTTPTransport
-	TransportName string
-}
-
-var allTransportsInfo = map[bool]httpTransportInfo{
-	false: {
-		Factory:       newSystemTransport,
-		TransportName: "tcp",
-	},
-	true: {
-		Factory:       newHTTP3Transport,
-		TransportName: "quic",
-	},
-}
-
-// NewDNSClient creates a new DNS client. The config argument is used to
-// create the underlying Dialer and/or HTTP transport, if needed. The URL
-// argument describes the kind of client that we want to make:
-//
-// - if the URL is `doh://powerdns`, `doh://google` or `doh://cloudflare` or the URL
-// starts with `https://`, then we create a DoH client.
-//
-// - if the URL is `` or `system:///`, then we create a system client,
-// i.e. a client using the system resolver.
-//
-// - if the URL starts with `udp://`, then we create a client using
-// a resolver that uses the specified UDP endpoint.
-//
-// We return error if the URL does not parse or the URL scheme does not
-// fall into one of the cases described above.
-//
-// If config.ResolveSaver is not nil and we're creating an underlying
-// resolver where this is possible, we will also save events.
-func NewDNSClient(config Config, URL string) (model.Resolver, error) {
-	return NewDNSClientWithOverrides(config, URL, "", "", "")
-}
-
-// NewDNSClientWithOverrides creates a new DNS client, similar to NewDNSClient,
-// with the option to override the default Hostname and SNI.
-func NewDNSClientWithOverrides(config Config, URL, hostOverride, SNIOverride,
-	TLSVersion string) (model.Resolver, error) {
-	switch URL {
-	case "doh://powerdns":
-		URL = "https://doh.powerdns.org/"
-	case "doh://google":
-		URL = "https://dns.google/dns-query"
-	case "doh://cloudflare":
-		URL = "https://cloudflare-dns.com/dns-query"
-	case "":
-		URL = "system:///"
-	}
-	resolverURL, err := url.Parse(URL)
-	if err != nil {
-		return nil, err
-	}
-	config.TLSConfig = &tls.Config{ServerName: SNIOverride}
-	if err := netxlite.ConfigureTLSVersion(config.TLSConfig, TLSVersion); err != nil {
-		return nil, err
-	}
-	switch resolverURL.Scheme {
-	case "system":
-		return netxlite.NewResolverSystem(), nil
-	case "https":
-		config.TLSConfig.NextProtos = []string{"h2", "http/1.1"}
-		httpClient := &http.Client{Transport: NewHTTPTransport(config)}
-		var txp model.DNSTransport = netxlite.NewUnwrappedDNSOverHTTPSTransportWithHostOverride(
-			httpClient, URL, hostOverride)
-		txp = config.Saver.WrapDNSTransport(txp) // safe when config.Saver == nil
-		return netxlite.NewUnwrappedSerialResolver(txp), nil
-	case "udp":
-		dialer := NewDialer(config)
-		endpoint, err := makeValidEndpoint(resolverURL)
-		if err != nil {
-			return nil, err
-		}
-		var txp model.DNSTransport = netxlite.NewUnwrappedDNSOverUDPTransport(
-			dialer, endpoint)
-		txp = config.Saver.WrapDNSTransport(txp) // safe when config.Saver == nil
-		return netxlite.NewUnwrappedSerialResolver(txp), nil
-	case "dot":
-		config.TLSConfig.NextProtos = []string{"dot"}
-		tlsDialer := NewTLSDialer(config)
-		endpoint, err := makeValidEndpoint(resolverURL)
-		if err != nil {
-			return nil, err
-		}
-		var txp model.DNSTransport = netxlite.NewUnwrappedDNSOverTLSTransport(
-			tlsDialer.DialTLSContext, endpoint)
-		txp = config.Saver.WrapDNSTransport(txp) // safe when config.Saver == nil
-		return netxlite.NewUnwrappedSerialResolver(txp), nil
-	case "tcp":
-		dialer := NewDialer(config)
-		endpoint, err := makeValidEndpoint(resolverURL)
-		if err != nil {
-			return nil, err
-		}
-		var txp model.DNSTransport = netxlite.NewUnwrappedDNSOverTCPTransport(
-			dialer.DialContext, endpoint)
-		txp = config.Saver.WrapDNSTransport(txp) // safe when config.Saver == nil
-		return netxlite.NewUnwrappedSerialResolver(txp), nil
-	default:
-		return nil, errors.New("unsupported resolver scheme")
-	}
-}
-
-// makeValidEndpoint makes a valid endpoint for DoT and Do53 given the
-// input URL representing such endpoint. Specifically, we are
-// concerned with the case where the port is missing. In such a
-// case, we ensure that we are using the default port 853 for DoT
-// and default port 53 for TCP and UDP.
-func makeValidEndpoint(URL *url.URL) (string, error) {
-	// Implementation note: when we're using a quoted IPv6
-	// address, URL.Host contains the quotes but instead the
-	// return value from URL.Hostname() does not.
-	//
-	// For example:
-	//
-	// - Host: [2620:fe::9]
-	// - Hostname(): 2620:fe::9
-	//
-	// We need to keep this in mind when trying to determine
-	// whether there is also a port or not.
-	//
-	// So the first step is to check whether URL.Host is already
-	// a whatever valid TCP/UDP endpoint and, if so, use it.
-	if _, _, err := net.SplitHostPort(URL.Host); err == nil {
-		return URL.Host, nil
-	}
-	// The second step is to assume that appending the default port
-	// to a host parsed by url.Parse should be giving us a valid
-	// endpoint. The possibilities in fact are:
-	//
-	// 1. domain w/o port
-	// 2. IPv4 w/o port
-	// 3. square bracket quoted IPv6 w/o port
-	// 4. other
-	//
-	// In the first three cases, appending a port leads us to a
-	// good endpoint. The fourth case does not.
-	//
-	// For this reason we check again whether we can split it using
-	// net.SplitHostPort. If we cannot, we were in case four.
-	host := URL.Host
-	if URL.Scheme == "dot" {
-		host += ":853"
-	} else {
-		host += ":53"
-	}
-	if _, _, err := net.SplitHostPort(host); err != nil {
-		return "", err
-	}
-	// Otherwise it's one of the three valid cases above.
-	return host, nil
-}

internal/engine/netx/quic.go

@@ -0,0 +1,21 @@
+package netx
+
+//
+// QUICDialer from Config.
+//
+
+import (
+	"github.com/ooni/probe-cli/v3/internal/model"
+	"github.com/ooni/probe-cli/v3/internal/netxlite"
+)
+
+// NewQUICDialer creates a new QUICDialer using the given Config.
+func NewQUICDialer(config Config) model.QUICDialer {
+	if config.FullResolver == nil {
+		config.FullResolver = NewResolver(config)
+	}
+	// TODO(bassosimone): we should count the bytes consumed by this QUIC dialer
+	ql := config.ReadWriteSaver.WrapQUICListener(netxlite.NewQUICListener())
+	logger := model.ValidLoggerOrDefault(config.Logger)
+	return netxlite.NewQUICDialerWithResolver(ql, logger, config.FullResolver, config.Saver)
+}

internal/engine/netx/resolver.go

@@ -0,0 +1,25 @@
+package netx
+
+//
+// Resolver from Config.
+//
+
+import (
+	"github.com/ooni/probe-cli/v3/internal/model"
+	"github.com/ooni/probe-cli/v3/internal/netxlite"
+)
+
+// NewResolver creates a new resolver from the specified config.
+func NewResolver(config Config) model.Resolver {
+	if config.BaseResolver == nil {
+		config.BaseResolver = netxlite.NewResolverSystem()
+	}
+	r := netxlite.WrapResolver(
+		model.ValidLoggerOrDefault(config.Logger),
+		config.BaseResolver,
+	)
+	r = netxlite.MaybeWrapWithCachingResolver(config.CacheResolutions, r)
+	r = netxlite.MaybeWrapWithStaticDNSCache(config.DNSCache, r)
+	r = netxlite.MaybeWrapWithBogonResolver(config.BogonIsError, r)
+	return config.Saver.WrapResolver(r) // WAI when config.Saver==nil
+}

internal/engine/netx/resolver_test.go

@@ -0,0 +1,33 @@
+package netx
+
+import (
+	"context"
+	"errors"
+	"testing"
+
+	"github.com/apex/log"
+	"github.com/ooni/probe-cli/v3/internal/netxlite"
+	"github.com/ooni/probe-cli/v3/internal/tracex"
+)
+
+func TestNewResolverBogonResolutionNotBroken(t *testing.T) {
+	saver := new(tracex.Saver)
+	r := NewResolver(Config{
+		BogonIsError: true,
+		DNSCache: map[string][]string{
+			"www.google.com": {"127.0.0.1"},
+		},
+		Saver:  saver,
+		Logger: log.Log,
+	})
+	addrs, err := r.LookupHost(context.Background(), "www.google.com")
+	if !errors.Is(err, netxlite.ErrDNSBogon) {
+		t.Fatal("not the error we expected")
+	}
+	if err.Error() != netxlite.FailureDNSBogonError {
+		t.Fatal("error not correctly wrapped")
+	}
+	if len(addrs) > 0 {
+		t.Fatal("expected no addresses here")
+	}
+}

internal/engine/netx/tls.go

@@ -0,0 +1,22 @@
+package netx
+
+//
+// TLSDialer from Config.
+//
+
+import (
+	"github.com/ooni/probe-cli/v3/internal/model"
+	"github.com/ooni/probe-cli/v3/internal/netxlite"
+)
+
+// NewTLSDialer creates a new TLSDialer from the specified config.
+func NewTLSDialer(config Config) model.TLSDialer {
+	if config.Dialer == nil {
+		config.Dialer = NewDialer(config)
+	}
+	logger := model.ValidLoggerOrDefault(config.Logger)
+	thx := netxlite.NewTLSHandshakerStdlib(logger)
+	thx = config.Saver.WrapTLSHandshaker(thx) // WAI even when config.Saver is nil
+	tlsConfig := netxlite.ClonedTLSConfigOrNewEmptyConfig(config.TLSConfig)
+	return netxlite.NewTLSDialerWithConfig(config.Dialer, thx, tlsConfig)
+}

internal/engine/netx/tls_test.go

@@ -0,0 +1,74 @@
+package netx
+
+import (
+	"context"
+	"crypto/tls"
+	"testing"
+
+	"github.com/ooni/probe-cli/v3/internal/netxlite"
+	"github.com/ooni/probe-cli/v3/internal/netxlite/filtering"
+	"github.com/ooni/probe-cli/v3/internal/tracex"
+)
+
+func TestNewTLSDialer(t *testing.T) {
+	t.Run("we always have error wrapping", func(t *testing.T) {
+		server := filtering.NewTLSServer(filtering.TLSActionReset)
+		defer server.Close()
+		tdx := NewTLSDialer(Config{})
+		conn, err := tdx.DialTLSContext(context.Background(), "tcp", server.Endpoint())
+		if err == nil || err.Error() != netxlite.FailureConnectionReset {
+			t.Fatal("unexpected err", err)
+		}
+		if conn != nil {
+			t.Fatal("expected nil conn")
+		}
+	})
+
+	t.Run("we can collect measurements", func(t *testing.T) {
+		server := filtering.NewTLSServer(filtering.TLSActionReset)
+		defer server.Close()
+		saver := &tracex.Saver{}
+		tdx := NewTLSDialer(Config{
+			Saver: saver,
+		})
+		conn, err := tdx.DialTLSContext(context.Background(), "tcp", server.Endpoint())
+		if err == nil || err.Error() != netxlite.FailureConnectionReset {
+			t.Fatal("unexpected err", err)
+		}
+		if conn != nil {
+			t.Fatal("expected nil conn")
+		}
+		if len(saver.Read()) <= 0 {
+			t.Fatal("did not read any event")
+		}
+	})
+
+	t.Run("we can skip TLS verification", func(t *testing.T) {
+		server := filtering.NewTLSServer(filtering.TLSActionBlockText)
+		defer server.Close()
+		tdx := NewTLSDialer(Config{TLSConfig: &tls.Config{
+			InsecureSkipVerify: true,
+		}})
+		conn, err := tdx.DialTLSContext(context.Background(), "tcp", server.Endpoint())
+		if err != nil {
+			t.Fatal(err.(*netxlite.ErrWrapper).WrappedErr)
+		}
+		conn.Close()
+	})
+
+	t.Run("we can set the cert pool", func(t *testing.T) {
+		server := filtering.NewTLSServer(filtering.TLSActionBlockText)
+		defer server.Close()
+		tdx := NewTLSDialer(Config{
+			TLSConfig: &tls.Config{
+				RootCAs:    server.CertPool(),
+				ServerName: "dns.google",
+			},
+		})
+		conn, err := tdx.DialTLSContext(context.Background(), "tcp", server.Endpoint())
+		if err != nil {
+			t.Fatal(err)
+		}
+		conn.Close()
+	})
+}