Merge pull request #5794 from yyforyongyu/5771-fix-tor

tor: enable lnd surviving Tor daemon restart

config.go

@@ -139,6 +139,15 @@ const (
 	defaultTLSBackoff  = time.Minute
 	defaultTLSAttempts = 0
 
+	// Set defaults for a health check which ensures that the tor
+	// connection is alive. Although this check is off by default (not all
+	// setups require it), we still set the other default values so that
+	// the health check can be easily enabled with sane defaults.
+	defaultTCInterval = time.Minute
+	defaultTCTimeout  = time.Second * 5
+	defaultTCBackoff  = time.Minute
+	defaultTCAttempts = 0
+
 	// defaultRemoteMaxHtlcs specifies the default limit for maximum
 	// concurrent HTLCs the remote party may add to commitment transactions.
 	// This value can be overridden with --default-remote-max-htlcs.
@@ -541,6 +550,12 @@ func DefaultConfig() Config {
 				Attempts: defaultTLSAttempts,
 				Backoff:  defaultTLSBackoff,
 			},
+			TorConnection: &lncfg.CheckConfig{
+				Interval: defaultTCInterval,
+				Timeout:  defaultTCTimeout,
+				Attempts: defaultTCAttempts,
+				Backoff:  defaultTCBackoff,
+			},
 		},
 		Gossip: &lncfg.Gossip{
 			MaxChannelUpdateBurst: discovery.DefaultMaxChannelUpdateBurst,

docs/release-notes/release-notes-0.14.0.md

@@ -2,11 +2,32 @@
 
 ## Networking & Tor
 
+### Connectivity mode
+
 A new flag has been added to enable a hybrid tor connectivity mode, where tor
 is only used for onion address connections, and clearnet for everything else.
 This new behavior can be added using the `tor.skip-proxy-for-clearnet-targets`
 flag.
 
+### Onion service
+
+The Onion service created upon lnd startup is [now deleted during lnd shutdown
+using `DEL_ONION`](https://github.com/lightningnetwork/lnd/pull/5794). 
+
+### Tor connection
+
+A new health check, tor connection, [is added to lnd's liveness monitor upon
+startup](https://github.com/lightningnetwork/lnd/pull/5794). This check will
+ensure the liveness of the connection between the Tor daemon and lnd's tor
+controller. To enable it, please use the following flags,
+```
+healthcheck.torconnection.attempts=xxx
+healthcheck.torconnection.timeout=xxx
+healthcheck.torconnection.backoff=xxx
+healthcheck.torconnection.internal=xxx
+```
+Read more about the usage of the flags in the `sample-lnd.conf`.
+
 ## LN Peer-to-Peer Netowrk
 
 ### Bitcoin Blockheaders in Ping Messages

healthcheck/tor_connection.go

@@ -0,0 +1,86 @@
+package healthcheck
+
+import (
+	"errors"
+	"fmt"
+	"io"
+	"syscall"
+
+	"github.com/lightningnetwork/lnd/tor"
+)
+
+// CheckTorServiceStatus checks whether the onion service is reachable by
+// sending a GETINFO command to the Tor daemon using our tor controller.
+// We will get an EOF or a broken pipe error if the Tor daemon is
+// stopped/restarted as the previously created socket connection is no longer
+// open. In this case, we will attempt a restart on our tor controller. If the
+// tor daemon comes back, a new socket connection will then be created.
+func CheckTorServiceStatus(tc *tor.Controller,
+	createService func() error) error {
+
+	// Send a cmd using GETINFO onions/current and checks that our known
+	// onion serviceID can be found.
+	err := tc.CheckOnionService()
+	if err == nil {
+		return nil
+	}
+
+	log.Debugf("Checking tor service got: %v", err)
+
+	switch {
+	// We will get an EOF if the connection is lost. In this case, we will
+	// return an error and wait for the Tor daemon to come back. We won't
+	// attempt to make a new connection since we know Tor daemon is down.
+	case errors.Is(err, io.EOF), errors.Is(err, syscall.ECONNREFUSED):
+		return fmt.Errorf("Tor daemon connection lost, " +
+			"check if Tor is up and running")
+
+	// Once Tor daemon is down, we will get a broken pipe error when we use
+	// the existing connection to make a GETINFO request since that socket
+	// has now been closed. As Tor daemon might not be running yet, we will
+	// attempt to make a new connection till Tor daemon is back.
+	case errors.Is(err, syscall.EPIPE):
+		log.Warnf("Tor connection lost, attempting a tor controller " +
+			"re-connection...")
+
+		// If the restart fails, we will attempt again during our next
+		// healthcheck cycle.
+		return restartTorController(tc, createService)
+
+	// If this is not a connection layer error, such as
+	// ErrServiceNotCreated or ErrServiceIDUnmatch, there's little we can
+	// do but to report the error to the user.
+	default:
+		return err
+	}
+}
+
+// restartTorController attempts to make a new connection to the Tor daemon and
+// re-create the Hidden Service.
+func restartTorController(tc *tor.Controller,
+	createService func() error) error {
+
+	err := tc.Reconnect()
+
+	// If we get a connection refused error, it means Tor daemon might not
+	// be started.
+	if errors.Is(err, syscall.ECONNREFUSED) {
+		return fmt.Errorf("check if Tor daemon is running")
+	}
+
+	// Otherwise, we get an unexpected and return it.
+	if err != nil {
+		log.Errorf("Re-connectting tor got err: %v", err)
+		return err
+	}
+
+	// Recreate the Hidden Service.
+	if err := createService(); err != nil {
+		log.Errorf("Re-create service tor got err: %v", err)
+		return err
+	}
+
+	log.Info("Successfully restarted tor connection!")
+
+	return nil
+}

lncfg/healthcheck.go

@@ -28,6 +28,8 @@ type HealthCheckConfig struct {
 	DiskCheck *DiskCheckConfig `group:"diskspace" namespace:"diskspace"`
 
 	TLSCheck *CheckConfig `group:"tls" namespace:"tls"`
+
+	TorConnection *CheckConfig `group:"torconnection" namespace:"torconnection"`
 }
 
 // Validate checks the values configured for our health checks.
@@ -50,6 +52,10 @@ func (h *HealthCheckConfig) Validate() error {
 		return errors.New("disk required ratio must be in [0:1)")
 	}
 
+	if err := h.TorConnection.validate("tor connection"); err != nil {
+		return err
+	}
+
 	return nil
 }
 

log.go

@@ -40,6 +40,7 @@ import (
 	"github.com/lightningnetwork/lnd/rpcperms"
 	"github.com/lightningnetwork/lnd/signal"
 	"github.com/lightningnetwork/lnd/sweep"
+	"github.com/lightningnetwork/lnd/tor"
 	"github.com/lightningnetwork/lnd/watchtower"
 	"github.com/lightningnetwork/lnd/watchtower/wtclient"
 )
@@ -161,6 +162,7 @@ func SetupLoggers(root *build.RotatingLogWriter, interceptor signal.Interceptor)
 	AddSubLogger(root, funding.Subsystem, interceptor, funding.UseLogger)
 	AddSubLogger(root, cluster.Subsystem, interceptor, cluster.UseLogger)
 	AddSubLogger(root, rpcperms.Subsystem, interceptor, rpcperms.UseLogger)
+	AddSubLogger(root, tor.Subsystem, interceptor, tor.UseLogger)
 }
 
 // AddSubLogger is a helper method to conveniently create and register the

sample-lnd.conf

@@ -990,6 +990,22 @@ litecoin.node=ltcd
 ; This value must be >= 1m.
 ; healthcheck.tls.interval=1m
 
+; The number of times we should attempt to check our tor connection before
+; gracefully shutting down. Set this value to 0 to disable this health check.
+; healthcheck.torconnection.attempts=3
+
+; The amount of time we allow a call to our tor connection to take before we 
+; fail the attempt. This value must be >= 1s.
+; healthcheck.torconnection.timeout=10s
+
+; The amount of time we should backoff between failed attempts to check tor 
+; connection. This value must be >= 1s.
+; healthcheck.torconnection.backoff=30s
+
+; The amount of time we should wait between tor connection health checks. This
+; value must be >= 1m.
+; healthcheck.torconnection.interval=1m
+
 
 [signrpc]
 

server.go

@@ -1470,9 +1470,40 @@ func newServer(cfg *Config, listenAddrs []net.Addr,
 		})
 	}
 
