Merge pull request #853 from hashicorp/issue-850

Make TokenHelper an interface and split exisiting functionality

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] == "--" {