From e6ca938bddb24b594de7f00a7912456116713aa6 Mon Sep 17 00:00:00 2001
From: Clint <catsby@users.noreply.github.com>
Date: Wed, 11 Dec 2019 09:32:22 -0600
Subject: [PATCH] Transit: error when restoring to a name that looks like a
 path (#7998)

* Add test to verify #7663

* Validate name in transit key restore to not be a path
---
 builtin/logical/transit/path_restore.go      | 15 ++++-
 builtin/logical/transit/path_restore_test.go | 67 +++++++++++++++++++-
 2 files changed, 80 insertions(+), 2 deletions(-)

diff --git a/builtin/logical/transit/path_restore.go b/builtin/logical/transit/path_restore.go
index f619d162588b..a8483c1554ec 100644
--- a/builtin/logical/transit/path_restore.go
+++ b/builtin/logical/transit/path_restore.go
@@ -2,6 +2,8 @@ package transit
 
 import (
 	"context"
+	"errors"
+	"strings"
 
 	"github.com/hashicorp/vault/sdk/framework"
 	"github.com/hashicorp/vault/sdk/logical"
@@ -42,8 +44,19 @@ func (b *backend) pathRestoreUpdate(ctx context.Context, req *logical.Request, d
 		return logical.ErrorResponse("'backup' must be supplied"), nil
 	}
 
-	return nil, b.lm.RestorePolicy(ctx, req.Storage, d.Get("name").(string), backupB64, force)
+	keyName := d.Get("name").(string)
+	// if a name is given, make sure it does not contain any slashes and look like
+	// a path
+	if keyName != "" {
+		if strings.Contains(keyName, "/") {
+			return nil, ErrInvalidKeyName
+		}
+	}
+
+	return nil, b.lm.RestorePolicy(ctx, req.Storage, keyName, backupB64, force)
 }
 
 const pathRestoreHelpSyn = `Restore the named key`
 const pathRestoreHelpDesc = `This path is used to restore the named key.`
+
+var ErrInvalidKeyName = errors.New("key names cannot be paths")
diff --git a/builtin/logical/transit/path_restore_test.go b/builtin/logical/transit/path_restore_test.go
index 5278d144ed99..c4c4583882e7 100644
--- a/builtin/logical/transit/path_restore_test.go
+++ b/builtin/logical/transit/path_restore_test.go
@@ -94,6 +94,9 @@ func TestTransit_Restore(t *testing.T) {
 		Force *bool
 		// The error we expect, if any
 		ExpectedErr error
+
+		// RestoreName is used to restore the key to a differnt name
+		RestoreName string
 	}{
 		{
 			// key does not already exist
@@ -116,6 +119,33 @@ func TestTransit_Restore(t *testing.T) {
 			Name:  "Restore-with-force-no-seed",
 			Force: boolPtr(true),
 		},
+		{
+			// key already exists, restore to new name
+			Name:        "Restore-new-name",
+			Seed:        true,
+			RestoreName: "new-key",
+		},
+		{
+			// key already exists, restore to bad path, should error
+			Name:        "Restore-new-name-bad-path",
+			Seed:        true,
+			RestoreName: "sub/path/new-key",
+			ExpectedErr: ErrInvalidKeyName,
+		},
+		{
+			// using force shouldn't matter if the restore key name is different
+			Name:        "Restore-with-force-seed-new-name",
+			Seed:        true,
+			Force:       boolPtr(true),
+			RestoreName: "other-key",
+		},
+		{
+			// using force shouldn't matter if the restore key name is different
+			Name:        "Restore-with-out-force-seed-new-name",
+			Seed:        true,
+			Force:       boolPtr(false),
+			RestoreName: "other-key",
+		},
 		{
 			// using force shouldn't matter if the key doesn't exist
 			Name:  "Restore-force-false",
@@ -154,8 +184,13 @@ func TestTransit_Restore(t *testing.T) {
 				}
 			}
 
+			restorePath := "restore"
+			if tc.RestoreName != "" {
+				restorePath = fmt.Sprintf("%s/%s", restorePath, tc.RestoreName)
+			}
+
 			restoreReq := &logical.Request{
-				Path:      "restore",
+				Path:      restorePath,
 				Operation: logical.UpdateOperation,
 				Storage:   s,
 				Data: map[string]interface{}{
@@ -184,12 +219,42 @@ func TestTransit_Restore(t *testing.T) {
 				}
 			}
 
+			readKeyName := keyName
+			if tc.RestoreName != "" {
+				readKeyName = tc.RestoreName
+			}
+
+			// read the key and make sure it's there
+			readReq := &logical.Request{
+				Path:      "keys/" + readKeyName,
+				Operation: logical.ReadOperation,
+				Storage:   s,
+			}
+
+			resp, err = b.HandleRequest(context.Background(), readReq)
+			if resp != nil && resp.IsError() {
+				t.Fatalf("resp: %#v\nerr: %v", resp, err)
+			}
+
+			if tc.ExpectedErr == nil && resp == nil {
+				t.Fatal("expected to find a key, but got none")
+			}
+
 			// cleanup / delete key after each run
 			keyReq.Operation = logical.DeleteOperation
 			resp, err = b.HandleRequest(context.Background(), keyReq)
 			if err != nil || (resp != nil && resp.IsError()) {
 				t.Fatalf("resp: %#v\nerr: %v", resp, err)
 			}
+
+			// cleanup / delete restore key after each run, if it was created
+			if tc.RestoreName != "" && tc.ExpectedErr == nil {
+				readReq.Operation = logical.DeleteOperation
+				resp, err = b.HandleRequest(context.Background(), readReq)
+				if err != nil || (resp != nil && resp.IsError()) {
+					t.Fatalf("resp: %#v\nerr: %v", resp, err)
+				}
+			}
 		})
 	}
 }