Merge branch 'master-oss' into approle-local-secretid

CHANGELOG.md

@@ -1,5 +1,18 @@
 ## 0.10.1 (Unreleased)
 
+DEPRECATIONS/CHANGES:
+
+ * `vault kv` and Vault versions: In 0.10.1 some issues with `vault kv` against
+   v1 K/V engine mounts are fixed. However, using 0.10.1 for both the server
+   and CLI versions is required.
+ * Mount information visiblity: Users that have access to any path within a
+   mount can now see information about that mount, such as its type and
+   options, via some API calls.
+ * Identity and Local Mounts: Local mounts would allow creating Identity
+   entities but these would not be able to be used successfully (even locally)
+   in replicated scenarios. We have now disallowed entities and groups from
+   being created for local mounts in the first place.
+
 FEATURES:
 
  * X-Forwarded-For support: `X-Forwarded-For` headers can now be used to set the
@@ -9,12 +22,16 @@ FEATURES:
  * CIDR IP Binding for Tokens: Tokens now support being bound to specific
    CIDR(s) for usage. Currently this is implemented in Token Roles; usage can be
    expanded to other authentication backends over time.
+ * `vault kv patch` command: A new `kv patch` helper command that allows
+   modifying only some values in existing data at a K/V path, but uses
+   check-and-set to ensure that this modification happens safely.
 
 IMPROVEMENTS:
 
  * auth/token: Add to the token lookup response, the policies inherited due to
    identity associations [GH-4366]
  * auth/token: Add CIDR binding to token roles [GH-815]
+ * cli: Add `vault kv patch` [GH-4432]
  * core: Add X-Forwarded-For support [GH-4380]
  * core: Add token CIDR-binding support [GH-815]
  * identity: Add the ability to disable an entity. Disabling an entity does not
@@ -27,9 +44,13 @@ IMPROVEMENTS:
  * physical/consul: Allow tuning of session TTL and lock wait time [GH-4352]
  * replication: Dynamically adjust WAL cleanup over a period of time based on
    the rate of writes committed
+ * secret/ssh: Update dynamic key install script to use shell locking to avoid
+   concurrent modifications [GH-4358]
 
 BUG FIXES:
 
+ * cli: Fix `vault kv` backwards compatibility with KV v1 engine mounts
+   [GH-4430]
  * identity: Persist entity memberships in external identity groups across
    mounts [GH-4365]
  * replication: Fix issue causing secondaries to not connect properly to a

api/renewer_integration_test.go

