netx: move beginning and handler at toplevel

DESIGN.md

@@ -2,7 +2,7 @@
 
 | Author       | Simone Basso |
 |--------------|--------------|
-| Last-Updated | 2019-10-04   |
+| Last-Updated | 2019-10-22   |
 | Status       | approved     |
 
 ## Introduction
@@ -179,7 +179,8 @@ type Measurement struct {
     HTTPResponseDone        *HTTPResponseDoneEvent
     Read                    *ReadEvent
     Resolve                 *ResolveEvent
-    TLSHandshake            *TLSHandshakeEvent
+    TLSHandshakeStart       *TLSHandshakeStartEvent
+    TLSHandshakeDone        *TLSHandshakeDoneEvent
     Write                   *WriteEvent
 }
 ```
@@ -193,9 +194,9 @@ The following network-level events will be defined:
 
 1. `CloseEvent`, indicating when a socket is closed
 2. `ConnectEvent`, indicating the result of connecting
-3. `Read`, indicating when a `read` completes
-4. `Resolve`, indicating when a name resolution completes
-5. `Write`, indicating when a `write` completes
+3. `ReadEvent`, indicating when a `read` completes
+4. `ResolveEvent`, indicating when a name resolution completes
+5. `WriteEvent`, indicating when a `write` completes
 
 The following DNS-level events will be defined:
 
@@ -425,13 +426,6 @@ As far as `ConfigureDNS` is concerned it will work as follows:
 used and no low-level events pertaining to the DNS will be
 emitted to the configured `handler`. This will be the default.
 
-* when `network` is `"netgo"`, we will try to use the DNS
-resolver written in Go within the standard library (which is
-know to work only on Unix), and we will use a bunch of
-hacks to observe the events occurring during name resolutions,
-such as `Read` and `Write` events. We will also be able to
-record the DNS messages sent and received.
-
 * when `network` is `"udp"`, `address` must be a valid
 string following the `"<ip_or_domain>(:<port>)*"` pattern. If
 `<ip_or_domain>` is IPv6, it must be quoted using `[]`. If

cmd/dnsclient/main.go

@@ -3,7 +3,7 @@
 // Usage:
 //
 //   dnsclient -type Addr|CNAME|Host|MX|NS -name <name>
-//             -transport system|godns|tcp|udp|dot|doh
+//             -transport system|tcp|udp|dot|doh
 //             -endpoint <transport-specific-endpoint>
 //
 //   dnsclient -help
@@ -18,7 +18,6 @@
 // Examples:
 //
 //   ./dnsclient -transport system ...
-//   ./dnsclient -transport godns ...
 //   ./dnsclient -transport doh -endpoint https://cloudflare-dns.com/dns-query ...
 //   ./dnsclient -transport dot -endpoint dns.quad9.net ...
 //   ./dnsclient -transport dot -endpoint 1.1.1.1:853 ...
@@ -66,7 +65,6 @@ func mainWithContext(ctx context.Context) error {
 		flag.PrintDefaults()
 		fmt.Printf("\nExamples:\n")
 		fmt.Printf("%s\n", "  ./dnsclient -transport system ...")
-		fmt.Printf("%s\n", "  ./dnsclient -transport godns ...")
 		fmt.Printf("%s\n", "  ./dnsclient -transport doh -endpoint https://cloudflare-dns.com/dns-query ...")
 		fmt.Printf("%s\n", "  ./dnsclient -transport dot -endpoint dns.quad9.net ...")
 		fmt.Printf("%s\n", "  ./dnsclient -transport dot -endpoint 1.1.1.1:853 ...")

cmd/httpclient/main.go

@@ -2,22 +2,20 @@
 //
 // Usage:
 //
-//   httpclient -dns-transport system|godns|tcp|udp|dot|doh -url <URL>
+//   httpclient -dns-server <URL> -url <URL>
 //
 //   httpclient -help
 //
-// The default is to use the system DNS. Use -dns-engine to force
-// a different type of DNS transport. We'll use a good default resolver
-// for the selected transport. This only works on Unix.
+// The default is to use the system DNS. Use different <URL>s to
+// configure using different DNS servers and transports.
 //
 // We emit JSONL messages on the stdout showing what we are
 // currently doing. We also print the final result on the stdout.
 //
 // Examples:
 //
 //   ./httpclient -dns-server system:/// -url https://ooni.org/ ...
-//   ./httpclient -dns-server godns:/// -url https://ooni.org/ ...
-//   ./httpclient -dns-server https://cloudflare-dns.com/dns-query -url https://ooni.org/ ...
+//   ./httpclient -dns-server https://cloudflare-dns.com/dns-query ...
 //   ./httpclient -dns-server dot://dns.quad9.net -url https://ooni.org/ ...
 //   ./httpclient -dns-server dot://1.1.1.1:853 -url https://ooni.org/ ...
 //   ./httpclient -dns-server tcp://8.8.8.8:53 -url https://ooni.org/ ...
@@ -67,9 +65,6 @@ func mainfunc() (err error) {
 		fmt.Printf("%s\n",
 			"  ./httpclient system:/// -url https://ooni.org/ ...",
 		)
-		fmt.Printf("%s\n",
-			"  ./httpclient -dns-server godns:/// -url https://ooni.org/ ...",
-		)
 		fmt.Printf("%s\n",
 			"  ./httpclient -dns-server https://cloudflare-dns.com/dns-query "+
 				"-url https://ooni.org/ ...",
@@ -86,8 +81,7 @@ func mainfunc() (err error) {
 		fmt.Printf("%s\n",
 			"  ./httpclient -dns-server udp://1.1.1.1:53 -url https://ooni.org/ ...",
 		)
-		fmt.Printf("\nWe'll select a suitable backend for each transport. Note\n")
-		fmt.Printf("that this only works on Unix.\n")
+		fmt.Printf("\nWe'll select a suitable backend for each transport.\n")
 		return nil
 	}
 
@@ -96,8 +90,6 @@ func mainfunc() (err error) {
 
 	if urlDNSServer.Scheme == "system" {
 		err = client.ConfigureDNS("system", "")
-	} else if urlDNSServer.Scheme == "godns" {
-		err = client.ConfigureDNS("godns", "")
 	} else if urlDNSServer.Scheme == "udp" {
 		err = client.ConfigureDNS("udp", urlDNSServer.Host)
 	} else if urlDNSServer.Scheme == "tcp" {

cmd/httpclient/main_test.go

@@ -30,17 +30,6 @@ func TestSystemTransport(t *testing.T) {
 	}
 }
 
-func TestGoDNSTransport(t *testing.T) {
-	*flagDNSServer = "godns:///"
-	defer func() {
-		*flagDNSServer = ""
-	}()
-	err := mainfunc()
-	if err != nil {
-		t.Fatal(err)
-	}
-}
-
 func TestUDPTransport(t *testing.T) {
 	*flagDNSServer = "udp://1.1.1.1:53"
 	defer func() {

dnsx/dnsx.go

@@ -1,32 +1,20 @@
-// Package dnsx contains OONI's DNS extensions
+// Package dnsx contains OONI's DNS extensions.
+//
+// Because this package was often causing import loop headaches we
+// have moved everything inside of model. Specifically, adding to
+// this file functions such as NewResolver or NewTransport will be
+// conducive to such loops. As such, this file is unusable unless
+// we move the definition, say, to model, and we just keep here the
+// names originally defined by the design document.
 package dnsx
 
-import (
-	"context"
-	"net"
-)
+import "github.com/ooni/netx/model"
 
 // Client is a DNS client. The *net.Resolver used by Go implements
 // this interface, but other implementations are possible.
-type Client interface {
-	// LookupAddr performs a reverse lookup of an address.
-	LookupAddr(ctx context.Context, addr string) (names []string, err error)
+type Client model.DNSClient
 
-	// LookupCNAME returns the canonical name of a given host.
-	LookupCNAME(ctx context.Context, host string) (cname string, err error)
-
-	// LookupHost resolves a hostname to a list of IP addresses.
-	LookupHost(ctx context.Context, hostname string) (addrs []string, err error)
-
-	// LookupMX resolves the DNS MX records for a given domain name.
-	LookupMX(ctx context.Context, name string) ([]*net.MX, error)
-
-	// LookupNS resolves the DNS NS records for a given domain name.
-	LookupNS(ctx context.Context, name string) ([]*net.NS, error)
-}
-
-// RoundTripper represents an abstract DNS transport.
-type RoundTripper interface {
-	// RoundTrip sends a DNS query and receives the reply.
-	RoundTrip(query []byte) (reply []byte, err error)
-}
+// RoundTripper represents an abstract DNS transport. This name
+// was not originally specified inside of the design document but
+// we've had it here for a while and may be useful later.
+type RoundTripper model.DNSRoundTripper

httpx/httpx.go

@@ -10,6 +10,7 @@ import (
 	"github.com/ooni/netx/internal/dialerapi"
 	"github.com/ooni/netx/internal/dnsconf"
 	"github.com/ooni/netx/internal/httptransport"
+	"github.com/ooni/netx/internal/tlsconf"
 	"github.com/ooni/netx/model"
 )
 
@@ -23,14 +24,10 @@ type Transport struct {
 // the time to use as zero for computing the elapsed time.
 func NewTransport(beginning time.Time, handler model.Handler) *Transport {
 	t := new(Transport)
-	t.dialer = dialerapi.NewDialer(beginning, handler)
+	t.dialer = dialerapi.NewDialer()
 	t.transport = httptransport.NewTransport(beginning, handler)
-	// make sure we use an http2 ready TLS config
-	t.dialer.TLSConfig = t.transport.TLSClientConfig
 	// make sure HTTP uses our dialer
-	t.transport.Dial = t.dialer.Dial
 	t.transport.DialContext = t.dialer.DialContext
-	t.transport.DialTLS = t.dialer.DialTLS
 	return t
 }
 
@@ -49,18 +46,24 @@ func (t *Transport) CloseIdleConnections() {
 
 // ConfigureDNS is exactly like netx.Dialer.ConfigureDNS.
 func (t *Transport) ConfigureDNS(network, address string) error {
-	return dnsconf.ConfigureDNS(t.dialer, network, address)
+	return dnsconf.ConfigureDNS(
+		t.dialer,
+		t.transport.Beginning,
+		t.transport.Handler,
+		network,
+		address,
+	)
 }
 
 // SetCABundle internally calls netx.Dialer.SetCABundle and
 // therefore it has the same caveats and limitations.
 func (t *Transport) SetCABundle(path string) error {
-	return t.dialer.SetCABundle(path)
+	return tlsconf.SetCABundle(t.transport.TLSClientConfig, path)
 }
 
 // ForceSpecificSNI forces using a specific SNI.
 func (t *Transport) ForceSpecificSNI(sni string) error {
-	return t.dialer.ForceSpecificSNI(sni)
+	return tlsconf.ForceSpecificSNI(t.transport.TLSClientConfig, sni)
 }
 
 // Client is a replacement for http.Client.

internal/connector/connector.go

@@ -0,0 +1,12 @@
+// Package connector contains the generic connector interface
+package connector
+
+import (
+	"context"
+	"net"
+)
+
+// Model is the model of any abstract connector
+type Model interface {
+	DialContext(context.Context, string, string) (net.Conn, error)
+}

internal/connector/emittingconnector/emittingconnector.go

@@ -0,0 +1,68 @@
+// Package emittingconnector contains a connector emitting events
+package emittingconnector
+
+import (
+	"context"
+	"net"
+	"time"
+
+	"github.com/ooni/netx/internal/connector"
+	"github.com/ooni/netx/internal/connx"
+	"github.com/ooni/netx/internal/tracing"
+	"github.com/ooni/netx/model"
+)
+
+// Connector is a connector emitting events
+type Connector struct {
+	connector connector.Model
+}
+
+// New returns a new emitting connector
+func New(connector connector.Model) *Connector {
+	return &Connector{connector: connector}
+}
+
+// DialContext creates a new connection.
+func (c *Connector) DialContext(
+	ctx context.Context, network, address string,
+) (net.Conn, error) {
+	start := time.Now()
+	conn, err := c.connector.DialContext(ctx, network, address)
+	stop := time.Now()
+	if info := tracing.ContextInfo(ctx); info != nil {
+		info.Handler.OnMeasurement(model.Measurement{
+			Connect: &model.ConnectEvent{
+				ConnID:        info.ConnID,
+				Duration:      stop.Sub(start),
+				Error:         err,
+				LocalAddress:  safeLocalAddress(conn),
+				Network:       network,
+				RemoteAddress: safeRemoteAddress(conn),
+				Time:          stop.Sub(info.Beginning),
+			},
+		})
+		if conn != nil {
+			conn = &connx.MeasuringConn{
+				Beginning: info.Beginning,
+				Conn:      conn,
+				Handler:   info.Handler,
+				ID:        info.ConnID,
+			}
+		}
+	}
+	return conn, err
+}
+
+func safeLocalAddress(conn net.Conn) (s string) {
+	if conn != nil && conn.LocalAddr() != nil {
+		s = conn.LocalAddr().String()
+	}
+	return
+}
+
+func safeRemoteAddress(conn net.Conn) (s string) {
+	if conn != nil && conn.RemoteAddr() != nil {
+		s = conn.RemoteAddr().String()
+	}
+	return
+}

internal/connector/emittingconnector/emittingconnector_test.go

@@ -0,0 +1,29 @@
+package emittingconnector
+
+import (
+	"context"
+	"testing"
+
+	"github.com/ooni/netx/internal/connector/ooconnector"
+	"github.com/ooni/netx/internal/handlers/counthandler"
+	"github.com/ooni/netx/internal/tracing"
+)
+
+func TestIntegration(t *testing.T) {
+	info := &tracing.Info{
+		Handler: &counthandler.Handler{},
+	}
+	ctx := tracing.WithInfo(context.Background(), info)
+	connector := New(ooconnector.New())
+	conn, err := connector.DialContext(ctx, "tcp", "8.8.8.8:53")
+	if err != nil {
+		t.Fatal(err)
+	}
+	if conn == nil {
+		t.Fatal("expected a nil conn")
+	}
+	conn.Close()
+	if info.Handler.(*counthandler.Handler).Count < 0 {
+		t.Fatal("no measurements saved")
+	}
+}

internal/connector/ooconnector/ooconnector.go

@@ -0,0 +1,26 @@
+// Package ooconnector contains OONI's connector
+package ooconnector
+
+import (
+	"context"
+	"errors"
+	"net"
+)
+
+// Connector is OONI's connector
+type Connector struct{}
+
+// New returns a new OONI connector
+func New() *Connector {
+	return new(Connector)
+}
+
+// DialContext creates a new connection.
+func (c *Connector) DialContext(
+	ctx context.Context, network, address string,
+) (net.Conn, error) {
+	if h, _, e := net.SplitHostPort(address); e != nil || net.ParseIP(h) == nil {
+		return nil, errors.New("ooconnector: didn't pass me a <ip>:<port>")
+	}
+	return (&net.Dialer{}).DialContext(ctx, network, address)
+}