refactor: use ooni/oocrypto instead of ooni/go (#751)

Rather than building for Android using ooni/go, we're now using
ooni/oocryto as the TLS dependency. Such a repository only forks
crypto/tls and some minor crypto packages and includes the
same set of patches that we have been using in ooni/go.

This new strategy should be better than the previous one in
terms of building for Android, because we can use the vanilla
go1.18.2 build. It also seems that it is easier to track and
merge from upstream with ooni/oocrypto than it is with ooni/go.

Should this assessment be wrong, we can revert back to the
previous scenario where we used ooni/go.

See https://github.com/ooni/probe/issues/2106 for extra context.

internal/engine/netx/httptransport/system.go

@@ -1,8 +1,7 @@
 package httptransport
 
 import (
-	"net/http"
-
+	oohttp "github.com/ooni/oohttp"
 	"github.com/ooni/probe-cli/v3/internal/model"
 )
 
@@ -13,7 +12,7 @@ import (
 //
 // New code should use netxlite.NewHTTPTransport instead.
 func NewSystemTransport(config Config) model.HTTPTransport {
-	txp := http.DefaultTransport.(*http.Transport).Clone()
+	txp := oohttp.DefaultTransport.(*oohttp.Transport).Clone()
 	txp.DialContext = config.Dialer.DialContext
 	txp.DialTLSContext = config.TLSDialer.DialTLSContext
 	// Better for Cloudflare DNS and also better because we have less
@@ -24,12 +23,12 @@ func NewSystemTransport(config Config) model.HTTPTransport {
 	// back the true headers, such as Content-Length. This change is
 	// functional to OONI's goal of observing the network.
 	txp.DisableCompression = true
-	return &SystemTransportWrapper{txp}
+	return &SystemTransportWrapper{&oohttp.StdlibTransport{Transport: txp}}
 }
 
 // SystemTransportWrapper adapts *http.Transport to have the .Network method
 type SystemTransportWrapper struct {
-	*http.Transport
+	*oohttp.StdlibTransport
 }
 
 func (txp *SystemTransportWrapper) Network() string {

internal/engine/netx/tlsdialer/integration_test.go

@@ -5,6 +5,7 @@ import (
 	"testing"
 
 	"github.com/apex/log"
+	oohttp "github.com/ooni/oohttp"
 	"github.com/ooni/probe-cli/v3/internal/netxlite"
 )
 
@@ -19,9 +20,11 @@ func TestTLSDialerSuccess(t *testing.T) {
 			DebugLogger:   log.Log,
 		},
 	}
-	txp := &http.Transport{
-		DialTLSContext:    dialer.DialTLSContext,
-		ForceAttemptHTTP2: true,
+	txp := &oohttp.StdlibTransport{
+		Transport: &oohttp.Transport{
+			DialTLSContext:    dialer.DialTLSContext,
+			ForceAttemptHTTP2: true,
+		},
 	}
 	client := &http.Client{Transport: txp}
 	resp, err := client.Get("https://www.google.com")

internal/netxlite/tls.go

@@ -13,6 +13,7 @@ import (
 	"net"
 	"time"
 
+	ootls "github.com/ooni/oocrypto/tls"
 	oohttp "github.com/ooni/oohttp"
 	"github.com/ooni/probe-cli/v3/internal/model"
 	"github.com/ooni/probe-cli/v3/internal/runtimex"
@@ -161,7 +162,7 @@ func newTLSHandshaker(th model.TLSHandshaker, logger model.DebugLogger) model.TL
 type tlsHandshakerConfigurable struct {
 	// NewConn is the OPTIONAL factory for creating a new connection. If
 	// this factory is not set, we'll use the stdlib.
-	NewConn func(conn net.Conn, config *tls.Config) TLSConn
+	NewConn func(conn net.Conn, config *tls.Config) (TLSConn, error)
 
 	// Timeout is the OPTIONAL timeout imposed on the TLS handshake. If zero
 	// or negative, we will use default timeout of 10 seconds.
@@ -190,7 +191,10 @@ func (h *tlsHandshakerConfigurable) Handshake(
 		config = config.Clone()
 		config.RootCAs = defaultCertPool
 	}
-	tlsconn := h.newConn(conn, config)
+	tlsconn, err := h.newConn(conn, config)
+	if err != nil {
+		return nil, tls.ConnectionState{}, err
+	}
 	if err := tlsconn.HandshakeContext(ctx); err != nil {
 		return nil, tls.ConnectionState{}, err
 	}
@@ -198,11 +202,11 @@ func (h *tlsHandshakerConfigurable) Handshake(
 }
 
 // newConn creates a new TLSConn.
-func (h *tlsHandshakerConfigurable) newConn(conn net.Conn, config *tls.Config) TLSConn {
+func (h *tlsHandshakerConfigurable) newConn(conn net.Conn, config *tls.Config) (TLSConn, error) {
 	if h.NewConn != nil {
 		return h.NewConn(conn, config)
 	}
-	return tls.Client(conn, config)
+	return ootls.NewClientConnStdlib(conn, config)
 }
 
 // defaultTLSHandshaker is the default TLS handshaker.

internal/netxlite/tls_test.go

@@ -202,13 +202,13 @@ func TestTLSHandshakerConfigurable(t *testing.T) {
 			expected := errors.New("mocked error")
 			var gotTLSConfig *tls.Config
 			handshaker := &tlsHandshakerConfigurable{
-				NewConn: func(conn net.Conn, config *tls.Config) TLSConn {
+				NewConn: func(conn net.Conn, config *tls.Config) (TLSConn, error) {
 					gotTLSConfig = config
 					return &mocks.TLSConn{
 						MockHandshakeContext: func(ctx context.Context) error {
 							return expected
 						},
-					}
+					}, nil
 				},
 			}
 			ctx := context.Background()
@@ -235,6 +235,32 @@ func TestTLSHandshakerConfigurable(t *testing.T) {
 				t.Fatal("gotTLSConfig.RootCAs has not been correctly set")
 			}
 		})
+
+		t.Run("we cannot create a new conn", func(t *testing.T) {
+			expected := errors.New("mocked error")
+			handshaker := &tlsHandshakerConfigurable{
+				NewConn: func(conn net.Conn, config *tls.Config) (TLSConn, error) {
+					return nil, expected
+				},
+			}
+			ctx := context.Background()
+			config := &tls.Config{}
+			conn := &mocks.Conn{
+				MockSetDeadline: func(t time.Time) error {
+					return nil
+				},
+			}
+			tlsConn, connState, err := handshaker.Handshake(ctx, conn, config)
+			if !errors.Is(err, expected) {
+				t.Fatal("not the error we expected", err)
+			}
+			if !reflect.ValueOf(connState).IsZero() {
+				t.Fatal("expected zero connState here")
+			}
+			if tlsConn != nil {
+				t.Fatal("expected nil tlsConn here")
+			}
+		})
 	})
 }
 

internal/netxlite/utls.go

@@ -8,7 +8,9 @@ import (
 	"context"
 	"crypto/tls"
 	"errors"
+	"fmt"
 	"net"
+	"reflect"
 
 	"github.com/ooni/probe-cli/v3/internal/model"
 	utls "gitlab.com/yawning/utls.git"
@@ -42,20 +44,50 @@ type utlsConn struct {
 var _ TLSConn = &utlsConn{}
 
 // newConnUTLS returns a NewConn function for creating utlsConn instances.
-func newConnUTLS(clientHello *utls.ClientHelloID) func(conn net.Conn, config *tls.Config) TLSConn {
-	return func(conn net.Conn, config *tls.Config) TLSConn {
-		uConfig := &utls.Config{
-			RootCAs:                     config.RootCAs,
-			NextProtos:                  config.NextProtos,
-			ServerName:                  config.ServerName,
-			InsecureSkipVerify:          config.InsecureSkipVerify,
-			DynamicRecordSizingDisabled: config.DynamicRecordSizingDisabled,
-		}
-		tlsConn := utls.UClient(conn, uConfig, *clientHello)
-		return &utlsConn{UConn: tlsConn}
+func newConnUTLS(clientHello *utls.ClientHelloID) func(conn net.Conn, config *tls.Config) (TLSConn, error) {
+	return func(conn net.Conn, config *tls.Config) (TLSConn, error) {
+		return newConnUTLSWithHelloID(conn, config, clientHello)
 	}
 }
 
+// errUTLSIncompatibleStdlibConfig indicates that the stdlib config you passed to
+// newConnUTLSWithHelloID contains some fields we don't support.
+var errUTLSIncompatibleStdlibConfig = errors.New("utls: incompatible stdlib config")
+
+// newConnUTLSWithHelloID creates a new connection with the given client hello ID.
+func newConnUTLSWithHelloID(conn net.Conn, config *tls.Config, cid *utls.ClientHelloID) (TLSConn, error) {
+	supportedFields := map[string]bool{
+		"DynamicRecordSizingDisabled": true,
+		"InsecureSkipVerify":          true,
+		"NextProtos":                  true,
+		"RootCAs":                     true,
+		"ServerName":                  true,
+	}
+	value := reflect.ValueOf(config).Elem()
+	kind := value.Type()
+	for idx := 0; idx < value.NumField(); idx++ {
+		field := value.Field(idx)
+		if field.IsZero() {
+			continue
+		}
+		fieldKind := kind.Field(idx)
+		if supportedFields[fieldKind.Name] {
+			continue
+		}
+		err := fmt.Errorf("%w: field %s is nonzero", errUTLSIncompatibleStdlibConfig, fieldKind.Name)
+		return nil, err
+	}
+	uConfig := &utls.Config{
+		DynamicRecordSizingDisabled: config.DynamicRecordSizingDisabled,
+		InsecureSkipVerify:          config.InsecureSkipVerify,
+		RootCAs:                     config.RootCAs,
+		NextProtos:                  config.NextProtos,
+		ServerName:                  config.ServerName,
+	}
+	tlsConn := utls.UClient(conn, uConfig, *cid)
+	return &utlsConn{UConn: tlsConn}, nil
+}
+
 // ErrUTLSHandshakePanic indicates that there was panic handshaking
 // when we were using the yawning/utls library for parroting.
 // See https://github.com/ooni/probe/issues/1770 for more information.

internal/netxlite/utls_test.go

@@ -2,7 +2,9 @@ package netxlite
 
 import (
 	"context"
+	"crypto/tls"
 	"errors"
+	"net"
 	"sync"
 	"testing"
 	"time"
@@ -92,3 +94,51 @@ func TestUTLSConn(t *testing.T) {
 		})
 	})
 }
+
+func Test_newConnUTLSWithHelloID(t *testing.T) {
+	tests := []struct {
+		name        string
+		config      *tls.Config
+		cid         *utls.ClientHelloID
+		wantNilConn bool
+		wantErr     error
+	}{{
+		name: "with only supported fields",
+		config: &tls.Config{
+			DynamicRecordSizingDisabled: true,
+			InsecureSkipVerify:          true,
+			NextProtos:                  []string{"h3"},
+			RootCAs:                     NewDefaultCertPool(),
+			ServerName:                  "ooni.org",
+		},
+		cid:         &utls.HelloFirefox_55,
+		wantNilConn: false,
+		wantErr:     nil,
+	}, {
+		name: "with unsupported fields",
+		config: &tls.Config{
+			Time: func() time.Time {
+				return time.Now()
+			},
+		},
+		cid:         &utls.HelloChrome_58,
+		wantNilConn: true,
+		wantErr:     errUTLSIncompatibleStdlibConfig,
+	}}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			conn, err := net.Dial("udp", "8.8.8.8:443") // we just need a conn
+			if err != nil {
+				t.Fatal(err)
+			}
+			defer conn.Close()
+			got, err := newConnUTLSWithHelloID(conn, tt.config, tt.cid)
+			if !errors.Is(err, tt.wantErr) {
+				t.Fatal("unexpected err", err)
+			}
+			if got != nil && tt.wantNilConn {
+				t.Fatal("expected nil conn here")
+			}
+		})
+	}
+}