diff --git a/internal/engine/tunnel/config.go b/internal/engine/tunnel/config.go index 6978182..2e3003a 100644 --- a/internal/engine/tunnel/config.go +++ b/internal/engine/tunnel/config.go @@ -10,14 +10,16 @@ import ( ) // Config contains the configuration for creating a Tunnel instance. You need -// to fill the mandatory fields. You SHOULD NOT modify the content of this +// 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 { // Name is the mandatory name of the tunnel. We support // "tor" and "psiphon" tunnels. Name string - // Session is the mandatory measurement session. + // Session is the mandatory measurement session, or a suitable + // mock of the required functionality. That is, the possibility + // of obtaining a valid psiphon configuration. Session Session // TorArgs contains the optional arguments that you want us to pass diff --git a/internal/engine/tunnel/psiphon.go b/internal/engine/tunnel/psiphon.go index 8a087ad..bafcf00 100644 --- a/internal/engine/tunnel/psiphon.go +++ b/internal/engine/tunnel/psiphon.go @@ -56,32 +56,21 @@ func psiphonStart(ctx context.Context, config *Config) (Tunnel, error) { return &psiphonTunnel{tunnel: tunnel, bootstrapTime: stop.Sub(start)}, nil } -// TODO(bassosimone): define the NullTunnel rather than relying on -// this magic that a nil psiphonTunnel works. - // Stop is an idempotent method that shuts down the tunnel func (t *psiphonTunnel) Stop() { - if t != nil { - t.tunnel.Stop() - } + t.tunnel.Stop() } // SOCKS5ProxyURL returns the SOCKS5 proxy URL. -func (t *psiphonTunnel) SOCKS5ProxyURL() (proxyURL *url.URL) { - if t != nil { - proxyURL = &url.URL{ - Scheme: "socks5", - Host: net.JoinHostPort( - "127.0.0.1", fmt.Sprintf("%d", t.tunnel.SOCKSProxyPort)), - } +func (t *psiphonTunnel) SOCKS5ProxyURL() *url.URL { + return &url.URL{ + Scheme: "socks5", + Host: net.JoinHostPort( + "127.0.0.1", fmt.Sprintf("%d", t.tunnel.SOCKSProxyPort)), } - return } // BootstrapTime returns the bootstrap time -func (t *psiphonTunnel) BootstrapTime() (duration time.Duration) { - if t != nil { - duration = t.bootstrapTime - } - return +func (t *psiphonTunnel) BootstrapTime() time.Duration { + return t.bootstrapTime } diff --git a/internal/engine/tunnel/psiphon_test.go b/internal/engine/tunnel/psiphon_test.go index d2e7a7f..cd9fa95 100644 --- a/internal/engine/tunnel/psiphon_test.go +++ b/internal/engine/tunnel/psiphon_test.go @@ -67,14 +67,3 @@ func TestPsiphonStartFailure(t *testing.T) { t.Fatal("expected nil tunnel here") } } - -func TestPsiphonNilTunnel(t *testing.T) { - var tunnel *psiphonTunnel - if tunnel.BootstrapTime() != 0 { - t.Fatal("expected zero bootstrap time") - } - if tunnel.SOCKS5ProxyURL() != nil { - t.Fatal("expected nil SOCKS Proxy URL") - } - tunnel.Stop() // must not crash -} diff --git a/internal/engine/tunnel/tor.go b/internal/engine/tunnel/tor.go index 992eb13..d28b5b7 100644 --- a/internal/engine/tunnel/tor.go +++ b/internal/engine/tunnel/tor.go @@ -30,26 +30,18 @@ type torTunnel struct { } // BootstrapTime returns the bootstrap time -func (tt *torTunnel) BootstrapTime() (duration time.Duration) { - if tt != nil { - duration = tt.bootstrapTime - } - return +func (tt *torTunnel) BootstrapTime() time.Duration { + return tt.bootstrapTime } // SOCKS5ProxyURL returns the URL of the SOCKS5 proxy -func (tt *torTunnel) SOCKS5ProxyURL() (url *url.URL) { - if tt != nil { - url = tt.proxy - } - return +func (tt *torTunnel) SOCKS5ProxyURL() *url.URL { + return tt.proxy } // Stop stops the Tor tunnel func (tt *torTunnel) Stop() { - if tt != nil { - tt.instance.Close() - } + tt.instance.Close() } // TODO(bassosimone): the current design is such that we have a bunch of diff --git a/internal/engine/tunnel/tor_test.go b/internal/engine/tunnel/tor_test.go index 3f59fb0..1496b40 100644 --- a/internal/engine/tunnel/tor_test.go +++ b/internal/engine/tunnel/tor_test.go @@ -43,17 +43,6 @@ func TestTorTunnelNonNil(t *testing.T) { } } -func TestTorTunnelNil(t *testing.T) { - var tun *torTunnel - if tun.BootstrapTime() != 0 { - t.Fatal("not the bootstrap time we expected") - } - if tun.SOCKS5ProxyURL() != nil { - t.Fatal("not the url we expected") - } - tun.Stop() // ensure we don't crash -} - func TestTorStartWithCancelledContext(t *testing.T) { ctx, cancel := context.WithCancel(context.Background()) cancel() // fail immediately diff --git a/internal/engine/tunnel/tunnel.go b/internal/engine/tunnel/tunnel.go index e1b2e38..0238334 100644 --- a/internal/engine/tunnel/tunnel.go +++ b/internal/engine/tunnel/tunnel.go @@ -1,5 +1,26 @@ // Package tunnel allows to create tunnels to speak // with OONI backends and other services. +// +// You need to fill a Config object and call Start to +// obtain an instance of Tunnel. The tunnel will expose +// a SOCKS5 proxy. You need to configure your HTTP +// code to use such a proxy. Remember to call the Stop +// method of a tunnel when you are done. +// +// There are two use cases for this package. The first +// use case is to enable urlgetter to perform measurements +// over tunnels (mainly psiphon). +// +// The second use case is to use tunnels to reach to the +// OONI backend when it's blocked. For the latter case +// we currently mainly use psiphon. In such a case, we'll +// use a psiphon configuration embedded into the OONI +// binary itself. When you are running a version of OONI +// that does not embed such a configuration, it won't +// be possible to address this use case. +// +// See session.go in the engine package for more details +// concerning this second use case. package tunnel import ( @@ -10,21 +31,27 @@ import ( "time" ) -// Session is the way in which this package sees a Session. +// Session is a measurement session. We filter for the only +// functionality we're interested to use. That is, fetching the +// psiphon configuration from the OONI backend (if possible). type Session interface { + // FetchPsiphonConfig should fetch and return the psiphon config + // as a serialized JSON, or fail with an error. FetchPsiphonConfig(ctx context.Context) ([]byte, error) } -// Tunnel is a tunnel used by the session +// Tunnel is a tunnel for communicating with OONI backends +// (and other services) to circumvent blocking. type Tunnel interface { // BootstrapTime returns the time it required to - // create an instance of the tunnel + // create a new tunnel instance. BootstrapTime() time.Duration - // SOCKS5ProxyURL returns the SOCSK5 proxy URL + // SOCKS5ProxyURL returns the SOCSK5 proxy URL. SOCKS5ProxyURL() *url.URL - // Stop stops the tunnel. This method is idempotent. + // Stop stops the tunnel. You should not attempt to + // use any other tunnel method after Stop. Stop() } @@ -35,30 +62,29 @@ var ErrEmptyTunnelDir = errors.New("TunnelDir is empty") // 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. +// Start starts a new tunnel by name or returns an error. We currently +// support the following tunnels: +// +// The "tor" tunnel requires the "tor" binary to be installed on +// your system. You can use config.TorArgs and config.TorBinary to +// select what binary to execute and with which arguments. +// +// The "psiphon" tunnel requires a configuration. Some builds of +// ooniprobe embed a configuration into the binary. When this +// is the case, the config.Session is a mocked object that just +// retuns such configuration. +// +// Otherwise, If there is no embedded psiphon configuration, the +// config.Session will must be an ordinary session. In such a +// case, fetching the Psiphon configuration from the backend may +// fail when the backend is not reachable. func Start(ctx context.Context, config *Config) (Tunnel, error) { switch config.Name { - case "": - return enforceNilContract(nil, nil) case "psiphon": - tun, err := psiphonStart(ctx, config) - return enforceNilContract(tun, err) + return psiphonStart(ctx, config) case "tor": - tun, err := torStart(ctx, config) - return enforceNilContract(tun, err) + return torStart(ctx, config) default: 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 - } - return tun, nil -} diff --git a/internal/engine/tunnel/tunnel_test.go b/internal/engine/tunnel/tunnel_test.go index c11d2f9..ad2b018 100644 --- a/internal/engine/tunnel/tunnel_test.go +++ b/internal/engine/tunnel/tunnel_test.go @@ -17,8 +17,8 @@ func TestStartNoTunnel(t *testing.T) { MockableLogger: log.Log, }, }) - if err != nil { - t.Fatal(err) + if !errors.Is(err, ErrUnsupportedTunnelName) { + t.Fatal("not the error we expected", err) } if tunnel != nil { t.Fatal("expected nil tunnel here")