diff --git a/api/sys_leases.go b/api/sys_leases.go index 34bd99e652..3b65ff094c 100644 --- a/api/sys_leases.go +++ b/api/sys_leases.go @@ -1,5 +1,7 @@ package api +import "errors" + func (c *Sys) Renew(id string, increment int) (*Secret, error) { r := c.c.NewRequest("PUT", "/v1/sys/leases/renew") @@ -46,3 +48,42 @@ func (c *Sys) RevokeForce(id string) error { } return err } + +func (c *Sys) RevokeWithOptions(opts *RevokeOptions) error { + if opts == nil { + return errors.New("nil options provided") + } + + // Construct path + path := "/v1/sys/leases/revoke/" + switch { + case opts.Force: + path = "/v1/sys/leases/revoke-force/" + case opts.Prefix: + path = "/v1/sys/leases/revoke-prefix/" + } + path += opts.LeaseID + + r := c.c.NewRequest("PUT", path) + if !opts.Force { + body := map[string]interface{}{ + "sync": opts.Sync, + } + if err := r.SetJSONBody(body); err != nil { + return err + } + } + + resp, err := c.c.RawRequest(r) + if err == nil { + defer resp.Body.Close() + } + return err +} + +type RevokeOptions struct { + LeaseID string + Force bool + Prefix bool + Sync bool +} diff --git a/builtin/credential/approle/backend.go b/builtin/credential/approle/backend.go index d57a3ec8a6..a16794e203 100644 --- a/builtin/credential/approle/backend.go +++ b/builtin/credential/approle/backend.go @@ -157,7 +157,7 @@ func (b *backend) invalidate(_ context.Context, key string) { func (b *backend) periodicFunc(ctx context.Context, req *logical.Request) error { // Initiate clean-up of expired SecretID entries if b.System().LocalMount() || !b.System().ReplicationState().HasState(consts.ReplicationPerformanceSecondary) { - b.tidySecretID(ctx, req.Storage) + b.tidySecretID(ctx, req) } return nil } diff --git a/builtin/credential/approle/path_tidy_user_id.go b/builtin/credential/approle/path_tidy_user_id.go index 6137e0594f..11693a1426 100644 --- a/builtin/credential/approle/path_tidy_user_id.go +++ b/builtin/credential/approle/path_tidy_user_id.go @@ -3,6 +3,7 @@ package approle import ( "context" "fmt" + "net/http" "sync/atomic" "time" @@ -26,13 +27,15 @@ func pathTidySecretID(b *backend) *framework.Path { } // tidySecretID is used to delete entries in the whitelist that are expired. -func (b *backend) tidySecretID(ctx context.Context, s logical.Storage) (*logical.Response, error) { +func (b *backend) tidySecretID(ctx context.Context, req *logical.Request) (*logical.Response, error) { if !atomic.CompareAndSwapUint32(b.tidySecretIDCASGuard, 0, 1) { resp := &logical.Response{} resp.AddWarning("Tidy operation already in progress.") return resp, nil } + s := req.Storage + go func() { defer atomic.StoreUint32(b.tidySecretIDCASGuard, 0) @@ -167,12 +170,12 @@ func (b *backend) tidySecretID(ctx context.Context, s logical.Storage) (*logical resp := &logical.Response{} resp.AddWarning("Tidy operation successfully started. Any information from the operation will be printed to Vault's server logs.") - return resp, nil + return logical.RespondWithStatusCode(resp, req, http.StatusAccepted) } // pathTidySecretIDUpdate is used to delete the expired SecretID entries func (b *backend) pathTidySecretIDUpdate(ctx context.Context, req *logical.Request, data *framework.FieldData) (*logical.Response, error) { - return b.tidySecretID(ctx, req.Storage) + return b.tidySecretID(ctx, req) } const pathTidySecretIDSyn = "Trigger the clean-up of expired SecretID entries." diff --git a/builtin/credential/aws/backend.go b/builtin/credential/aws/backend.go index 992b295987..809beaefe4 100644 --- a/builtin/credential/aws/backend.go +++ b/builtin/credential/aws/backend.go @@ -165,7 +165,7 @@ func (b *backend) periodicFunc(ctx context.Context, req *logical.Request) error } // tidy role tags if explicitly not disabled if !skipBlacklistTidy { - b.tidyBlacklistRoleTag(ctx, req.Storage, safety_buffer) + b.tidyBlacklistRoleTag(ctx, req, safety_buffer) } } @@ -189,7 +189,7 @@ func (b *backend) periodicFunc(ctx context.Context, req *logical.Request) error } // tidy identities if explicitly not disabled if !skipWhitelistTidy { - b.tidyWhitelistIdentity(ctx, req.Storage, safety_buffer) + b.tidyWhitelistIdentity(ctx, req, safety_buffer) } // Update the time at which to run the tidy functions again. diff --git a/builtin/credential/aws/path_tidy_identity_whitelist.go b/builtin/credential/aws/path_tidy_identity_whitelist.go index a8e7c98d30..c09c97de33 100644 --- a/builtin/credential/aws/path_tidy_identity_whitelist.go +++ b/builtin/credential/aws/path_tidy_identity_whitelist.go @@ -3,6 +3,7 @@ package awsauth import ( "context" "fmt" + "net/http" "sync/atomic" "time" @@ -33,13 +34,15 @@ expiration, before it is removed from the backend storage.`, } // tidyWhitelistIdentity is used to delete entries in the whitelist that are expired. -func (b *backend) tidyWhitelistIdentity(ctx context.Context, s logical.Storage, safetyBuffer int) (*logical.Response, error) { +func (b *backend) tidyWhitelistIdentity(ctx context.Context, req *logical.Request, safetyBuffer int) (*logical.Response, error) { if !atomic.CompareAndSwapUint32(b.tidyWhitelistCASGuard, 0, 1) { resp := &logical.Response{} resp.AddWarning("Tidy operation already in progress.") return resp, nil } + s := req.Storage + go func() { defer atomic.StoreUint32(b.tidyWhitelistCASGuard, 0) @@ -93,12 +96,12 @@ func (b *backend) tidyWhitelistIdentity(ctx context.Context, s logical.Storage, resp := &logical.Response{} resp.AddWarning("Tidy operation successfully started. Any information from the operation will be printed to Vault's server logs.") - return resp, nil + return logical.RespondWithStatusCode(resp, req, http.StatusAccepted) } // pathTidyIdentityWhitelistUpdate is used to delete entries in the whitelist that are expired. func (b *backend) pathTidyIdentityWhitelistUpdate(ctx context.Context, req *logical.Request, data *framework.FieldData) (*logical.Response, error) { - return b.tidyWhitelistIdentity(ctx, req.Storage, data.Get("safety_buffer").(int)) + return b.tidyWhitelistIdentity(ctx, req, data.Get("safety_buffer").(int)) } const pathTidyIdentityWhitelistSyn = ` diff --git a/builtin/credential/aws/path_tidy_roletag_blacklist.go b/builtin/credential/aws/path_tidy_roletag_blacklist.go index e84862f48c..7f9a65230c 100644 --- a/builtin/credential/aws/path_tidy_roletag_blacklist.go +++ b/builtin/credential/aws/path_tidy_roletag_blacklist.go @@ -3,6 +3,7 @@ package awsauth import ( "context" "fmt" + "net/http" "sync/atomic" "time" @@ -33,13 +34,15 @@ expiration, before it is removed from the backend storage.`, } // tidyBlacklistRoleTag is used to clean-up the entries in the role tag blacklist. -func (b *backend) tidyBlacklistRoleTag(ctx context.Context, s logical.Storage, safetyBuffer int) (*logical.Response, error) { +func (b *backend) tidyBlacklistRoleTag(ctx context.Context, req *logical.Request, safetyBuffer int) (*logical.Response, error) { if !atomic.CompareAndSwapUint32(b.tidyBlacklistCASGuard, 0, 1) { resp := &logical.Response{} resp.AddWarning("Tidy operation already in progress.") return resp, nil } + s := req.Storage + go func() { defer atomic.StoreUint32(b.tidyBlacklistCASGuard, 0) @@ -93,12 +96,12 @@ func (b *backend) tidyBlacklistRoleTag(ctx context.Context, s logical.Storage, s resp := &logical.Response{} resp.AddWarning("Tidy operation successfully started. Any information from the operation will be printed to Vault's server logs.") - return resp, nil + return logical.RespondWithStatusCode(resp, req, http.StatusAccepted) } // pathTidyRoletagBlacklistUpdate is used to clean-up the entries in the role tag blacklist. func (b *backend) pathTidyRoletagBlacklistUpdate(ctx context.Context, req *logical.Request, data *framework.FieldData) (*logical.Response, error) { - return b.tidyBlacklistRoleTag(ctx, req.Storage, data.Get("safety_buffer").(int)) + return b.tidyBlacklistRoleTag(ctx, req, data.Get("safety_buffer").(int)) } const pathTidyRoletagBlacklistSyn = ` diff --git a/builtin/logical/pki/path_tidy.go b/builtin/logical/pki/path_tidy.go index 4d9ea992db..3ae30fd4da 100644 --- a/builtin/logical/pki/path_tidy.go +++ b/builtin/logical/pki/path_tidy.go @@ -4,6 +4,7 @@ import ( "context" "crypto/x509" "fmt" + "net/http" "sync/atomic" "time" @@ -188,7 +189,7 @@ func (b *backend) pathTidyWrite(ctx context.Context, req *logical.Request, d *fr resp := &logical.Response{} resp.AddWarning("Tidy operation successfully started. Any information from the operation will be printed to Vault's server logs.") - return resp, nil + return logical.RespondWithStatusCode(resp, req, http.StatusAccepted) } const pathTidyHelpSyn = ` diff --git a/command/lease_revoke.go b/command/lease_revoke.go index 45f5dc3a76..6077f053b7 100644 --- a/command/lease_revoke.go +++ b/command/lease_revoke.go @@ -4,6 +4,7 @@ import ( "fmt" "strings" + "github.com/hashicorp/vault/api" "github.com/mitchellh/cli" "github.com/posener/complete" ) @@ -16,6 +17,7 @@ type LeaseRevokeCommand struct { flagForce bool flagPrefix bool + flagSync bool } func (c *LeaseRevokeCommand) Synopsis() string { @@ -29,6 +31,12 @@ Usage: vault lease revoke [options] ID Revokes secrets by their lease ID. This command can revoke a single secret or multiple secrets based on a path-matched prefix. + The default behavior when not using -force is to revoke asynchronously; Vault + will queue the revocation and keep trying if it fails (including across + restarts). The -sync flag can be used to force a synchronous operation, but + it is then up to the caller to retry on failure. Force mode always operates + synchronously. + Revoke a single lease: $ vault lease revoke database/creds/readonly/2f6a614c... @@ -72,6 +80,14 @@ func (c *LeaseRevokeCommand) Flags() *FlagSets { "revoke multiple leases simultaneously.", }) + f.BoolVar(&BoolVar{ + Name: "sync", + Target: &c.flagSync, + Default: false, + Usage: "Force a synchronous operation; on failure it is up to the client " + + "to retry.", + }) + return set } @@ -114,29 +130,47 @@ func (c *LeaseRevokeCommand) Run(args []string) int { leaseID := strings.TrimSpace(args[0]) - switch { - case c.flagForce && c.flagPrefix: + revokeOpts := &api.RevokeOptions{ + LeaseID: leaseID, + Force: c.flagForce, + Prefix: c.flagPrefix, + Sync: c.flagSync, + } + + if c.flagForce { c.UI.Warn(wrapAtLength("Warning! Force-removing leases can cause Vault " + "to become out of sync with secret engines!")) - if err := client.Sys().RevokeForce(leaseID); err != nil { + } + + err = client.Sys().RevokeWithOptions(revokeOpts) + if err != nil { + switch { + case c.flagForce: c.UI.Error(fmt.Sprintf("Error force revoking leases with prefix %s: %s", leaseID, err)) return 2 - } - c.UI.Output(fmt.Sprintf("Success! Force revoked any leases with prefix: %s", leaseID)) - return 0 - case c.flagPrefix: - if err := client.Sys().RevokePrefix(leaseID); err != nil { + case c.flagPrefix: c.UI.Error(fmt.Sprintf("Error revoking leases with prefix %s: %s", leaseID, err)) return 2 - } - c.UI.Output(fmt.Sprintf("Success! Revoked any leases with prefix: %s", leaseID)) - return 0 - default: - if err := client.Sys().Revoke(leaseID); err != nil { + default: c.UI.Error(fmt.Sprintf("Error revoking lease %s: %s", leaseID, err)) return 2 } + } + + if c.flagForce { + c.UI.Output(fmt.Sprintf("Success! Force revoked any leases with prefix: %s", leaseID)) + return 0 + } + + if c.flagSync { + if c.flagPrefix { + c.UI.Output(fmt.Sprintf("Success! Revoked any leases with prefix: %s", leaseID)) + return 0 + } c.UI.Output(fmt.Sprintf("Success! Revoked lease: %s", leaseID)) return 0 } + + c.UI.Output("All revocation operations queued successfully!") + return 0 } diff --git a/command/server.go b/command/server.go index ede3611661..32be0f197f 100644 --- a/command/server.go +++ b/command/server.go @@ -1151,22 +1151,24 @@ func (c *ServerCommand) enableDev(core *vault.Core, coreConfig *vault.CoreConfig } // Upgrade the default K/V store - req := &logical.Request{ - Operation: logical.UpdateOperation, - ClientToken: init.RootToken, - Path: "sys/mounts/secret/tune", - Data: map[string]interface{}{ - "options": map[string]string{ - "version": "2", + if !c.flagDevLeasedKV { + req := &logical.Request{ + Operation: logical.UpdateOperation, + ClientToken: init.RootToken, + Path: "sys/mounts/secret/tune", + Data: map[string]interface{}{ + "options": map[string]string{ + "version": "2", + }, }, - }, - } - resp, err := core.HandleRequest(req) - if err != nil { - return nil, errwrap.Wrapf("error upgrading default K/V store: {{err}}", err) - } - if resp.IsError() { - return nil, errwrap.Wrapf("failed to upgrade default K/V store: {{err}}", resp.Error()) + } + resp, err := core.HandleRequest(req) + if err != nil { + return nil, errwrap.Wrapf("error upgrading default K/V store: {{err}}", err) + } + if resp.IsError() { + return nil, errwrap.Wrapf("failed to upgrade default K/V store: {{err}}", resp.Error()) + } } return init, nil diff --git a/http/handler_test.go b/http/handler_test.go index 3760b4c98b..34ef71f3aa 100644 --- a/http/handler_test.go +++ b/http/handler_test.go @@ -130,6 +130,31 @@ func TestHandler_CacheControlNoStore(t *testing.T) { } } +func TestHandler_Accepted(t *testing.T) { + core, _, token := vault.TestCoreUnsealed(t) + ln, addr := TestServer(t, core) + defer ln.Close() + + req, err := http.NewRequest("POST", addr+"/v1/auth/token/tidy", nil) + if err != nil { + t.Fatalf("err: %s", err) + } + req.Header.Set(AuthHeaderName, token) + + client := cleanhttp.DefaultClient() + resp, err := client.Do(req) + if err != nil { + t.Fatalf("err: %s", err) + } + + t.Logf("%#v", resp) + + testResponseStatus(t, resp, 202) + if resp.Body != http.NoBody { + t.Fatal("got non-empty body") + } +} + // We use this test to verify header auth func TestSysMounts_headerAuth(t *testing.T) { core, _, token := vault.TestCoreUnsealed(t) diff --git a/http/logical.go b/http/logical.go index 7b31c8751d..fb60010d19 100644 --- a/http/logical.go +++ b/http/logical.go @@ -268,8 +268,7 @@ func respondRaw(w http.ResponseWriter, r *http.Request, resp *logical.Response) // Get the body bodyRaw, ok := resp.Data[logical.HTTPRawBody] if !ok { - retErr(w, "no body given") - return + goto WRITE_RESPONSE } switch bodyRaw.(type) { @@ -290,6 +289,7 @@ func respondRaw(w http.ResponseWriter, r *http.Request, resp *logical.Response) } } +WRITE_RESPONSE: // Write the response if contentType != "" { w.Header().Set("Content-Type", contentType) diff --git a/logical/error.go b/logical/error.go index 3c017bdf94..b033e0eac5 100644 --- a/logical/error.go +++ b/logical/error.go @@ -1,5 +1,27 @@ package logical +import "errors" + +var ( + // ErrUnsupportedOperation is returned if the operation is not supported + // by the logical backend. + ErrUnsupportedOperation = errors.New("unsupported operation") + + // ErrUnsupportedPath is returned if the path is not supported + // by the logical backend. + ErrUnsupportedPath = errors.New("unsupported path") + + // ErrInvalidRequest is returned if the request is invalid + ErrInvalidRequest = errors.New("invalid request") + + // ErrPermissionDenied is returned if the client is not authorized + ErrPermissionDenied = errors.New("permission denied") + + // ErrMultiAuthzPending is returned if the the request needs more + // authorizations + ErrMultiAuthzPending = errors.New("request needs further approval") +) + type HTTPCodedError interface { Error() string Code() int diff --git a/logical/request.go b/logical/request.go index 4c395370d6..4bc7f50a5a 100644 --- a/logical/request.go +++ b/logical/request.go @@ -1,7 +1,6 @@ package logical import ( - "errors" "fmt" "strings" "time" @@ -273,23 +272,3 @@ const ( RenewOperation = "renew" RollbackOperation = "rollback" ) - -var ( - // ErrUnsupportedOperation is returned if the operation is not supported - // by the logical backend. - ErrUnsupportedOperation = errors.New("unsupported operation") - - // ErrUnsupportedPath is returned if the path is not supported - // by the logical backend. - ErrUnsupportedPath = errors.New("unsupported path") - - // ErrInvalidRequest is returned if the request is invalid - ErrInvalidRequest = errors.New("invalid request") - - // ErrPermissionDenied is returned if the client is not authorized - ErrPermissionDenied = errors.New("permission denied") - - // ErrMultiAuthzPending is returned if the the request needs more - // authorizations - ErrMultiAuthzPending = errors.New("request needs further approval") -) diff --git a/logical/response.go b/logical/response.go index 723f88e7d0..8a6402df12 100644 --- a/logical/response.go +++ b/logical/response.go @@ -141,22 +141,27 @@ func ListResponseWithInfo(keys []string, keyInfo map[string]interface{}) *Respon // RespondWithStatusCode takes a response and converts it to a raw response with // the provided Status Code. func RespondWithStatusCode(resp *Response, req *Request, code int) (*Response, error) { - httpResp := LogicalResponseToHTTPResponse(resp) - httpResp.RequestID = req.ID - - body, err := json.Marshal(httpResp) - if err != nil { - return nil, err - } - - return &Response{ + ret := &Response{ Data: map[string]interface{}{ HTTPContentType: "application/json", - // We default to string here so that the value is HMAC'd via audit. - // Since this function is always marshaling to JSON, this is - // appropriate. - HTTPRawBody: string(body), - HTTPStatusCode: code, + HTTPStatusCode: code, }, - }, nil + } + + if resp != nil { + httpResp := LogicalResponseToHTTPResponse(resp) + httpResp.RequestID = req.ID + + body, err := json.Marshal(httpResp) + if err != nil { + return nil, err + } + + // We default to string here so that the value is HMAC'd via audit. + // Since this function is always marshaling to JSON, this is + // appropriate. + ret.Data[HTTPRawBody] = string(body) + } + + return ret, nil } diff --git a/vault/auth.go b/vault/auth.go index a50320dc01..a0c9d0f861 100644 --- a/vault/auth.go +++ b/vault/auth.go @@ -189,7 +189,7 @@ func (c *Core) disableCredential(ctx context.Context, path string) error { if backend != nil { // Revoke credentials from this path - if err := c.expiration.RevokePrefix(fullPath); err != nil { + if err := c.expiration.RevokePrefix(fullPath, true); err != nil { return err } diff --git a/vault/expiration.go b/vault/expiration.go index 7924a40170..a56e7fcb2e 100644 --- a/vault/expiration.go +++ b/vault/expiration.go @@ -489,6 +489,38 @@ func (m *ExpirationManager) Revoke(leaseID string) error { return m.revokeCommon(leaseID, false, false) } +// LazyRevoke is used to queue revocation for a secret named by the given +// LeaseID. If the lease was not found it returns nil; if the lease was found +// it triggers a return of a 202. +func (m *ExpirationManager) LazyRevoke(leaseID string) error { + defer metrics.MeasureSince([]string{"expire", "lazy-revoke"}, time.Now()) + + // Load the entry + le, err := m.loadEntry(leaseID) + if err != nil { + return err + } + + // If there is no entry, nothing to revoke + if le == nil { + return nil + } + + le.ExpireTime = time.Now() + { + m.pendingLock.Lock() + if err := m.persistEntry(le); err != nil { + m.pendingLock.Unlock() + return err + } + + m.updatePendingInternal(le, 0) + m.pendingLock.Unlock() + } + + return nil +} + // revokeCommon does the heavy lifting. If force is true, we ignore a problem // during revocation and still remove entries/index/lease timers func (m *ExpirationManager) revokeCommon(leaseID string, force, skipToken bool) error { @@ -550,16 +582,16 @@ func (m *ExpirationManager) revokeCommon(leaseID string, force, skipToken bool) func (m *ExpirationManager) RevokeForce(prefix string) error { defer metrics.MeasureSince([]string{"expire", "revoke-force"}, time.Now()) - return m.revokePrefixCommon(prefix, true) + return m.revokePrefixCommon(prefix, true, true) } // RevokePrefix is used to revoke all secrets with a given prefix. // The prefix maps to that of the mount table to make this simpler // to reason about. -func (m *ExpirationManager) RevokePrefix(prefix string) error { +func (m *ExpirationManager) RevokePrefix(prefix string, sync bool) error { defer metrics.MeasureSince([]string{"expire", "revoke-prefix"}, time.Now()) - return m.revokePrefixCommon(prefix, false) + return m.revokePrefixCommon(prefix, false, sync) } // RevokeByToken is used to revoke all the secrets issued with a given token. @@ -623,7 +655,7 @@ func (m *ExpirationManager) RevokeByToken(te *logical.TokenEntry) error { return nil } -func (m *ExpirationManager) revokePrefixCommon(prefix string, force bool) error { +func (m *ExpirationManager) revokePrefixCommon(prefix string, force, sync bool) error { if m.inRestoreMode() { m.restoreRequestLock.Lock() defer m.restoreRequestLock.Unlock() @@ -634,10 +666,13 @@ func (m *ExpirationManager) revokePrefixCommon(prefix string, force bool) error if !strings.HasSuffix(prefix, "/") { le, err := m.loadEntry(prefix) if err == nil && le != nil { - if err := m.revokeCommon(prefix, force, false); err != nil { - return errwrap.Wrapf(fmt.Sprintf("failed to revoke %q: {{err}}", prefix), err) + if sync { + if err := m.revokeCommon(prefix, force, false); err != nil { + return errwrap.Wrapf(fmt.Sprintf("failed to revoke %q: {{err}}", prefix), err) + } + return nil } - return nil + return m.LazyRevoke(prefix) } prefix = prefix + "/" } @@ -652,10 +687,18 @@ func (m *ExpirationManager) revokePrefixCommon(prefix string, force bool) error // Revoke all the keys for idx, suffix := range existing { leaseID := prefix + suffix - if err := m.revokeCommon(leaseID, force, false); err != nil { - return errwrap.Wrapf(fmt.Sprintf("failed to revoke %q (%d / %d): {{err}}", leaseID, idx+1, len(existing)), err) + switch { + case sync: + if err := m.revokeCommon(leaseID, force, false); err != nil { + return errwrap.Wrapf(fmt.Sprintf("failed to revoke %q (%d / %d): {{err}}", leaseID, idx+1, len(existing)), err) + } + default: + if err := m.LazyRevoke(leaseID); err != nil { + return errwrap.Wrapf(fmt.Sprintf("failed to revoke %q (%d / %d): {{err}}", leaseID, idx+1, len(existing)), err) + } } } + return nil } diff --git a/vault/expiration_test.go b/vault/expiration_test.go index 6a5578a00a..f9f360a077 100644 --- a/vault/expiration_test.go +++ b/vault/expiration_test.go @@ -3,11 +3,13 @@ package vault import ( "bytes" "context" + "errors" "fmt" "reflect" "sort" "strings" "sync" + "sync/atomic" "testing" "time" @@ -1438,6 +1440,97 @@ func TestExpiration_renewEntry(t *testing.T) { } } +func TestExpiration_revokeEntry_rejected(t *testing.T) { + core, _, _ := TestCoreUnsealed(t) + exp := core.expiration + + rejected := new(uint32) + + noop := &NoopBackend{ + RequestHandler: func(ctx context.Context, req *logical.Request) (*logical.Response, error) { + if req.Operation == logical.RevokeOperation { + if atomic.CompareAndSwapUint32(rejected, 0, 1) { + t.Logf("denying revocation") + return nil, errors.New("nope") + } + t.Logf("allowing revocation") + } + return nil, nil + }, + } + _, barrier, _ := mockBarrier(t) + view := NewBarrierView(barrier, "logical/") + meUUID, err := uuid.GenerateUUID() + if err != nil { + t.Fatal(err) + } + err = exp.router.Mount(noop, "foo/bar/", &MountEntry{Path: "foo/bar/", Type: "noop", UUID: meUUID, Accessor: "noop-accessor"}, view) + if err != nil { + t.Fatal(err) + } + + le := &leaseEntry{ + LeaseID: "foo/bar/1234", + Path: "foo/bar", + Data: map[string]interface{}{ + "testing": true, + }, + Secret: &logical.Secret{ + LeaseOptions: logical.LeaseOptions{ + TTL: time.Minute, + }, + }, + IssueTime: time.Now(), + ExpireTime: time.Now().Add(time.Minute), + } + + err = exp.persistEntry(le) + if err != nil { + t.Fatal(err) + } + + err = exp.Revoke(le.LeaseID) + if err != nil { + t.Fatal(err) + } + + // Give time to let the request be handled + time.Sleep(1 * time.Second) + + if atomic.LoadUint32(rejected) != 1 { + t.Fatal("unexpected val for rejected") + } + + err = exp.Stop() + if err != nil { + t.Fatal(err) + } + + err = core.setupExpiration() + if err != nil { + t.Fatal(err) + } + exp = core.expiration + + for { + if !exp.inRestoreMode() { + break + } + time.Sleep(100 * time.Millisecond) + } + + // Now let the revocation actually process + time.Sleep(1 * time.Second) + + le, err = exp.FetchLeaseTimes(le.LeaseID) + if err != nil { + t.Fatal(err) + } + if le != nil { + t.Fatal("ugh") + } +} + func TestExpiration_renewAuthEntry(t *testing.T) { exp := mockExpiration(t) diff --git a/vault/logical_system.go b/vault/logical_system.go index fd9fe69d8f..913da8a34a 100644 --- a/vault/logical_system.go +++ b/vault/logical_system.go @@ -536,6 +536,11 @@ func NewSystemBackend(core *Core, logger log.Logger) *SystemBackend { Type: framework.TypeString, Description: strings.TrimSpace(sysHelp["lease_id"][0]), }, + "sync": &framework.FieldSchema{ + Type: framework.TypeBool, + Default: true, + Description: strings.TrimSpace(sysHelp["revoke-sync"][0]), + }, }, Callbacks: map[logical.Operation]framework.OperationFunc{ @@ -572,6 +577,11 @@ func NewSystemBackend(core *Core, logger log.Logger) *SystemBackend { Type: framework.TypeString, Description: strings.TrimSpace(sysHelp["revoke-prefix-path"][0]), }, + "sync": &framework.FieldSchema{ + Type: framework.TypeBool, + Default: true, + Description: strings.TrimSpace(sysHelp["revoke-sync"][0]), + }, }, Callbacks: map[logical.Operation]framework.OperationFunc{ @@ -1192,7 +1202,7 @@ func (b *SystemBackend) handleTidyLeases(ctx context.Context, req *logical.Reque resp := &logical.Response{} resp.AddWarning("Tidy operation successfully started. Any information from the operation will be printed to Vault's server logs.") - return resp, nil + return logical.RespondWithStatusCode(resp, req, http.StatusAccepted) } func (b *SystemBackend) invalidate(ctx context.Context, key string) { @@ -2306,27 +2316,37 @@ func (b *SystemBackend) handleRevoke(ctx context.Context, req *logical.Request, logical.ErrInvalidRequest } - // Invoke the expiration manager directly - if err := b.Core.expiration.Revoke(leaseID); err != nil { + if data.Get("sync").(bool) { + // Invoke the expiration manager directly + if err := b.Core.expiration.Revoke(leaseID); err != nil { + b.Backend.Logger().Error("lease revocation failed", "lease_id", leaseID, "error", err) + return handleErrorNoReadOnlyForward(err) + } + + return nil, nil + } + + if err := b.Core.expiration.LazyRevoke(leaseID); err != nil { b.Backend.Logger().Error("lease revocation failed", "lease_id", leaseID, "error", err) return handleErrorNoReadOnlyForward(err) } - return nil, nil + + return logical.RespondWithStatusCode(nil, nil, http.StatusAccepted) } // handleRevokePrefix is used to revoke a prefix with many LeaseIDs func (b *SystemBackend) handleRevokePrefix(ctx context.Context, req *logical.Request, data *framework.FieldData) (*logical.Response, error) { - return b.handleRevokePrefixCommon(req, data, false) + return b.handleRevokePrefixCommon(req, data, false, data.Get("sync").(bool)) } // handleRevokeForce is used to revoke a prefix with many LeaseIDs, ignoring errors func (b *SystemBackend) handleRevokeForce(ctx context.Context, req *logical.Request, data *framework.FieldData) (*logical.Response, error) { - return b.handleRevokePrefixCommon(req, data, true) + return b.handleRevokePrefixCommon(req, data, true, true) } // handleRevokePrefixCommon is used to revoke a prefix with many LeaseIDs func (b *SystemBackend) handleRevokePrefixCommon( - req *logical.Request, data *framework.FieldData, force bool) (*logical.Response, error) { + req *logical.Request, data *framework.FieldData, force, sync bool) (*logical.Response, error) { // Get all the options prefix := data.Get("prefix").(string) @@ -2335,13 +2355,18 @@ func (b *SystemBackend) handleRevokePrefixCommon( if force { err = b.Core.expiration.RevokeForce(prefix) } else { - err = b.Core.expiration.RevokePrefix(prefix) + err = b.Core.expiration.RevokePrefix(prefix, sync) } if err != nil { b.Backend.Logger().Error("revoke prefix failed", "prefix", prefix, "error", err) return handleErrorNoReadOnlyForward(err) } - return nil, nil + + if sync { + return nil, nil + } + + return logical.RespondWithStatusCode(nil, nil, http.StatusAccepted) } // handleAuthTable handles the "auth" endpoint to provide the auth table @@ -3960,6 +3985,17 @@ used to revoke the secret with the given Lease ID. `, }, + "revoke-sync": { + "Whether or not to perform the revocation synchronously", + ` +If false, the call will return immediately and revocation will be queued; if it +fails, Vault will keep trying. If true, if the revocation fails, Vault will not +automatically try again and will return an error. For revoke-prefix, this +setting will apply to all leases being revoked. For revoke-force, since errors +are ignored, this setting is not supported. +`, + }, + "revoke-prefix": { "Revoke all secrets generated in a given prefix", ` diff --git a/vault/mount.go b/vault/mount.go index 5d89e1285c..ff0f20150e 100644 --- a/vault/mount.go +++ b/vault/mount.go @@ -408,7 +408,7 @@ func (c *Core) unmountInternal(ctx context.Context, path string) error { } // Revoke all the dynamic keys - if err := c.expiration.RevokePrefix(path); err != nil { + if err := c.expiration.RevokePrefix(path, true); err != nil { return err } @@ -555,7 +555,7 @@ func (c *Core) remount(ctx context.Context, src, dst string) error { } // Revoke all the dynamic keys - if err := c.expiration.RevokePrefix(src); err != nil { + if err := c.expiration.RevokePrefix(src, true); err != nil { return err } diff --git a/vault/token_store.go b/vault/token_store.go index 9526c72c07..aade87a052 100644 --- a/vault/token_store.go +++ b/vault/token_store.go @@ -5,6 +5,7 @@ import ( "encoding/json" "errors" "fmt" + "net/http" "sync" "sync/atomic" @@ -1507,7 +1508,7 @@ func (ts *TokenStore) handleTidy(ctx context.Context, req *logical.Request, data resp := &logical.Response{} resp.AddWarning("Tidy operation successfully started. Any information from the operation will be printed to Vault's server logs.") - return resp, nil + return logical.RespondWithStatusCode(resp, req, http.StatusAccepted) } // handleUpdateLookupAccessor handles the auth/token/lookup-accessor path for returning