From 19e50617799443ed037ff965ca7dae76de1473b0 Mon Sep 17 00:00:00 2001 From: Jeff Mitchell Date: Tue, 3 Apr 2018 22:35:45 -0400 Subject: [PATCH] Allow returning warnings and other data in 404s in the Go API (#4256) * Allow returning list information and other data in 404s. On read it'll output data and/or warnings on a 404 if they exist. On list, the same behavior; the actual 'vault list' command doesn't change behavior though in terms of output unless there are no actual keys (so it doesn't just magically show other data). This corrects some assumptions in response_util and wrapping.go; it also corrects a few places in the latter where it could leak a (useless) token in some error cases. * Use same 404 logic in delete/put too * Add the same secret parsing logic to the KV request functions --- api/logical.go | 51 ++++++++++++++++++++++++++++++++++++++++ api/response.go | 8 +++++-- command/kv_helpers.go | 51 ++++++++++++++++++++++++++++++++++++++++ command/list.go | 7 +++++- logical/response_util.go | 12 +++++++++- vault/wrapping.go | 12 +++++++++- 6 files changed, 136 insertions(+), 5 deletions(-) diff --git a/api/logical.go b/api/logical.go index a492b5ab92..4e5d7ce5a0 100644 --- a/api/logical.go +++ b/api/logical.go @@ -3,6 +3,7 @@ package api import ( "bytes" "fmt" + "io" "net/http" "os" @@ -50,6 +51,17 @@ func (c *Logical) Read(path string) (*Secret, error) { defer resp.Body.Close() } if resp != nil && resp.StatusCode == 404 { + secret, err := ParseSecret(resp.Body) + switch err { + case nil: + case io.EOF: + return nil, nil + default: + return nil, err + } + if secret != nil && (len(secret.Warnings) > 0 || len(secret.Data) > 0) { + return secret, nil + } return nil, nil } if err != nil { @@ -70,6 +82,17 @@ func (c *Logical) List(path string) (*Secret, error) { defer resp.Body.Close() } if resp != nil && resp.StatusCode == 404 { + secret, err := ParseSecret(resp.Body) + switch err { + case nil: + case io.EOF: + return nil, nil + default: + return nil, err + } + if secret != nil && (len(secret.Warnings) > 0 || len(secret.Data) > 0) { + return secret, nil + } return nil, nil } if err != nil { @@ -89,6 +112,20 @@ func (c *Logical) Write(path string, data map[string]interface{}) (*Secret, erro if resp != nil { defer resp.Body.Close() } + if resp != nil && resp.StatusCode == 404 { + secret, err := ParseSecret(resp.Body) + switch err { + case nil: + case io.EOF: + return nil, nil + default: + return nil, err + } + if secret != nil && (len(secret.Warnings) > 0 || len(secret.Data) > 0) { + return secret, nil + } + return nil, nil + } if err != nil { return nil, err } @@ -106,6 +143,20 @@ func (c *Logical) Delete(path string) (*Secret, error) { if resp != nil { defer resp.Body.Close() } + if resp != nil && resp.StatusCode == 404 { + secret, err := ParseSecret(resp.Body) + switch err { + case nil: + case io.EOF: + return nil, nil + default: + return nil, err + } + if secret != nil && (len(secret.Warnings) > 0 || len(secret.Data) > 0) { + return secret, nil + } + return nil, nil + } if err != nil { return nil, err } diff --git a/api/response.go b/api/response.go index 05502e1b0f..053a277238 100644 --- a/api/response.go +++ b/api/response.go @@ -4,6 +4,7 @@ import ( "bytes" "fmt" "io" + "io/ioutil" "net/http" "github.com/hashicorp/vault/helper/jsonutil" @@ -33,11 +34,14 @@ func (r *Response) Error() error { // We have an error. Let's copy the body into our own buffer first, // so that if we can't decode JSON, we can at least copy it raw. - var bodyBuf bytes.Buffer - if _, err := io.Copy(&bodyBuf, r.Body); err != nil { + bodyBuf := &bytes.Buffer{} + if _, err := io.Copy(bodyBuf, r.Body); err != nil { return err } + r.Body.Close() + r.Body = ioutil.NopCloser(bodyBuf) + // Decode the error response if we can. Note that we wrap the bodyBuf // in a bytes.Reader here so that the JSON decoder doesn't move the // read pointer for the original buffer. diff --git a/command/kv_helpers.go b/command/kv_helpers.go index 0743caf2a8..86282625de 100644 --- a/command/kv_helpers.go +++ b/command/kv_helpers.go @@ -3,6 +3,7 @@ package command import ( "errors" "fmt" + "io" "net/http" "path" "strings" @@ -27,6 +28,17 @@ func kvReadRequest(client *api.Client, path string, params map[string]string) (* defer resp.Body.Close() } if resp != nil && resp.StatusCode == 404 { + secret, err := api.ParseSecret(resp.Body) + switch err { + case nil: + case io.EOF: + return nil, nil + default: + return nil, err + } + if secret != nil && (len(secret.Warnings) > 0 || len(secret.Data) > 0) { + return secret, nil + } return nil, nil } if err != nil { @@ -52,6 +64,17 @@ func kvListRequest(client *api.Client, path string) (*api.Secret, error) { defer resp.Body.Close() } if resp != nil && resp.StatusCode == 404 { + secret, err := api.ParseSecret(resp.Body) + switch err { + case nil: + case io.EOF: + return nil, nil + default: + return nil, err + } + if secret != nil && (len(secret.Warnings) > 0 || len(secret.Data) > 0) { + return secret, nil + } return nil, nil } if err != nil { @@ -75,6 +98,20 @@ func kvWriteRequest(client *api.Client, path string, data map[string]interface{} if resp != nil { defer resp.Body.Close() } + if resp != nil && resp.StatusCode == 404 { + secret, err := api.ParseSecret(resp.Body) + switch err { + case nil: + case io.EOF: + return nil, nil + default: + return nil, err + } + if secret != nil && (len(secret.Warnings) > 0 || len(secret.Data) > 0) { + return secret, nil + } + return nil, nil + } if err != nil { return nil, err } @@ -96,6 +133,20 @@ func kvDeleteRequest(client *api.Client, path string) (*api.Secret, error) { if resp != nil { defer resp.Body.Close() } + if resp != nil && resp.StatusCode == 404 { + secret, err := api.ParseSecret(resp.Body) + switch err { + case nil: + case io.EOF: + return nil, nil + default: + return nil, err + } + if secret != nil && (len(secret.Warnings) > 0 || len(secret.Data) > 0) { + return secret, nil + } + return nil, nil + } if err != nil { return nil, err } diff --git a/command/list.go b/command/list.go index 151b46fc5b..56762151ac 100644 --- a/command/list.go +++ b/command/list.go @@ -82,10 +82,15 @@ func (c *ListCommand) Run(args []string) int { c.UI.Error(fmt.Sprintf("Error listing %s: %s", path, err)) return 2 } - if secret == nil || secret.Data == nil { + if secret == nil { c.UI.Error(fmt.Sprintf("No value found at %s", path)) return 2 } + if secret.Data == nil { + // If secret wasn't nil, we have warnings, so output them anyways. We + // may also have non-keys info. + return OutputSecret(c.UI, secret) + } // If the secret is wrapped, return the wrapped response. if secret.WrapInfo != nil && secret.WrapInfo.TTL != 0 { diff --git a/logical/response_util.go b/logical/response_util.go index 41e617a8b4..803ae9fbd3 100644 --- a/logical/response_util.go +++ b/logical/response_util.go @@ -24,11 +24,21 @@ func RespondErrorCommon(req *Request, resp *Response, err error) (int, error) { // Basically: if we have empty "keys" or no keys at all, 404. This // provides consistency with GET. case req.Operation == ListOperation && resp.WrapInfo == nil: - if resp == nil || len(resp.Data) == 0 { + if resp == nil { + return http.StatusNotFound, nil + } + if len(resp.Data) == 0 { + if len(resp.Warnings) > 0 { + return 0, nil + } return http.StatusNotFound, nil } keysRaw, ok := resp.Data["keys"] if !ok || keysRaw == nil { + // If we don't have keys but have other data, return as-is + if len(resp.Data) > 0 || len(resp.Warnings) > 0 { + return 0, nil + } return http.StatusNotFound, nil } diff --git a/vault/wrapping.go b/vault/wrapping.go index fd48d60293..925e80b44c 100644 --- a/vault/wrapping.go +++ b/vault/wrapping.go @@ -77,13 +77,20 @@ func (c *Core) wrapInCubbyhole(ctx context.Context, req *logical.Request, resp * // Before wrapping, obey special rules for listing: if no entries are // found, 404. This prevents unwrapping only to find empty data. if req.Operation == logical.ListOperation { - if resp == nil || len(resp.Data) == 0 { + if resp == nil || (len(resp.Data) == 0 && len(resp.Warnings) == 0) { return nil, logical.ErrUnsupportedPath } + keysRaw, ok := resp.Data["keys"] if !ok || keysRaw == nil { + if len(resp.Data) > 0 || len(resp.Warnings) > 0 { + // We could be returning extra metadata on a list, or returning + // warnings with no data, so handle these cases + goto DONELISTHANDLING + } return nil, logical.ErrUnsupportedPath } + keys, ok := keysRaw.([]string) if !ok { return nil, logical.ErrUnsupportedPath @@ -93,6 +100,7 @@ func (c *Core) wrapInCubbyhole(ctx context.Context, req *logical.Request, resp * } } +DONELISTHANDLING: var err error sealWrap := resp.WrapInfo.SealWrap @@ -151,6 +159,7 @@ func (c *Core) wrapInCubbyhole(ctx context.Context, req *logical.Request, resp * jwt := jws.NewJWT(claims, crypto.SigningMethodES512) serWebToken, err := jwt.Serialize(c.wrappingJWTKey) if err != nil { + c.tokenStore.Revoke(ctx, te.ID) c.logger.Error("failed to serialize JWT", "error", err) return nil, ErrInternalError } @@ -191,6 +200,7 @@ func (c *Core) wrapInCubbyhole(ctx context.Context, req *logical.Request, resp * marshaledResponse, err := json.Marshal(httpResponse) if err != nil { + c.tokenStore.Revoke(ctx, te.ID) c.logger.Error("failed to marshal wrapped response", "error", err) return nil, ErrInternalError }