Make TokenHelper an interface and split exisiting functionality

Functionality is split into ExternalTokenHelper, which is used if a path
is given in a configuration file, and InternalTokenHelper which is used
otherwise. The internal helper no longer shells out to the same Vault
binary, instead performing the same actions with internal code. This
avoids problems using dev mode when there are spaces in paths or when
the binary is built in a container without a shell.

Fixes #850 among others

builtin/token/disk/command.go

@@ -1,100 +0,0 @@
-package disk
-
-import (
-	"flag"
-	"fmt"
-	"io"
-	"os"
-	"strings"
-
-	"github.com/mitchellh/go-homedir"
-)
-
-// DefaultPath is the default path where the Vault token is stored.
-const DefaultPath = "~/.vault-token"
-
-type Command struct {
-	Path string
-}
-
-func (c *Command) Run(args []string) int {
-	var path string
-	pathDefault := DefaultPath
-	if c.Path != "" {
-		pathDefault = c.Path
-	}
-
-	f := flag.NewFlagSet("token-disk", flag.ContinueOnError)
-	f.StringVar(&path, "path", pathDefault, "")
-	f.Usage = func() { fmt.Fprintf(os.Stderr, c.Help()+"\n") }
-	if err := f.Parse(args); err != nil {
-		fmt.Fprintf(os.Stderr, "\n%s\n", err)
-		return 1
-	}
-
-	path, err := homedir.Expand(path)
-	if err != nil {
-		fmt.Fprintf(os.Stderr, "Error expanding directory: %s\n", err)
-		return 1
-	}
-
-	args = f.Args()
-	switch args[0] {
-	case "get":
-		f, err := os.Open(path)
-		if os.IsNotExist(err) {
-			return 0
-		}
-		if err != nil {
-			fmt.Fprintf(os.Stderr, "%s\n", err)
-			return 1
-		}
-		defer f.Close()
-
-		if _, err := io.Copy(os.Stdout, f); err != nil {
-			fmt.Fprintf(os.Stderr, "%s\n", err)
-			return 1
-		}
-	case "store":
-		f, err := os.OpenFile(path, os.O_CREATE|os.O_TRUNC|os.O_WRONLY, 0600)
-		if err != nil {
-			fmt.Fprintf(os.Stderr, "%s\n", err)
-			return 1
-		}
-		defer f.Close()
-
-		if _, err := io.Copy(f, os.Stdin); err != nil {
-			fmt.Fprintf(os.Stderr, "%s\n", err)
-			return 1
-		}
-	case "erase":
-		if err := os.Remove(path); err != nil && !os.IsNotExist(err) {
-			fmt.Fprintf(os.Stderr, "%s\n", err)
-			return 1
-		}
-	default:
-		fmt.Fprintf(os.Stderr, "Error: unknown subcommand: %s\n", args[0])
-		return 1
-	}
-
-	return 0
-}
-
-func (c *Command) Synopsis() string {
-	return "Stores Vault tokens on disk"
-}
-
-func (c *Command) Help() string {
-	helpText := `
-Usage: vault token-disk [options] [operation]
-
-  Vault token helper (see vault config "token_helper") that writes
-  authenticated tokens to disk unencrypted.
-
-Options:
-
-  -path=path      Path to store the token.
-
-`
-	return strings.TrimSpace(helpText)
-}

builtin/token/disk/command_test.go

@@ -1,15 +0,0 @@
-package disk
-
-import (
-	"testing"
-
-	"github.com/hashicorp/vault/command/token"
-)
-
-func TestCommand(t *testing.T) {
-	token.TestProcess(t)
-}
-
-func TestHelperProcess(t *testing.T) {
-	token.TestHelperProcessCLI(t, new(Command))
-}

cli/commands.go

@@ -25,7 +25,6 @@ import (
 	"github.com/hashicorp/vault/builtin/logical/transit"
 
 	"github.com/hashicorp/vault/audit"
-	tokenDisk "github.com/hashicorp/vault/builtin/token/disk"
 	"github.com/hashicorp/vault/command"
 	"github.com/hashicorp/vault/logical"
 	"github.com/mitchellh/cli"
@@ -275,11 +274,6 @@ func Commands(metaPtr *command.Meta) map[string]cli.CommandFactory {
 				Ui:          meta.Ui,
 			}, nil
 		},
-
-		// The commands below are hidden from the help output
-		"token-disk": func() (cli.Command, error) {
-			return &tokenDisk.Command{}, nil
-		},
 	}
 }
 

command/auth_test.go