@@ -13,9 +13,6 @@ func TestRenewer_Renew(t *testing.T) {
 	client, vaultDone := testVaultServer(t)
 	defer vaultDone()
 
-	pgURL, pgDone := testPostgresDB(t)
-	defer pgDone()
-
 	t.Run("group", func(t *testing.T) {
 		t.Run("kv", func(t *testing.T) {
 			t.Parallel()
@@ -92,6 +89,9 @@ func TestRenewer_Renew(t *testing.T) {
 		t.Run("database", func(t *testing.T) {
 			t.Parallel()
 
+			pgURL, pgDone := testPostgresDB(t)
+			defer pgDone()
+
 			if err := client.Sys().Mount("database", &api.MountInput{
 				Type: "database",
 			}); err != nil {
@@ -129,37 +129,37 @@ func TestRenewer_Renew(t *testing.T) {
 			go v.Renew()
 			defer v.Stop()
 
-			select {
-			case err := <-v.DoneCh():
-				t.Errorf("should have renewed once before returning: %s", err)
-			case renew := <-v.RenewCh():
-				if renew == nil {
-					t.Fatal("renew is nil")
-				}
-				if !renew.Secret.Renewable {
-					t.Errorf("expected lease to be renewable: %#v", renew)
-				}
-				if renew.Secret.LeaseDuration > 5 {
-					t.Errorf("expected lease to <= 5s: %#v", renew)
-				}
-			case <-time.After(5 * time.Second):
-				t.Errorf("no renewal")
-			}
-
-		outer:
+			done, renewed := false, false
 			for {
+				if done {
+					break
+				}
 				select {
 				case err := <-v.DoneCh():
-					if err != nil {
-						t.Fatal(err)
+					if renewed {
+						// If we renewed but there's an error, we fail
+						if err != nil {
+							t.Fatalf("renewal failed with an error: %v", err)
+						}
+					} else {
+						t.Errorf("should have renewed once before returning: %s", err)
 					}
-					break outer
 				case renew := <-v.RenewCh():
-					t.Logf("renew called, remaining lease duration: %d", renew.Secret.LeaseDuration)
-					continue outer
+					if renew == nil {
+						t.Fatal("renew is nil")
+					}
+					if !renew.Secret.Renewable {
+						t.Errorf("expected lease to be renewable: %#v", renew)
+					}
+					if renew.Secret.LeaseDuration > 5 {
+						t.Errorf("expected lease to <= 5s: %#v", renew)
+					}
+					renewed = true
 				case <-time.After(5 * time.Second):
-					t.Errorf("no data")
-					break outer
+					if !renewed {
+						t.Errorf("no renewal")
+					}
+					done = true
 				}
 			}
 		})
@@ -169,7 +169,7 @@ func TestRenewer_Renew(t *testing.T) {
 
 			secret, err := client.Auth().Token().Create(&api.TokenCreateRequest{
 				Policies:       []string{"default"},
-				TTL:            "3s",
+				TTL:            "5s",
 				ExplicitMaxTTL: "10s",
 			})
 			if err != nil {
@@ -185,45 +185,47 @@ func TestRenewer_Renew(t *testing.T) {
 			go v.Renew()
 			defer v.Stop()
 
-			select {
-			case err := <-v.DoneCh():
-				t.Errorf("should have renewed once before returning: %s", err)
-			case renew := <-v.RenewCh():
-				if renew == nil {
-					t.Fatal("renew is nil")
-				}
-				if renew.Secret.Auth == nil {
-					t.Fatal("renew auth is nil")
-				}
-				if !renew.Secret.Auth.Renewable {
-					t.Errorf("expected lease to be renewable: %#v", renew)
-				}
-				if renew.Secret.Auth.LeaseDuration > 3 {
-					t.Errorf("expected lease to < 3s: %#v", renew)
-				}
-				if renew.Secret.Auth.ClientToken == "" {
-					t.Error("expected a client token")
-				}
-				if renew.Secret.Auth.Accessor == "" {
-					t.Error("expected an accessor")
-				}
-			case <-time.After(5 * time.Second):
-				t.Errorf("no renewal")
-			}
-
-		outer:
+			renewed, done := false, false
 			for {
+				if done {
+					break
+				}
 				select {
 				case err := <-v.DoneCh():
-					if err != nil {
-						t.Fatal(err)
+					if renewed {
+						// If we renewed but there's an error, we fail
+						if err != nil {
+							t.Fatalf("renewal failed with an error: %v", err)
+						}
+					} else {
+						t.Errorf("should have renewed once before returning: %s", err)
 					}
-					break outer
-				case <-v.RenewCh():
-					continue outer
-				case <-time.After(5 * time.Second):
-					t.Errorf("no data")
-					break outer
+					done = true
+				case renew := <-v.RenewCh():
+					if renew == nil {
+						t.Fatal("renew is nil")
+					}
+					if renew.Secret.Auth == nil {
+						t.Fatal("renew auth is nil")
+					}
+					if !renew.Secret.Auth.Renewable {
+						t.Errorf("expected lease to be renewable: %#v", renew)
+					}
+					if renew.Secret.Auth.LeaseDuration > 5 {
+						t.Errorf("expected lease to <= 5s: %#v", renew)
+					}
+					if renew.Secret.Auth.ClientToken == "" {
+						t.Error("expected a client token")
+					}
+					if renew.Secret.Auth.Accessor == "" {
+						t.Error("expected an accessor")
+					}
+					renewed = true
+				case <-time.After(10 * time.Second):
+					if !renewed {
+						t.Errorf("no renewal")
+					}
+					done = true
 				}
 			}
 		})

builtin/logical/ssh/linux_install_script.go

@@ -49,21 +49,23 @@ if [ "$INSTALL_OPTION" != "install" ] && [ "$INSTALL_OPTION" != "uninstall" ]; t
 	exit 1
 fi
 
-# Create the .ssh directory and authorized_keys file if it does not exist
-SSH_DIR=$(dirname $AUTH_KEYS_FILE)
-sudo mkdir -p "$SSH_DIR"
-sudo touch "$AUTH_KEYS_FILE"
-
-# Remove the key from authorized_keys file if it is already present.
-# This step is common for both install and uninstall.  Note that grep's
-# return code is ignored, thus if grep fails all keys will be removed
-# rather than none and it fails secure
-sudo grep -vFf "$PUBLIC_KEY_FILE" "$AUTH_KEYS_FILE" > temp_$PUBLIC_KEY_FILE || true
-cat temp_$PUBLIC_KEY_FILE | sudo tee "$AUTH_KEYS_FILE"
-
-# Append the new public key to authorized_keys file
-if [ "$INSTALL_OPTION" == "install" ]; then
-	cat "$PUBLIC_KEY_FILE" | sudo tee --append "$AUTH_KEYS_FILE"
-fi
+# use locking to avoid parallel script execution
+(
+	flock --timeout 10 200
+	# Create the .ssh directory and authorized_keys file if it does not exist
+	SSH_DIR=$(dirname $AUTH_KEYS_FILE)
+	sudo mkdir -p "$SSH_DIR"
+	sudo touch "$AUTH_KEYS_FILE"
+	# Remove the key from authorized_keys file if it is already present.
+	# This step is common for both install and uninstall.  Note that grep's
+	# return code is ignored, thus if grep fails all keys will be removed
+	# rather than none and it fails secure
+	sudo grep -vFf "$PUBLIC_KEY_FILE" "$AUTH_KEYS_FILE" > temp_$PUBLIC_KEY_FILE || true
+	cat temp_$PUBLIC_KEY_FILE | sudo tee "$AUTH_KEYS_FILE"
+	# Append the new public key to authorized_keys file
+	if [ "$INSTALL_OPTION" == "install" ]; then
+		cat "$PUBLIC_KEY_FILE" | sudo tee --append "$AUTH_KEYS_FILE"
+	fi
+) 200> ${AUTH_KEYS_FILE}.lock
 `
 )

command/command_test.go

@@ -10,6 +10,7 @@ import (
 	"time"
 
 	log "github.com/hashicorp/go-hclog"
+	kv "github.com/hashicorp/vault-plugin-secrets-kv"
 	"github.com/hashicorp/vault/api"
 	"github.com/hashicorp/vault/audit"
 	"github.com/hashicorp/vault/builtin/logical/pki"
@@ -41,6 +42,7 @@ var (
 		"pki":            pki.Factory,
 		"ssh":            ssh.Factory,
 		"transit":        transit.Factory,
+		"kv":             kv.Factory,
 	}
 )
 

command/commands.go

@@ -695,6 +695,13 @@ func initCommands(ui, serverCmdUi cli.Ui, runOpts *RunOptions) {
 				},
 			}, nil
 		},
+		"kv patch": func() (cli.Command, error) {
+			return &KVPatchCommand{
+				BaseCommand: &BaseCommand{
+					UI: ui,
+				},
+			}, nil
+		},
 		"kv get": func() (cli.Command, error) {
 			return &KVGetCommand{
 				BaseCommand: &BaseCommand{

command/kv_delete.go

@@ -4,6 +4,7 @@ import (
 	"fmt"
 	"strings"
 
+	"github.com/hashicorp/vault/api"
 	"github.com/mitchellh/cli"
 	"github.com/posener/complete"
 )
@@ -87,13 +88,25 @@ func (c *KVDeleteCommand) Run(args []string) int {
 		return 1
 	}
 
+	client, err := c.Client()
+	if err != nil {
+		c.UI.Error(err.Error())
+		return 2
+	}
+
 	path := sanitizePath(args[0])
-	var err error
-	if len(c.flagVersions) > 0 {
-		err = c.deleteVersions(path, kvParseVersionsFlags(c.flagVersions))
+	mountPath, v2, err := isKVv2(path, client)
+	if err != nil {
+		c.UI.Error(err.Error())
+		return 2
+	}
+
+	if v2 {
+		err = c.deleteV2(path, mountPath, client)
 	} else {
-		err = c.deleteLatest(path)
+		_, err = client.Logical().Delete(path)
 	}
+
 	if err != nil {
 		c.UI.Error(fmt.Sprintf("Error deleting %s: %s", path, err))
 		return 2
@@ -103,39 +116,29 @@ func (c *KVDeleteCommand) Run(args []string) int {
 	return 0
 }
 
-func (c *KVDeleteCommand) deleteLatest(path string) error {
+func (c *KVDeleteCommand) deleteV2(path, mountPath string, client *api.Client) error {
 	var err error
-	path, err = addPrefixToVKVPath(path, "data")
-	if err != nil {
-		return err
-	}
-
-	client, err := c.Client()
-	if err != nil {
-		return err
-	}
-
-	_, err = kvDeleteRequest(client, path)
+	switch {
+	case len(c.flagVersions) > 0:
+		path = addPrefixToVKVPath(path, mountPath, "delete")
+		if err != nil {
+			return err
+		}
 
-	return err
-}
+		data := map[string]interface{}{
+			"versions": kvParseVersionsFlags(c.flagVersions),
+		}
 
-func (c *KVDeleteCommand) deleteVersions(path string, versions []string) error {
-	var err error
-	path, err = addPrefixToVKVPath(path, "delete")
-	if err != nil {
-		return err
-	}
+		_, err = client.Logical().Write(path, data)
+	default:
 
-	data := map[string]interface{}{
-		"versions": versions,
-	}
+		path = addPrefixToVKVPath(path, mountPath, "data")
+		if err != nil {
+			return err
+		}
 
-	client, err := c.Client()
-	if err != nil {
-		return err
+		_, err = client.Logical().Delete(path)
 	}
 
-	_, err = kvWriteRequest(client, path, data)
 	return err
 }