Skip to content

Commit

Permalink
Transit: error when restoring to a name that looks like a path (#7998)
Browse files Browse the repository at this point in the history
* Add test to verify #7663

* Validate name in transit key restore to not be a path
  • Loading branch information
catsby committed Dec 11, 2019
1 parent dd3dcd7 commit 3f62e7f
Show file tree
Hide file tree
Showing 2 changed files with 80 additions and 2 deletions.
15 changes: 14 additions & 1 deletion builtin/logical/transit/path_restore.go
Expand Up @@ -2,6 +2,8 @@ package transit

import (
"context"
"errors"
"strings"

"github.com/hashicorp/vault/sdk/framework"
"github.com/hashicorp/vault/sdk/logical"
Expand Down Expand Up @@ -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")
67 changes: 66 additions & 1 deletion builtin/logical/transit/path_restore_test.go
Expand Up @@ -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
Expand All @@ -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",
Expand Down Expand Up @@ -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{}{
Expand Down Expand Up @@ -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)
}
}
})
}
}

0 comments on commit 3f62e7f

Please sign in to comment.