@@ -10,8 +10,6 @@ import (
 	"testing"
 
 	"github.com/hashicorp/vault/api"
-	tokenDisk "github.com/hashicorp/vault/builtin/token/disk"
-	"github.com/hashicorp/vault/command/token"
 	"github.com/hashicorp/vault/http"
 	"github.com/hashicorp/vault/vault"
 	"github.com/mitchellh/cli"
@@ -188,14 +186,10 @@ func testAuthInit(t *testing.T) {
 
 	// Write a .vault config to use our custom token helper
 	config := fmt.Sprintf(
-		"token_helper = \"%s\"\n", token.TestProcessPath(t))
+		"token_helper = \"\"\n")
 	ioutil.WriteFile(filepath.Join(td, ".vault"), []byte(config), 0644)
 }
 
-func TestHelperProcess(t *testing.T) {
-	token.TestHelperProcessCLI(t, &tokenDisk.Command{})
-}
-
 type testAuthHandler struct{}
 
 func (h *testAuthHandler) Auth(c *api.Client, m map[string]string) (string, error) {

command/config.go

@@ -23,8 +23,8 @@ const (
 type Config struct {
 	// TokenHelper is the executable/command that is executed for storing
 	// and retrieving the authentication token for the Vault CLI. If this
-	// is not specified, then vault token-disk will be used, which stores
+	// is not specified, then vault's internal token store will be used, which
-	// the token on disk unencrypted.
+	// stores the token on disk unencrypted.
 	TokenHelper string `hcl:"token_helper"`
 }
 

command/meta.go

@@ -194,7 +194,7 @@ func (m *Meta) FlagSet(n string, fs FlagSetFlags) *flag.FlagSet {
 }
 
 // TokenHelper returns the token helper that is configured for Vault.
-func (m *Meta) TokenHelper() (*token.Helper, error) {
+func (m *Meta) TokenHelper() (token.TokenHelper, error) {
 	config, err := m.Config()
 	if err != nil {
 		return nil, err
@@ -202,11 +202,14 @@ func (m *Meta) TokenHelper() (*token.Helper, error) {
 
 	path := config.TokenHelper
 	if path == "" {
-		path = "disk"
+		return &token.InternalTokenHelper{}, nil
 	}
 
-	path = token.HelperPath(path)
+	path, err = token.ExternalTokenHelperPath(path)
-	return &token.Helper{Path: path}, nil
+	if err != nil {
+		return nil, err
+	}
+	return &token.ExternalTokenHelper{BinaryPath: path}, nil
 }
 
 func (m *Meta) loadCACert(path string) (*x509.CertPool, error) {

command/token/helper.go

@@ -1,132 +1,14 @@
 package token
 
-import (
+// TokenHelper is an interface that contains basic operations that must be
-	"bytes"
+// implemented by a token helper
-	"fmt"
+type TokenHelper interface {
-	"os"
+	// Path displays a backend-specific path; for the internal helper this
-	"os/exec"
+	// is the location of the token stored on disk; for the external helper
-	"path/filepath"
+	// this is the location of the binary being invoked
-	"runtime"
+	Path() string
-	"strings"
 
-	"github.com/kardianos/osext"
+	Erase() error
-)
+	Get() (string, error)
-
+	Store(string) error
-var exePath string
-
-func init() {
-	var err error
-	exePath, err = osext.Executable()
-	if err != nil {
-		panic("failed to detect self path: " + err.Error())
-	}
-}
-
-// HelperPath takes the configured path to a helper and expands it to
-// a full absolute path that can be executed. If the path is relative then
-// a prefix of "vault token-" will be prepended to the path.
-func HelperPath(path string) string {
-	space := strings.Index(path, " ")
-	if space == -1 {
-		space = len(path)
-	}
-
-	// Get the binary name. If it isn't absolute, prepend "vault token-"
-	binary := path[0:space]
-	if !filepath.IsAbs(binary) {
-		binary = exePath + " token-" + binary
-	}
-
-	// Return the resulting string
-	return fmt.Sprintf("%s%s", binary, path[space:])
-}
-
-// Helper is the struct that has all the logic for storing and retrieving
-// tokens from the token helper. The API for the helpers is simple: the
-// Path is executed within a shell with environment Env. The last argument
-// appended will be the operation, which is:
-//
-//   * "get" - Read the value of the token and write it to stdout.
-//   * "store" - Store the value of the token which is on stdin. Output
-//       nothing.
-//   * "erase" - Erase the contents stored. Output nothing.
-//
-// Any errors can be written on stdout. If the helper exits with a non-zero
-// exit code then the stderr will be made part of the error value.
-type Helper struct {
-	Path string
-	Env  []string
-}
-
-// Erase deletes the contents from the helper.
-func (h *Helper) Erase() error {
-	cmd, err := h.cmd("erase")
-	if err != nil {
-		return fmt.Errorf("Error: %s", err)
-	}
-	if output, err := cmd.CombinedOutput(); err != nil {
-		return fmt.Errorf(
-			"Error: %s\n\n%s", err, string(output))
-	}
-	return nil
-}
-
-// Get gets the token value from the helper.
-func (h *Helper) Get() (string, error) {
-	var buf, stderr bytes.Buffer
-	cmd, err := h.cmd("get")
-	if err != nil {
-		return "", fmt.Errorf("Error: %s", err)
-	}
-	cmd.Stdout = &buf
-	cmd.Stderr = &stderr
-	if err := cmd.Run(); err != nil {
-		return "", fmt.Errorf(
-			"Error: %s\n\n%s", err, stderr.String())
-	}
-
-	return buf.String(), nil
-}
-
-// Store stores the token value into the helper.
-func (h *Helper) Store(v string) error {
-	buf := bytes.NewBufferString(v)
-	cmd, err := h.cmd("store")
-	if err != nil {
-		return fmt.Errorf("Error: %s", err)
-	}
-	cmd.Stdin = buf
-	if output, err := cmd.CombinedOutput(); err != nil {
-		return fmt.Errorf(
-			"Error: %s\n\n%s", err, string(output))
-	}
-
-	return nil
-}
-
-func (h *Helper) cmd(op string) (*exec.Cmd, error) {
-	script := strings.Replace(h.Path, "\\", "\\\\", -1) + " " + op
-	cmd, err := ExecScript(script)
-	if err != nil {
-		return nil, err
-	}
-	cmd.Env = h.Env
-	return cmd, nil
-}
-
-// ExecScript returns a command to execute a script
-func ExecScript(script string) (*exec.Cmd, error) {
-	var shell, flag string
-	if runtime.GOOS == "windows" {
-		shell = "cmd"
-		flag = "/C"
-	} else {
-		shell = "/bin/sh"
-		flag = "-c"
-	}
-	if other := os.Getenv("SHELL"); other != "" {
-		shell = other
-	}
-	cmd := exec.Command(shell, flag, script)
-	return cmd, nil
 }

command/token/helper_external.go

@@ -0,0 +1,131 @@
+package token
+
+import (
+	"bytes"
+	"fmt"
+	"os"
+	"os/exec"
+	"path/filepath"
+	"runtime"
+	"strings"
+)
+
+// ExternalTokenHelperPath takes the configured path to a helper and expands it to
+// a full absolute path that can be executed. As of 0.5, the default token
+// helper is internal, to avoid problems running in dev mode (see GH-850 and
+// GH-783), so special assumptions of prepending "vault token-" no longer
+// apply.
+//
+// As an additional result, only absolute paths are now allowed. Looking in the
+// path or a current directory for an arbitrary executable could allow someone
+// to switch the expected binary for one further up the path (or in the current
+// directory), potentially opening up execution of an arbitrary binary.
+func ExternalTokenHelperPath(path string) (string, error) {
+	if !filepath.IsAbs(path) {
+		var err error
+		path, err = filepath.Abs(path)
+		if err != nil {
+			return "", err
+		}
+	}
+
+	if _, err := os.Stat(path); err != nil {
+		return path, nil
+	}
+
+	return "", fmt.Errorf("unknown error getting the external helper path")
+}
+
+// ExternalTokenHelper is the struct that has all the logic for storing and retrieving
+// tokens from the token helper. The API for the helpers is simple: the
+// BinaryPath is executed within a shell with environment Env. The last argument
+// appended will be the operation, which is:
+//
+//   * "get" - Read the value of the token and write it to stdout.
+//   * "store" - Store the value of the token which is on stdin. Output
+//       nothing.
+//   * "erase" - Erase the contents stored. Output nothing.
+//
+// Any errors can be written on stdout. If the helper exits with a non-zero
+// exit code then the stderr will be made part of the error value.
+type ExternalTokenHelper struct {
+	BinaryPath string
+	Env        []string
+}
+
+// Erase deletes the contents from the helper.
+func (h *ExternalTokenHelper) Erase() error {
+	cmd, err := h.cmd("erase")
+	if err != nil {
+		return fmt.Errorf("Error: %s", err)
+	}
+	if output, err := cmd.CombinedOutput(); err != nil {
+		return fmt.Errorf(
+			"Error: %s\n\n%s", err, string(output))
+	}
+	return nil
+}
+
+// Get gets the token value from the helper.
+func (h *ExternalTokenHelper) Get() (string, error) {
+	var buf, stderr bytes.Buffer
+	cmd, err := h.cmd("get")
+	if err != nil {
+		return "", fmt.Errorf("Error: %s", err)
+	}
+	cmd.Stdout = &buf
+	cmd.Stderr = &stderr
+	if err := cmd.Run(); err != nil {
+		return "", fmt.Errorf(
+			"Error: %s\n\n%s", err, stderr.String())
+	}
+
+	return buf.String(), nil
+}
+
+// Store stores the token value into the helper.
+func (h *ExternalTokenHelper) Store(v string) error {
+	buf := bytes.NewBufferString(v)
+	cmd, err := h.cmd("store")
+	if err != nil {
+		return fmt.Errorf("Error: %s", err)
+	}
+	cmd.Stdin = buf
+	if output, err := cmd.CombinedOutput(); err != nil {
+		return fmt.Errorf(
+			"Error: %s\n\n%s", err, string(output))
+	}
+
+	return nil
+}
+
+func (h *ExternalTokenHelper) Path() string {
+	return h.BinaryPath
+}
+
+func (h *ExternalTokenHelper) cmd(op string) (*exec.Cmd, error) {
+	script := strings.Replace(h.BinaryPath, "\\", "\\\\", -1) + " " + op
+	cmd, err := ExecScript(script)
+	if err != nil {
+		return nil, err
+	}
+	cmd.Env = h.Env
+	return cmd, nil
+}
+
+// ExecScript returns a command to execute a script
+func ExecScript(script string) (*exec.Cmd, error) {
+	var shell, flag string
+	if runtime.GOOS == "windows" {
+		shell = "cmd"
+		flag = "/C"
+	} else {
+		shell = "/bin/sh"
+		flag = "-c"
+	}
+	if other := os.Getenv("SHELL"); other != "" {
+		shell = other
+	}
+	cmd := exec.Command(shell, flag, script)
+	return cmd, nil
+}

command/token/helper_external_test.go

@@ -10,16 +10,13 @@ import (
 	"testing"
 )
 
-func TestHelperPath(t *testing.T) {
+func TestExternalTokenHelperPath(t *testing.T) {
-	cases := map[string]string{
+	cases := map[string]string{}
-		"foo": exePath + " token-foo",
-	}
 
 	unixCases := map[string]string{
 		"/foo": "/foo",
 	}
 	windowsCases := map[string]string{
-		"/foo":             exePath + " token-/foo",
 		"C:/foo":           "C:/foo",
 		`C:\Program Files`: `C:\Program Files`,
 	}
@@ -36,7 +33,10 @@ func TestHelperPath(t *testing.T) {
 	}
 
 	for k, v := range cases {
-		actual := HelperPath(k)
+		actual, err := ExternalTokenHelperPath(k)
+		if err != nil {
+			t.Fatalf("error getting external helper path: %v", err)
+		}
 		if actual != v {
 			t.Fatalf(
 				"input: %s, expected: %s, got: %s",
@@ -45,16 +45,16 @@ func TestHelperPath(t *testing.T) {
 	}
 }
 
-func TestHelper(t *testing.T) {
+func TestExternalTokenHelper(t *testing.T) {
-	Test(t, testHelper(t))
+	Test(t, testExternalTokenHelper(t))
 }
 
-func testHelper(t *testing.T) *Helper {
+func testExternalTokenHelper(t *testing.T) *ExternalTokenHelper {
-	return &Helper{Path: helperPath("helper"), Env: helperEnv()}
+	return &ExternalTokenHelper{BinaryPath: helperPath("helper"), Env: helperEnv()}
 }
 
 func helperPath(s ...string) string {
-	cs := []string{"-test.run=TestHelperProcess", "--"}
+	cs := []string{"-test.run=TestExternalTokenHelperProcess", "--"}
 	cs = append(cs, s...)
 	return fmt.Sprintf(
 		"%s %s",
@@ -76,7 +76,7 @@ func helperEnv() []string {
 }
 
 // This is not a real test. This is just a helper process kicked off by tests.
-func TestHelperProcess(*testing.T) {
+func TestExternalTokenHelperProcess(*testing.T) {
 	if os.Getenv("GO_WANT_HELPER_PROCESS") != "1" {
 		return
 	}

command/token/helper_internal.go

@@ -0,0 +1,77 @@
+package token
+
+import (
+	"bytes"
+	"fmt"
+	"io"
+	"os"
+
+	"github.com/mitchellh/go-homedir"
+)
+
+// InternalTokenHelper fulfills the TokenHelper interface when no external
+// token-helper is configured, and avoids shelling out
+type InternalTokenHelper struct {
+	tokenPath string
+}
+
+// populateTokenPath figures out the token path using homedir to get the user's
+// home directory
+func (i *InternalTokenHelper) populateTokenPath() {
+	homePath, err := homedir.Dir()
+	if err != nil {
+		panic(fmt.Errorf("error getting user's home directory: %v", err))
+	}
+	i.tokenPath = homePath + "/.vault-token"
+}
+
+func (i *InternalTokenHelper) Path() string {
+	return i.tokenPath
+}
+
+// Get gets the value of the stored token, if any
+func (i *InternalTokenHelper) Get() (string, error) {
+	i.populateTokenPath()
+	f, err := os.Open(i.tokenPath)
+	if os.IsNotExist(err) {
+		return "", nil
+	}
+	if err != nil {
+		return "", err
+	}
+	defer f.Close()
+
+	buf := bytes.NewBuffer(nil)
+	if _, err := io.Copy(buf, f); err != nil {
+		return "", err
+	}
+
+	return buf.String(), nil
+}
+
+// Store stores the value of the token to the file
+func (i *InternalTokenHelper) Store(input string) error {
+	i.populateTokenPath()
+	f, err := os.OpenFile(i.tokenPath, os.O_CREATE|os.O_TRUNC|os.O_WRONLY, 0600)
+	if err != nil {
+		return err
+	}
+	defer f.Close()
+
+	buf := bytes.NewBufferString(input)
+	if _, err := io.Copy(f, buf); err != nil {
+		return err
+	}
+
+	return nil
+}
+
+// Erase erases the value of the token
+func (i *InternalTokenHelper) Erase() error {
+	i.populateTokenPath()
+	if err := os.Remove(i.tokenPath); err != nil && !os.IsNotExist(err) {
+		return err
+	}
+
+	return nil
+}

command/token/helper_internal_test.go

@@ -0,0 +1,11 @@
+package token
+
+import (
+	"testing"
+)
+
+// TestCommand re-uses the existing Test function to ensure proper behavior of
+// the internal token helper
+func TestCommand(t *testing.T) {
+	Test(t, &InternalTokenHelper{})
+}

command/token/testing.go

@@ -11,7 +11,7 @@ import (
 
 // Test is a public function that can be used in other tests to
 // test that a helper is functioning properly.
-func Test(t *testing.T, h *Helper) {
+func Test(t *testing.T, h TokenHelper) {
 	if err := h.Store("foo"); err != nil {
 		t.Fatalf("err: %s", err)
 	}
@@ -40,16 +40,16 @@ func Test(t *testing.T, h *Helper) {
 }
 
 // TestProcess is used to re-execute this test in order to use it as the
-// helper process. For this to work, the TestHelperProcess function must
+// helper process. For this to work, the TestExternalTokenHelperProcess function must
 // exist.
 func TestProcess(t *testing.T, s ...string) {
-	h := &Helper{Path: TestProcessPath(t, s...)}
+	h := &ExternalTokenHelper{BinaryPath: TestProcessPath(t, s...)}
 	Test(t, h)
 }
 
 // TestProcessPath returns the path to the test process.
 func TestProcessPath(t *testing.T, s ...string) string {
-	cs := []string{"-test.run=TestHelperProcess", "--", "GO_WANT_HELPER_PROCESS"}
+	cs := []string{"-test.run=TestExternalTokenHelperProcess", "--", "GO_WANT_HELPER_PROCESS"}
 	cs = append(cs, s...)
 	return fmt.Sprintf(
 		"%s %s",
@@ -57,9 +57,9 @@ func TestProcessPath(t *testing.T, s ...string) string {
 		strings.Join(cs, " "))
 }
 
-// TestHelperProcessCLI can be called to implement TestHelperProcess
+// TestExternalTokenHelperProcessCLI can be called to implement TestExternalTokenHelperProcess
 // for TestProcess that just executes a CLI command.
-func TestHelperProcessCLI(t *testing.T, cmd cli.Command) {
+func TestExternalTokenHelperProcessCLI(t *testing.T, cmd cli.Command) {
 	args := os.Args
 	for len(args) > 0 {
 		if args[0] == "--" {