tor: patch unit tests for tor controller

tor/add_onion.go

@@ -105,19 +105,9 @@ 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) {
-	// 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.
-	if cfg.Type == V3 {
-		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,
@@ -125,6 +115,7 @@ func (c *Controller) AddOnion(cfg AddOnionConfig) (*OnionAddr, error) {
 	// private key.
 	var keyParam string
 	switch cfg.Type {
+	// TODO(yy): drop support for v2.
 	case V2:
 		keyParam = "NEW:RSA1024"
 	case V3:
@@ -142,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.
@@ -174,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

tor/add_onion_test.go

@@ -2,9 +2,13 @@ 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
@@ -49,3 +53,160 @@ func TestOnionFile(t *testing.T) {
 		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)
+}