refactor(tunnel): simplify tor implementation (#290)

Simplify interaction within the package by avoiding to have
a tor specific config. Use a Config instead.

Part of https://github.com/ooni/probe/issues/985.
This commit is contained in:
Simone Basso 2021-04-03 21:25:08 +02:00 committed by GitHub
parent f739450370
commit d9aff19be5
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
6 changed files with 109 additions and 92 deletions

View File

@ -4,6 +4,8 @@ import (
"context" "context"
"os" "os"
"github.com/cretz/bine/control"
"github.com/cretz/bine/tor"
"github.com/ooni/psiphon/oopsi/github.com/Psiphon-Labs/psiphon-tunnel-core/ClientLibrary/clientlib" "github.com/ooni/psiphon/oopsi/github.com/Psiphon-Labs/psiphon-tunnel-core/ClientLibrary/clientlib"
) )
@ -29,6 +31,17 @@ type Config struct {
// testStartPsiphon allows us to mock psiphon's clientlib.StartTunnel. // testStartPsiphon allows us to mock psiphon's clientlib.StartTunnel.
testStartPsiphon func(ctx context.Context, config []byte, testStartPsiphon func(ctx context.Context, config []byte,
workdir string) (*clientlib.PsiphonTunnel, error) workdir string) (*clientlib.PsiphonTunnel, error)
// testTorStart allows us to mock tor.Start.
testTorStart func(ctx context.Context, conf *tor.StartConf) (*tor.Tor, error)
// testTorEnableNetwork allows us to fake a failure when
// telling to the tor daemon to enable the network.
testTorEnableNetwork func(ctx context.Context, tor *tor.Tor, wait bool) error
// testTorGetInfo allows us to fake a failure when
// getting info from the tor control port.
testTorGetInfo func(ctrl *control.Conn, keys ...string) ([]*control.KeyVal, error)
} }
// mkdirAll calls either testMkdirAll or os.MkdirAll. // mkdirAll calls either testMkdirAll or os.MkdirAll.
@ -56,3 +69,27 @@ func (c *Config) startPsiphon(ctx context.Context, config []byte,
return clientlib.StartTunnel(ctx, config, "", clientlib.Parameters{ return clientlib.StartTunnel(ctx, config, "", clientlib.Parameters{
DataRootDirectory: &workdir}, nil, nil) DataRootDirectory: &workdir}, nil, nil)
} }
// torStart calls either testTorStart or tor.Start.
func (c *Config) torStart(ctx context.Context, conf *tor.StartConf) (*tor.Tor, error) {
if c.testTorStart != nil {
return c.testTorStart(ctx, conf)
}
return tor.Start(ctx, conf)
}
// torEnableNetwork calls either testTorEnableNetwork or tor.EnableNetwork.
func (c *Config) torEnableNetwork(ctx context.Context, tor *tor.Tor, wait bool) error {
if c.testTorEnableNetwork != nil {
return c.testTorEnableNetwork(ctx, tor, wait)
}
return tor.EnableNetwork(ctx, wait)
}
// torGetInfo calls either testTorGetInfo or ctrl.GetInfo.
func (c *Config) torGetInfo(ctrl *control.Conn, keys ...string) ([]*control.KeyVal, error) {
if c.testTorGetInfo != nil {
return c.testTorGetInfo(ctrl, keys...)
}
return ctrl.GetInfo(keys...)
}

View File

