From 82d1f28fb6588454f099efe356b9880b8b279ed5 Mon Sep 17 00:00:00 2001 From: Jeff Mitchell Date: Thu, 17 Sep 2015 18:49:50 -0400 Subject: [PATCH] Remove enable/disable and make deletion_allowed a configurable property. On read, return the version and creation time of each key --- builtin/logical/transit/backend.go | 4 +- builtin/logical/transit/backend_test.go | 59 ++++++---- builtin/logical/transit/path_config.go | 27 ++++- .../logical/transit/path_enable_disable.go | 111 ------------------ builtin/logical/transit/path_encrypt.go | 5 - builtin/logical/transit/path_keys.go | 24 ++-- builtin/logical/transit/policy.go | 6 +- 7 files changed, 77 insertions(+), 159 deletions(-) delete mode 100644 builtin/logical/transit/path_enable_disable.go diff --git a/builtin/logical/transit/backend.go b/builtin/logical/transit/backend.go index 68a805d66b..1510fbaffa 100644 --- a/builtin/logical/transit/backend.go +++ b/builtin/logical/transit/backend.go @@ -19,10 +19,8 @@ func Backend() *framework.Backend { }, Paths: []*framework.Path{ - // Rotate/Enable/Disable needs to come before Keys + // Rotate/Config needs to come before Keys // as the handler is greedy - pathEnable(), - pathDisable(), pathConfig(), pathRotate(), pathRewrap(), diff --git a/builtin/logical/transit/backend_test.go b/builtin/logical/transit/backend_test.go index f95c7847fe..ada1fc61f3 100644 --- a/builtin/logical/transit/backend_test.go +++ b/builtin/logical/transit/backend_test.go @@ -26,13 +26,13 @@ func TestBackend_basic(t *testing.T) { testAccStepEncrypt(t, "test", testPlaintext, decryptData), testAccStepDecrypt(t, "test", testPlaintext, decryptData), testAccStepDeleteNotDisabledPolicy(t, "test"), - testAccStepDisablePolicy(t, "test"), + testAccStepEnableDeletion(t, "test"), testAccStepDeletePolicy(t, "test"), testAccStepWritePolicy(t, "test", false), - testAccStepDisablePolicy(t, "test"), - testAccStepEnablePolicy(t, "test"), + testAccStepEnableDeletion(t, "test"), + testAccStepDisableDeletion(t, "test"), testAccStepDeleteNotDisabledPolicy(t, "test"), - testAccStepDisablePolicy(t, "test"), + testAccStepEnableDeletion(t, "test"), testAccStepDeletePolicy(t, "test"), testAccStepReadPolicy(t, "test", true, false), }, @@ -90,7 +90,7 @@ func TestBackend_rotation(t *testing.T) { testAccStepDecrypt(t, "test", testPlaintext, decryptData), testAccStepRewrap(t, "test", decryptData, 4), testAccStepDecrypt(t, "test", testPlaintext, decryptData), - testAccStepDisablePolicy(t, "test"), + testAccStepEnableDeletion(t, "test"), testAccStepDeletePolicy(t, "test"), testAccStepReadPolicy(t, "test", true, false), }, @@ -106,7 +106,7 @@ func TestBackend_upsert(t *testing.T) { testAccStepEncrypt(t, "test", testPlaintext, decryptData), testAccStepReadPolicy(t, "test", false, false), testAccStepDecrypt(t, "test", testPlaintext, decryptData), - testAccStepDisablePolicy(t, "test"), + testAccStepEnableDeletion(t, "test"), testAccStepDeletePolicy(t, "test"), testAccStepReadPolicy(t, "test", true, false), }, @@ -122,7 +122,7 @@ func TestBackend_basic_derived(t *testing.T) { testAccStepReadPolicy(t, "test", false, true), testAccStepEncryptContext(t, "test", testPlaintext, "my-cool-context", decryptData), testAccStepDecrypt(t, "test", testPlaintext, decryptData), - testAccStepDisablePolicy(t, "test"), + testAccStepEnableDeletion(t, "test"), testAccStepDeletePolicy(t, "test"), testAccStepReadPolicy(t, "test", true, true), }, @@ -139,13 +139,6 @@ func testAccStepWritePolicy(t *testing.T, name string, derived bool) logicaltest } } -func testAccStepEnablePolicy(t *testing.T, name string) logicaltest.TestStep { - return logicaltest.TestStep{ - Operation: logical.WriteOperation, - Path: "keys/" + name + "/enable", - } -} - func testAccStepAdjustPolicy(t *testing.T, name string, minVer int) logicaltest.TestStep { return logicaltest.TestStep{ Operation: logical.WriteOperation, @@ -156,10 +149,23 @@ func testAccStepAdjustPolicy(t *testing.T, name string, minVer int) logicaltest. } } -func testAccStepDisablePolicy(t *testing.T, name string) logicaltest.TestStep { +func testAccStepDisableDeletion(t *testing.T, name string) logicaltest.TestStep { return logicaltest.TestStep{ Operation: logical.WriteOperation, - Path: "keys/" + name + "/disable", + Path: "keys/" + name + "/config", + Data: map[string]interface{}{ + "deletion_allowed": false, + }, + } +} + +func testAccStepEnableDeletion(t *testing.T, name string) logicaltest.TestStep { + return logicaltest.TestStep{ + Operation: logical.WriteOperation, + Path: "keys/" + name + "/config", + Data: map[string]interface{}{ + "deletion_allowed": true, + }, } } @@ -176,6 +182,9 @@ func testAccStepDeleteNotDisabledPolicy(t *testing.T, name string) logicaltest.T Path: "keys/" + name, ErrorOk: true, Check: func(resp *logical.Response) error { + if resp == nil { + return fmt.Errorf("Got nil response instead of error") + } if resp.IsError() { return nil } @@ -198,13 +207,13 @@ func testAccStepReadPolicy(t *testing.T, name string, expectNone, derived bool) return nil } var d struct { - Name string `mapstructure:"name"` - Key []byte `mapstructure:"key"` - Keys [][]byte `mapstructure:"keys"` - CipherMode string `mapstructure:"cipher_mode"` - Derived bool `mapstructure:"derived"` - KDFMode string `mapstructure:"kdf_mode"` - Disabled bool `mapstructure:"disabled"` + Name string `mapstructure:"name"` + Key []byte `mapstructure:"key"` + Keys map[string]int64 `mapstructure:"keys"` + CipherMode string `mapstructure:"cipher_mode"` + Derived bool `mapstructure:"derived"` + KDFMode string `mapstructure:"kdf_mode"` + DeletionAllowed bool `mapstructure:"deletion_allowed"` } if err := mapstructure.Decode(resp.Data, &d); err != nil { return err @@ -220,10 +229,10 @@ func testAccStepReadPolicy(t *testing.T, name string, expectNone, derived bool) if d.Key != nil { return fmt.Errorf("bad: %#v", d) } - if d.Keys != nil { + if d.Keys == nil { return fmt.Errorf("bad: %#v", d) } - if d.Disabled == true { + if d.DeletionAllowed == true { return fmt.Errorf("bad: %#v", d) } if d.Derived != derived { diff --git a/builtin/logical/transit/path_config.go b/builtin/logical/transit/path_config.go index 9e64530b05..d368c64b9a 100644 --- a/builtin/logical/transit/path_config.go +++ b/builtin/logical/transit/path_config.go @@ -21,6 +21,11 @@ func pathConfig() *framework.Path { Description: `If set, the minimum version of the key allowed to be decrypted.`, }, + + "deletion_allowed": &framework.FieldSchema{ + Type: framework.TypeBool, + Description: "Whether to allow deletion of the key", + }, }, Callbacks: map[logical.Operation]framework.OperationFunc{ @@ -47,13 +52,27 @@ func pathConfigWrite( logical.ErrInvalidRequest } + persistNeeded := false + minDecryptionVersion := d.Get("min_decryption_version").(int) - if minDecryptionVersion == 0 || - minDecryptionVersion == policy.MinDecryptionVersion { - return nil, nil + if minDecryptionVersion != 0 && + minDecryptionVersion != policy.MinDecryptionVersion { + policy.MinDecryptionVersion = minDecryptionVersion + persistNeeded = true } - policy.MinDecryptionVersion = minDecryptionVersion + allowDeletionInt, ok := d.GetOk("deletion_allowed") + if ok { + allowDeletion := allowDeletionInt.(bool) + if allowDeletion != policy.DeletionAllowed { + policy.DeletionAllowed = allowDeletion + persistNeeded = true + } + } + + if !persistNeeded { + return nil, nil + } return nil, policy.Persist(req.Storage, name) } diff --git a/builtin/logical/transit/path_enable_disable.go b/builtin/logical/transit/path_enable_disable.go deleted file mode 100644 index d936821945..0000000000 --- a/builtin/logical/transit/path_enable_disable.go +++ /dev/null @@ -1,111 +0,0 @@ -package transit - -import ( - "fmt" - - "github.com/hashicorp/vault/logical" - "github.com/hashicorp/vault/logical/framework" -) - -func pathEnable() *framework.Path { - return &framework.Path{ - Pattern: "keys/" + framework.GenericNameRegex("name") + "/enable", - Fields: map[string]*framework.FieldSchema{ - "name": &framework.FieldSchema{ - Type: framework.TypeString, - Description: "Name of the key", - }, - }, - - Callbacks: map[logical.Operation]framework.OperationFunc{ - logical.WriteOperation: pathEnableWrite, - }, - - HelpSynopsis: pathEnableHelpSyn, - HelpDescription: pathEnableHelpDesc, - } -} - -func pathDisable() *framework.Path { - return &framework.Path{ - Pattern: "keys/" + framework.GenericNameRegex("name") + "/disable", - Fields: map[string]*framework.FieldSchema{ - "name": &framework.FieldSchema{ - Type: framework.TypeString, - Description: "Name of the key", - }, - }, - - Callbacks: map[logical.Operation]framework.OperationFunc{ - logical.WriteOperation: pathDisableWrite, - }, - - HelpSynopsis: pathDisableHelpSyn, - HelpDescription: pathDisableHelpDesc, - } -} - -func pathEnableWrite( - req *logical.Request, d *framework.FieldData) (*logical.Response, error) { - name := d.Get("name").(string) - - // Check if the policy already exists - policy, err := getPolicy(req, name) - if err != nil { - return nil, err - } - if policy == nil { - return logical.ErrorResponse( - fmt.Sprintf("no existing role named %s could be found", name)), - logical.ErrInvalidRequest - } - - if !policy.Disabled { - return nil, nil - } - - policy.Disabled = false - - return nil, policy.Persist(req.Storage, name) -} - -func pathDisableWrite( - req *logical.Request, d *framework.FieldData) (*logical.Response, error) { - name := d.Get("name").(string) - - // Check if the policy already exists - policy, err := getPolicy(req, name) - if err != nil { - return nil, err - } - if policy == nil { - return logical.ErrorResponse( - fmt.Sprintf("no existing role named %s could be found", name)), - logical.ErrInvalidRequest - } - - if policy.Disabled { - return nil, nil - } - - policy.Disabled = true - - return nil, policy.Persist(req.Storage, name) -} - -const pathEnableHelpSyn = `Enable a named encryption key` - -const pathEnableHelpDesc = ` -This path is used to enable the named key. After enabling, -the key will be available for use for encryption. -` - -const pathDisableHelpSyn = `Disable a named encryption key` - -const pathDisableHelpDesc = ` -This path is used to disable the named key. After disabling, -the key cannot be used to encrypt values. This is useful when -when switching to a new named key, but wanting to be able to -decrypt against old keys while guarding against additional -data being encrypted with the old key. -` diff --git a/builtin/logical/transit/path_encrypt.go b/builtin/logical/transit/path_encrypt.go index 822cf63578..7a402292dd 100644 --- a/builtin/logical/transit/path_encrypt.go +++ b/builtin/logical/transit/path_encrypt.go @@ -52,11 +52,6 @@ func pathEncryptWrite( return nil, err } - // Fast track disable checking - if p != nil && p.Disabled { - return logical.ErrorResponse("key is disabled and cannot be used for encryption"), logical.ErrInvalidRequest - } - // Decode the context if any contextRaw := d.Get("context").(string) var context []byte diff --git a/builtin/logical/transit/path_keys.go b/builtin/logical/transit/path_keys.go index cb12d6677d..7c6d1d1573 100644 --- a/builtin/logical/transit/path_keys.go +++ b/builtin/logical/transit/path_keys.go @@ -2,6 +2,7 @@ package transit import ( "fmt" + "strconv" "github.com/hashicorp/vault/logical" "github.com/hashicorp/vault/logical/framework" @@ -67,15 +68,22 @@ func pathPolicyRead( // Return the response resp := &logical.Response{ Data: map[string]interface{}{ - "name": p.Name, - "cipher_mode": p.CipherMode, - "derived": p.Derived, - "disabled": p.Disabled, + "name": p.Name, + "cipher_mode": p.CipherMode, + "derived": p.Derived, + "deletion_allowed": p.DeletionAllowed, }, } if p.Derived { resp.Data["kdf_mode"] = p.KDFMode } + + retKeys := map[string]int64{} + for k, v := range p.Keys { + retKeys[strconv.Itoa(k)] = v.CreationTime + } + resp.Data["keys"] = retKeys + return resp, nil } @@ -85,19 +93,19 @@ func pathPolicyDelete( p, err := getPolicy(req, name) if err != nil { - return nil, err + return logical.ErrorResponse(fmt.Sprintf("error looking up policy %s, error is %s", name, err)), err } if p == nil { return logical.ErrorResponse(fmt.Sprintf("no such key %s", name)), logical.ErrInvalidRequest } - if !p.Disabled { - return logical.ErrorResponse(fmt.Sprintf("key must be disabled before deletion")), logical.ErrInvalidRequest + if !p.DeletionAllowed { + return logical.ErrorResponse(fmt.Sprintf("'allow_deletion' config value is not set")), logical.ErrInvalidRequest } err = req.Storage.Delete("policy/" + name) if err != nil { - return nil, err + return logical.ErrorResponse(fmt.Sprintf("error deleting policy %s: %s", name, err)), err } return nil, nil } diff --git a/builtin/logical/transit/policy.go b/builtin/logical/transit/policy.go index 2587fbaec3..fbda93111e 100644 --- a/builtin/logical/transit/policy.go +++ b/builtin/logical/transit/policy.go @@ -68,12 +68,12 @@ type Policy struct { Derived bool `json:"derived"` KDFMode string `json:"kdf_mode"` - // Whether the key can be used for encryption - Disabled bool `json:"disabled"` - // The minimum version of the key allowed to be used // for decryption MinDecryptionVersion int `json:"min_decryption_version"` + + // Whether the key is allowed to be deleted + DeletionAllowed bool `json:"deletion_allowed"` } func (p *Policy) Persist(storage logical.Storage, name string) error {