feature: use go-libtor on mobile, OONI_TOR_BINARY env on desktop (#614)

This branch adds support for running:

1. `go-libtor` on mobile.

2. the tor provided by the desktop app via the `OONI_TOR_BINARY` environment variable.

See https://github.com/ooni/ooni.org/issues/761.

Co-authored-by: Simone Basso <bassosimone@gmail.com>

internal/tunnel/config.go

@@ -140,13 +140,34 @@ func (c *Config) startPsiphon(ctx context.Context, config []byte,
 		DataRootDirectory: &workdir}, nil, nil)
 }
 
-// torBinary returns the tor binary path, if configured, or
-// the default path, otherwise.
-func (c *Config) torBinary() string {
+// ooniTorBinaryEnv is the name of the environment variable
+// we're using to get the path to the tor binary when we are
+// being run by the ooni/probe-desktop application.
+const ooniTorBinaryEnv = "OONI_TOR_BINARY"
+
+// torBinary returns the tor binary path.
+//
+// Here's is the algorithm:
+//
+// 1. if c.TorBinary is set, we use its value;
+//
+// 2. if os.Getenv("OONI_TOR_BINARY") is set, we use its value;
+//
+// 3. otherwise, we return "tor".
+//
+// Implementation note: in cases 1 and 3 we use execabs.LookPath
+// to guarantee we're not going to execute a binary outside of the
+// PATH (see https://blog.golang.org/path-security for more info
+// on how this bug could affect Windows). In case 2, we're instead
+// just going to trust the binary set by the probe-desktop app.
+func (c *Config) torBinary() (string, error) {
 	if c.TorBinary != "" {
-		return c.TorBinary
+		return c.execabsLookPath(c.TorBinary)
 	}
-	return "tor"
+	if binary := os.Getenv(ooniTorBinaryEnv); binary != "" {
+		return binary, nil
+	}
+	return c.execabsLookPath("tor")
 }
 
 // torStart calls either testTorStart or tor.Start.

internal/tunnel/config_test.go

@@ -1,6 +1,8 @@
 package tunnel
 
 import (
+	"errors"
+	"os"
 	"testing"
 
 	"github.com/apex/log"
@@ -20,17 +22,88 @@ func TestConfigLoggerCustom(t *testing.T) {
 	}
 }
 
-func TestTorBinaryNotSet(t *testing.T) {
-	config := &Config{}
-	if config.torBinary() != "tor" {
-		t.Fatal("not the result we expected")
+func TestConfigTorBinary(t *testing.T) {
+	// newConfig is a factory for creating a new config
+	//
+	// Arguments:
+	//
+	// - torBinaryPath is the possibly-empty config.TorBinary to use;
+	//
+	// - realBinaryPath is the possibly-empty path execabs.LookupPath should return;
+	//
+	// - err is the possbly-nil error that execabs.LookupPath should return.
+	//
+	// Returns a new *Config.
+	newConfig := func(binaryPath, realBinaryPath string, err error) *Config {
+		return &Config{
+			TorBinary: binaryPath,
+			testExecabsLookPath: func(name string) (string, error) {
+				if err != nil {
+					return "", err
+				}
+				return realBinaryPath, nil
+			},
+		}
 	}
-}
 
-func TestTorBinarySet(t *testing.T) {
-	path := "/usr/local/bin/tor"
-	config := &Config{TorBinary: path}
-	if config.torBinary() != path {
-		t.Fatal("not the result we expected")
+	// verifyExpectations ensures that config.torBinary() produces in
+	// output the expectPath and expectErr result.
+	verifyExpectations := func(
+		t *testing.T, config *Config, expectPath string, expectErr error) {
+		path, err := config.torBinary()
+		if !errors.Is(err, expectErr) {
+			t.Fatal("not the error we expected", err)
+		}
+		if path != expectPath {
+			t.Fatal("not the path we expected", path)
+		}
 	}
+
+	t.Run("with empty TorBinary and no tor in PATH", func(t *testing.T) {
+		expected := errors.New("no such binary in PATH")
+		config := newConfig("", "", expected)
+		verifyExpectations(t, config, "", expected)
+	})
+
+	t.Run("with empty TorBinary and tor in PATH", func(t *testing.T) {
+		expected := "/usr/bin/tor"
+		config := newConfig("", expected, nil)
+		verifyExpectations(t, config, expected, nil)
+	})
+
+	t.Run("with TorBinary and no such binary in PATH", func(t *testing.T) {
+		expected := errors.New("no such binary in PATH")
+		config := newConfig("tor-real", "", expected)
+		verifyExpectations(t, config, "", expected)
+	})
+
+	t.Run("with TorBinary and the binary is in PATH", func(t *testing.T) {
+		expected := "/usr/bin/tor-real"
+		config := newConfig("tor-real", expected, nil)
+		verifyExpectations(t, config, expected, nil)
+	})
+
+	t.Run("with OONI_TOR_BINARY and empty TorBinary", func(t *testing.T) {
+		expected := "./tor.exe"
+		os.Setenv(ooniTorBinaryEnv, expected)
+		defer os.Unsetenv(ooniTorBinaryEnv)
+		config := newConfig("", expected, errors.New("should not be seen"))
+		verifyExpectations(t, config, expected, nil)
+	})
+
+	t.Run("with OONI_TOR_BINARY and TorBinary not in PATH", func(t *testing.T) {
+		expected := errors.New("no such binary in PATH")
+		os.Setenv(ooniTorBinaryEnv, "./tor.exe")
+		defer os.Unsetenv(ooniTorBinaryEnv)
+		config := newConfig("tor-real", "", expected)
+		verifyExpectations(t, config, "", expected)
+	})
+
+	t.Run("with OONI_TOR_BINARY and TorBinary in PATH", func(t *testing.T) {
+		expected := "/usr/bin/tor-real"
+		os.Setenv(ooniTorBinaryEnv, "./tor.exe")
+		defer os.Unsetenv(ooniTorBinaryEnv)
+		config := newConfig("tor-real", expected, nil)
+		verifyExpectations(t, config, expected, nil)
+	})
 }

internal/tunnel/tor.go

@@ -10,8 +10,6 @@ import (
 	"path/filepath"
 	"strings"
 	"time"
-
-	"github.com/cretz/bine/tor"
 )
 
 // torProcess is a running tor process.
@@ -74,20 +72,12 @@ func torStart(ctx context.Context, config *Config) (Tunnel, error) {
 	extraArgs = append(extraArgs, "notice stderr")
 	extraArgs = append(extraArgs, "Log")
 	extraArgs = append(extraArgs, fmt.Sprintf(`notice file %s`, logfile))
-	// Implementation note: here we make sure that we're not going to
-	// execute a binary called "tor" in the current directory on Windows
-	// as documented in https://blog.golang.org/path-security.
-	exePath, err := config.execabsLookPath(config.torBinary())
+	config.logger().Infof("tunnel: tor: exec params: %s %+v", stateDir, extraArgs)
+	torStartConf, err := getTorStartConf(config, stateDir, extraArgs)
 	if err != nil {
 		return nil, err
 	}
-	config.logger().Infof("tunnel: exec: %s %+v", exePath, extraArgs)
-	instance, err := config.torStart(ctx, &tor.StartConf{
-		DataDir:   stateDir,
-		ExtraArgs: extraArgs,
-		ExePath:   exePath,
-		NoHush:    true,
-	})
+	instance, err := config.torStart(ctx, torStartConf)
 	if err != nil {
 		return nil, err
 	}

internal/tunnel/tordesktop.go

@@ -0,0 +1,23 @@
+//go:build !android && !ios
+
+package tunnel
+
+// This file implements our strategy for running tor on desktop.
+
+import "github.com/cretz/bine/tor"
+
+// getTorStartConf in this configuration uses torExePath to get a
+// suitable tor binary and then executes it.
+func getTorStartConf(config *Config, dataDir string, extraArgs []string) (*tor.StartConf, error) {
+	exePath, err := config.torBinary()
+	if err != nil {
+		return nil, err
+	}
+	config.logger().Infof("tunnel: tor: exec binary: %s", exePath)
+	return &tor.StartConf{
+		ExePath:   exePath,
+		DataDir:   dataDir,
+		ExtraArgs: extraArgs,
+		NoHush:    true,
+	}, nil
+}

internal/tunnel/tormobile.go

@@ -0,0 +1,21 @@
+//go:build ios || android
+
+package tunnel
+
+// This file implements our strategy for running tor on mobile.
+
+import (
+	"github.com/cretz/bine/tor"
+	"github.com/ooni/go-libtor"
+)
+
+// getTorStartConf in this configuration uses github.com/ooni/go-libtor.
+func getTorStartConf(config *Config, dataDir string, extraArgs []string) (*tor.StartConf, error) {
+	config.logger().Info("tunnel: tor: using ooni/go-libtor")
+	return &tor.StartConf{
+		ProcessCreator: libtor.Creator,
+		DataDir:        dataDir,
+		ExtraArgs:      extraArgs,
+		NoHush:         true,
+	}, nil
+}

internal/tunnel/tunnel.go

@@ -19,6 +19,14 @@
 // that does not embed such a configuration, it won't
 // be possible to address this use case.
 //
+// For tor tunnels, we have two distinct configurations: on
+// mobile we use github.com/ooni/go-libtor; on desktop we use
+// a more complex strategy. If the OONI_TOR_BINARY environment
+// variable is set, we assume its value is the path to the
+// tor binary and use it. Otherwise, we search for an executable
+// called "tor" in the PATH and use it. Those two strategies
+// are implemented, respectively by tormobile.go and tordesktop.go.
+//
 // See session.go in the engine package for more details
 // concerning this second use case.
 package tunnel