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
This commit is contained in:
Jeff Mitchell 2018-04-03 22:35:45 -04:00 committed by GitHub
parent 69a8158913
commit 599f691141
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
6 changed files with 136 additions and 5 deletions

View File

@ -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
}

View File

@ -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.

View File

@ -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
}

View File

@ -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 {

View File

@ -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
}

View File

@ -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
}