From a849213b597c6fd8046810a5711b9a85ed5ec5bd Mon Sep 17 00:00:00 2001
From: Simone Basso <bassosimone@gmail.com>
Date: Mon, 5 Apr 2021 12:02:35 +0200
Subject: [PATCH] fix(engine): break circular dep betwen session and tunnel
 (#295)

This diff breaks the circular dependency between session and
tunnel, by introducing the concept of early session.

An early session is a session that is able to fetch the psiphon
configuration file _only_ if it's embedded in the binary.

This breaks `miniooni --tunnel=psiphon` for users who have
access to the OONI backend. They are not the users we are
writing this feature for, though, so I think this is reasonable.

At the same time, this opens up the possibility of creating
a psiphon tunnel when constructing a session, which is the
approach I was following in https://github.com/ooni/probe-cli/pull/286.

This work is part of https://github.com/ooni/probe/issues/985.

Once this diff is in, I can land https://github.com/ooni/probe-cli/pull/286.
---
 internal/cmd/miniooni/main.go        |  2 +-
 internal/engine/session.go           |  3 +--
 internal/engine/session_nopsiphon.go | 14 +++++++++++++-
 internal/engine/session_psiphon.go   | 13 ++++++++++++-
 internal/engine/tunnel/config.go     |  3 +--
 5 files changed, 28 insertions(+), 7 deletions(-)

diff --git a/internal/cmd/miniooni/main.go b/internal/cmd/miniooni/main.go
index e46620f..dccef00 100644
--- a/internal/cmd/miniooni/main.go
+++ b/internal/cmd/miniooni/main.go
@@ -10,7 +10,7 @@ import (
 func main() {
 	defer func() {
 		if s := recover(); s != nil {
-			fmt.Fprintf(os.Stderr, "%s", s)
+			fmt.Fprintf(os.Stderr, "FATAL: %s\n", s)
 		}
 	}()
 	Main()
diff --git a/internal/engine/session.go b/internal/engine/session.go
index 7f6391e..c34f0df 100644
--- a/internal/engine/session.go
+++ b/internal/engine/session.go
@@ -369,13 +369,12 @@ func (s *Session) MaybeStartTunnel(ctx context.Context, name string) error {
 	s.logger.Infof("starting '%s' tunnel; please be patient...", name)
 	tunnel, err := tunnel.Start(ctx, &tunnel.Config{
 		Name:      name,
-		Session:   s,
+		Session:   &sessionTunnelEarlySession{},
 		TorArgs:   s.TorArgs(),
 		TorBinary: s.TorBinary(),
 		TunnelDir: s.tunnelDir,
 	})
 	if err != nil {
-		s.logger.Warnf("cannot start tunnel: %+v", err)
 		return err
 	}
 	// Implementation note: tunnel _may_ be NIL here if name is ""
diff --git a/internal/engine/session_nopsiphon.go b/internal/engine/session_nopsiphon.go
index 41a18e7..ac43b55 100644
--- a/internal/engine/session_nopsiphon.go
+++ b/internal/engine/session_nopsiphon.go
@@ -2,7 +2,10 @@
 
 package engine
 
-import "context"
+import (
+	"context"
+	"errors"
+)
 
 // FetchPsiphonConfig fetches psiphon config from the API.
 func (s *Session) FetchPsiphonConfig(ctx context.Context) ([]byte, error) {
@@ -12,3 +15,12 @@ func (s *Session) FetchPsiphonConfig(ctx context.Context) ([]byte, error) {
 	}
 	return clnt.FetchPsiphonConfig(ctx)
 }
+
+// sessionTunnelEarlySession is the early session that we pass
+// to tunnel.Start to fetch the Psiphon configuration.
+type sessionTunnelEarlySession struct{}
+
+// FetchPsiphonConfig implements tunnel.Session.FetchPsiphonConfig.
+func (s *sessionTunnelEarlySession) FetchPsiphonConfig(ctx context.Context) ([]byte, error) {
+	return nil, errors.New("no embedded configuration file")
+}
diff --git a/internal/engine/session_psiphon.go b/internal/engine/session_psiphon.go
index 733726a..a6ee33e 100644
--- a/internal/engine/session_psiphon.go
+++ b/internal/engine/session_psiphon.go
@@ -17,9 +17,13 @@ var psiphonConfigJSONAge []byte
 //go:embed psiphon-config.key
 var psiphonConfigSecretKey string
 
+// sessionTunnelEarlySession is the early session that we pass
+// to tunnel.Start to fetch the Psiphon configuration.
+type sessionTunnelEarlySession struct{}
+
 // FetchPsiphonConfig decrypts psiphonConfigJSONAge using
 // filippo.io/age _and_ psiphonConfigSecretKey.
-func (s *Session) FetchPsiphonConfig(ctx context.Context) ([]byte, error) {
+func (s *sessionTunnelEarlySession) FetchPsiphonConfig(ctx context.Context) ([]byte, error) {
 	key := "AGE-SECRET-KEY-1" + psiphonConfigSecretKey
 	identity, err := age.ParseX25519Identity(key)
 	if err != nil {
@@ -32,3 +36,10 @@ func (s *Session) FetchPsiphonConfig(ctx context.Context) ([]byte, error) {
 	}
 	return ioutil.ReadAll(output)
 }
+
+// FetchPsiphonConfig decrypts psiphonConfigJSONAge using
+// filippo.io/age _and_ psiphonConfigSecretKey.
+func (s *Session) FetchPsiphonConfig(ctx context.Context) ([]byte, error) {
+	child := &sessionTunnelEarlySession{}
+	return child.FetchPsiphonConfig(ctx)
+}
diff --git a/internal/engine/tunnel/config.go b/internal/engine/tunnel/config.go
index 5518c2f..6978182 100644
--- a/internal/engine/tunnel/config.go
+++ b/internal/engine/tunnel/config.go
@@ -17,8 +17,7 @@ type Config struct {
 	// "tor" and "psiphon" tunnels.
 	Name string
 
-	// Session is the current measurement session. This
-	// field is mandatory.
+	// Session is the mandatory measurement session.
 	Session Session
 
 	// TorArgs contains the optional arguments that you want us to pass