From 8fe4e5410d2d1c9aea62afda277e29e5bc79ba6b Mon Sep 17 00:00:00 2001 From: Simone Basso Date: Mon, 5 Apr 2021 11:27:41 +0200 Subject: [PATCH] feat(tunnel): introduce persistent tunnel state dir (#294) * feat(tunnel): introduce persistent tunnel state dir This diff introduces a persistent state directory for tunnels, so that we can bootstrap them more quickly after the first time. Part of https://github.com/ooni/probe/issues/985 * fix: make tunnel dir optional We have many tests where it does not make sense to explicitly provide a tunnel dir because we're not using tunnels. This should simplify setting up a session. * fix(tunnel): repair tests * final changes * more cleanups --- cmd/ooniprobe/internal/ooni/ooni.go | 9 ++++- cmd/ooniprobe/internal/utils/paths.go | 5 +++ internal/cmd/miniooni/libminiooni.go | 5 +++ .../engine/experiment/urlgetter/getter.go | 21 ++++++++++- internal/engine/session.go | 10 +++++ internal/engine/tunnel/config.go | 29 ++++++--------- internal/engine/tunnel/integration_test.go | 20 ++++++---- internal/engine/tunnel/psiphon.go | 28 +++++--------- internal/engine/tunnel/psiphon_test.go | 28 +++----------- internal/engine/tunnel/testdata/.gitignore | 1 + internal/engine/tunnel/tor.go | 25 +++++++------ internal/engine/tunnel/tor_test.go | 37 +++++++++++++------ internal/engine/tunnel/tunnel.go | 21 ++++++++++- internal/engine/tunnel/tunnel_test.go | 20 +++++----- pkg/oonimkall/internal/tasks/runner.go | 1 + pkg/oonimkall/internal/tasks/settings.go | 5 +++ pkg/oonimkall/session.go | 6 +++ 17 files changed, 166 insertions(+), 105 deletions(-) create mode 100644 internal/engine/tunnel/testdata/.gitignore diff --git a/cmd/ooniprobe/internal/ooni/ooni.go b/cmd/ooniprobe/internal/ooni/ooni.go index 7dfb079..61f354d 100644 --- a/cmd/ooniprobe/internal/ooni/ooni.go +++ b/cmd/ooniprobe/internal/ooni/ooni.go @@ -45,8 +45,9 @@ type Probe struct { db sqlbuilder.Database isBatch bool - home string - tempDir string + home string + tempDir string + tunnelDir string dbPath string configPath string @@ -207,12 +208,16 @@ func (p *Probe) NewSession() (*engine.Session, error) { if err != nil { return nil, errors.Wrap(err, "creating engine's kvstore") } + if err := os.MkdirAll(utils.TunnelDir(p.home), 0700); err != nil { + return nil, errors.Wrap(err, "creating tunnel dir") + } return engine.NewSession(engine.SessionConfig{ KVStore: kvstore, Logger: enginex.Logger, SoftwareName: p.softwareName, SoftwareVersion: p.softwareVersion, TempDir: p.tempDir, + TunnelDir: p.tunnelDir, }) } diff --git a/cmd/ooniprobe/internal/utils/paths.go b/cmd/ooniprobe/internal/utils/paths.go index 7d7db8e..a9aea3f 100644 --- a/cmd/ooniprobe/internal/utils/paths.go +++ b/cmd/ooniprobe/internal/utils/paths.go @@ -30,6 +30,11 @@ func AssetsDir(home string) string { return filepath.Join(home, "assets") } +// TunnelDir returns the directory where to store tunnels state +func TunnelDir(home string) string { + return filepath.Join(home, "tunnel") +} + // EngineDir returns the directory where ooni/probe-engine should // store its private data given a specific OONI Home. func EngineDir(home string) string { diff --git a/internal/cmd/miniooni/libminiooni.go b/internal/cmd/miniooni/libminiooni.go index 25b3a65..70d734e 100644 --- a/internal/cmd/miniooni/libminiooni.go +++ b/internal/cmd/miniooni/libminiooni.go @@ -333,6 +333,10 @@ func MainWithConfiguration(experimentName string, currentOptions Options) { kvstore, err := engine.NewFileSystemKVStore(kvstore2dir) fatalOnError(err, "cannot create kvstore2 directory") + tunnelDir := filepath.Join(miniooniDir, "tunnel") + err = os.MkdirAll(tunnelDir, 0700) + fatalOnError(err, "cannot create tunnelDir") + config := engine.SessionConfig{ KVStore: kvstore, Logger: logger, @@ -341,6 +345,7 @@ func MainWithConfiguration(experimentName string, currentOptions Options) { SoftwareVersion: softwareVersion, TorArgs: currentOptions.TorArgs, TorBinary: currentOptions.TorBinary, + TunnelDir: tunnelDir, } if currentOptions.ProbeServicesURL != "" { config.AvailableProbeServices = []model.Service{{ diff --git a/internal/engine/experiment/urlgetter/getter.go b/internal/engine/experiment/urlgetter/getter.go index de373a5..12b5855 100644 --- a/internal/engine/experiment/urlgetter/getter.go +++ b/internal/engine/experiment/urlgetter/getter.go @@ -2,8 +2,10 @@ package urlgetter import ( "context" + "fmt" "net/url" "path/filepath" + "sync" "time" "github.com/ooni/probe-cli/v3/internal/engine/model" @@ -80,6 +82,15 @@ func (g Getter) Get(ctx context.Context) (TestKeys, error) { return tk, err } +var ( + // tunnelDirCount counts the number of tunnels started by + // the urlgetter package so far. + tunnelDirCount int64 + + // tunnelDirMu protects tunnelDirCount + tunnelDirMu sync.Mutex +) + func (g Getter) get(ctx context.Context, saver *trace.Saver) (TestKeys, error) { tk := TestKeys{ Agent: "redirect", @@ -94,12 +105,20 @@ func (g Getter) get(ctx context.Context, saver *trace.Saver) (TestKeys, error) { // start tunnel var proxyURL *url.URL if g.Config.Tunnel != "" { + // Every new instance of the tunnel goes into a separate + // directory within the temporary directory. Calling + // Session.Close will delete such a directory. + tunnelDirMu.Lock() + count := tunnelDirCount + tunnelDirCount++ + tunnelDirMu.Unlock() tun, err := tunnel.Start(ctx, &tunnel.Config{ Name: g.Config.Tunnel, Session: g.Session, TorArgs: g.Session.TorArgs(), TorBinary: g.Session.TorBinary(), - WorkDir: filepath.Join(g.Session.TempDir(), "urlgetter-tunnel"), + TunnelDir: filepath.Join( + g.Session.TempDir(), fmt.Sprintf("urlgetter-tunnel-%d", count)), }) if err != nil { return tk, err diff --git a/internal/engine/session.go b/internal/engine/session.go index 12d8275..7f6391e 100644 --- a/internal/engine/session.go +++ b/internal/engine/session.go @@ -34,6 +34,13 @@ type SessionConfig struct { TempDir string TorArgs []string TorBinary string + + // TunnelDir is the directory where we should store + // the state of persistent tunnels. This field is + // optional _unless_ you want to use tunnels. In such + // case, starting a tunnel will fail because there + // is no directory where to store state. + TunnelDir string } // Session is a measurement session. It contains shared information @@ -58,6 +65,7 @@ type Session struct { tempDir string torArgs []string torBinary string + tunnelDir string tunnelMu sync.Mutex tunnelName string tunnel tunnel.Tunnel @@ -126,6 +134,7 @@ func NewSession(config SessionConfig) (*Session, error) { tempDir: tempDir, torArgs: config.TorArgs, torBinary: config.TorBinary, + tunnelDir: config.TunnelDir, } httpConfig := netx.Config{ ByteCounter: sess.byteCounter, @@ -363,6 +372,7 @@ func (s *Session) MaybeStartTunnel(ctx context.Context, name string) error { Session: s, TorArgs: s.TorArgs(), TorBinary: s.TorBinary(), + TunnelDir: s.tunnelDir, }) if err != nil { s.logger.Warnf("cannot start tunnel: %+v", err) diff --git a/internal/engine/tunnel/config.go b/internal/engine/tunnel/config.go index d40e73d..5518c2f 100644 --- a/internal/engine/tunnel/config.go +++ b/internal/engine/tunnel/config.go @@ -9,35 +9,36 @@ import ( "github.com/ooni/psiphon/oopsi/github.com/Psiphon-Labs/psiphon-tunnel-core/ClientLibrary/clientlib" ) -// Config contains the configuration for creating a Tunnel instance. +// Config contains the configuration for creating a Tunnel instance. You need +// to fill 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 { // Name is the mandatory name of the tunnel. We support // "tor" and "psiphon" tunnels. Name string - // Session is the current measurement session. + // Session is the current measurement session. This + // field is mandatory. Session Session - // TorArgs contains the arguments that you want us to pass + // TorArgs contains the optional arguments that you want us to pass // to the tor binary when invoking it. By default we do not // pass any extra argument. This flag might be useful to // configure pluggable transports. TorArgs []string - // TorBinary is the path of the TorBinary we SHOULD be + // TorBinary is the optional path of the TorBinary we SHOULD be // executing. When not set, we execute `tor`. TorBinary string - // WorkDir is the directory in which the tunnel SHOULD - // store its state, if any. - WorkDir string + // TunnelDir is the mandatory directory in which the tunnel SHOULD + // store its state, if any. If this field is empty, the + // Start function fails with ErrEmptyTunnelDir. + TunnelDir string // testMkdirAll allows us to mock os.MkdirAll in testing code. testMkdirAll func(path string, perm os.FileMode) error - // testRemoveAll allows us to mock os.RemoveAll in testing code. - testRemoveAll func(path string) error - // testStartPsiphon allows us to mock psiphon's clientlib.StartTunnel. testStartPsiphon func(ctx context.Context, config []byte, workdir string) (*clientlib.PsiphonTunnel, error) @@ -62,14 +63,6 @@ func (c *Config) mkdirAll(path string, perm os.FileMode) error { return os.MkdirAll(path, perm) } -// removeAll calls either testRemoveAll or os.RemoveAll. -func (c *Config) removeAll(path string) error { - if c.testRemoveAll != nil { - return c.testRemoveAll(path) - } - return os.RemoveAll(path) -} - // startPsiphon calls either testStartPsiphon or psiphon's clientlib.StartTunnel. func (c *Config) startPsiphon(ctx context.Context, config []byte, workdir string) (*clientlib.PsiphonTunnel, error) { diff --git a/internal/engine/tunnel/integration_test.go b/internal/engine/tunnel/integration_test.go index 61e54b3..b0de4ec 100644 --- a/internal/engine/tunnel/integration_test.go +++ b/internal/engine/tunnel/integration_test.go @@ -11,19 +11,23 @@ import ( ) func TestPsiphonStartWithCancelledContext(t *testing.T) { + // TODO(bassosimone): this test can use a mockable session so we + // can move it inside of the internal tests. ctx, cancel := context.WithCancel(context.Background()) - cancel() + cancel() // fail immediately sess, err := engine.NewSession(engine.SessionConfig{ Logger: log.Log, - SoftwareName: "ooniprobe-engine", - SoftwareVersion: "0.0.1", + SoftwareName: "miniooni", + SoftwareVersion: "0.1.0-dev", + TunnelDir: "testdata", }) if err != nil { t.Fatal(err) } tunnel, err := tunnel.Start(ctx, &tunnel.Config{ - Name: "psiphon", - Session: sess, + Name: "psiphon", + Session: sess, + TunnelDir: "testdata", }) if !errors.Is(err, context.Canceled) { t.Fatal("not the error we expected") @@ -41,13 +45,15 @@ func TestPsiphonStartStop(t *testing.T) { Logger: log.Log, SoftwareName: "ooniprobe-engine", SoftwareVersion: "0.0.1", + TunnelDir: "testdata", }) if err != nil { t.Fatal(err) } tunnel, err := tunnel.Start(context.Background(), &tunnel.Config{ - Name: "psiphon", - Session: sess, + Name: "psiphon", + Session: sess, + TunnelDir: "testdata", }) if err != nil { t.Fatal(err) diff --git a/internal/engine/tunnel/psiphon.go b/internal/engine/tunnel/psiphon.go index d05bd09..8a087ad 100644 --- a/internal/engine/tunnel/psiphon.go +++ b/internal/engine/tunnel/psiphon.go @@ -13,29 +13,16 @@ import ( // psiphonTunnel is a psiphon tunnel type psiphonTunnel struct { + // bootstrapTime is the bootstrapTime of the bootstrap + bootstrapTime time.Duration + // tunnel is the underlying psiphon tunnel tunnel *clientlib.PsiphonTunnel - - // duration is the duration of the bootstrap - duration time.Duration } -// TODO(bassosimone): _always_ wiping the state directory -// here is absolutely wrong. This prevents us from reusing -// an existing psiphon cache existing on disk. We want to -// delete the directory _only_ in the psiphon nettest. - // psiphonMakeWorkingDir creates the working directory func psiphonMakeWorkingDir(config *Config) (string, error) { - const testdirname = "oonipsiphon" - baseWorkDir := config.WorkDir - if baseWorkDir == "" { - baseWorkDir = config.Session.TempDir() - } - workdir := filepath.Join(baseWorkDir, testdirname) - if err := config.removeAll(workdir); err != nil { - return "", err - } + workdir := filepath.Join(config.TunnelDir, config.Name) if err := config.mkdirAll(workdir, 0700); err != nil { return "", err } @@ -49,6 +36,9 @@ func psiphonStart(ctx context.Context, config *Config) (Tunnel, error) { return nil, ctx.Err() // simplifies unit testing this code default: } + if config.TunnelDir == "" { + return nil, ErrEmptyTunnelDir + } configJSON, err := config.Session.FetchPsiphonConfig(ctx) if err != nil { return nil, err @@ -63,7 +53,7 @@ func psiphonStart(ctx context.Context, config *Config) (Tunnel, error) { return nil, err } stop := time.Now() - return &psiphonTunnel{tunnel: tunnel, duration: stop.Sub(start)}, nil + return &psiphonTunnel{tunnel: tunnel, bootstrapTime: stop.Sub(start)}, nil } // TODO(bassosimone): define the NullTunnel rather than relying on @@ -91,7 +81,7 @@ func (t *psiphonTunnel) SOCKS5ProxyURL() (proxyURL *url.URL) { // BootstrapTime returns the bootstrap time func (t *psiphonTunnel) BootstrapTime() (duration time.Duration) { if t != nil { - duration = t.duration + duration = t.bootstrapTime } return } diff --git a/internal/engine/tunnel/psiphon_test.go b/internal/engine/tunnel/psiphon_test.go index 74ccba9..d2e7a7f 100644 --- a/internal/engine/tunnel/psiphon_test.go +++ b/internal/engine/tunnel/psiphon_test.go @@ -16,7 +16,8 @@ func TestPsiphonFetchPsiphonConfigFailure(t *testing.T) { MockableFetchPsiphonConfigErr: expected, } tunnel, err := psiphonStart(context.Background(), &Config{ - Session: sess, + Session: sess, + TunnelDir: "testdata", }) if !errors.Is(err, expected) { t.Fatal("not the error we expected") @@ -32,7 +33,8 @@ func TestPsiphonMkdirAllFailure(t *testing.T) { MockableFetchPsiphonConfigResult: []byte(`{}`), } tunnel, err := psiphonStart(context.Background(), &Config{ - Session: sess, + Session: sess, + TunnelDir: "testdata", testMkdirAll: func(path string, perm os.FileMode) error { return expected }, @@ -45,32 +47,14 @@ func TestPsiphonMkdirAllFailure(t *testing.T) { } } -func TestPsiphonRemoveAllFailure(t *testing.T) { - expected := errors.New("mocked error") - sess := &mockable.Session{ - MockableFetchPsiphonConfigResult: []byte(`{}`), - } - tunnel, err := psiphonStart(context.Background(), &Config{ - Session: sess, - testRemoveAll: func(path string) error { - return expected - }, - }) - if !errors.Is(err, expected) { - t.Fatal("not the error we expected") - } - if tunnel != nil { - t.Fatal("expected nil tunnel here") - } -} - func TestPsiphonStartFailure(t *testing.T) { expected := errors.New("mocked error") sess := &mockable.Session{ MockableFetchPsiphonConfigResult: []byte(`{}`), } tunnel, err := psiphonStart(context.Background(), &Config{ - Session: sess, + Session: sess, + TunnelDir: "testdata", testStartPsiphon: func(ctx context.Context, config []byte, workdir string) (*clientlib.PsiphonTunnel, error) { return nil, expected diff --git a/internal/engine/tunnel/testdata/.gitignore b/internal/engine/tunnel/testdata/.gitignore new file mode 100644 index 0000000..72e8ffc --- /dev/null +++ b/internal/engine/tunnel/testdata/.gitignore @@ -0,0 +1 @@ +* diff --git a/internal/engine/tunnel/tor.go b/internal/engine/tunnel/tor.go index c5f3361..992eb13 100644 --- a/internal/engine/tunnel/tor.go +++ b/internal/engine/tunnel/tor.go @@ -3,18 +3,18 @@ package tunnel import ( "context" "fmt" + "io" "net/url" - "path" + "path/filepath" "strings" "time" "github.com/cretz/bine/tor" ) -// torProcess is a running tor process +// torProcess is a running tor process. type torProcess interface { - // Close kills the running tor process - Close() error + io.Closer } // torTunnel is the Tor tunnel @@ -52,6 +52,9 @@ func (tt *torTunnel) Stop() { } } +// TODO(bassosimone): the current design is such that we have a bunch of +// torrc-$number and a growing tor.log file inside of stateDir. + // torStart starts the tor tunnel. func torStart(ctx context.Context, config *Config) (Tunnel, error) { select { @@ -59,14 +62,18 @@ func torStart(ctx context.Context, config *Config) (Tunnel, error) { return nil, ctx.Err() // allows to write unit tests using this code default: } - logfile := LogFile(config.Session) + if config.TunnelDir == "" { + return nil, ErrEmptyTunnelDir + } + stateDir := filepath.Join(config.TunnelDir, "tor") + logfile := filepath.Join(stateDir, "tor.log") extraArgs := append([]string{}, config.TorArgs...) extraArgs = append(extraArgs, "Log") extraArgs = append(extraArgs, "notice stderr") extraArgs = append(extraArgs, "Log") extraArgs = append(extraArgs, fmt.Sprintf(`notice file %s`, logfile)) instance, err := config.torStart(ctx, &tor.StartConf{ - DataDir: path.Join(config.Session.TempDir(), "tor"), + DataDir: stateDir, ExtraArgs: extraArgs, ExePath: config.TorBinary, NoHush: true, @@ -102,9 +109,3 @@ func torStart(ctx context.Context, config *Config) (Tunnel, error) { proxy: &url.URL{Scheme: "socks5", Host: proxyAddress}, }, nil } - -// LogFile returns the name of tor logs given a specific session. The file -// is always located somewhere inside the sess.TempDir() directory. -func LogFile(sess Session) string { - return path.Join(sess.TempDir(), "tor.log") -} diff --git a/internal/engine/tunnel/tor_test.go b/internal/engine/tunnel/tor_test.go index c0b9fda..3f59fb0 100644 --- a/internal/engine/tunnel/tor_test.go +++ b/internal/engine/tunnel/tor_test.go @@ -11,17 +11,20 @@ import ( "github.com/ooni/probe-cli/v3/internal/engine/internal/mockable" ) -type Closer struct { +// torCloser is used to mock a running tor process, which +// we abstract as a io.Closer in tor.go. +type torCloser struct { counter int } -func (c *Closer) Close() error { +// Close implements io.Closer.Close. +func (c *torCloser) Close() error { c.counter++ return errors.New("mocked mocked mocked") } func TestTorTunnelNonNil(t *testing.T) { - closer := new(Closer) + closer := new(torCloser) proxy := &url.URL{Scheme: "x", Host: "10.0.0.1:443"} tun := &torTunnel{ bootstrapTime: 128, @@ -53,8 +56,11 @@ func TestTorTunnelNil(t *testing.T) { func TestTorStartWithCancelledContext(t *testing.T) { ctx, cancel := context.WithCancel(context.Background()) - cancel() - tun, err := torStart(ctx, &Config{Session: &mockable.Session{}}) + cancel() // fail immediately + tun, err := torStart(ctx, &Config{ + Session: &mockable.Session{}, + TunnelDir: "testdata", + }) if !errors.Is(err, context.Canceled) { t.Fatal("not the error we expected") } @@ -67,7 +73,8 @@ func TestTorStartStartFailure(t *testing.T) { expected := errors.New("mocked error") ctx := context.Background() tun, err := torStart(ctx, &Config{ - Session: &mockable.Session{}, + Session: &mockable.Session{}, + TunnelDir: "testdata", testTorStart: func(ctx context.Context, conf *tor.StartConf) (*tor.Tor, error) { return nil, expected }, @@ -84,7 +91,8 @@ func TestTorStartEnableNetworkFailure(t *testing.T) { expected := errors.New("mocked error") ctx := context.Background() tun, err := torStart(ctx, &Config{ - Session: &mockable.Session{}, + Session: &mockable.Session{}, + TunnelDir: "testdata", testTorStart: func(ctx context.Context, conf *tor.StartConf) (*tor.Tor, error) { return &tor.Tor{}, nil }, @@ -104,7 +112,8 @@ func TestTorStartGetInfoFailure(t *testing.T) { expected := errors.New("mocked error") ctx := context.Background() tun, err := torStart(ctx, &Config{ - Session: &mockable.Session{}, + Session: &mockable.Session{}, + TunnelDir: "testdata", testTorStart: func(ctx context.Context, conf *tor.StartConf) (*tor.Tor, error) { return &tor.Tor{}, nil }, @@ -126,7 +135,8 @@ func TestTorStartGetInfoFailure(t *testing.T) { func TestTorStartGetInfoInvalidNumberOfKeys(t *testing.T) { ctx := context.Background() tun, err := torStart(ctx, &Config{ - Session: &mockable.Session{}, + Session: &mockable.Session{}, + TunnelDir: "testdata", testTorStart: func(ctx context.Context, conf *tor.StartConf) (*tor.Tor, error) { return &tor.Tor{}, nil }, @@ -148,7 +158,8 @@ func TestTorStartGetInfoInvalidNumberOfKeys(t *testing.T) { func TestTorStartGetInfoInvalidKey(t *testing.T) { ctx := context.Background() tun, err := torStart(ctx, &Config{ - Session: &mockable.Session{}, + Session: &mockable.Session{}, + TunnelDir: "testdata", testTorStart: func(ctx context.Context, conf *tor.StartConf) (*tor.Tor, error) { return &tor.Tor{}, nil }, @@ -170,7 +181,8 @@ func TestTorStartGetInfoInvalidKey(t *testing.T) { func TestTorStartGetInfoInvalidProxyType(t *testing.T) { ctx := context.Background() tun, err := torStart(ctx, &Config{ - Session: &mockable.Session{}, + Session: &mockable.Session{}, + TunnelDir: "testdata", testTorStart: func(ctx context.Context, conf *tor.StartConf) (*tor.Tor, error) { return &tor.Tor{}, nil }, @@ -192,7 +204,8 @@ func TestTorStartGetInfoInvalidProxyType(t *testing.T) { func TestTorStartUnsupportedProxy(t *testing.T) { ctx := context.Background() tun, err := torStart(ctx, &Config{ - Session: &mockable.Session{}, + Session: &mockable.Session{}, + TunnelDir: "testdata", testTorStart: func(ctx context.Context, conf *tor.StartConf) (*tor.Tor, error) { return &tor.Tor{}, nil }, diff --git a/internal/engine/tunnel/tunnel.go b/internal/engine/tunnel/tunnel.go index f56ef51..e1b2e38 100644 --- a/internal/engine/tunnel/tunnel.go +++ b/internal/engine/tunnel/tunnel.go @@ -5,6 +5,7 @@ package tunnel import ( "context" "errors" + "fmt" "net/url" "time" ) @@ -12,16 +13,28 @@ import ( // Session is the way in which this package sees a Session. type Session interface { FetchPsiphonConfig(ctx context.Context) ([]byte, error) - TempDir() string } // Tunnel is a tunnel used by the session type Tunnel interface { + // BootstrapTime returns the time it required to + // create an instance of the tunnel BootstrapTime() time.Duration + + // SOCKS5ProxyURL returns the SOCSK5 proxy URL SOCKS5ProxyURL() *url.URL + + // Stop stops the tunnel. This method is idempotent. Stop() } +// ErrEmptyTunnelDir indicates that config.TunnelDir is empty. +var ErrEmptyTunnelDir = errors.New("TunnelDir is empty") + +// ErrUnsupportedTunnelName indicates that the given tunnel name +// is not supported by this package. +var ErrUnsupportedTunnelName = errors.New("unsupported tunnel name") + // Start starts a new tunnel by name or returns an error. Note that if you // pass to this function the "" tunnel, you get back nil, nil. func Start(ctx context.Context, config *Config) (Tunnel, error) { @@ -35,11 +48,15 @@ func Start(ctx context.Context, config *Config) (Tunnel, error) { tun, err := torStart(ctx, config) return enforceNilContract(tun, err) default: - return nil, errors.New("unsupported tunnel") + return nil, fmt.Errorf("%w: %s", ErrUnsupportedTunnelName, config.Name) } } +// enforceNilContract ensures that either the tunnel is nil +// or the error is nil. func enforceNilContract(tun Tunnel, err error) (Tunnel, error) { + // TODO(bassosimone): we currently allow returning nil, nil but + // we want to change this to return a fake NilTunnel. if err != nil { return nil, err } diff --git a/internal/engine/tunnel/tunnel_test.go b/internal/engine/tunnel/tunnel_test.go index b46d9c9..c11d2f9 100644 --- a/internal/engine/tunnel/tunnel_test.go +++ b/internal/engine/tunnel/tunnel_test.go @@ -10,8 +10,7 @@ import ( ) func TestStartNoTunnel(t *testing.T) { - ctx, cancel := context.WithCancel(context.Background()) - cancel() + ctx := context.Background() tunnel, err := Start(ctx, &Config{ Name: "", Session: &mockable.Session{ @@ -26,14 +25,15 @@ func TestStartNoTunnel(t *testing.T) { } } -func TestStartPsiphonTunnel(t *testing.T) { +func TestStartPsiphonWithCancelledContext(t *testing.T) { ctx, cancel := context.WithCancel(context.Background()) - cancel() + cancel() // fail immediately tunnel, err := Start(ctx, &Config{ Name: "psiphon", Session: &mockable.Session{ MockableLogger: log.Log, }, + TunnelDir: "testdata", }) if !errors.Is(err, context.Canceled) { t.Fatal("not the error we expected") @@ -43,14 +43,15 @@ func TestStartPsiphonTunnel(t *testing.T) { } } -func TestStartTorTunnel(t *testing.T) { +func TestStartTorWithCancelledContext(t *testing.T) { ctx, cancel := context.WithCancel(context.Background()) - cancel() + cancel() // fail immediately tunnel, err := Start(ctx, &Config{ Name: "tor", Session: &mockable.Session{ MockableLogger: log.Log, }, + TunnelDir: "testdata", }) if !errors.Is(err, context.Canceled) { t.Fatal("not the error we expected") @@ -61,18 +62,17 @@ func TestStartTorTunnel(t *testing.T) { } func TestStartInvalidTunnel(t *testing.T) { - ctx, cancel := context.WithCancel(context.Background()) - cancel() + ctx := context.Background() tunnel, err := Start(ctx, &Config{ Name: "antani", Session: &mockable.Session{ MockableLogger: log.Log, }, + TunnelDir: "testdata", }) - if err == nil || err.Error() != "unsupported tunnel" { + if !errors.Is(err, ErrUnsupportedTunnelName) { t.Fatal("not the error we expected") } - t.Log(tunnel) if tunnel != nil { t.Fatal("expected nil tunnel here") } diff --git a/pkg/oonimkall/internal/tasks/runner.go b/pkg/oonimkall/internal/tasks/runner.go index ccdc9a9..afeacab 100644 --- a/pkg/oonimkall/internal/tasks/runner.go +++ b/pkg/oonimkall/internal/tasks/runner.go @@ -80,6 +80,7 @@ func (r *Runner) newsession(logger *ChanLogger) (*engine.Session, error) { SoftwareName: r.settings.Options.SoftwareName, SoftwareVersion: r.settings.Options.SoftwareVersion, TempDir: r.settings.TempDir, + TunnelDir: r.settings.TunnelDir, } if r.settings.Options.ProbeServicesBaseURL != "" { config.AvailableProbeServices = []model.Service{{ diff --git a/pkg/oonimkall/internal/tasks/settings.go b/pkg/oonimkall/internal/tasks/settings.go index 5a90553..1093637 100644 --- a/pkg/oonimkall/internal/tasks/settings.go +++ b/pkg/oonimkall/internal/tasks/settings.go @@ -44,6 +44,11 @@ type Settings struct { // for iOS and does not work for Android. TempDir string `json:"temp_dir"` + // TunnelDir is the directory where to store persistent state + // related to circumvention tunnels. This directory is required + // only if you want to use the tunnels. Added since 3.10.0. + TunnelDir string `json:"tunnel_dir"` + // Version indicates the version of this structure. Version int64 `json:"version"` } diff --git a/pkg/oonimkall/session.go b/pkg/oonimkall/session.go index 537339b..3b73b21 100644 --- a/pkg/oonimkall/session.go +++ b/pkg/oonimkall/session.go @@ -87,6 +87,11 @@ type SessionConfig struct { // remove any temporary file created within this Session. TempDir string + // TunnelDir is the directory where the Session shall store + // persistent data regarding circumvention tunnels. This directory + // is mandatory if you want to use tunnels. + TunnelDir string + // Verbose is optional. If there is a non-null Logger and this // field is true, then the Logger will also receive Debug messages, // otherwise it will not receive such messages. @@ -143,6 +148,7 @@ func NewSession(config *SessionConfig) (*Session, error) { SoftwareName: config.SoftwareName, SoftwareVersion: config.SoftwareVersion, TempDir: config.TempDir, + TunnelDir: config.TunnelDir, } sessp, err := engine.NewSession(engineConfig) if err != nil {