Ensure that the .vault-token file writen by `vault login` always has the correct permissions and ownership. (#8867)

command/config/util.go

@@ -13,7 +13,7 @@ func DefaultTokenHelper() (token.TokenHelper, error) {
 
 	path := config.TokenHelper
 	if path == "" {
-		return &token.InternalTokenHelper{}, nil
+		return token.NewInternalTokenHelper()
 	}
 
 	path, err = token.ExternalTokenHelperPath(path)

command/token/helper_internal.go

@@ -9,6 +9,7 @@ import (
 	"strings"
 
 	homedir "github.com/mitchellh/go-homedir"
+	"github.com/natefinch/atomic"
 )
 
 var _ TokenHelper = (*InternalTokenHelper)(nil)
@@ -17,16 +18,21 @@ var _ TokenHelper = (*InternalTokenHelper)(nil)
 // token-helper is configured, and avoids shelling out
 type InternalTokenHelper struct {
 	tokenPath string
+	homeDir   string
+}
+
+func NewInternalTokenHelper() (*InternalTokenHelper, error) {
+	homeDir, err := homedir.Dir()
+	if err != nil {
+		panic(fmt.Sprintf("error getting user's home directory: %v", err))
+	}
+	return &InternalTokenHelper{homeDir: homeDir}, err
 }
 
 // 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.Sprintf("error getting user's home directory: %v", err))
-	}
-	i.tokenPath = filepath.Join(homePath, ".vault-token")
+	i.tokenPath = filepath.Join(i.homeDir, ".vault-token")
 }
 
 func (i *InternalTokenHelper) Path() string {
@@ -53,21 +59,35 @@ func (i *InternalTokenHelper) Get() (string, error) {
 	return strings.TrimSpace(buf.String()), nil
 }
 
-// Store stores the value of the token to the file
+// Store stores the value of the token to the file.  We always overwrite any
+// existing file atomically to ensure that ownership and permissions are set
+// appropriately.
 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)
+	tmpFile := i.tokenPath + ".tmp"
+	f, err := os.OpenFile(tmpFile, os.O_CREATE|os.O_TRUNC|os.O_WRONLY, 0600)
 	if err != nil {
 		return err
 	}
 	defer f.Close()
+	defer os.Remove(tmpFile)
 
-	buf := bytes.NewBufferString(input)
-	if _, err := io.Copy(f, buf); err != nil {
+	_, err = io.WriteString(f, input)
+	if err != nil {
+		return err
+	}
+	err = f.Close()
+	if err != nil {
 		return err
 	}
 
-	return nil
+	// We don't care so much about atomic writes here.  We're using this package
+	// because we don't have a portable way of verifying that the target file
+	// is owned by the correct user.  The simplest way of ensuring that is
+	// to simply re-write it, and the simplest way to ensure that we don't
+	// damage an existing working file due to error is the write-rename pattern.
+	// os.Rename on Windows will return an error if the target already exists.
+	return atomic.ReplaceFile(tmpFile, i.tokenPath)
 }
 
 // Erase erases the value of the token

command/token/helper_internal_test.go

@@ -1,11 +1,62 @@
 package token
 
 import (
+	"io/ioutil"
+	"os"
+	"path/filepath"
 	"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{})
+	helper, err := NewInternalTokenHelper()
+	if err != nil {
+		t.Fatal(err)
+	}
+	Test(t, helper)
+}
+
+func TestInternalHelperFilePerms(t *testing.T) {
+	tmpDir, err := ioutil.TempDir("", t.Name())
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer os.RemoveAll(tmpDir)
+
+	helper, err := NewInternalTokenHelper()
+	if err != nil {
+		t.Fatal(err)
+	}
+	helper.homeDir = tmpDir
+
+	tmpFile := filepath.Join(tmpDir, ".vault-token")
+	f, err := os.Create(tmpFile)
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer f.Close()
+
+	fi, err := os.Stat(tmpFile)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	if fi.Mode().Perm()&004 != 004 {
+		t.Fatalf("expected world-readable/writable permission bits, got: %o", fi.Mode().Perm())
+	}
+
+	err = helper.Store("bogus_token")
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	fi, err = os.Stat(tmpFile)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	if fi.Mode().Perm()&004 != 0 {
+		t.Fatalf("expected no world-readable/writable permission bits, got: %o", fi.Mode().Perm())
+	}
 }

