diff --git a/internal/engine/session.go b/internal/engine/session.go index 079d1f6..11e42b9 100644 --- a/internal/engine/session.go +++ b/internal/engine/session.go @@ -171,6 +171,7 @@ func NewSession(ctx context.Context, config SessionConfig) (*Session, error) { config.Logger.Infof( "starting '%s' tunnel; please be patient...", proxyURL.Scheme) tunnel, err := tunnel.Start(ctx, &tunnel.Config{ + Logger: config.Logger, Name: proxyURL.Scheme, Session: &sessionTunnelEarlySession{}, TorArgs: config.TorArgs, diff --git a/internal/engine/tunnel/config.go b/internal/engine/tunnel/config.go index 9ccd1aa..a85056f 100644 --- a/internal/engine/tunnel/config.go +++ b/internal/engine/tunnel/config.go @@ -9,12 +9,24 @@ import ( "github.com/cretz/bine/control" "github.com/cretz/bine/tor" "github.com/ooni/psiphon/oopsi/github.com/Psiphon-Labs/psiphon-tunnel-core/ClientLibrary/clientlib" + "golang.org/x/sys/execabs" ) +// Logger is the logger to use. Its signature is compatibile +// with the apex/log logger signature. +type Logger interface { + // Infof formats and emits an informative message + Infof(format string, v ...interface{}) +} + // Config contains the configuration for creating a Tunnel instance. You need // to fill all the mandatory fields. You SHOULD NOT modify the content of this // structure while in use, because that may lead to data races. type Config struct { + // Logger is the logger to use. If empty we use a default + // implementation that does not emit any output. + Logger Logger + // Name is the mandatory name of the tunnel. We support // "tor", "psiphon", and "fake" tunnels. You SHOULD // use "fake" tunnels only for testing: they don't provide @@ -41,6 +53,9 @@ type Config struct { // Start function fails with ErrEmptyTunnelDir. TunnelDir string + // testExecabsLookPath allows us to mock exeabs.LookPath + testExecabsLookPath func(name string) (string, error) + // testMkdirAll allows us to mock os.MkdirAll in testing code. testMkdirAll func(path string, perm os.FileMode) error @@ -66,6 +81,31 @@ type Config struct { testTorGetInfo func(ctrl *control.Conn, keys ...string) ([]*control.KeyVal, error) } +// silentLogger is a logger that does not emit output. +type silentLogger struct{} + +// Infof implements Logger.Infof. +func (sl *silentLogger) Infof(format string, v ...interface{}) {} + +// defaultLogger is the default logger. +var defaultLogger = &silentLogger{} + +// logger returns the logger to use. +func (c *Config) logger() Logger { + if c.Logger != nil { + return c.Logger + } + return defaultLogger +} + +// execabsLookPath calls either testExeabsLookPath or execabs.LookPath +func (c *Config) execabsLookPath(name string) (string, error) { + if c.testExecabsLookPath != nil { + return c.testExecabsLookPath(name) + } + return execabs.LookPath(name) +} + // mkdirAll calls either testMkdirAll or os.MkdirAll. func (c *Config) mkdirAll(path string, perm os.FileMode) error { if c.testMkdirAll != nil { @@ -100,6 +140,15 @@ 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 { + if c.TorBinary != "" { + return c.TorBinary + } + return "tor" +} + // torStart calls either testTorStart or tor.Start. func (c *Config) torStart(ctx context.Context, conf *tor.StartConf) (*tor.Tor, error) { if c.testTorStart != nil { diff --git a/internal/engine/tunnel/config_test.go b/internal/engine/tunnel/config_test.go new file mode 100644 index 0000000..0654038 --- /dev/null +++ b/internal/engine/tunnel/config_test.go @@ -0,0 +1,36 @@ +package tunnel + +import ( + "testing" + + "github.com/apex/log" +) + +func TestConfigLoggerDefault(t *testing.T) { + config := &Config{} + if config.logger() != defaultLogger { + t.Fatal("not the logger we expected") + } +} + +func TestConfigLoggerCustom(t *testing.T) { + config := &Config{Logger: log.Log} + if config.logger() != log.Log { + t.Fatal("not the logger we expected") + } +} + +func TestTorBinaryNotSet(t *testing.T) { + config := &Config{} + if config.torBinary() != "tor" { + t.Fatal("not the result we expected") + } +} + +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") + } +} diff --git a/internal/engine/tunnel/tor.go b/internal/engine/tunnel/tor.go index 96e299c..fdc59ff 100644 --- a/internal/engine/tunnel/tor.go +++ b/internal/engine/tunnel/tor.go @@ -74,10 +74,18 @@ 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()) + 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: config.TorBinary, + ExePath: exePath, NoHush: true, }) if err != nil { diff --git a/internal/engine/tunnel/tor_integration_test.go b/internal/engine/tunnel/tor_integration_test.go index 15ed457..f52702c 100644 --- a/internal/engine/tunnel/tor_integration_test.go +++ b/internal/engine/tunnel/tor_integration_test.go @@ -3,21 +3,21 @@ package tunnel_test import ( "context" "io/ioutil" - "os" "testing" "github.com/apex/log" "github.com/ooni/probe-cli/v3/internal/engine" "github.com/ooni/probe-cli/v3/internal/engine/tunnel" + "golang.org/x/sys/execabs" ) func TestTorStartStop(t *testing.T) { if testing.Short() { t.Skip("skip test in short mode") } - torBinaryPath := "/usr/bin/tor" - if m, err := os.Stat(torBinaryPath); err != nil || !m.Mode().IsRegular() { - t.Skip("missing precondition for the test") + torBinaryPath, err := execabs.LookPath("tor") + if err != nil { + t.Skip("missing precondition for the test: tor not in PATH") } tunnelDir, err := ioutil.TempDir("testdata", "tor") if err != nil { diff --git a/internal/engine/tunnel/tor_test.go b/internal/engine/tunnel/tor_test.go index c180041..35dc6be 100644 --- a/internal/engine/tunnel/tor_test.go +++ b/internal/engine/tunnel/tor_test.go @@ -9,6 +9,7 @@ import ( "os" "path/filepath" "strings" + "syscall" "testing" "github.com/cretz/bine/control" @@ -77,12 +78,30 @@ func TestTorWithEmptyTunnelDir(t *testing.T) { } } +func TestTorBinaryNotFoundFailure(t *testing.T) { + ctx := context.Background() + tun, err := torStart(ctx, &Config{ + Session: &mockable.Session{}, + TorBinary: "/nonexistent/directory/tor", + TunnelDir: "testdata", + }) + if !errors.Is(err, syscall.ENOENT) { + t.Fatal("not the error we expected", err) + } + if tun != nil { + t.Fatal("expected nil tunnel here") + } +} + func TestTorStartFailure(t *testing.T) { expected := errors.New("mocked error") ctx := context.Background() tun, err := torStart(ctx, &Config{ Session: &mockable.Session{}, TunnelDir: "testdata", + testExecabsLookPath: func(name string) (string, error) { + return "/usr/local/bin/tor", nil + }, testTorStart: func(ctx context.Context, conf *tor.StartConf) (*tor.Tor, error) { return nil, expected }, @@ -101,6 +120,9 @@ func TestTorEnableNetworkFailure(t *testing.T) { tun, err := torStart(ctx, &Config{ Session: &mockable.Session{}, TunnelDir: "testdata", + testExecabsLookPath: func(name string) (string, error) { + return "/usr/local/bin/tor", nil + }, testTorStart: func(ctx context.Context, conf *tor.StartConf) (*tor.Tor, error) { return &tor.Tor{}, nil }, @@ -122,6 +144,9 @@ func TestTorGetInfoFailure(t *testing.T) { tun, err := torStart(ctx, &Config{ Session: &mockable.Session{}, TunnelDir: "testdata", + testExecabsLookPath: func(name string) (string, error) { + return "/usr/local/bin/tor", nil + }, testTorStart: func(ctx context.Context, conf *tor.StartConf) (*tor.Tor, error) { return &tor.Tor{}, nil }, @@ -145,6 +170,9 @@ func TestTorGetInfoInvalidNumberOfKeys(t *testing.T) { tun, err := torStart(ctx, &Config{ Session: &mockable.Session{}, TunnelDir: "testdata", + testExecabsLookPath: func(name string) (string, error) { + return "/usr/local/bin/tor", nil + }, testTorStart: func(ctx context.Context, conf *tor.StartConf) (*tor.Tor, error) { return &tor.Tor{}, nil }, @@ -168,6 +196,9 @@ func TestTorGetInfoInvalidKey(t *testing.T) { tun, err := torStart(ctx, &Config{ Session: &mockable.Session{}, TunnelDir: "testdata", + testExecabsLookPath: func(name string) (string, error) { + return "/usr/local/bin/tor", nil + }, testTorStart: func(ctx context.Context, conf *tor.StartConf) (*tor.Tor, error) { return &tor.Tor{}, nil }, @@ -191,6 +222,9 @@ func TestTorGetInfoInvalidProxyType(t *testing.T) { tun, err := torStart(ctx, &Config{ Session: &mockable.Session{}, TunnelDir: "testdata", + testExecabsLookPath: func(name string) (string, error) { + return "/usr/local/bin/tor", nil + }, testTorStart: func(ctx context.Context, conf *tor.StartConf) (*tor.Tor, error) { return &tor.Tor{}, nil }, @@ -214,6 +248,9 @@ func TestTorUnsupportedProxy(t *testing.T) { tun, err := torStart(ctx, &Config{ Session: &mockable.Session{}, TunnelDir: "testdata", + testExecabsLookPath: func(name string) (string, error) { + return "/usr/local/bin/tor", nil + }, testTorStart: func(ctx context.Context, conf *tor.StartConf) (*tor.Tor, error) { return &tor.Tor{}, nil },