diff --git a/CHANGELOG.md b/CHANGELOG.md index 41aecd277f67..462328bbe29d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,10 @@ DEPRECATIONS/CHANGES: + * As of this release, the Vault CLI (via `vault unwrap`) and Go API (via + `Logical().Unwrap()`) can no longer unwrap response-wrapped tokens produced + by Vault prior to 0.6.2. These can still be read manually by performing a + read on `cubbyhole/response` and decoding the JSON-encoded value. * PKI duration return types: The PKI backend now returns durations (e.g. when reading a role) as an integer number of seconds instead of a Go-style string, in line with how the rest of Vault's API returns durations. diff --git a/api/client.go b/api/client.go index 1471b6185629..0780baddcffa 100644 --- a/api/client.go +++ b/api/client.go @@ -389,6 +389,15 @@ func (c *Client) SetClientTimeout(timeout time.Duration) { c.config.Timeout = timeout } +// CurrentWrappingLookupFunc sets a lookup function that returns desired wrap TTLs +// for a given operation and path +func (c *Client) CurrentWrappingLookupFunc() WrappingLookupFunc { + c.modifyLock.RLock() + defer c.modifyLock.RUnlock() + + return c.wrappingLookupFunc +} + // SetWrappingLookupFunc sets a lookup function that returns desired wrap TTLs // for a given operation and path func (c *Client) SetWrappingLookupFunc(lookupFunc WrappingLookupFunc) { diff --git a/api/logical.go b/api/logical.go index d5e5afae867a..ceb2b72f8e62 100644 --- a/api/logical.go +++ b/api/logical.go @@ -1,14 +1,8 @@ package api import ( - "bytes" - "fmt" "io" - "net/http" "os" - - "github.com/hashicorp/errwrap" - "github.com/hashicorp/vault/helper/jsonutil" ) const ( @@ -188,49 +182,23 @@ func (c *Logical) Unwrap(wrappingToken string) (*Secret, error) { if resp != nil { defer resp.Body.Close() } - - // Return all errors except those that are from a 404 as we handle the not - // found error as a special case. - if err != nil && (resp == nil || resp.StatusCode != 404) { - return nil, err - } - if resp == nil { - return nil, nil - } - - switch resp.StatusCode { - case http.StatusOK: // New method is supported - return ParseSecret(resp.Body) - case http.StatusNotFound: // Fall back to old method - default: + if resp != nil && resp.StatusCode == 404 { + secret, parseErr := ParseSecret(resp.Body) + switch parseErr { + 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 wrappingToken != "" { - origToken := c.c.Token() - defer c.c.SetToken(origToken) - c.c.SetToken(wrappingToken) - } - - secret, err := c.Read(wrappedResponseLocation) if err != nil { - return nil, errwrap.Wrapf(fmt.Sprintf("error reading %q: {{err}}", wrappedResponseLocation), err) - } - if secret == nil { - return nil, fmt.Errorf("no value found at %q", wrappedResponseLocation) - } - if secret.Data == nil { - return nil, fmt.Errorf("\"data\" not found in wrapping response") - } - if _, ok := secret.Data["response"]; !ok { - return nil, fmt.Errorf("\"response\" not found in wrapping response \"data\" map") - } - - wrappedSecret := new(Secret) - buf := bytes.NewBufferString(secret.Data["response"].(string)) - if err := jsonutil.DecodeJSONFromReader(buf, wrappedSecret); err != nil { - return nil, errwrap.Wrapf("error unmarshalling wrapped secret: {{err}}", err) + return nil, err } - return wrappedSecret, nil + return ParseSecret(resp.Body) } diff --git a/command/kv_helpers.go b/command/kv_helpers.go index b0e984783595..1c3c74a8c907 100644 --- a/command/kv_helpers.go +++ b/command/kv_helpers.go @@ -41,6 +41,12 @@ func kvReadRequest(client *api.Client, path string, params map[string]string) (* } func kvPreflightVersionRequest(client *api.Client, path string) (string, int, error) { + // We don't want to use a wrapping call here so save any custom value and + // restore after + currentWrappingLookupFunc := client.CurrentWrappingLookupFunc() + client.SetWrappingLookupFunc(nil) + defer client.SetWrappingLookupFunc(currentWrappingLookupFunc) + r := client.NewRequest("GET", "/v1/sys/internal/ui/mounts/"+path) resp, err := client.RawRequest(r) if resp != nil { diff --git a/http/logical.go b/http/logical.go index 2d09f67e5147..7b31c8751d8c 100644 --- a/http/logical.go +++ b/http/logical.go @@ -274,11 +274,13 @@ func respondRaw(w http.ResponseWriter, r *http.Request, resp *logical.Response) switch bodyRaw.(type) { case string: - var err error - body, err = base64.StdEncoding.DecodeString(bodyRaw.(string)) - if err != nil { - retErr(w, "cannot decode body") - return + // This is best effort. The value may already be base64-decoded so + // if it doesn't work we just use as-is + bodyDec, err := base64.StdEncoding.DecodeString(bodyRaw.(string)) + if err == nil { + body = bodyDec + } else { + body = []byte(bodyRaw.(string)) } case []byte: body = bodyRaw.([]byte) diff --git a/http/unwrapping_raw_body_test.go b/http/unwrapping_raw_body_test.go new file mode 100644 index 000000000000..358c81f5e882 --- /dev/null +++ b/http/unwrapping_raw_body_test.go @@ -0,0 +1,62 @@ +package http + +import ( + "testing" + + kv "github.com/hashicorp/vault-plugin-secrets-kv" + "github.com/hashicorp/vault/api" + "github.com/hashicorp/vault/logical" + "github.com/hashicorp/vault/vault" +) + +func TestUnwrapping_Raw_Body(t *testing.T) { + coreConfig := &vault.CoreConfig{ + LogicalBackends: map[string]logical.Factory{ + "kv": kv.Factory, + }, + } + cluster := vault.NewTestCluster(t, coreConfig, &vault.TestClusterOptions{ + HandlerFunc: Handler, + }) + cluster.Start() + defer cluster.Cleanup() + + core := cluster.Cores[0].Core + vault.TestWaitActive(t, core) + client := cluster.Cores[0].Client + + // Mount a k/v backend, version 2 + err := client.Sys().Mount("kv", &api.MountInput{ + Type: "kv", + Options: map[string]string{"version": "2"}, + }) + if err != nil { + t.Fatal(err) + } + + client.SetWrappingLookupFunc(func(operation, path string) string { + return "5m" + }) + secret, err := client.Logical().Write("kv/foo/bar", map[string]interface{}{ + "a": "b", + }) + if err != nil { + t.Fatal(err) + } + if secret == nil { + t.Fatal("nil secret") + } + if secret.WrapInfo == nil { + t.Fatal("nil wrap info") + } + wrapToken := secret.WrapInfo.Token + + client.SetWrappingLookupFunc(nil) + secret, err = client.Logical().Unwrap(wrapToken) + if err != nil { + t.Fatal(err) + } + if len(secret.Warnings) != 1 { + t.Fatal("expected 1 warning") + } +} diff --git a/logical/response.go b/logical/response.go index acfe63945c91..723f88e7d07f 100644 --- a/logical/response.go +++ b/logical/response.go @@ -24,6 +24,12 @@ const ( // This can only be specified for non-secrets, and should should be similarly // avoided like the HTTPContentType. The value must be an integer. HTTPStatusCode = "http_status_code" + + // For unwrapping we may need to know whether the value contained in the + // raw body is already JSON-unmarshaled. The presence of this key indicates + // that it has already been unmarshaled. That way we don't need to simply + // ignore errors. + HTTPRawBodyAlreadyJSONDecoded = "http_raw_body_already_json_decoded" ) // Response is a struct that stores the response of a request. @@ -146,8 +152,11 @@ func RespondWithStatusCode(resp *Response, req *Request, code int) (*Response, e return &Response{ Data: map[string]interface{}{ HTTPContentType: "application/json", - HTTPRawBody: body, - HTTPStatusCode: code, + // 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, }, }, nil } diff --git a/vault/logical_system.go b/vault/logical_system.go index f565241a03b2..afd395eaa355 100644 --- a/vault/logical_system.go +++ b/vault/logical_system.go @@ -23,6 +23,7 @@ import ( "github.com/hashicorp/vault/helper/compressutil" "github.com/hashicorp/vault/helper/consts" "github.com/hashicorp/vault/helper/identity" + "github.com/hashicorp/vault/helper/jsonutil" "github.com/hashicorp/vault/helper/parseutil" "github.com/hashicorp/vault/helper/strutil" "github.com/hashicorp/vault/helper/wrapping" @@ -3116,6 +3117,52 @@ func (b *SystemBackend) handleWrappingUnwrap(ctx context.Context, req *logical.R resp := &logical.Response{ Data: map[string]interface{}{}, } + + // Most of the time we want to just send over the marshalled HTTP bytes. + // However there is a sad separate case: if the original response was using + // bare values we need to use those or else what comes back is garbled. + httpResp := &logical.HTTPResponse{} + err = jsonutil.DecodeJSON([]byte(response), httpResp) + if err != nil { + return nil, errwrap.Wrapf("error decoding wrapped response: {{err}}", err) + } + if httpResp.Data != nil && + (httpResp.Data[logical.HTTPStatusCode] != nil || + httpResp.Data[logical.HTTPRawBody] != nil || + httpResp.Data[logical.HTTPContentType] != nil) { + if httpResp.Data[logical.HTTPStatusCode] != nil { + resp.Data[logical.HTTPStatusCode] = httpResp.Data[logical.HTTPStatusCode] + } + if httpResp.Data[logical.HTTPContentType] != nil { + resp.Data[logical.HTTPContentType] = httpResp.Data[logical.HTTPContentType] + } + + rawBody := httpResp.Data[logical.HTTPRawBody] + if rawBody != nil { + // Decode here so that we can audit properly + switch rawBody.(type) { + case string: + // Best effort decoding; if this works, the original value was + // probably a []byte instead of a string, but was marshaled + // when the value was saved, so this restores it as it was + decBytes, err := base64.StdEncoding.DecodeString(rawBody.(string)) + if err == nil { + // We end up with []byte, will not be HMAC'd + resp.Data[logical.HTTPRawBody] = decBytes + } else { + // We end up with string, will be HMAC'd + resp.Data[logical.HTTPRawBody] = rawBody + } + default: + b.Core.Logger().Error("unexpected type of raw body when decoding wrapped token", "type", fmt.Sprintf("%T", rawBody)) + } + + resp.Data[logical.HTTPRawBodyAlreadyJSONDecoded] = true + } + + return resp, nil + } + if len(response) == 0 { resp.Data[logical.HTTPStatusCode] = 204 } else { diff --git a/vault/request_handling.go b/vault/request_handling.go index 022952a16e56..0e2a7892bed3 100644 --- a/vault/request_handling.go +++ b/vault/request_handling.go @@ -105,14 +105,18 @@ func (c *Core) HandleRequest(req *logical.Request) (resp *logical.Response, err resp.Data[logical.HTTPRawBody] != nil { // Decode the JSON - httpResp := &logical.HTTPResponse{} - err := jsonutil.DecodeJSON(resp.Data[logical.HTTPRawBody].([]byte), httpResp) - if err != nil { - c.logger.Error("failed to unmarshal wrapped HTTP response for audit logging", "error", err) - return nil, ErrInternalError - } + if resp.Data[logical.HTTPRawBodyAlreadyJSONDecoded] != nil { + delete(resp.Data, logical.HTTPRawBodyAlreadyJSONDecoded) + } else { + httpResp := &logical.HTTPResponse{} + err := jsonutil.DecodeJSON(resp.Data[logical.HTTPRawBody].([]byte), httpResp) + if err != nil { + c.logger.Error("failed to unmarshal wrapped HTTP response for audit logging", "error", err) + return nil, ErrInternalError + } - auditResp = logical.HTTPResponseToLogicalResponse(httpResp) + auditResp = logical.HTTPResponseToLogicalResponse(httpResp) + } } var nonHMACReqDataKeys []string