From c15974cbdd7f23301698ce911f2dc589c5770e3e Mon Sep 17 00:00:00 2001 From: Jeff Mitchell Date: Mon, 14 Dec 2015 16:23:04 -0500 Subject: [PATCH] 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 | 100 ------------- builtin/token/disk/command_test.go | 15 -- cli/commands.go | 6 - command/auth_test.go | 8 +- command/config.go | 4 +- command/meta.go | 11 +- command/token/helper.go | 138 ++---------------- command/token/helper_external.go | 131 +++++++++++++++++ ...helper_test.go => helper_external_test.go} | 24 +-- command/token/helper_internal.go | 77 ++++++++++ command/token/helper_internal_test.go | 11 ++ command/token/testing.go | 12 +- 12 files changed, 257 insertions(+), 280 deletions(-) delete mode 100644 builtin/token/disk/command.go delete mode 100644 builtin/token/disk/command_test.go create mode 100644 command/token/helper_external.go rename command/token/{helper_test.go => helper_external_test.go} (77%) create mode 100644 command/token/helper_internal.go create mode 100644 command/token/helper_internal_test.go diff --git a/builtin/token/disk/command.go b/builtin/token/disk/command.go deleted file mode 100644 index b26b083d8d..0000000000 --- a/builtin/token/disk/command.go +++ /dev/null @@ -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) -} diff --git a/builtin/token/disk/command_test.go b/builtin/token/disk/command_test.go deleted file mode 100644 index 0263141f60..0000000000 --- a/builtin/token/disk/command_test.go +++ /dev/null @@ -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)) -} diff --git a/cli/commands.go b/cli/commands.go index f8dc707df5..8fa09dbf4b 100644 --- a/cli/commands.go +++ b/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 - }, } } diff --git a/command/auth_test.go b/command/auth_test.go index 563d400027..05781f4ce3 100644 --- a/command/auth_test.go +++ b/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) { diff --git a/command/config.go b/command/config.go index 95342bf85b..73b0a4a9c1 100644 --- a/command/config.go +++ b/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 - // the token on disk unencrypted. + // is not specified, then vault's internal token store will be used, which + // stores the token on disk unencrypted. TokenHelper string `hcl:"token_helper"` } diff --git a/command/meta.go b/command/meta.go index 7087a2cc03..ec0cdc5832 100644 --- a/command/meta.go +++ b/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) - return &token.Helper{Path: path}, nil + path, err = token.ExternalTokenHelperPath(path) + if err != nil { + return nil, err + } + return &token.ExternalTokenHelper{BinaryPath: path}, nil } func (m *Meta) loadCACert(path string) (*x509.CertPool, error) { diff --git a/command/token/helper.go b/command/token/helper.go index 5efda9a69c..db068beb29 100644 --- a/command/token/helper.go +++ b/command/token/helper.go @@ -1,132 +1,14 @@ package token -import ( - "bytes" - "fmt" - "os" - "os/exec" - "path/filepath" - "runtime" - "strings" +// TokenHelper is an interface that contains basic operations that must be +// implemented by a token helper +type TokenHelper interface { + // Path displays a backend-specific path; for the internal helper this + // is the location of the token stored on disk; for the external helper + // this is the location of the binary being invoked + Path() string - "github.com/kardianos/osext" -) - -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 + Erase() error + Get() (string, error) + Store(string) error } diff --git a/command/token/helper_external.go b/command/token/helper_external.go new file mode 100644 index 0000000000..9f65f2f3f4 --- /dev/null +++ b/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 +} diff --git a/command/token/helper_test.go b/command/token/helper_external_test.go similarity index 77% rename from command/token/helper_test.go rename to command/token/helper_external_test.go index ded8b9a181..b1a9ff5c65 100644 --- a/command/token/helper_test.go +++ b/command/token/helper_external_test.go @@ -10,16 +10,13 @@ import ( "testing" ) -func TestHelperPath(t *testing.T) { - cases := map[string]string{ - "foo": exePath + " token-foo", - } +func TestExternalTokenHelperPath(t *testing.T) { + cases := map[string]string{} 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) { - Test(t, testHelper(t)) +func TestExternalTokenHelper(t *testing.T) { + Test(t, testExternalTokenHelper(t)) } -func testHelper(t *testing.T) *Helper { - return &Helper{Path: helperPath("helper"), Env: helperEnv()} +func testExternalTokenHelper(t *testing.T) *ExternalTokenHelper { + 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 } diff --git a/command/token/helper_internal.go b/command/token/helper_internal.go new file mode 100644 index 0000000000..4327be4194 --- /dev/null +++ b/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 +} diff --git a/command/token/helper_internal_test.go b/command/token/helper_internal_test.go new file mode 100644 index 0000000000..4ac527ce49 --- /dev/null +++ b/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{}) +} diff --git a/command/token/testing.go b/command/token/testing.go index 63e3a27d38..725f1276a0 100644 --- a/command/token/testing.go +++ b/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] == "--" {