From 41cf4a867176c8828a6ee9199f2fa33b3ab3b0f4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Arturo=20Filast=C3=B2?= Date: Wed, 15 Dec 2021 14:16:22 +0100 Subject: [PATCH] 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 --- go.mod | 1 + go.sum | 5 ++ internal/tunnel/config.go | 31 ++++++++++-- internal/tunnel/config_test.go | 93 ++++++++++++++++++++++++++++++---- internal/tunnel/tor.go | 16 ++---- internal/tunnel/tordesktop.go | 23 +++++++++ internal/tunnel/tormobile.go | 21 ++++++++ internal/tunnel/tunnel.go | 8 +++ 8 files changed, 170 insertions(+), 28 deletions(-) create mode 100644 internal/tunnel/tordesktop.go create mode 100644 internal/tunnel/tormobile.go diff --git a/go.mod b/go.mod index c758cf0..83855d8 100644 --- a/go.mod +++ b/go.mod @@ -32,6 +32,7 @@ require ( github.com/mitchellh/go-wordwrap v1.0.1 github.com/montanaflynn/stats v0.6.6 github.com/onsi/ginkgo v1.16.5 // indirect + github.com/ooni/go-libtor v1.1.4 github.com/ooni/oohttp v0.0.0-20211206150145-a14131163a5e github.com/ooni/probe-assets v0.6.0 github.com/ooni/psiphon v0.9.0 diff --git a/go.sum b/go.sum index 95ff75b..6df2c7c 100644 --- a/go.sum +++ b/go.sum @@ -93,6 +93,7 @@ github.com/creack/goselect v0.1.1/go.mod h1:a/NhLweNvqIYMuxcMOuWY516Cimucms3DglD github.com/creack/goselect v0.1.2 h1:2DNy14+JPjRBgPzAd1thbQp4BSIihxcBf0IXhQXDRa0= github.com/creack/goselect v0.1.2/go.mod h1:a/NhLweNvqIYMuxcMOuWY516Cimucms3DglDzQP3hKY= github.com/creack/pty v1.1.7/go.mod h1:lj5s0c3V2DBrqTV7llrYr5NG6My20zk30Fl46Y7DoTY= +github.com/cretz/bine v0.1.0/go.mod h1:6PF6fWAvYtwjRGkAuDEJeWNOv3a2hUouSP/yRYXmvHw= github.com/cretz/bine v0.2.0 h1:8GiDRGlTgz+o8H9DSnsl+5MeBK4HsExxgl6WgzOCuZo= github.com/cretz/bine v0.2.0/go.mod h1:WU4o9QR9wWp8AVKtTM1XD5vUHkEqnf2vVSo6dBqbetI= github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= @@ -418,6 +419,8 @@ github.com/onsi/gomega v1.10.3/go.mod h1:V9xEwhxec5O8UDM77eCW8vLymOMltsqPVYWrpDs github.com/onsi/gomega v1.11.0/go.mod h1:azGKhqFUon9Vuj0YmTfLSmx0FUwqXYSTl5re8lQLTUg= github.com/onsi/gomega v1.13.0 h1:7lLHu94wT9Ij0o6EWWclhu0aOh32VxhkwEJvzuWPeak= github.com/onsi/gomega v1.13.0/go.mod h1:lRk9szgn8TxENtWd0Tp4c3wjlRfMTMH27I+3Je41yGY= +github.com/ooni/go-libtor v1.1.4 h1:awp/ZUTO4sI85ufR5/GNmZ4YK41lNlWpjeKJkpBQ41Q= +github.com/ooni/go-libtor v1.1.4/go.mod h1:q1YyLwRD9GeMyeerVvwc0vJ2YgwDLTp2bdVcrh/JXyI= github.com/ooni/oohttp v0.0.0-20211206150145-a14131163a5e h1:hGKGXauBg3FpVw+gwUsGTNAwDz/TwSJL7eobvzDKnYs= github.com/ooni/oohttp v0.0.0-20211206150145-a14131163a5e/go.mod h1:IhQrrBqPX6/Pb8mGKdw3wskNOk5r8ngqmqAXrH3j7vw= github.com/ooni/probe-assets v0.6.0 h1:ZL1CTO1Z3CBYieVG6wuwXHLoeZmmFUR2AhtZlBFctTk= @@ -691,6 +694,7 @@ golang.org/x/crypto v0.0.0-20181030102418-4d3f4d9ffa16/go.mod h1:6SG95UA2DQfeDnf golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2/go.mod h1:djNgcEr1/C05ACkg1iLfiJU5Ep61QUkGW8qpdssI0+w= golang.org/x/crypto v0.0.0-20190313024323-a1f597ede03a/go.mod h1:djNgcEr1/C05ACkg1iLfiJU5Ep61QUkGW8qpdssI0+w= golang.org/x/crypto v0.0.0-20190325154230-a5d413f7728c/go.mod h1:djNgcEr1/C05ACkg1iLfiJU5Ep61QUkGW8qpdssI0+w= +golang.org/x/crypto v0.0.0-20190404164418-38d8ce5564a5/go.mod h1:WFFai1msRO1wXaEeE5yQxYXgSfI8pQAWXbQop6sCtWE= golang.org/x/crypto v0.0.0-20190426145343-a29dc8fdc734/go.mod h1:yigFU9vqHzYiE8UmvKecakEJjdnWj3jj499lnFckfCI= golang.org/x/crypto v0.0.0-20190510104115-cbcb75029529/go.mod h1:yigFU9vqHzYiE8UmvKecakEJjdnWj3jj499lnFckfCI= golang.org/x/crypto v0.0.0-20190530122614-20be4c3c3ed5/go.mod h1:yigFU9vqHzYiE8UmvKecakEJjdnWj3jj499lnFckfCI= @@ -802,6 +806,7 @@ golang.org/x/sys v0.0.0-20190215142949-d0b11bdaac8a/go.mod h1:STP8DvDyc/dI5b8T5h golang.org/x/sys v0.0.0-20190222072716-a9d3bda3a223/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY= golang.org/x/sys v0.0.0-20190316082340-a2f829d7f35f/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20190329044733-9eb1bfa1ce65/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= +golang.org/x/sys v0.0.0-20190403152447-81d4e9dc473e/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20190412213103-97732733099d/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20190422165155-953cdadca894/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20190502145724-3ef323f4f1fd/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= diff --git a/internal/tunnel/config.go b/internal/tunnel/config.go index b44983b..f5fe1c8 100644 --- a/internal/tunnel/config.go +++ b/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. diff --git a/internal/tunnel/config_test.go b/internal/tunnel/config_test.go index 0654038..6861a42 100644 --- a/internal/tunnel/config_test.go +++ b/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) + }) } diff --git a/internal/tunnel/tor.go b/internal/tunnel/tor.go index fdc59ff..4ba4759 100644 --- a/internal/tunnel/tor.go +++ b/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 } diff --git a/internal/tunnel/tordesktop.go b/internal/tunnel/tordesktop.go new file mode 100644 index 0000000..902a4e6 --- /dev/null +++ b/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 +} diff --git a/internal/tunnel/tormobile.go b/internal/tunnel/tormobile.go new file mode 100644 index 0000000..6a28291 --- /dev/null +++ b/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 +} diff --git a/internal/tunnel/tunnel.go b/internal/tunnel/tunnel.go index 1261b4d..9825c5d 100644 --- a/internal/tunnel/tunnel.go +++ b/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