-	// Create a set of health checks using our configured values. If a
+	// Create liveliness monitor.
-	// health check has been disabled by setting attempts to 0, our monitor
+	s.createLivenessMonitor(cfg, cc)
+
+	// Create the connection manager which will be responsible for
+	// maintaining persistent outbound connections and also accepting new
+	// incoming connections
+	cmgr, err := connmgr.New(&connmgr.Config{
+		Listeners:      listeners,
+		OnAccept:       s.InboundPeerConnected,
+		RetryDuration:  time.Second * 5,
+		TargetOutbound: 100,
+		Dial: noiseDial(
+			nodeKeyECDH, s.cfg.net, s.cfg.ConnectionTimeout,
+		),
+		OnConnection: s.OutboundPeerConnected,
+	})
+	if err != nil {
+		return nil, err
+	}
+	s.connMgr = cmgr
+
+	return s, nil
+}
+
+// createLivenessMonitor creates a set of health checks using our configured
+// values and uses these checks to create a liveliness monitor. Available
+// health checks,
+//   - chainHealthCheck
+//   - diskCheck
+//   - tlsHealthCheck
+//   - torController, only created when tor is enabled.
+// If a health check has been disabled by setting attempts to 0, our monitor
 // will not run it.
+func (s *server) createLivenessMonitor(cfg *Config, cc *chainreg.ChainControl) {
 	chainHealthCheck := healthcheck.NewObservation(
 		"chain backend",
 		cc.HealthCheck,
@@ -1521,11 +1552,12 @@ func newServer(cfg *Config, listenAddrs []net.Addr,
 			// If the current time is passed the certificate's
 			// expiry time, then it is considered expired
 			if time.Now().After(parsedCert.NotAfter) {
-				return fmt.Errorf("TLS certificate is expired as of %v", parsedCert.NotAfter)
+				return fmt.Errorf("TLS certificate is "+
+					"expired as of %v", parsedCert.NotAfter)
 			}
 
-			// If the certificate is not outdated, no error needs to
+			// If the certificate is not outdated, no error needs
-			// be returned
+			// to be returned
 			return nil
 		},
 		cfg.HealthChecks.TLSCheck.Interval,
@@ -1534,36 +1566,36 @@ func newServer(cfg *Config, listenAddrs []net.Addr,
 		cfg.HealthChecks.TLSCheck.Attempts,
 	)
 
+	checks := []*healthcheck.Observation{
+		chainHealthCheck, diskCheck, tlsHealthCheck,
+	}
+
+	// If Tor is enabled, add the healthcheck for tor connection.
+	if s.torController != nil {
+		torConnectionCheck := healthcheck.NewObservation(
+			"tor connection",
+			func() error {
+				return healthcheck.CheckTorServiceStatus(
+					s.torController,
+					s.createNewHiddenService,
+				)
+			},
+			cfg.HealthChecks.TorConnection.Interval,
+			cfg.HealthChecks.TorConnection.Timeout,
+			cfg.HealthChecks.TorConnection.Backoff,
+			cfg.HealthChecks.TorConnection.Attempts,
+		)
+		checks = append(checks, torConnectionCheck)
+	}
+
 	// If we have not disabled all of our health checks, we create a
 	// liveliness monitor with our configured checks.
 	s.livelinessMonitor = healthcheck.NewMonitor(
 		&healthcheck.Config{
-			Checks: []*healthcheck.Observation{
+			Checks:   checks,
-				chainHealthCheck, diskCheck, tlsHealthCheck,
-			},
 			Shutdown: srvrLog.Criticalf,
 		},
 	)
-
-	// Create the connection manager which will be responsible for
-	// maintaining persistent outbound connections and also accepting new
-	// incoming connections
-	cmgr, err := connmgr.New(&connmgr.Config{
-		Listeners:      listeners,
-		OnAccept:       s.InboundPeerConnected,
-		RetryDuration:  time.Second * 5,
-		TargetOutbound: 100,
-		Dial: noiseDial(
-			nodeKeyECDH, s.cfg.net, s.cfg.ConnectionTimeout,
-		),
-		OnConnection: s.OutboundPeerConnected,
-	})
-	if err != nil {
-		return nil, err
-	}
-	s.connMgr = cmgr
-
-	return s, nil
 }
 
 // Started returns true if the server has been started, and false otherwise.

tor/add_onion_test.go

@@ -1,51 +0,0 @@
-package tor
-
-import (
-	"bytes"
-	"io/ioutil"
-	"path/filepath"
-	"testing"
-)
-
-// TestOnionFile tests that the OnionFile implementation of the OnionStore
-// interface behaves as expected.
-func TestOnionFile(t *testing.T) {
-	t.Parallel()
-
-	tempDir, err := ioutil.TempDir("", "onion_store")
-	if err != nil {
-		t.Fatalf("unable to create temp dir: %v", err)
-	}
-
-	privateKey := []byte("hide_me_plz")
-	privateKeyPath := filepath.Join(tempDir, "secret")
-
-	// Create a new file-based onion store. A private key should not exist
-	// yet.
-	onionFile := NewOnionFile(privateKeyPath, 0600)
-	if _, err := onionFile.PrivateKey(V2); err != ErrNoPrivateKey {
-		t.Fatalf("expected ErrNoPrivateKey, got \"%v\"", err)
-	}
-
-	// Store the private key and ensure what's stored matches.
-	if err := onionFile.StorePrivateKey(V2, privateKey); err != nil {
-		t.Fatalf("unable to store private key: %v", err)
-	}
-	storePrivateKey, err := onionFile.PrivateKey(V2)
-	if err != nil {
-		t.Fatalf("unable to retrieve private key: %v", err)
-	}
-	if !bytes.Equal(storePrivateKey, privateKey) {
-		t.Fatalf("expected private key \"%v\", got \"%v\"",
-			string(privateKey), string(storePrivateKey))
-	}
-
-	// Finally, delete the private key. We should no longer be able to
-	// retrieve it.
-	if err := onionFile.DeletePrivateKey(V2); err != nil {
-		t.Fatalf("unable to delete private key: %v", err)
-	}
-	if _, err := onionFile.PrivateKey(V2); err != ErrNoPrivateKey {
-		t.Fatal("found deleted private key")
-	}
-}

tor/cmd_info.go

@@ -0,0 +1,72 @@
+package tor
+
+import (
+	"errors"
+	"fmt"
+)
+
+var (
+	// ErrServiceNotCreated is used when we want to query info on an onion
+	// service while it's not been created yet.
+	ErrServiceNotCreated = errors.New("onion service hasn't been created")
+
+	// ErrServiceIDUnmatch is used when the serviceID the controller has
+	// doesn't match the serviceID the Tor daemon has.
+	ErrServiceIDUnmatch = errors.New("onion serviceIDs not match")
+
+	// ErrNoServiceFound is used when the Tor daemon replies no active
+	// onion services found for the current control connection while we
+	// expect one.
+	ErrNoServiceFound = errors.New("no active service found")
+)
+
+// CheckOnionService checks that the onion service created by the controller
+// is active. It queries the Tor daemon using the endpoint "onions/current" to
+// get the current onion service and checks that service ID matches the
+// activeServiceID.
+func (c *Controller) CheckOnionService() error {
+	// Check that we have a hidden service created.
+	if c.activeServiceID == "" {
+		return ErrServiceNotCreated
+	}
+
+	// Fetch the onion services that live in current control connection.
+	cmd := "GETINFO onions/current"
+	code, reply, err := c.sendCommand(cmd)
+
+	// Exit early if we got an error or Tor daemon didn't respond success.
+	// TODO(yy): unify the usage of err and code so we could rely on a
+	// single source to change our state.
+	if err != nil || code != success {
+		log.Debugf("query service:%v got err:%v, reply:%v",
+			c.activeServiceID, err, reply)
+
+		return fmt.Errorf("%w: %v", err, reply)
+	}
+
+	// Parse the reply, which should have the following format,
+	//      onions/current=serviceID
+	// After parsing, we get a map as,
+	// 	[onion/current: serviceID]
+	//
+	// NOTE: our current tor controller does NOT support multiple onion
+	// services to be created at the same time, thus we expect the reply to
+	// only contain one serviceID. If multiple serviceIDs are returned, we
+	// would expected the reply to have the following format,
+	//      onions/current=serviceID1, serviceID2, serviceID3,...
+	// Thus a new parser is need to parse that reply.
+	resp := parseTorReply(reply)
+	serviceID, ok := resp["onions/current"]
+	if !ok {
+		return ErrNoServiceFound
+	}
+
+	// Check that our active service is indeed the service acknowledged by
+	// Tor daemon.
+	if c.activeServiceID != serviceID {
+		return fmt.Errorf("%w: controller has: %v, Tor daemon has: %v",
+			ErrServiceIDUnmatch, c.activeServiceID, serviceID)
+	}
+
+	return nil
+}

tor/cmd_info_test.go

@@ -0,0 +1,87 @@
+package tor
+
+import (
+	"errors"
+	"io"
+	"syscall"
+	"testing"
+
+	"github.com/stretchr/testify/require"
+)
+
+func TestCheckOnionServiceFailOnServiceNotCreated(t *testing.T) {
+	t.Parallel()
+
+	// Create a dummy tor controller.
+	c := &Controller{}
+
+	// Check that CheckOnionService returns an error when the service
+	// hasn't been created.
+	require.Equal(t, ErrServiceNotCreated, c.CheckOnionService())
+}
+
+func TestCheckOnionServiceSucceed(t *testing.T) {
+	t.Parallel()
+
+	// Create mock server and client connection.
+	proxy := createTestProxy(t)
+	defer proxy.cleanUp()
+	server := proxy.serverConn
+
+	// Assign a fake service ID to the controller.
+	c := &Controller{conn: proxy.clientConn, activeServiceID: "fakeID"}
+
+	// Test a successful response.
+	serverResp := "250-onions/current=fakeID\n250 OK\n"
+
+	// Let the server mocks a given response.
+	_, err := server.Write([]byte(serverResp))
+	require.NoError(t, err, "server failed to write")
+
+	// For a successful response, we expect no error.
+	require.NoError(t, c.CheckOnionService())
+}
+
+func TestCheckOnionServiceFailOnServiceIDNotMatch(t *testing.T) {
+	t.Parallel()
+
+	// Create mock server and client connection.
+	proxy := createTestProxy(t)
+	defer proxy.cleanUp()
+	server := proxy.serverConn
+
+	// Assign a fake service ID to the controller.
+	c := &Controller{conn: proxy.clientConn, activeServiceID: "fakeID"}
+
+	// Mock a response with a different serviceID.
+	serverResp := "250-onions/current=unmatchedID\n250 OK\n"
+
+	// Let the server mocks a given response.
+	_, err := server.Write([]byte(serverResp))
+	require.NoError(t, err, "server failed to write")
+
+	// Check the error returned from GetServiceInfo is expected.
+	require.ErrorIs(t, c.CheckOnionService(), ErrServiceIDUnmatch)
+}
+
+func TestCheckOnionServiceFailOnClosedConnection(t *testing.T) {
+	t.Parallel()
+
+	// Create mock server and client connection.
+	proxy := createTestProxy(t)
+	defer proxy.cleanUp()
+	server := proxy.serverConn
+
+	// Assign a fake service ID to the controller.
+	c := &Controller{conn: proxy.clientConn, activeServiceID: "fakeID"}
+
+	// Close the connection from the server side.
+	require.NoError(t, server.Close(), "server failed to close conn")
+
+	// Check the error returned from GetServiceInfo is expected.
+	err := c.CheckOnionService()
+	eof := errors.Is(err, io.EOF)
+	reset := errors.Is(err, syscall.ECONNRESET)
+	require.Truef(t, eof || reset,
+		"must of EOF or RESET error, instead got: %v", err)
+}

tor/cmd_onion.go

@@ -52,7 +52,9 @@ var _ OnionStore = (*OnionFile)(nil)
 
 // NewOnionFile creates a file-based implementation of the OnionStore interface
 // to store an onion service's private key.
-func NewOnionFile(privateKeyPath string, privateKeyPerm os.FileMode) *OnionFile {
+func NewOnionFile(privateKeyPath string,
+	privateKeyPerm os.FileMode) *OnionFile {
+
 	return &OnionFile{
 		privateKeyPath: privateKeyPath,
 		privateKeyPerm: privateKeyPerm,
@@ -64,8 +66,8 @@ func (f *OnionFile) StorePrivateKey(_ OnionType, privateKey []byte) error {
 	return ioutil.WriteFile(f.privateKeyPath, privateKey, f.privateKeyPerm)
 }
 
-// PrivateKey retrieves the private key from its expected path. If the file does
+// PrivateKey retrieves the private key from its expected path. If the file
-// not exist, then ErrNoPrivateKey is returned.
+// does not exist, then ErrNoPrivateKey is returned.
 func (f *OnionFile) PrivateKey(_ OnionType) ([]byte, error) {
 	if _, err := os.Stat(f.privateKeyPath); os.IsNotExist(err) {
 		return nil, ErrNoPrivateKey
@@ -78,8 +80,8 @@ func (f *OnionFile) DeletePrivateKey(_ OnionType) error {
 	return os.Remove(f.privateKeyPath)
 }
 
-// AddOnionConfig houses all of the required parameters in order to successfully
+// AddOnionConfig houses all of the required parameters in order to
-// create a new onion service or restore an existing one.
+// successfully create a new onion service or restore an existing one.
 type AddOnionConfig struct {
 	// Type denotes the type of the onion service that should be created.
 	Type OnionType
@@ -87,9 +89,9 @@ type AddOnionConfig struct {
 	// VirtualPort is the externally reachable port of the onion address.
 	VirtualPort int
 
-	// TargetPorts is the set of ports that the service will be listening on
+	// TargetPorts is the set of ports that the service will be listening
-	// locally. The Tor server will use choose a random port from this set
+	// on locally. The Tor server will use choose a random port from this
-	// to forward the traffic from the virtual port.
+	// set to forward the traffic from the virtual port.
 	//
 	// NOTE: If nil/empty, the virtual port will be used as the only target
 	// port.
@@ -103,25 +105,17 @@ type AddOnionConfig struct {
 	Store OnionStore
 }
 
-// AddOnion creates an onion service and returns its onion address. Once
+// prepareKeyparam takes a config and prepares the key param to be used inside
-// created, the new onion service will remain active until the connection
+// ADD_ONION.
-// between the controller and the Tor server is closed.
+func (c *Controller) prepareKeyparam(cfg AddOnionConfig) (string, error) {
-func (c *Controller) AddOnion(cfg AddOnionConfig) (*OnionAddr, error) {
+	// We'll start off by checking if the store contains an existing
-	// Before sending the request to create an onion service to the Tor
+	// private key. If it does not, then we should request the server to
-	// server, we'll make sure that it supports V3 onion services if that
+	// create a new onion service and return its private key. Otherwise,
-	// was the type requested.
+	// we'll request the server to recreate the onion server from our
-	if cfg.Type == V3 {
+	// private key.
-		if err := supportsV3(c.version); err != nil {
-			return nil, err
-		}
-	}
-
-	// We'll start off by checking if the store contains an existing private
-	// key. If it does not, then we should request the server to create a
-	// new onion service and return its private key. Otherwise, we'll
-	// request the server to recreate the onion server from our private key.
 	var keyParam string
 	switch cfg.Type {
+	// TODO(yy): drop support for v2.
 	case V2:
 		keyParam = "NEW:RSA1024"
 	case V3:
@@ -139,10 +133,22 @@ func (c *Controller) AddOnion(cfg AddOnionConfig) (*OnionAddr, error) {
 			keyParam = string(privateKey)
 
 		default:
-			return nil, err
+			return "", err
 		}
 	}
 
+	return keyParam, nil
+}
+
+// prepareAddOnion constructs a cmd command string based on the specified
+// config.
+func (c *Controller) prepareAddOnion(cfg AddOnionConfig) (string, error) {
+	// Create the keyParam.
+	keyParam, err := c.prepareKeyparam(cfg)
+	if err != nil {
+		return "", err
+	}
+
 	// Now, we'll create a mapping from the virtual port to each target
 	// port. If no target ports were specified, we'll use the virtual port
 	// to provide a one-to-one mapping.
@@ -155,8 +161,8 @@ func (c *Controller) AddOnion(cfg AddOnionConfig) (*OnionAddr, error) {
 			portParam += fmt.Sprintf("Port=%d,%d ", cfg.VirtualPort,
 				targetPort)
 		} else {
-			portParam += fmt.Sprintf("Port=%d,%s:%d ", cfg.VirtualPort,
+			portParam += fmt.Sprintf("Port=%d,%s:%d ",
-				c.targetIPAddress, targetPort)
+				cfg.VirtualPort, c.targetIPAddress, targetPort)
 		}
 	}
 
@@ -171,6 +177,37 @@ func (c *Controller) AddOnion(cfg AddOnionConfig) (*OnionAddr, error) {
 	// Send the command to create the onion service to the Tor server and
 	// await its response.
 	cmd := fmt.Sprintf("ADD_ONION %s %s", keyParam, portParam)
+
+	return cmd, nil
+}
+
+// AddOnion creates an ephemeral onion service and returns its onion address.
+// Once created, the new onion service will remain active until either,
+//   - the onion service is removed via `DEL_ONION`.
+//   - the Tor daemon terminates.
+//   - the controller connection that originated the `ADD_ONION` is closed.
+// Each connection can only see its own ephemeral services. If a service needs
+// to survive beyond current controller connection, use the "Detach" flag when
+// creating new service via `ADD_ONION`.
+func (c *Controller) AddOnion(cfg AddOnionConfig) (*OnionAddr, error) {
+	// Before sending the request to create an onion service to the Tor
+	// server, we'll make sure that it supports V3 onion services if that
+	// was the type requested.
+	// TODO(yy): drop support for v2.
+	if cfg.Type == V3 {
+		if err := supportsV3(c.version); err != nil {
+			return nil, err
+		}
+	}
+
+	// Construct the cmd command.
+	cmd, err := c.prepareAddOnion(cfg)
+	if err != nil {
+		return nil, err
+	}
+
+	// Send the command to create the onion service to the Tor server and
+	// await its response.
 	_, reply, err := c.sendCommand(cmd)
 	if err != nil {
 		return nil, err
@@ -207,6 +244,9 @@ func (c *Controller) AddOnion(cfg AddOnionConfig) (*OnionAddr, error) {
 		}
 	}
 
+	c.activeServiceID = serviceID
+	log.Debugf("serviceID:%s added to tor controller", serviceID)
+
 	// Finally, we'll return the onion address composed of the service ID,
 	// along with the onion suffix, and the port this onion service can be
 	// reached at externally.
@@ -215,3 +255,47 @@ func (c *Controller) AddOnion(cfg AddOnionConfig) (*OnionAddr, error) {
 		Port:         cfg.VirtualPort,
 	}, nil
 }
+
+// DelOnion tells the Tor daemon to remove an onion service, which satisfies
+// either,
+//   - the onion service was created on the same control connection as the
+//     "DEL_ONION" command.
+//   - the onion service was created using the "Detach" flag.
+func (c *Controller) DelOnion(serviceID string) error {
+	log.Debugf("removing serviceID:%s from tor controller", serviceID)
+
+	cmd := fmt.Sprintf("DEL_ONION %s", serviceID)
+
+	// Send the command to create the onion service to the Tor server and
+	// await its response.
+	code, _, err := c.sendCommand(cmd)
+
+	// Tor replies with "250 OK" on success, or a 512 if there are an
+	// invalid number of arguments, or a 552 if it doesn't recognize the
+	// ServiceID.
+	switch code {
+	// Replied 250 OK.
+	case success:
+		return nil
+
+	// Replied 512 for invalid arguments. This is most likely that the
+	// serviceID is not set(empty string).
+	case invalidNumOfArguments:
+		return fmt.Errorf("invalid arguments: %w", err)
+
+	// Replied 552, which means either,
+	//   - the serviceID is invalid.
+	//   - the onion service is not owned by the current control connection
+	//     and,
+	//   - the onion service is not a detached service.
+	// In either case, we will ignore the error and log a warning as there
+	// not much we can do from the controller side.
+	case serviceIDNotRecognized:
+		log.Warnf("removing serviceID:%v not found", serviceID)
+		return nil
+
+	default:
+		return fmt.Errorf("undefined response code: %v, err: %w",
+			code, err)
+	}
+}

tor/cmd_onion_test.go

@@ -0,0 +1,212 @@
+package tor
+
+import (
+	"bytes"
+	"errors"
+	"io/ioutil"
+	"path/filepath"
+	"testing"
+
+	"github.com/stretchr/testify/mock"
+	"github.com/stretchr/testify/require"
+)
+
+// TestOnionFile tests that the OnionFile implementation of the OnionStore
+// interface behaves as expected.
+func TestOnionFile(t *testing.T) {
+	t.Parallel()
+
+	tempDir, err := ioutil.TempDir("", "onion_store")
+	if err != nil {
+		t.Fatalf("unable to create temp dir: %v", err)
+	}
+
+	privateKey := []byte("hide_me_plz")
+	privateKeyPath := filepath.Join(tempDir, "secret")
+
+	// Create a new file-based onion store. A private key should not exist
+	// yet.
+	onionFile := NewOnionFile(privateKeyPath, 0600)
+	if _, err := onionFile.PrivateKey(V2); err != ErrNoPrivateKey {
+		t.Fatalf("expected ErrNoPrivateKey, got \"%v\"", err)
+	}
+
+	// Store the private key and ensure what's stored matches.
+	if err := onionFile.StorePrivateKey(V2, privateKey); err != nil {
+		t.Fatalf("unable to store private key: %v", err)
+	}
+	storePrivateKey, err := onionFile.PrivateKey(V2)
+	if err != nil {
+		t.Fatalf("unable to retrieve private key: %v", err)
+	}
+	if !bytes.Equal(storePrivateKey, privateKey) {
+		t.Fatalf("expected private key \"%v\", got \"%v\"",
+			string(privateKey), string(storePrivateKey))
+	}
+
+	// Finally, delete the private key. We should no longer be able to
+	// retrieve it.
+	if err := onionFile.DeletePrivateKey(V2); err != nil {
+		t.Fatalf("unable to delete private key: %v", err)
+	}
+	if _, err := onionFile.PrivateKey(V2); err != ErrNoPrivateKey {
+		t.Fatal("found deleted private key")
+	}
+}
+
+// TestPrepareKeyParam checks that the key param is created as expected.
+func TestPrepareKeyParam(t *testing.T) {
+	testKey := []byte("hide_me_plz")
+	dummyErr := errors.New("dummy")
+
+	// Create a dummy controller.
+	controller := NewController("", "", "")
+
+	// Test that a V3 keyParam is used.
+	cfg := AddOnionConfig{Type: V3}
+	keyParam, err := controller.prepareKeyparam(cfg)
+
+	require.Equal(t, "NEW:ED25519-V3", keyParam)
+	require.NoError(t, err)
+
+	// Create a mock store which returns the test private key.
+	store := &mockStore{}
+	store.On("PrivateKey", cfg.Type).Return(testKey, nil)
+
+	// Check that the test private is returned.
+	cfg = AddOnionConfig{Type: V3, Store: store}
+	keyParam, err = controller.prepareKeyparam(cfg)
+
+	require.Equal(t, string(testKey), keyParam)
+	require.NoError(t, err)
+	store.AssertExpectations(t)
+
+	// Create a mock store which returns ErrNoPrivateKey.
+	store = &mockStore{}
+	store.On("PrivateKey", cfg.Type).Return(nil, ErrNoPrivateKey)
+
+	// Check that the V3 keyParam is returned.
+	cfg = AddOnionConfig{Type: V3, Store: store}
+	keyParam, err = controller.prepareKeyparam(cfg)
+
+	require.Equal(t, "NEW:ED25519-V3", keyParam)
+	require.NoError(t, err)
+	store.AssertExpectations(t)
+
+	// Create a mock store which returns an dummy error.
+	store = &mockStore{}
+	store.On("PrivateKey", cfg.Type).Return(nil, dummyErr)
+
+	// Check that an error is returned.
+	cfg = AddOnionConfig{Type: V3, Store: store}
+	keyParam, err = controller.prepareKeyparam(cfg)
+
+	require.Empty(t, keyParam)
+	require.ErrorIs(t, dummyErr, err)
+	store.AssertExpectations(t)
+}
+
+// TestPrepareAddOnion checks that the cmd used to add onion service is created
+// as expected.
+func TestPrepareAddOnion(t *testing.T) {
+	t.Parallel()
+
+	// Create a mock store.
+	store := &mockStore{}
+	testKey := []byte("hide_me_plz")
+
+	testCases := []struct {
+		name            string
+		targetIPAddress string
+		cfg             AddOnionConfig
+		expectedCmd     string
+		expectedErr     error
+	}{
+		{
+			name:            "empty target IP and ports",
+			targetIPAddress: "",
+			cfg:             AddOnionConfig{VirtualPort: 9735},
+			expectedCmd:     "ADD_ONION NEW:RSA1024 Port=9735,9735 ",
+			expectedErr:     nil,
+		},
+		{
+			name:            "specified target IP and empty ports",
+			targetIPAddress: "127.0.0.1",
+			cfg:             AddOnionConfig{VirtualPort: 9735},
+			expectedCmd: "ADD_ONION NEW:RSA1024 " +
+				"Port=9735,127.0.0.1:9735 ",
+			expectedErr: nil,
+		},
+		{
+			name:            "specified target IP and ports",
+			targetIPAddress: "127.0.0.1",
+			cfg: AddOnionConfig{
+				VirtualPort: 9735,
+				TargetPorts: []int{18000, 18001},
+			},
+			expectedCmd: "ADD_ONION NEW:RSA1024 " +
+				"Port=9735,127.0.0.1:18000 " +
+				"Port=9735,127.0.0.1:18001 ",
+			expectedErr: nil,
+		},
+		{
+			name:            "specified private key from store",
+			targetIPAddress: "",
+			cfg: AddOnionConfig{
+				VirtualPort: 9735,
+				Store:       store,
+			},
+			expectedCmd: "ADD_ONION hide_me_plz " +
+				"Port=9735,9735 ",
+			expectedErr: nil,
+		},
+	}
+
+	for _, tc := range testCases {
+		tc := tc
+
+		if tc.cfg.Store != nil {
+			store.On("PrivateKey", tc.cfg.Type).Return(
+				testKey, tc.expectedErr,
+			)
+		}
+
+		controller := NewController("", tc.targetIPAddress, "")
+		t.Run(tc.name, func(t *testing.T) {
+			cmd, err := controller.prepareAddOnion(tc.cfg)
+			require.Equal(t, tc.expectedErr, err)
+			require.Equal(t, tc.expectedCmd, cmd)
+
+			// Check that the mocker is satisfied.
+			store.AssertExpectations(t)
+		})
+	}
+
+}
+
+// mockStore implements a mock of the interface OnionStore.
+type mockStore struct {
+	mock.Mock
+}
+
+// A compile-time constraint to ensure mockStore satisfies the OnionStore
+// interface.
+var _ OnionStore = (*mockStore)(nil)
+
+func (m *mockStore) StorePrivateKey(ot OnionType, key []byte) error {
+	args := m.Called(ot, key)
+	return args.Error(0)
+}
+
+func (m *mockStore) PrivateKey(ot OnionType) ([]byte, error) {
+	args := m.Called(ot)
+	if args.Get(0) == nil {
+		return nil, args.Error(1)
+	}
+	return args.Get(0).([]byte), args.Error(1)
+}
+
+func (m *mockStore) DeletePrivateKey(ot OnionType) error {
+	args := m.Called(ot)
+	return args.Error(0)
+}

tor/controller.go

@@ -20,6 +20,14 @@ const (
 	// request.
 	success = 250
 
+	// invalidNumOfArguments is the Tor Control response code representing
+	// there being an invalid number of arguments.
+	invalidNumOfArguments = 512
+
+	// serviceIDNotRecognized is the Tor Control response code representing
+	// the specified ServiceID is not recognized.
+	serviceIDNotRecognized = 552
+
 	// nonceLen is the length of a nonce generated by either the controller
 	// or the Tor server
 	nonceLen = 32
@@ -57,6 +65,18 @@ var (
 	// message from the controller.
 	controllerKey = []byte("Tor safe cookie authentication " +
 		"controller-to-server hash")
+
+	// errCodeNotMatch is used when an expected response code is not
+	// returned.
+	errCodeNotMatch = errors.New("unexpected code")
+
+	// errTCNotStarted is used when we require the tor controller to be
+	// started while it's not.
+	errTCNotStarted = errors.New("tor controller must be started")
+
+	// errTCNotStarted is used when we require the tor controller to be
+	// not stopped while it is.
+	errTCStopped = errors.New("tor controller must not be stopped")
 )
 
 // Controller is an implementation of the Tor Control protocol. This is used in
@@ -100,6 +120,9 @@ type Controller struct {
 	// to connect to the LND node.  This is required when the Tor server
 	// runs on another host, otherwise the service will not be reachable.
 	targetIPAddress string
+
+	// activeServiceID is the Onion ServiceID created by ADD_ONION.
+	activeServiceID string
 }
 
 // NewController returns a new Tor controller that will be able to interact with
@@ -114,14 +137,16 @@ func NewController(controlAddr string, targetIPAddress string,
 	}
 }
 
-// Start establishes and authenticates the connection between the controller and
+// Start establishes and authenticates the connection between the controller
-// a Tor server. Once done, the controller will be able to send commands and
+// and a Tor server. Once done, the controller will be able to send commands
-// expect responses.
+// and expect responses.
 func (c *Controller) Start() error {
 	if !atomic.CompareAndSwapInt32(&c.started, 0, 1) {
 		return nil
 	}
 
+	log.Info("Starting tor controller")
+
 	conn, err := textproto.Dial("tcp", c.controlAddr)
 	if err != nil {
 		return fmt.Errorf("unable to connect to Tor server: %v", err)
@@ -138,23 +163,207 @@ func (c *Controller) Stop() error {
 		return nil
 	}
 
+	log.Info("Stopping tor controller")
+
+	// Remove the onion service.
+	if err := c.DelOnion(c.activeServiceID); err != nil {
+		log.Errorf("DEL_ONION got error: %v", err)
+		return err
+	}
+
+	// Reset service ID.
+	c.activeServiceID = ""
+
 	return c.conn.Close()
 }
 
+// Reconnect makes a new socket connection between the tor controller and
+// daemon. It will attempt to close the old connection, make a new connection
+// and authenticate, and finally reset the activeServiceID that the controller
+// is aware of.
+//
+// NOTE: Any old onion services will be removed once this function is called.
+// In the case of a Tor daemon restart, previously created onion services will
+// no longer be there. If the function is called without a Tor daemon restart,
+// because the control connection is reset, all the onion services belonging to
+// the old connection will be removed.
+func (c *Controller) Reconnect() error {
+	// Require the tor controller to be running when we want to reconnect.
+	// This means the started flag must be 1 and the stopped flag must be
+	// 0.
+	if c.started != 1 {
+		return errTCNotStarted
+	}
+	if c.stopped != 0 {
+		return errTCStopped
+	}
+
+	log.Info("Re-connectting tor controller")
+
+	// If we have an old connection, try to close it. We might receive an
+	// error if the connection has already been closed by Tor daemon(ie,
+	// daemon restarted), so we ignore the error here.
+	if c.conn != nil {
+		if err := c.conn.Close(); err != nil {
+			log.Debugf("closing old conn got err: %v", err)
+		}
+	}
+
+	// Make a new connection and authenticate.
+	conn, err := textproto.Dial("tcp", c.controlAddr)
+	if err != nil {
+		return fmt.Errorf("unable to connect to Tor server: %w", err)
+	}
+
+	c.conn = conn
+
+	// Authenticate the connection between the controller and Tor daemon.
+	if err := c.authenticate(); err != nil {
+		return err
+	}
+
+	// Reset the activeServiceID. This value would only be set if a
+	// previous onion service was created. Because the old connection has
+	// been closed at this point, the old onion service is no longer
+	// active.
+	c.activeServiceID = ""
+
+	return nil
+}
+
 // sendCommand sends a command to the Tor server and returns its response, as a
 // single space-delimited string, and code.
 func (c *Controller) sendCommand(command string) (int, string, error) {
-	if err := c.conn.Writer.PrintfLine(command); err != nil {
+	id, err := c.conn.Cmd(command)
+	if err != nil {
 		return 0, "", err
 	}
 
-	// We'll use ReadResponse as it has built-in support for multi-line
+	// Make sure our reader only process the response returned from the
-	// text protocol responses.
+	// above command.
-	code, reply, err := c.conn.Reader.ReadResponse(success)
+	c.conn.StartResponse(id)
+	defer c.conn.EndResponse(id)
+
+	code, reply, err := c.readResponse(success)
+	if err != nil {
+		log.Debugf("sendCommand:%s got err:%v, reply:%v",
+			command, err, reply)
+		return code, reply, err
+	}
+
+	return code, reply, nil
+}
+
+// readResponse reads the replies from Tor to the controller. The reply has the
+// following format,
+//
+//   Reply = SyncReply / AsyncReply
+//   SyncReply = *(MidReplyLine / DataReplyLine) EndReplyLine
+//   AsyncReply = *(MidReplyLine / DataReplyLine) EndReplyLine
+//
+//   MidReplyLine = StatusCode "-" ReplyLine
+//   DataReplyLine = StatusCode "+" ReplyLine CmdData
+//   EndReplyLine = StatusCode SP ReplyLine
+//   ReplyLine = [ReplyText] CRLF
+//   ReplyText = XXXX
+//   StatusCode = 3DIGIT
+//
+// Unless specified otherwise, multiple lines in a single reply from Tor daemon
+// to the controller are guaranteed to share the same status code. Read more on
+// this topic:
+//   https://gitweb.torproject.org/torspec.git/tree/control-spec.txt#n158
+//
+// NOTE: this code is influenced by https://github.com/Yawning/bulb.
+func (c *Controller) readResponse(expected int) (int, string, error) {
+	// Clean the buffer inside the conn. This is needed when we encountered
+	// an error while reading the response, the remaining lines need to be
+	// cleaned before next read.
+	defer func() {
+		if _, err := c.conn.R.Discard(c.conn.R.Buffered()); err != nil {
+			log.Errorf("clean read buffer failed: %v", err)
+		}
+	}()
+
+	reply, code := "", 0
+	hasMoreLines := true
+
+	for hasMoreLines {
+		line, err := c.conn.Reader.ReadLine()
+		if err != nil {
+			return 0, reply, err
+		}
+		log.Tracef("Reading line: %v", line)
+
+		// Line being shortter than 4 is not allowed.
+		if len(line) < 4 {
+			err = textproto.ProtocolError("short line: " + line)
+			return 0, reply, err
+		}
+
+		// Parse the status code.
+		code, err = strconv.Atoi(line[0:3])
 		if err != nil {
 			return code, reply, err
 		}
 
+		switch line[3] {
+		// EndReplyLine = StatusCode SP ReplyLine.
+		// Example: 250 OK
+		// This is the end of the response, so we mark hasMoreLines to
+		// be false to exit the loop.
+		case ' ':
+			reply += line[4:]
+			hasMoreLines = false
+
+		// MidReplyLine = StatusCode "-" ReplyLine.
+		// Example: 250-version=...
+		// This is a continued response, so we keep reading the next
+		// line.
+		case '-':
+			reply += line[4:]
+
+		// DataReplyLine = StatusCode "+" ReplyLine CmdData.
+		// Example: 250+config-text=
+		//	    line1
+		//	    line2
+		//          more lines...
+		//          .
+		// This is a data response, meaning the following multiple
+		// lines are the actual data, and a dot(.) in the end means the
+		// end of the data response. The response will be formatted as,
+		// 	key=line1,line2,...
+		// The above example will then be,
+		// 	config-text=line1,line2,...
+		case '+':
+			// Add the key(config-text=)
+			reply += line[4:]
+
+			// Add the values.
+			resp, err := c.conn.Reader.ReadDotLines()
+			if err != nil {
+				return code, reply, err
+			}
+			reply += strings.Join(resp, ",")
+
+		// Invalid line separator found.
+		default:
+			err = textproto.ProtocolError("invalid line: " + line)
+			return code, reply, err
+		}
+
+		// We check the code here so that the error message is parsed
+		// from the line.
+		if code != expected {
+			return code, reply, errCodeNotMatch
+		}
+
+		// Separate each line using "\n".
+		if hasMoreLines {
+			reply += "\n"
+		}
+	}
+
+	log.Tracef("Parsed reply: %v", reply)
 	return code, reply, nil
 }
 
@@ -196,6 +405,8 @@ func (c *Controller) authenticate() error {
 		return err
 	}
 
+	log.Debugf("received protocol info: %v", protocolInfo)
+
 	// With the version retrieved, we'll cache it now in case it needs to be
 	// used later on.
 	c.version = protocolInfo.version()

tor/controller_test.go

@@ -1,6 +1,12 @@
 package tor
 
-import "testing"
+import (
+	"net"
+	"net/textproto"
+	"testing"
+
+	"github.com/stretchr/testify/require"
+)
 
 // TestParseTorVersion is a series of tests for different version strings that
 // check the correctness of determining whether they support creating v3 onion
@@ -74,3 +80,283 @@ func TestParseTorVersion(t *testing.T) {
 		}
 	}
 }
+
+// testProxy emulates a Tor daemon and contains the info used for the tor
+// controller to make connections.
+type testProxy struct {
+	// server is the proxy listener.
+	server net.Listener
+
+	// serverConn is the established connection from the server side.
+	serverConn net.Conn
+
+	// serverAddr is the tcp address the proxy is listening on.
+	serverAddr string
+
+	// clientConn is the established connection from the client side.
+	clientConn *textproto.Conn
+}
+
+// cleanUp is used after each test to properly close the ports/connections.
+func (tp *testProxy) cleanUp() {
+	// Don't bother cleanning if there's no a server created.
+	if tp.server == nil {
+		return
+	}
+
+	if err := tp.clientConn.Close(); err != nil {
+		log.Errorf("closing client conn got err: %v", err)
+	}
+	if err := tp.server.Close(); err != nil {
+		log.Errorf("closing proxy server got err: %v", err)
+	}
+}
+
+// createTestProxy creates a proxy server to listen on a random address,
+// creates a server and a client connection, and initializes a testProxy using
+// these params.
+func createTestProxy(t *testing.T) *testProxy {
+	// Set up the proxy to listen on given port.
+	//
+	// NOTE: we use a port 0 here to indicate we want a free port selected
+	// by the system.
+	proxy, err := net.Listen("tcp", ":0")
+	require.NoError(t, err, "failed to create proxy")
+
+	t.Logf("created proxy server to listen on address: %v", proxy.Addr())
+
+	// Accept the connection inside a goroutine.
+	serverChan := make(chan net.Conn, 1)
+	go func(result chan net.Conn) {
+		conn, err := proxy.Accept()
+		require.NoError(t, err, "failed to accept")
+
+		result <- conn
+	}(serverChan)
+
+	// Create the connection using tor controller.
+	client, err := textproto.Dial("tcp", proxy.Addr().String())
+	require.NoError(t, err, "failed to create connection")
+
+	tc := &testProxy{
+		server:     proxy,
+		serverConn: <-serverChan,
+		serverAddr: proxy.Addr().String(),
+		clientConn: client,
+	}
+
+	return tc
+}
+
+// TestReadResponse contructs a series of possible responses returned by Tor
+// and asserts the readResponse can handle them correctly.
+func TestReadResponse(t *testing.T) {
+	// Create mock server and client connection.
+	proxy := createTestProxy(t)
+	defer proxy.cleanUp()
+	server := proxy.serverConn
+
+	// Create a dummy tor controller.
+	c := &Controller{conn: proxy.clientConn}
+
+	testCase := []struct {
+		name       string
+		serverResp string
+
+		// expectedReply is the reply we expect the readResponse to
+		// return.
+		expectedReply string
+
+		// expectedCode is the code we expect the server to return.
+		expectedCode int
+
+		// returnedCode is the code we expect the readResponse to
+		// return.
+		returnedCode int
+
+		// expectedErr is the error we expect the readResponse to
+		// return.
+		expectedErr error
+	}{
+		{
+			// Test a simple response.
+			name:          "succeed on 250",
+			serverResp:    "250 OK\n",
+			expectedReply: "OK",
+			expectedCode:  250,
+			returnedCode:  250,
+			expectedErr:   nil,
+		},
+		{
+			// Test a mid reply(-) response.
+			name: "succeed on mid reply line",
+			serverResp: "250-field=value\n" +
+				"250 OK\n",
+			expectedReply: "field=value\nOK",
+			expectedCode:  250,
+			returnedCode:  250,
+			expectedErr:   nil,
+		},
+		{
+			// Test a data reply(+) response.
+			name: "succeed on data reply line",
+			serverResp: "250+field=\n" +
+				"line1\n" +
+				"line2\n" +
+				".\n" +
+				"250 OK\n",
+			expectedReply: "field=line1,line2\nOK",
+			expectedCode:  250,
+			returnedCode:  250,
+			expectedErr:   nil,
+		},
+		{
+			// Test a mixed reply response.
+			name: "succeed on mixed reply line",
+			serverResp: "250-field=value\n" +
+				"250+field=\n" +
+				"line1\n" +
+				"line2\n" +
+				".\n" +
+				"250 OK\n",
+			expectedReply: "field=value\nfield=line1,line2\nOK",
+			expectedCode:  250,
+			returnedCode:  250,
+			expectedErr:   nil,
+		},
+		{
+			// Test unexpected code.
+			name:          "fail on codes not matched",
+			serverResp:    "250 ERR\n",
+			expectedReply: "ERR",
+			expectedCode:  500,
+			returnedCode:  250,
+			expectedErr:   errCodeNotMatch,
+		},
+		{
+			// Test short response error.
+			name:          "fail on short response",
+			serverResp:    "123\n250 OK\n",
+			expectedReply: "",
+			expectedCode:  250,
+			returnedCode:  0,
+			expectedErr: textproto.ProtocolError(
+				"short line: 123"),
+		},
+		{
+			// Test short response error.
+			name:          "fail on invalid response",
+			serverResp:    "250?OK\n",
+			expectedReply: "",
+			expectedCode:  250,
+			returnedCode:  250,
+			expectedErr: textproto.ProtocolError(
+				"invalid line: 250?OK"),
+		},
+	}
+
+	for _, tc := range testCase {
+		tc := tc
+		t.Run(tc.name, func(t *testing.T) {
+
+			// Let the server mocks a given response.
+			_, err := server.Write([]byte(tc.serverResp))
+			require.NoError(t, err, "server failed to write")
+
+			// Read the response and checks all expectations
+			// satisfied.
+			code, reply, err := c.readResponse(tc.expectedCode)
+			require.Equal(t, tc.expectedErr, err)
+			require.Equal(t, tc.returnedCode, code)
+			require.Equal(t, tc.expectedReply, reply)
+
+			// Check that the read buffer is cleaned.
+			require.Zero(t, c.conn.R.Buffered(),
+				"read buffer not empty")
+		})
+	}
+}
+
+// TestReconnectTCMustBeRunning checks that the tor controller must be running
+// while calling Reconnect.
+func TestReconnectTCMustBeRunning(t *testing.T) {
+	// Create a dummy controller.
+	c := &Controller{}
+
+	// Reconnect should fail because the TC is not started.
+	require.Equal(t, errTCNotStarted, c.Reconnect())
+
+	// Set the started flag.
+	c.started = 1
+
+	// Set the stopped flag so the TC is stopped.
+	c.stopped = 1
+
+	// Reconnect should fail because the TC is stopped.
+	require.Equal(t, errTCStopped, c.Reconnect())
+}
+
+// TestReconnectSucceed tests a reconnection will succeed when the tor
+// controller is up and running.
+func TestReconnectSucceed(t *testing.T) {
+	// Create mock server and client connection.
+	proxy := createTestProxy(t)
+	defer proxy.cleanUp()
+
+	// Create a tor controller and mark the controller as started.
+	c := &Controller{
+		conn:        proxy.clientConn,
+		started:     1,
+		controlAddr: proxy.serverAddr,
+	}
+
+	// Accept the connection inside a goroutine. We will also write some
+	// data so that the reconnection can succeed. We will mock three writes
+	// and two reads inside our proxy server,
+	//   - write protocol info
+	//   - read auth info
+	//   - write auth challenge
+	//   - read auth challenge
+	//   - write OK
+	go func() {
+		// Accept the new connection.
+		server, err := proxy.server.Accept()
+		require.NoError(t, err, "failed to accept")
+
+		// Write the protocol info.
+		resp := "250-PROTOCOLINFO 1\n" +
+			"250-AUTH METHODS=NULL\n" +
+			"250 OK\n"
+		_, err = server.Write([]byte(resp))
+		require.NoErrorf(t, err, "failed to write protocol info")
+
+		// Read the auth info from the client.
+		buf := make([]byte, 65535)
+		_, err = server.Read(buf)
+		require.NoError(t, err)
+
+		// Write the auth challenge.
+		resp = "250 AUTHCHALLENGE SERVERHASH=fake\n"
+		_, err = server.Write([]byte(resp))
+		require.NoErrorf(t, err, "failed to write auth challenge")
+
+		// Read the auth challenge resp from the client.
+		_, err = server.Read(buf)
+		require.NoError(t, err)
+
+		// Write OK resp.
+		resp = "250 OK\n"
+		_, err = server.Write([]byte(resp))
+		require.NoErrorf(t, err, "failed to write response auth")
+	}()
+
+	// Reconnect should succeed.
+	require.NoError(t, c.Reconnect())
+
+	// Check that the old connection is closed.
+	_, err := proxy.clientConn.ReadLine()
+	require.Contains(t, err.Error(), "use of closed network connection")
+
+	// Check that the connection has been updated.
+	require.NotEqual(t, proxy.clientConn, c.conn)
+}

tor/log.go

@@ -0,0 +1,26 @@
+package tor
+
+import (
+	"github.com/btcsuite/btclog"
+)
+
+// Subsystem defines the logging code for this subsystem.
+const Subsystem = "TORC" // TORC as in Tor Controller.
+
+// log is a logger that is initialized with no output filters.  This
+// means the package will not perform any logging by default until the caller
+// requests it.
+var log = btclog.Disabled
+
+// DisableLog disables all library log output.  Logging output is disabled
+// by default until UseLogger is called.
+func DisableLog() {
+	UseLogger(btclog.Disabled)
+}
+
+// UseLogger uses a specified Logger to output package logging info.
+// This should be used in preference to SetLogWriter if the caller is also
+// using btclog.
+func UseLogger(logger btclog.Logger) {
+	log = logger
+}

tor/tor.go

@@ -66,14 +66,15 @@ func (c *proxyConn) RemoteAddr() net.Addr {
 // around net.Conn in order to expose the actual remote address we're dialing,
 // rather than the proxy's address.
 func Dial(address, socksAddr string, streamIsolation bool,
-	skipProxyForClearNetTargets bool, timeout time.Duration) (net.Conn, error) {
+	skipProxyForClearNetTargets bool,
+	timeout time.Duration) (net.Conn, error) {
 
-	conn, err := dial(
+	conn, err := dialProxy(
 		address, socksAddr, streamIsolation,
 		skipProxyForClearNetTargets, timeout,
 	)
 	if err != nil {
-		return nil, err
+		return nil, fmt.Errorf("dial proxy failed: %w", err)
 	}
 
 	// Now that the connection is established, we'll create our internal
@@ -90,7 +91,7 @@ func Dial(address, socksAddr string, streamIsolation bool,
 	}, nil
 }
 
-// dial establishes a connection to the address via the provided TOR SOCKS
+// dialProxy establishes a connection to the address via the provided TOR SOCKS
 // proxy. Only TCP traffic may be routed via Tor.
 //
 // streamIsolation determines if we should force stream isolation for this new
@@ -100,8 +101,9 @@ func Dial(address, socksAddr string, streamIsolation bool,
 // skipProxyForClearNetTargets argument allows the dialer to directly connect
 // to the provided address if it does not represent an union service, skipping
 // the SOCKS proxy.
-func dial(address, socksAddr string, streamIsolation bool,
+func dialProxy(address, socksAddr string, streamIsolation bool,
-	skipProxyForClearNetTargets bool, timeout time.Duration) (net.Conn, error) {
+	skipProxyForClearNetTargets bool,
+	timeout time.Duration) (net.Conn, error) {
 
 	// If we were requested to force stream isolation for this connection,
 	// we'll populate the authentication credentials with random data as
@@ -136,7 +138,7 @@ func dial(address, socksAddr string, streamIsolation bool,
 	// Establish the connection through Tor's SOCKS proxy.
 	dialer, err := proxy.SOCKS5("tcp", socksAddr, auth, clearDialer)
 	if err != nil {
-		return nil, err
+		return nil, fmt.Errorf("establish sock proxy: %w", err)
 	}
 
 	return dialer.Dial("tcp", address)
@@ -163,7 +165,7 @@ func LookupSRV(service, proto, name, socksAddr,
 	timeout time.Duration) (string, []*net.SRV, error) {
 
 	// Connect to the DNS server we'll be using to query SRV records.
-	conn, err := dial(
+	conn, err := dialProxy(
 		dnsServer, socksAddr, streamIsolation,
 		skipProxyForClearNetTargets, timeout,
 	)