go.mod

@@ -106,6 +106,7 @@ require (
 	github.com/mitchellh/go-testing-interface v1.0.0
 	github.com/mitchellh/mapstructure v1.1.2
 	github.com/mitchellh/reflectwalk v1.0.1
+	github.com/natefinch/atomic v0.0.0-20150920032501-a62ce929ffcc
 	github.com/ncw/swift v1.0.47
 	github.com/nwaples/rardecode v1.0.0 // indirect
 	github.com/oklog/run v1.0.0

go.sum

@@ -578,6 +578,8 @@ github.com/mwielbut/pointy v1.1.0 h1:U5/YEfoIkaGCHv0St3CgjduqXID4FNRoyZgLM1kY9vg
 github.com/mwielbut/pointy v1.1.0/go.mod h1:MvvO+uMFj9T5DMda33HlvogsFBX7pWWKAkFIn4teYwY=
 github.com/mwitkow/go-conntrack v0.0.0-20161129095857-cc309e4a2223/go.mod h1:qRWi+5nqEBWmkhHvq77mSJWrCKwh8bxhgT7d/eI7P4U=
 github.com/mxk/go-flowrate v0.0.0-20140419014527-cca7078d478f/go.mod h1:ZdcZmHo+o7JKHSa8/e818NopupXU1YMK5fe1lsApnBw=
+github.com/natefinch/atomic v0.0.0-20150920032501-a62ce929ffcc h1:7xGrl4tTpBQu5Zjll08WupHyq+Sp0Z/adtyf1cfk3Q8=
+github.com/natefinch/atomic v0.0.0-20150920032501-a62ce929ffcc/go.mod h1:1rLVY/DWf3U6vSZgH16S7pymfrhK2lcUlXjgGglw/lY=
 github.com/ncw/swift v1.0.47 h1:4DQRPj35Y41WogBxyhOXlrI37nzGlyEcsforeudyYPQ=
 github.com/ncw/swift v1.0.47/go.mod h1:23YIA4yWVnGwv2dQlN4bB7egfYX6YLn0Yo/S6zZO/ZM=
 github.com/nwaples/rardecode v1.0.0 h1:r7vGuS5akxOnR4JQSkko62RJ1ReCMXxQRPtxsiFMBOs=

vendor/github.com/hashicorp/vault/api/sys_raft.go

@@ -92,6 +92,10 @@ func (c *Sys) RaftSnapshot(snapWriter io.Writer) error {
 	// to determine if the body contains error message.
 	var result *Response
 	resp, err := c.c.config.HttpClient.Do(req)
+	if err != nil {
+		return err
+	}
+
 	if resp == nil {
 		return nil
 	}

vendor/github.com/hashicorp/vault/sdk/database/dbplugin/databasemiddleware.go

@@ -8,10 +8,10 @@ import (
 	"sync"
 	"time"
 
-	"github.com/hashicorp/errwrap"
-
 	metrics "github.com/armon/go-metrics"
+	"github.com/hashicorp/errwrap"
 	log "github.com/hashicorp/go-hclog"
+	"google.golang.org/grpc/status"
 )
 
 // ---- Tracing Middleware Domain ----
@@ -318,6 +318,15 @@ func (mw *DatabaseErrorSanitizerMiddleware) sanitize(err error) error {
 			if k == "" {
 				continue
 			}
+
+			// Attempt to keep the status code attached to the
+			// error without changing the actual error message
+			s, ok := status.FromError(err)
+			if ok {
+				err = status.Error(s.Code(), strings.Replace(s.Message(), k, v.(string), -1))
+				continue
+			}
+
 			err = errors.New(strings.Replace(err.Error(), k, v.(string), -1))
 		}
 	}

vendor/github.com/natefinch/atomic/.gitignore

@@ -0,0 +1,24 @@
+# Compiled Object files, Static and Dynamic libs (Shared Objects)
+*.o
+*.a
+*.so
+
+# Folders
+_obj
+_test
+
+# Architecture specific extensions/prefixes
+*.[568vq]
+[568vq].out
+
+*.cgo1.go
+*.cgo2.c
+_cgo_defun.c
+_cgo_gotypes.go
+_cgo_export.*
+
+_testmain.go
+
+*.exe
+*.test
+*.prof

vendor/github.com/natefinch/atomic/LICENSE

@@ -0,0 +1,22 @@
+The MIT License (MIT) 
+
+Copyright (c) 2015 Nate Finch
+
+Permission is hereby granted, free of charge, to any person obtaining a copy
+of this software and associated documentation files (the "Software"), to deal
+in the Software without restriction, including without limitation the rights
+to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+copies of the Software, and to permit persons to whom the Software is
+furnished to do so, subject to the following conditions:
+
+The above copyright notice and this permission notice shall be included in all
+copies or substantial portions of the Software.
+
+THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
+SOFTWARE.
+

vendor/github.com/natefinch/atomic/README.md

@@ -0,0 +1,35 @@
+# atomic
+    import "github.com/natefinch/atomic"
+atomic is a go package for atomic file writing
+
+By default, writing to a file in go (and generally any language) can fail
+partway through... you then have a partially written file, which probably was
+truncated when the write began, and bam, now you've lost data.
+
+This go package avoids this problem, by writing first to a temp file, and then
+overwriting the target file in an atomic way.  This is easy on linux, os.Rename
+just is atomic.  However, on Windows, os.Rename is not atomic, and so bad things
+can happen.  By wrapping the windows API moveFileEx, we can ensure that the move
+is atomic, and we can be safe in knowing that either the move succeeds entirely,
+or neither file will be modified.
+
+
+## func ReplaceFile
+``` go
+func ReplaceFile(source, destination string) error
+```
+ReplaceFile atomically replaces the destination file or directory with the
+source.  It is guaranteed to either replace the target file entirely, or not
+change either file.
+
+
+## func WriteFile
+``` go
+func WriteFile(filename string, r io.Reader) (err error)
+```
+WriteFile atomically writes the contents of r to the specified filepath.  If
+an error occurs, the target file is guaranteed to be either fully written, or
+not written at all.  WriteFile overwrites any file that exists at the
+location (but only if the write fully succeeds, otherwise the existing file
+is unmodified).
+

vendor/github.com/natefinch/atomic/atomic.go

@@ -0,0 +1,58 @@
+// package atomic provides functions to atomically change files.
+package atomic
+
+import (
+	"fmt"
+	"io"
+	"io/ioutil"
+	"os"
+	"path/filepath"
+)
+
+// WriteFile atomically writes the contents of r to the specified filepath.  If
+// an error occurs, the target file is guaranteed to be either fully written, or
+// not written at all.  WriteFile overwrites any file that exists at the
+// location (but only if the write fully succeeds, otherwise the existing file
+// is unmodified).
+func WriteFile(filename string, r io.Reader) (err error) {
+	// write to a temp file first, then we'll atomically replace the target file
+	// with the temp file.
+	dir, file := filepath.Split(filename)
+	f, err := ioutil.TempFile(dir, file)
+	if err != nil {
+		return fmt.Errorf("cannot create temp file: %v", err)
+	}
+	defer func() {
+		if err != nil {
+			// Don't leave the temp file lying around on error.
+			_ = os.Remove(f.Name()) // yes, ignore the error, not much we can do about it.
+		}
+	}()
+	// ensure we always close f. Note that this does not conflict with  the
+	// close below, as close is idempotent.
+	defer f.Close()
+	name := f.Name()
+	if _, err := io.Copy(f, r); err != nil {
+		return fmt.Errorf("cannot write data to tempfile %q: %v", name, err)
+	}
+	if err := f.Close(); err != nil {
+		return fmt.Errorf("can't close tempfile %q: %v", name, err)
+	}
+
+	// get the file mode from the original file and use that for the replacement
+	// file, too.
+	info, err := os.Stat(filename)
+	if os.IsNotExist(err) {
+		// no original file
+	} else if err != nil {
+		return err
+	} else {
+		if err := os.Chmod(name, info.Mode()); err != nil {
+			return fmt.Errorf("can't set filemode on tempfile %q: %v", name, err)
+		}
+	}
+	if err := ReplaceFile(name, filename); err != nil {
+		return fmt.Errorf("cannot replace %q with tempfile %q: %v", filename, name, err)
+	}
+	return nil
+}

vendor/github.com/natefinch/atomic/file_unix.go

@@ -0,0 +1,14 @@
+// +build !windows
+
+package atomic
+
+import (
+	"os"
+)
+
+// ReplaceFile atomically replaces the destination file or directory with the
+// source.  It is guaranteed to either replace the target file entirely, or not
+// change either file.
+func ReplaceFile(source, destination string) error {
+	return os.Rename(source, destination)
+}

vendor/github.com/natefinch/atomic/file_windows.go

@@ -0,0 +1,33 @@
+package atomic
+
+import (
+	"os"
+	"syscall"
+)
+
+const (
+	movefile_replace_existing = 0x1
+	movefile_write_through    = 0x8
+)
+
+//sys moveFileEx(lpExistingFileName *uint16, lpNewFileName *uint16, dwFlags uint32) (err error) = MoveFileExW
+
+// ReplaceFile atomically replaces the destination file or directory with the
+// source.  It is guaranteed to either replace the target file entirely, or not
+// change either file.
+func ReplaceFile(source, destination string) error {
+	src, err := syscall.UTF16PtrFromString(source)
+	if err != nil {
+		return &os.LinkError{"replace", source, destination, err}
+	}
+	dest, err := syscall.UTF16PtrFromString(destination)
+	if err != nil {
+		return &os.LinkError{"replace", source, destination, err}
+	}
+
+	// see http://msdn.microsoft.com/en-us/library/windows/desktop/aa365240(v=vs.85).aspx
+	if err := moveFileEx(src, dest, movefile_replace_existing|movefile_write_through); err != nil {
+		return &os.LinkError{"replace", source, destination, err}
+	}
+	return nil
+}

vendor/github.com/natefinch/atomic/zfile_windows.go

@@ -0,0 +1,27 @@
+// mksyscall_windows -l32 file_windows.go
+// MACHINE GENERATED BY THE COMMAND ABOVE; DO NOT EDIT
+
+package atomic
+
+import (
+	"syscall"
+	"unsafe"
+)
+
+var (
+	modkernel32 = syscall.NewLazyDLL("kernel32.dll")
+
+	procMoveFileExW = modkernel32.NewProc("MoveFileExW")
+)
+
+func moveFileEx(lpExistingFileName *uint16, lpNewFileName *uint16, dwFlags uint32) (err error) {
+	r1, _, e1 := syscall.Syscall(procMoveFileExW.Addr(), 3, uintptr(unsafe.Pointer(lpExistingFileName)), uintptr(unsafe.Pointer(lpNewFileName)), uintptr(dwFlags))
+	if r1 == 0 {
+		if e1 != 0 {
+			err = error(e1)
+		} else {
+			err = syscall.EINVAL
+		}
+	}
+	return
+}

vendor/modules.txt

@@ -437,6 +437,7 @@ github.com/hashicorp/vault/sdk/database/helper/connutil
 github.com/hashicorp/vault/sdk/database/helper/credsutil
 github.com/hashicorp/vault/sdk/database/helper/dbutil
 github.com/hashicorp/vault/sdk/framework
+github.com/hashicorp/vault/sdk/helper/authmetadata
 github.com/hashicorp/vault/sdk/helper/awsutil
 github.com/hashicorp/vault/sdk/helper/base62
 github.com/hashicorp/vault/sdk/helper/certutil
@@ -613,6 +614,8 @@ github.com/modern-go/reflect2
 github.com/mongodb/go-client-mongodb-atlas/mongodbatlas
 # github.com/mwielbut/pointy v1.1.0
 github.com/mwielbut/pointy
+# github.com/natefinch/atomic v0.0.0-20150920032501-a62ce929ffcc
+github.com/natefinch/atomic
 # github.com/ncw/swift v1.0.47
 github.com/ncw/swift
 # github.com/nwaples/rardecode v1.0.0