@ -13,10 +13,14 @@ import (
// psiphonTunnel is a psiphon tunnel // psiphonTunnel is a psiphon tunnel
type psiphonTunnel struct { type psiphonTunnel struct {
// tunnel is the underlying psiphon tunnel
tunnel *clientlib.PsiphonTunnel tunnel *clientlib.PsiphonTunnel
// duration is the duration of the bootstrap
duration time.Duration duration time.Duration
} }
// makeworkingdir creates the working directory
func makeworkingdir(config *Config) (string, error) { func makeworkingdir(config *Config) (string, error) {
const testdirname = "oonipsiphon" const testdirname = "oonipsiphon"
workdir := filepath.Join(config.WorkDir, testdirname) workdir := filepath.Join(config.WorkDir, testdirname)

View File

@ -26,7 +26,7 @@ func TestPsiphonFetchPsiphonConfigFailure(t *testing.T) {
} }
} }
func TestPsiphonMakeMkdirAllFailure(t *testing.T) { func TestPsiphonMkdirAllFailure(t *testing.T) {
expected := errors.New("mocked error") expected := errors.New("mocked error")
sess := &mockable.Session{ sess := &mockable.Session{
MockableFetchPsiphonConfigResult: []byte(`{}`), MockableFetchPsiphonConfigResult: []byte(`{}`),
@ -45,7 +45,7 @@ func TestPsiphonMakeMkdirAllFailure(t *testing.T) {
} }
} }
func TestPsiphonMakeRemoveAllFailure(t *testing.T) { func TestPsiphonRemoveAllFailure(t *testing.T) {
expected := errors.New("mocked error") expected := errors.New("mocked error")
sess := &mockable.Session{ sess := &mockable.Session{
MockableFetchPsiphonConfigResult: []byte(`{}`), MockableFetchPsiphonConfigResult: []byte(`{}`),
@ -64,7 +64,7 @@ func TestPsiphonMakeRemoveAllFailure(t *testing.T) {
} }
} }
func TestPsiphonMakeStartFailure(t *testing.T) { func TestPsiphonStartFailure(t *testing.T) {
expected := errors.New("mocked error") expected := errors.New("mocked error")
sess := &mockable.Session{ sess := &mockable.Session{
MockableFetchPsiphonConfigResult: []byte(`{}`), MockableFetchPsiphonConfigResult: []byte(`{}`),

View File

@ -8,23 +8,28 @@ import (
"strings" "strings"
"time" "time"
"github.com/cretz/bine/control"
"github.com/cretz/bine/tor" "github.com/cretz/bine/tor"
) )
// torProcess is a running tor process // torProcess is a running tor process
type torProcess interface { type torProcess interface {
// Close kills the running tor process
Close() error Close() error
} }
// torTunnel is the Tor tunnel // torTunnel is the Tor tunnel
type torTunnel struct { type torTunnel struct {
// bootstrapTime is the duration of the bootstrap
bootstrapTime time.Duration bootstrapTime time.Duration
// instance is the running tor instance
instance torProcess instance torProcess
// proxy is the SOCKS5 proxy URL
proxy *url.URL proxy *url.URL
} }
// BootstrapTime is the bootstrsap time // BootstrapTime returns the bootstrap time
func (tt *torTunnel) BootstrapTime() (duration time.Duration) { func (tt *torTunnel) BootstrapTime() (duration time.Duration) {
if tt != nil { if tt != nil {
duration = tt.bootstrapTime duration = tt.bootstrapTime
@ -47,47 +52,23 @@ func (tt *torTunnel) Stop() {
} }
} }
// torStartConfig contains the configuration for StartWithConfig // torStart starts the tor tunnel.
type torStartConfig struct { func torStart(ctx context.Context, config *Config) (Tunnel, error) {
Sess Session
Start func(ctx context.Context, conf *tor.StartConf) (*tor.Tor, error)
EnableNetwork func(ctx context.Context, tor *tor.Tor, wait bool) error
GetInfo func(ctrl *control.Conn, keys ...string) ([]*control.KeyVal, error)
}
// torStart starts the tor tunnel
func torStart(ctx context.Context, sess Session) (Tunnel, error) {
return torStartWithConfig(ctx, torStartConfig{
Sess: sess,
Start: func(ctx context.Context, conf *tor.StartConf) (*tor.Tor, error) {
return tor.Start(ctx, conf)
},
EnableNetwork: func(ctx context.Context, tor *tor.Tor, wait bool) error {
return tor.EnableNetwork(ctx, wait)
},
GetInfo: func(ctrl *control.Conn, keys ...string) ([]*control.KeyVal, error) {
return ctrl.GetInfo(keys...)
},
})
}
// torStartWithConfig is a configurable torStart for testing
func torStartWithConfig(ctx context.Context, config torStartConfig) (Tunnel, error) {
select { select {
case <-ctx.Done(): case <-ctx.Done():
return nil, ctx.Err() // allows to write unit tests using this code return nil, ctx.Err() // allows to write unit tests using this code
default: default:
} }
logfile := LogFile(config.Sess) logfile := LogFile(config.Session)
extraArgs := append([]string{}, config.Sess.TorArgs()...) extraArgs := append([]string{}, config.Session.TorArgs()...)
extraArgs = append(extraArgs, "Log") extraArgs = append(extraArgs, "Log")
extraArgs = append(extraArgs, "notice stderr") extraArgs = append(extraArgs, "notice stderr")
extraArgs = append(extraArgs, "Log") extraArgs = append(extraArgs, "Log")
extraArgs = append(extraArgs, fmt.Sprintf(`notice file %s`, logfile)) extraArgs = append(extraArgs, fmt.Sprintf(`notice file %s`, logfile))
instance, err := config.Start(ctx, &tor.StartConf{ instance, err := config.torStart(ctx, &tor.StartConf{
DataDir: path.Join(config.Sess.TempDir(), "tor"), DataDir: path.Join(config.Session.TempDir(), "tor"),
ExtraArgs: extraArgs, ExtraArgs: extraArgs,
ExePath: config.Sess.TorBinary(), ExePath: config.Session.TorBinary(),
NoHush: true, NoHush: true,
}) })
if err != nil { if err != nil {
@ -95,13 +76,13 @@ func torStartWithConfig(ctx context.Context, config torStartConfig) (Tunnel, err
} }
instance.StopProcessOnClose = true instance.StopProcessOnClose = true
start := time.Now() start := time.Now()
if err := config.EnableNetwork(ctx, instance, true); err != nil { if err := config.torEnableNetwork(ctx, instance, true); err != nil {
instance.Close() instance.Close()
return nil, err return nil, err
} }
stop := time.Now() stop := time.Now()
// Adapted from <https://git.io/Jfc7N> // Adapted from <https://git.io/Jfc7N>
info, err := config.GetInfo(instance.Control, "net/listeners/socks") info, err := config.torGetInfo(instance.Control, "net/listeners/socks")
if err != nil { if err != nil {
instance.Close() instance.Close()
return nil, err return nil, err
@ -127,12 +108,3 @@ func torStartWithConfig(ctx context.Context, config torStartConfig) (Tunnel, err
func LogFile(sess Session) string { func LogFile(sess Session) string {
return path.Join(sess.TempDir(), "tor.log") return path.Join(sess.TempDir(), "tor.log")
} }
// newTorTunnel creates a new torTunnel
func newTorTunnel(bootstrapTime time.Duration, instance torProcess, proxy *url.URL) *torTunnel {
return &torTunnel{
bootstrapTime: bootstrapTime,
instance: instance,
proxy: proxy,
}
}

View File

@ -23,7 +23,11 @@ func (c *Closer) Close() error {
func TestTorTunnelNonNil(t *testing.T) { func TestTorTunnelNonNil(t *testing.T) {
closer := new(Closer) closer := new(Closer)
proxy := &url.URL{Scheme: "x", Host: "10.0.0.1:443"} proxy := &url.URL{Scheme: "x", Host: "10.0.0.1:443"}
tun := newTorTunnel(128, closer, proxy) tun := &torTunnel{
bootstrapTime: 128,
instance: closer,
proxy: proxy,
}
if tun.BootstrapTime() != 128 { if tun.BootstrapTime() != 128 {
t.Fatal("not the bootstrap time we expected") t.Fatal("not the bootstrap time we expected")
} }
@ -50,7 +54,7 @@ func TestTorTunnelNil(t *testing.T) {
func TestTorStartWithCancelledContext(t *testing.T) { func TestTorStartWithCancelledContext(t *testing.T) {
ctx, cancel := context.WithCancel(context.Background()) ctx, cancel := context.WithCancel(context.Background())
cancel() cancel()
tun, err := torStart(ctx, &mockable.Session{}) tun, err := torStart(ctx, &Config{Session: &mockable.Session{}})
if !errors.Is(err, context.Canceled) { if !errors.Is(err, context.Canceled) {
t.Fatal("not the error we expected") t.Fatal("not the error we expected")
} }
@ -59,12 +63,12 @@ func TestTorStartWithCancelledContext(t *testing.T) {
} }
} }
func TestTorStartWithConfigStartFailure(t *testing.T) { func TestTorStartStartFailure(t *testing.T) {
expected := errors.New("mocked error") expected := errors.New("mocked error")
ctx := context.Background() ctx := context.Background()
tun, err := torStartWithConfig(ctx, torStartConfig{ tun, err := torStart(ctx, &Config{
Sess: &mockable.Session{}, Session: &mockable.Session{},
Start: func(ctx context.Context, conf *tor.StartConf) (*tor.Tor, error) { testTorStart: func(ctx context.Context, conf *tor.StartConf) (*tor.Tor, error) {
return nil, expected return nil, expected
}, },
}) })
@ -76,15 +80,15 @@ func TestTorStartWithConfigStartFailure(t *testing.T) {
} }
} }
func TestTorStartWithConfigEnableNetworkFailure(t *testing.T) { func TestTorStartEnableNetworkFailure(t *testing.T) {
expected := errors.New("mocked error") expected := errors.New("mocked error")
ctx := context.Background() ctx := context.Background()
tun, err := torStartWithConfig(ctx, torStartConfig{ tun, err := torStart(ctx, &Config{
Sess: &mockable.Session{}, Session: &mockable.Session{},
Start: func(ctx context.Context, conf *tor.StartConf) (*tor.Tor, error) { testTorStart: func(ctx context.Context, conf *tor.StartConf) (*tor.Tor, error) {
return &tor.Tor{}, nil return &tor.Tor{}, nil
}, },
EnableNetwork: func(ctx context.Context, tor *tor.Tor, wait bool) error { testTorEnableNetwork: func(ctx context.Context, tor *tor.Tor, wait bool) error {
return expected return expected
}, },
}) })
@ -96,18 +100,18 @@ func TestTorStartWithConfigEnableNetworkFailure(t *testing.T) {
} }
} }
func TestTorStartWithConfigGetInfoFailure(t *testing.T) { func TestTorStartGetInfoFailure(t *testing.T) {
expected := errors.New("mocked error") expected := errors.New("mocked error")
ctx := context.Background() ctx := context.Background()
tun, err := torStartWithConfig(ctx, torStartConfig{ tun, err := torStart(ctx, &Config{
Sess: &mockable.Session{}, Session: &mockable.Session{},
Start: func(ctx context.Context, conf *tor.StartConf) (*tor.Tor, error) { testTorStart: func(ctx context.Context, conf *tor.StartConf) (*tor.Tor, error) {
return &tor.Tor{}, nil return &tor.Tor{}, nil
}, },
EnableNetwork: func(ctx context.Context, tor *tor.Tor, wait bool) error { testTorEnableNetwork: func(ctx context.Context, tor *tor.Tor, wait bool) error {
return nil return nil
}, },
GetInfo: func(ctrl *control.Conn, keys ...string) ([]*control.KeyVal, error) { testTorGetInfo: func(ctrl *control.Conn, keys ...string) ([]*control.KeyVal, error) {
return nil, expected return nil, expected
}, },
}) })
@ -119,17 +123,17 @@ func TestTorStartWithConfigGetInfoFailure(t *testing.T) {
} }
} }
func TestTorStartWithConfigGetInfoInvalidNumberOfKeys(t *testing.T) { func TestTorStartGetInfoInvalidNumberOfKeys(t *testing.T) {
ctx := context.Background() ctx := context.Background()
tun, err := torStartWithConfig(ctx, torStartConfig{ tun, err := torStart(ctx, &Config{
Sess: &mockable.Session{}, Session: &mockable.Session{},
Start: func(ctx context.Context, conf *tor.StartConf) (*tor.Tor, error) { testTorStart: func(ctx context.Context, conf *tor.StartConf) (*tor.Tor, error) {
return &tor.Tor{}, nil return &tor.Tor{}, nil
}, },
EnableNetwork: func(ctx context.Context, tor *tor.Tor, wait bool) error { testTorEnableNetwork: func(ctx context.Context, tor *tor.Tor, wait bool) error {
return nil return nil
}, },
GetInfo: func(ctrl *control.Conn, keys ...string) ([]*control.KeyVal, error) { testTorGetInfo: func(ctrl *control.Conn, keys ...string) ([]*control.KeyVal, error) {
return nil, nil return nil, nil
}, },
}) })
@ -141,17 +145,17 @@ func TestTorStartWithConfigGetInfoInvalidNumberOfKeys(t *testing.T) {
} }
} }
func TestTorStartWithConfigGetInfoInvalidKey(t *testing.T) { func TestTorStartGetInfoInvalidKey(t *testing.T) {
ctx := context.Background() ctx := context.Background()
tun, err := torStartWithConfig(ctx, torStartConfig{ tun, err := torStart(ctx, &Config{
Sess: &mockable.Session{}, Session: &mockable.Session{},
Start: func(ctx context.Context, conf *tor.StartConf) (*tor.Tor, error) { testTorStart: func(ctx context.Context, conf *tor.StartConf) (*tor.Tor, error) {
return &tor.Tor{}, nil return &tor.Tor{}, nil
}, },
EnableNetwork: func(ctx context.Context, tor *tor.Tor, wait bool) error { testTorEnableNetwork: func(ctx context.Context, tor *tor.Tor, wait bool) error {
return nil return nil
}, },
GetInfo: func(ctrl *control.Conn, keys ...string) ([]*control.KeyVal, error) { testTorGetInfo: func(ctrl *control.Conn, keys ...string) ([]*control.KeyVal, error) {
return []*control.KeyVal{{}}, nil return []*control.KeyVal{{}}, nil
}, },
}) })
@ -163,17 +167,17 @@ func TestTorStartWithConfigGetInfoInvalidKey(t *testing.T) {
} }
} }
func TestTorStartWithConfigGetInfoInvalidProxyType(t *testing.T) { func TestTorStartGetInfoInvalidProxyType(t *testing.T) {
ctx := context.Background() ctx := context.Background()
tun, err := torStartWithConfig(ctx, torStartConfig{ tun, err := torStart(ctx, &Config{
Sess: &mockable.Session{}, Session: &mockable.Session{},
Start: func(ctx context.Context, conf *tor.StartConf) (*tor.Tor, error) { testTorStart: func(ctx context.Context, conf *tor.StartConf) (*tor.Tor, error) {
return &tor.Tor{}, nil return &tor.Tor{}, nil
}, },
EnableNetwork: func(ctx context.Context, tor *tor.Tor, wait bool) error { testTorEnableNetwork: func(ctx context.Context, tor *tor.Tor, wait bool) error {
return nil return nil
}, },
GetInfo: func(ctrl *control.Conn, keys ...string) ([]*control.KeyVal, error) { testTorGetInfo: func(ctrl *control.Conn, keys ...string) ([]*control.KeyVal, error) {
return []*control.KeyVal{{Key: "net/listeners/socks", Val: "127.0.0.1:9050"}}, nil return []*control.KeyVal{{Key: "net/listeners/socks", Val: "127.0.0.1:9050"}}, nil
}, },
}) })
@ -185,17 +189,17 @@ func TestTorStartWithConfigGetInfoInvalidProxyType(t *testing.T) {
} }
} }
func TestTorStartWithConfigSuccess(t *testing.T) { func TestTorStartUnsupportedProxy(t *testing.T) {
ctx := context.Background() ctx := context.Background()
tun, err := torStartWithConfig(ctx, torStartConfig{ tun, err := torStart(ctx, &Config{
Sess: &mockable.Session{}, Session: &mockable.Session{},
Start: func(ctx context.Context, conf *tor.StartConf) (*tor.Tor, error) { testTorStart: func(ctx context.Context, conf *tor.StartConf) (*tor.Tor, error) {
return &tor.Tor{}, nil return &tor.Tor{}, nil
}, },
EnableNetwork: func(ctx context.Context, tor *tor.Tor, wait bool) error { testTorEnableNetwork: func(ctx context.Context, tor *tor.Tor, wait bool) error {
return nil return nil
}, },
GetInfo: func(ctrl *control.Conn, keys ...string) ([]*control.KeyVal, error) { testTorGetInfo: func(ctrl *control.Conn, keys ...string) ([]*control.KeyVal, error) {
return []*control.KeyVal{{Key: "net/listeners/socks", Val: "unix:/foo/bar"}}, nil return []*control.KeyVal{{Key: "net/listeners/socks", Val: "unix:/foo/bar"}}, nil
}, },
}) })

View File

@ -41,7 +41,7 @@ func Start(ctx context.Context, config *Config) (Tunnel, error) {
return enforceNilContract(tun, err) return enforceNilContract(tun, err)
case "tor": case "tor":
logger.Infof("starting %s tunnel; please be patient...", config.Name) logger.Infof("starting %s tunnel; please be patient...", config.Name)
tun, err := torStart(ctx, config.Session) tun, err := torStart(ctx, config)
return enforceNilContract(tun, err) return enforceNilContract(tun, err)
default: default:
return nil, errors.New("unsupported tunnel") return nil, errors.New("unsupported tunnel")