From 8b92037ae38dff92890690d44db4618c2c4570d4 Mon Sep 17 00:00:00 2001
From: Simone Basso <bassosimone@gmail.com>
Date: Mon, 5 Apr 2021 19:18:00 +0200
Subject: [PATCH] fix(tunnel/tor): keep tunneldir clean (#300)

* fix(tunnel/tor): keep tunneldir clean

This diff ensures that we don't keep the log file growing and
we also remove the temporary files created by the library we
are currently using for running tor from golang.

Part of https://github.com/ooni/probe/issues/985

* fix(session.go): tell use we're using a tunnel
---
 internal/engine/session.go         |  3 ++
 internal/engine/tunnel/tor.go      | 21 ++++++++++--
 internal/engine/tunnel/tor_test.go | 55 ++++++++++++++++++++++++++++++
 3 files changed, 76 insertions(+), 3 deletions(-)

diff --git a/internal/engine/session.go b/internal/engine/session.go
index e78c999..a0adfea 100644
--- a/internal/engine/session.go
+++ b/internal/engine/session.go
@@ -168,6 +168,8 @@ func NewSession(ctx context.Context, config SessionConfig) (*Session, error) {
 	if proxyURL != nil {
 		switch proxyURL.Scheme {
 		case "psiphon", "tor", "fake":
+			config.Logger.Infof(
+				"starting '%s' tunnel; please be patient...", proxyURL.Scheme)
 			tunnel, err := tunnel.Start(ctx, &tunnel.Config{
 				Name:      proxyURL.Scheme,
 				Session:   &sessionTunnelEarlySession{},
@@ -178,6 +180,7 @@ func NewSession(ctx context.Context, config SessionConfig) (*Session, error) {
 			if err != nil {
 				return nil, err
 			}
+			config.Logger.Infof("tunnel '%s' running...", proxyURL.Scheme)
 			sess.tunnel = tunnel
 			proxyURL = tunnel.SOCKS5ProxyURL()
 		}
diff --git a/internal/engine/tunnel/tor.go b/internal/engine/tunnel/tor.go
index 71e8496..96e299c 100644
--- a/internal/engine/tunnel/tor.go
+++ b/internal/engine/tunnel/tor.go
@@ -6,6 +6,7 @@ import (
 	"fmt"
 	"io"
 	"net/url"
+	"os"
 	"path/filepath"
 	"strings"
 	"time"
@@ -45,9 +46,6 @@ func (tt *torTunnel) Stop() {
 	tt.instance.Close()
 }
 
-// TODO(bassosimone): the current design is such that we have a bunch of
-// torrc-$number and a growing tor.log file inside of stateDir.
-
 // ErrTorUnableToGetSOCKSProxyAddress indicates that we could not
 // get the SOCKS proxy address via the control port.
 var ErrTorUnableToGetSOCKSProxyAddress = errors.New(
@@ -70,6 +68,7 @@ func torStart(ctx context.Context, config *Config) (Tunnel, error) {
 	}
 	stateDir := filepath.Join(config.TunnelDir, "tor")
 	logfile := filepath.Join(stateDir, "tor.log")
+	maybeCleanupTunnelDir(stateDir, logfile)
 	extraArgs := append([]string{}, config.TorArgs...)
 	extraArgs = append(extraArgs, "Log")
 	extraArgs = append(extraArgs, "notice stderr")
@@ -112,3 +111,19 @@ func torStart(ctx context.Context, config *Config) (Tunnel, error) {
 		proxy:         &url.URL{Scheme: "socks5", Host: proxyAddress},
 	}, nil
 }
+
+// maybeCleanupTunnelDir removes stale files inside
+// of the tunnel directory.
+func maybeCleanupTunnelDir(dir, logfile string) {
+	os.Remove(logfile)
+	removeWithGlob(filepath.Join(dir, "torrc-*"))
+	removeWithGlob(filepath.Join(dir, "control-port-*"))
+}
+
+// removeWithGlob globs and removes files.
+func removeWithGlob(pattern string) {
+	files, _ := filepath.Glob(pattern)
+	for _, file := range files {
+		os.Remove(file)
+	}
+}
diff --git a/internal/engine/tunnel/tor_test.go b/internal/engine/tunnel/tor_test.go
index 8e27e5c..c180041 100644
--- a/internal/engine/tunnel/tor_test.go
+++ b/internal/engine/tunnel/tor_test.go
@@ -3,7 +3,12 @@ package tunnel
 import (
 	"context"
 	"errors"
+	"fmt"
+	"io/ioutil"
 	"net/url"
+	"os"
+	"path/filepath"
+	"strings"
 	"testing"
 
 	"github.com/cretz/bine/control"
@@ -226,3 +231,53 @@ func TestTorUnsupportedProxy(t *testing.T) {
 		t.Fatal("expected nil tunnel here")
 	}
 }
+
+func TestMaybeCleanupTunnelDir(t *testing.T) {
+	fakeTunDir := filepath.Join("testdata", "fake-tun-dir")
+	if err := os.RemoveAll(fakeTunDir); err != nil {
+		t.Fatal(err)
+	}
+	if err := os.MkdirAll(fakeTunDir, 0700); err != nil {
+		t.Fatal(err)
+	}
+	fakeData := []byte("deadbeef\n")
+	logfile := filepath.Join(fakeTunDir, "tor.log")
+	if err := ioutil.WriteFile(logfile, fakeData, 0600); err != nil {
+		t.Fatal(err)
+	}
+	for idx := 0; idx < 3; idx++ {
+		filename := filepath.Join(fakeTunDir, fmt.Sprintf("torrc-%d", idx))
+		if err := ioutil.WriteFile(filename, fakeData, 0600); err != nil {
+			t.Fatal(err)
+		}
+		filename = filepath.Join(fakeTunDir, fmt.Sprintf("control-port-%d", idx))
+		if err := ioutil.WriteFile(filename, fakeData, 0600); err != nil {
+			t.Fatal(err)
+		}
+		filename = filepath.Join(fakeTunDir, fmt.Sprintf("antani-%d", idx))
+		if err := ioutil.WriteFile(filename, fakeData, 0600); err != nil {
+			t.Fatal(err)
+		}
+	}
+	files, err := filepath.Glob(filepath.Join(fakeTunDir, "*"))
+	if err != nil {
+		t.Fatal(err)
+	}
+	if len(files) != 10 {
+		t.Fatal("unexpected number of files")
+	}
+	maybeCleanupTunnelDir(fakeTunDir, logfile)
+	files, err = filepath.Glob(filepath.Join(fakeTunDir, "*"))
+	if err != nil {
+		t.Fatal(err)
+	}
+	if len(files) != 3 {
+		t.Fatal("unexpected number of files")
+	}
+	expectPrefix := filepath.Join(fakeTunDir, "antani-")
+	for _, file := range files {
+		if !strings.HasPrefix(file, expectPrefix) {
+			t.Fatal("unexpected file name: ", file)
+		}
+	}
+}