cleanup: remove UnderlyingNetworkLibrary and TProxy (#874)

* cleanup: remove UnderlyingNetworkLibrary and TProxy

While there, replace mixture of mocking and real connections inside
quicping with pure mocking of network connections.

Closes https://github.com/ooni/probe/issues/2224

* cleanup: we don't need a SimpleResolver now

This type was only used by UnderlyingNetworkLibrary and all the
rest of the code uses Resolver. So, let's avoid complexity by zapping
the SimpleResolver type and merging it inside Resolver.

internal/engine/experiment/quicping/quicping.go

@@ -18,7 +18,6 @@ import (
 	_ "crypto/sha256"
 
 	"github.com/ooni/probe-cli/v3/internal/model"
-	"github.com/ooni/probe-cli/v3/internal/netxlite"
 	"github.com/ooni/probe-cli/v3/internal/tracex"
 )
 
@@ -44,8 +43,8 @@ type Config struct {
 	// Port is the port to test.
 	Port int64 `ooni:"port is the port to test"`
 
-	// networkLibrary is the underlying network library. Can be used for testing.
+	// netListenUDP allows mocking the real net.ListenUDP call
-	networkLib model.UnderlyingNetworkLibrary
+	netListenUDP func(network string, laddr *net.UDPAddr) (model.UDPLikeConn, error)
 }
 
 func (c *Config) repetitions() int64 {
@@ -62,11 +61,11 @@ func (c *Config) port() string {
 	return "443"
 }
 
-func (c *Config) networkLibrary() model.UnderlyingNetworkLibrary {
+func (c *Config) doListenUDP(network string, laddr *net.UDPAddr) (model.UDPLikeConn, error) {
-	if c.networkLib != nil {
+	if c.netListenUDP != nil {
-		return c.networkLib
+		return c.netListenUDP(network, laddr)
 	}
-	return &netxlite.TProxyStdlib{}
+	return net.ListenUDP(network, laddr)
 }
 
 // TestKeys contains the experiment results.
@@ -246,7 +245,7 @@ func (m *Measurer) Run(
 	measurement.TestKeys = tk
 
 	// create UDP socket
-	pconn, err := m.config.networkLibrary().ListenUDP("udp", &net.UDPAddr{})
+	pconn, err := m.config.doListenUDP("udp", &net.UDPAddr{})
 	if err != nil {
 		return err
 	}

internal/engine/experiment/quicping/quicping_test.go

@@ -15,76 +15,6 @@ import (
 	"github.com/ooni/probe-cli/v3/internal/model/mocks"
 )
 
-// FailStdLib is a failing model.UnderlyingNetworkLibrary.
-type FailStdLib struct {
-	conn     model.UDPLikeConn
-	err      error
-	writeErr error
-	readErr  error
-}
-
-// ListenUDP implements model.UnderlyingNetworkLibrary.ListenUDP.
-func (f *FailStdLib) ListenUDP(network string, laddr *net.UDPAddr) (model.UDPLikeConn, error) {
-	conn, _ := net.ListenUDP(network, laddr)
-	f.conn = model.UDPLikeConn(conn)
-	if f.err != nil {
-		return nil, f.err
-	}
-	if f.writeErr != nil {
-		return &mocks.UDPLikeConn{
-			MockWriteTo: func(p []byte, addr net.Addr) (int, error) {
-				return 0, f.writeErr
-			},
-			MockReadFrom: func(p []byte) (int, net.Addr, error) {
-				return f.conn.ReadFrom(p)
-			},
-			MockSetDeadline: func(t time.Time) error {
-				return f.conn.SetDeadline(t)
-			},
-			MockClose: func() error {
-				return f.conn.Close()
-			},
-		}, nil
-	}
-	if f.readErr != nil {
-		return &mocks.UDPLikeConn{
-			MockWriteTo: func(p []byte, addr net.Addr) (int, error) {
-				return f.conn.WriteTo(p, addr)
-			},
-			MockReadFrom: func(p []byte) (int, net.Addr, error) {
-				return 0, nil, f.readErr
-			},
-			MockSetDeadline: func(t time.Time) error {
-				return f.conn.SetDeadline(t)
-			},
-			MockClose: func() error {
-				return f.conn.Close()
-			},
-		}, nil
-	}
-	return &mocks.UDPLikeConn{}, nil
-}
-
-// DefaultResolver implements model.UnderlyingNetworkLibrary.DefaultResolver.
-func (f *FailStdLib) DefaultResolver() model.SimpleResolver {
-	return f
-}
-
-// LookupHost implements model.SimpleResolver.LookupHost.
-func (f *FailStdLib) LookupHost(ctx context.Context, domain string) ([]string, error) {
-	return nil, f.err
-}
-
-// Network implements model.SimpleResolver.Network.
-func (f *FailStdLib) Network() string {
-	return "fail_stdlib"
-}
-
-// NewSimpleDialer implements UnderlyingNetworkLibrary.NewSimpleDialer.
-func (f *FailStdLib) NewSimpleDialer(timeout time.Duration) model.SimpleDialer {
-	return nil
-}
-
 func TestNewExperimentMeasurer(t *testing.T) {
 	measurer := NewExperimentMeasurer(Config{})
 	if measurer.ExperimentName() != "quicping" {
@@ -201,7 +131,9 @@ func TestWithCancelledContext(t *testing.T) {
 func TestListenFails(t *testing.T) {
 	expected := errors.New("expected")
 	measurer := NewExperimentMeasurer(Config{
-		networkLib: &FailStdLib{err: expected, readErr: nil, writeErr: nil},
+		netListenUDP: func(network string, laddr *net.UDPAddr) (model.UDPLikeConn, error) {
+			return nil, expected
+		},
 	})
 	measurement := new(model.Measurement)
 	measurement.Input = model.MeasurementTarget("google.com")
@@ -221,8 +153,30 @@ func TestWriteFails(t *testing.T) {
 		t.Skip("skip test in short mode")
 	}
 	expected := errors.New("expected")
+	setDeadlineCalled := false
+	closeCalled := false
+	pconn := &mocks.UDPLikeConn{
+		MockReadFrom: func(p []byte) (int, net.Addr, error) {
+			source := make([]byte, len(p))
+			copy(p, source)
+			return len(p), &mocks.Addr{}, nil
+		},
+		MockSetDeadline: func(t time.Time) error {
+			setDeadlineCalled = true
+			return nil
+		},
+		MockClose: func() error {
+			closeCalled = true
+			return nil
+		},
+		MockWriteTo: func(p []byte, addr net.Addr) (int, error) {
+			return 0, expected
+		},
+	}
 	measurer := NewExperimentMeasurer(Config{
-		networkLib:  &FailStdLib{err: nil, readErr: nil, writeErr: expected},
+		netListenUDP: func(network string, laddr *net.UDPAddr) (model.UDPLikeConn, error) {
+			return pconn, nil
+		},
 		Repetitions: 1,
 	})
 	measurement := new(model.Measurement)
@@ -245,15 +199,41 @@ func TestWriteFails(t *testing.T) {
 			t.Fatal("ping: unexpected error type", i, *ping.Failure)
 		}
 	}
+	if !setDeadlineCalled {
+		t.Fatal("did not call set deadline")
+	}
+	if !closeCalled {
+		t.Fatal("did not call close")
+	}
 }
 
 func TestReadFails(t *testing.T) {
 	if testing.Short() {
 		t.Skip("skip test in short mode")
 	}
+	setDeadlineCalled := false
+	closeCalled := false
 	expected := errors.New("expected")
+	pconn := &mocks.UDPLikeConn{
+		MockReadFrom: func(p []byte) (int, net.Addr, error) {
+			return 0, nil, expected
+		},
+		MockSetDeadline: func(t time.Time) error {
+			setDeadlineCalled = true
+			return nil
+		},
+		MockClose: func() error {
+			closeCalled = true
+			return nil
+		},
+		MockWriteTo: func(p []byte, addr net.Addr) (int, error) {
+			return len(p), nil
+		},
+	}
 	measurer := NewExperimentMeasurer(Config{
-		networkLib:  &FailStdLib{err: nil, readErr: expected, writeErr: nil},
+		netListenUDP: func(network string, laddr *net.UDPAddr) (model.UDPLikeConn, error) {
+			return pconn, nil
+		},
 		Repetitions: 1,
 	})
 	measurement := new(model.Measurement)
@@ -273,6 +253,12 @@ func TestReadFails(t *testing.T) {
 			t.Fatal("expected an error here, ping", i)
 		}
 	}
+	if !setDeadlineCalled {
+		t.Fatal("did not call set deadline")
+	}
+	if !closeCalled {
+		t.Fatal("did not call close")
+	}
 }
 
 func TestNoResponse(t *testing.T) {
@@ -303,7 +289,7 @@ func TestNoResponse(t *testing.T) {
 }
 
 func TestDissect(t *testing.T) {
-	//                             destID--srcID: 040b9649d3fd4c038ab6c073966f3921--44d064031288e97646451f
+	// destID--srcID: 040b9649d3fd4c038ab6c073966f3921--44d064031288e97646451f
 	versionNegotiationResponse, _ := hex.DecodeString("eb0000000010040b9649d3fd4c038ab6c073966f39210b44d064031288e97646451f00000001ff00001dff00001cff00001b")
 	measurer := NewExperimentMeasurer(Config{})
 	destID := "040b9649d3fd4c038ab6c073966f3921"

internal/model/netx.go

@@ -211,9 +211,8 @@ type QUICDialer interface {
 	CloseIdleConnections()
 }
 
-// SimpleResolver is a simplified resolver that only allows to perform
+// Resolver performs domain name resolutions.
-// an ordinary lookup operation and to know the resolver's name.
+type Resolver interface {
-type SimpleResolver interface {
 	// LookupHost behaves like net.Resolver.LookupHost.
 	LookupHost(ctx context.Context, hostname string) (addrs []string, err error)
 
@@ -239,12 +238,6 @@ type SimpleResolver interface {
 	// for an explanation of why it would not be proper to call "netgo" the
 	// resolver we get by default from the standard library.
 	Network() string
-}
-
-// Resolver performs domain name resolutions.
-type Resolver interface {
-	// A Resolver is also a SimpleResolver.
-	SimpleResolver
 
 	// Address returns the resolver address (e.g., 8.8.8.8:53).
 	Address() string
@@ -484,18 +477,3 @@ type UDPLikeConn interface {
 	// which is also instrumental to setting the read buffer.
 	SyscallConn() (syscall.RawConn, error)
 }
-
-// UnderlyingNetworkLibrary defines the basic functionality from
-// which the network extensions depend. By changing the default
-// implementation of this interface, we can implement a wide array
-// of tests, including self censorship tests.
-type UnderlyingNetworkLibrary interface {
-	// ListenUDP creates a new model.UDPLikeConn conn.
-	ListenUDP(network string, laddr *net.UDPAddr) (UDPLikeConn, error)
-
-	// DefaultResolver returns the default resolver.
-	DefaultResolver() SimpleResolver
-
-	// NewSimpleDialer returns a new SimpleDialer.
-	NewSimpleDialer(timeout time.Duration) SimpleDialer
-}

internal/netxlite/dialer.go

@@ -143,7 +143,7 @@ func NewDialerWithoutResolver(dl model.DebugLogger, w ...model.DialerWrapper) mo
 	return NewDialerWithResolver(dl, &NullResolver{}, w...)
 }
 
-// DialerSystem is a model.Dialer that uses TProxy.NewSimplerDialer
+// DialerSystem is a model.Dialer that uses the stdlib's net.Dialer
 // to construct the new SimpleDialer used for dialing. This dialer has
 // a fixed timeout for each connect operation equal to 15 seconds.
 type DialerSystem struct {
@@ -160,7 +160,7 @@ func (d *DialerSystem) newUnderlyingDialer() model.SimpleDialer {
 	if t <= 0 {
 		t = dialerDefaultTimeout
 	}
-	return TProxy.NewSimpleDialer(t)
+	return &net.Dialer{Timeout: t}
 }
 
 func (d *DialerSystem) DialContext(ctx context.Context, network, address string) (net.Conn, error) {

internal/netxlite/dnsovergetaddrinfo.go

@@ -81,7 +81,7 @@ func (txp *dnsOverGetaddrinfoTransport) lookupfn() func(ctx context.Context, dom
 	if txp.testableLookupHost != nil {
 		return txp.testableLookupHost
 	}
-	return TProxy.DefaultResolver().LookupHost
+	return getaddrinfoLookupHost
 }
 
 func (txp *dnsOverGetaddrinfoTransport) RequiresPadding() bool {
@@ -89,7 +89,7 @@ func (txp *dnsOverGetaddrinfoTransport) RequiresPadding() bool {
 }
 
 func (txp *dnsOverGetaddrinfoTransport) Network() string {
-	return TProxy.DefaultResolver().Network()
+	return getaddrinfoResolverNetwork()
 }
 
 func (txp *dnsOverGetaddrinfoTransport) Address() string {

internal/netxlite/dnsovergetaddrinfo_test.go

@@ -21,7 +21,7 @@ func TestDNSOverGetaddrinfo(t *testing.T) {
 
 	t.Run("Network", func(t *testing.T) {
 		txp := &dnsOverGetaddrinfoTransport{}
-		if txp.Network() != TProxy.DefaultResolver().Network() {
+		if txp.Network() != getaddrinfoResolverNetwork() {
 			t.Fatal("unexpected Network")
 		}
 	})

internal/netxlite/quic.go

@@ -29,7 +29,7 @@ var _ model.QUICListener = &quicListenerStdlib{}
 
 // Listen implements QUICListener.Listen.
 func (qls *quicListenerStdlib) Listen(addr *net.UDPAddr) (model.UDPLikeConn, error) {
-	return TProxy.ListenUDP("udp", addr)
+	return net.ListenUDP("udp", addr)
 }
 
 // NewQUICDialerWithResolver is the WrapDialer equivalent for QUIC where

internal/netxlite/tproxy.go

@@ -1,52 +0,0 @@
-package netxlite
-
-//
-// Transparent proxy (for integration testing)
-//
-
-import (
-	"context"
-	"net"
-	"time"
-
-	"github.com/ooni/probe-cli/v3/internal/model"
-)
-
-// TProxy is the fundamental variable controlling how netxlite creates
-// net.Conn and model.UDPLikeConn, as well as how it uses the stdlib
-// resolver. By modifying this variable, you can effectively transparently
-// proxy netxlite (and hence OONI) activities to other services. This is
-// quite convenient when performing quality assurance tests.
-var TProxy model.UnderlyingNetworkLibrary = &TProxyStdlib{}
-
-// TProxyStdlib is the default model.UnderlyingNetworkLibrary using
-// the stdlib in the most obvious way for every functionality.
-type TProxyStdlib struct{}
-
-// ListenUDP calls net.ListenUDP.
-func (*TProxyStdlib) ListenUDP(network string, laddr *net.UDPAddr) (model.UDPLikeConn, error) {
-	return net.ListenUDP(network, laddr)
-}
-
-// DefaultResolver returns the default resolver.
-func (*TProxyStdlib) DefaultResolver() model.SimpleResolver {
-	return &tproxyDefaultResolver{}
-}
-
-// NewSimpleDialer returns a &net.Dialer{Timeout: timeout} instance.
-func (*TProxyStdlib) NewSimpleDialer(timeout time.Duration) model.SimpleDialer {
-	return &net.Dialer{Timeout: timeout}
-}
-
-// tproxyDefaultResolver is the resolver we use by default.
-type tproxyDefaultResolver struct{}
-
-// LookupHost implements model.SimpleResolver.LookupHost.
-func (r *tproxyDefaultResolver) LookupHost(ctx context.Context, domain string) ([]string, error) {
-	return getaddrinfoLookupHost(ctx, domain)
-}
-
-// Network implements model.SimpleResolver.Network.
-func (r *tproxyDefaultResolver) Network() string {
-	return getaddrinfoResolverNetwork()
-}