Implement partial_failure_response_code_override for batch requests (#17118)

* Implement partial_failure_response_code_override for batch requests

* docs

* changelog

* one more test case
This commit is contained in:
Scott Miller 2022-09-13 11:51:09 -06:00 committed by GitHub
parent 6d94dd991d
commit 12a8ef1cfd
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 86 additions and 19 deletions

View File

@ -4,8 +4,6 @@ import (
"context"
"encoding/base64"
"fmt"
"net/http"
"github.com/hashicorp/vault/sdk/framework"
"github.com/hashicorp/vault/sdk/helper/errutil"
"github.com/hashicorp/vault/sdk/helper/keysutil"
@ -51,6 +49,14 @@ Base64 encoded nonce value used during encryption. Must be provided if
convergent encryption is enabled for this key and the key was generated with
Vault 0.6.1. Not required for keys created in 0.6.2+.`,
},
"partial_failure_response_code": {
Type: framework.TypeInt,
Description: `
Ordinarily, if a batch item fails to decrypt due to a bad input, but other batch items succeed,
the HTTP response code is 400 (Bad Request). Some applications may want to treat partial failures differently.
Providing the parameter returns the given response code integer instead of a 400 in this case. If all values fail
HTTP 400 is still returned.`,
},
},
Callbacks: map[logical.Operation]framework.OperationFunc{
@ -142,6 +148,7 @@ func (b *backend) pathDecryptWrite(ctx context.Context, req *logical.Request, d
p.Lock(false)
}
successesInBatch := false
for i, item := range batchInputItems {
if batchResponseItems[i].Error != "" {
continue
@ -158,6 +165,7 @@ func (b *backend) pathDecryptWrite(ctx context.Context, req *logical.Request, d
batchResponseItems[i].Error = err.Error()
continue
}
successesInBatch = true
batchResponseItems[i].Plaintext = plaintext
}
@ -183,18 +191,7 @@ func (b *backend) pathDecryptWrite(ctx context.Context, req *logical.Request, d
p.Unlock()
// Depending on the errors in the batch, different status codes should be returned. User errors
// will return a 400 and precede internal errors which return a 500. The reasoning behind this is
// that user errors are non-retryable without making changes to the request, and should be surfaced
// to the user first.
switch {
case userErrorInBatch:
return logical.RespondWithStatusCode(resp, req, http.StatusBadRequest)
case internalErrorInBatch:
return logical.RespondWithStatusCode(resp, req, http.StatusInternalServerError)
}
return resp, nil
return batchRequestResponse(d, resp, req, successesInBatch, userErrorInBatch, internalErrorInBatch)
}
const pathDecryptHelpSyn = `Decrypt a ciphertext value using a named key`

View File

@ -119,6 +119,7 @@ func TestTransit_BatchDecryption_DerivedKey(t *testing.T) {
want []DecryptBatchResponseItem
shouldErr bool
wantHTTPStatus int
params map[string]interface{}
}{
{
name: "nil-input",
@ -182,6 +183,19 @@ func TestTransit_BatchDecryption_DerivedKey(t *testing.T) {
},
wantHTTPStatus: http.StatusBadRequest,
},
{
name: "batch-partial-success-overridden-response",
in: []interface{}{
map[string]interface{}{"ciphertext": encryptedItems[0].Ciphertext, "context": plaintextItems[1].context},
map[string]interface{}{"ciphertext": encryptedItems[1].Ciphertext, "context": plaintextItems[1].context},
},
want: []DecryptBatchResponseItem{
{Error: "cipher: message authentication failed"},
{Plaintext: plaintextItems[1].plaintext},
},
params: map[string]interface{}{"partial_failure_response_code": http.StatusAccepted},
wantHTTPStatus: http.StatusAccepted,
},
{
name: "batch-full-failure",
in: []interface{}{
@ -194,6 +208,20 @@ func TestTransit_BatchDecryption_DerivedKey(t *testing.T) {
},
wantHTTPStatus: http.StatusBadRequest,
},
{
name: "batch-full-failure-overridden-response",
in: []interface{}{
map[string]interface{}{"ciphertext": encryptedItems[0].Ciphertext, "context": plaintextItems[1].context},
map[string]interface{}{"ciphertext": encryptedItems[1].Ciphertext, "context": plaintextItems[0].context},
},
want: []DecryptBatchResponseItem{
{Error: "cipher: message authentication failed"},
{Error: "cipher: message authentication failed"},
},
params: map[string]interface{}{"partial_failure_response_code": http.StatusAccepted},
// Full failure, shouldn't affect status code
wantHTTPStatus: http.StatusBadRequest,
},
}
for _, tt := range tests {
@ -206,6 +234,9 @@ func TestTransit_BatchDecryption_DerivedKey(t *testing.T) {
"batch_input": tt.in,
},
}
for k, v := range tt.params {
req.Data[k] = v
}
resp, err = b.HandleRequest(context.Background(), req)
didErr := err != nil || (resp != nil && resp.IsError())

View File

@ -113,6 +113,14 @@ will severely impact the ciphertext's security.`,
Must be 0 (for latest) or a value greater than or equal
to the min_encryption_version configured on the key.`,
},
"partial_failure_response_code": {
Type: framework.TypeInt,
Description: `
Ordinarily, if a batch item fails to encrypt due to a bad input, but other batch items succeed,
the HTTP response code is 400 (Bad Request). Some applications may want to treat partial failures differently.
Providing the parameter returns the given response code integer instead of a 400 in this case. If all values fail
HTTP 400 is still returned.`,
},
},
Callbacks: map[logical.Operation]framework.OperationFunc{
@ -375,6 +383,7 @@ func (b *backend) pathEncryptWrite(ctx context.Context, req *logical.Request, d
// item fails, respectively mark the error in the response
// collection and continue to process other items.
warnAboutNonceUsage := false
successesInBatch := false
for i, item := range batchInputItems {
if batchResponseItems[i].Error != "" {
continue
@ -402,6 +411,7 @@ func (b *backend) pathEncryptWrite(ctx context.Context, req *logical.Request, d
continue
}
successesInBatch = true
keyVersion := item.KeyVersion
if keyVersion == 0 {
keyVersion = p.LatestVersion
@ -443,13 +453,27 @@ func (b *backend) pathEncryptWrite(ctx context.Context, req *logical.Request, d
p.Unlock()
// Depending on the errors in the batch, different status codes should be returned. User errors
// will return a 400 and precede internal errors which return a 500. The reasoning behind this is
// that user errors are non-retryable without making changes to the request, and should be surfaced
// to the user first.
return batchRequestResponse(d, resp, req, successesInBatch, userErrorInBatch, internalErrorInBatch)
}
// Depending on the errors in the batch, different status codes should be returned. User errors
// will return a 400 and precede internal errors which return a 500. The reasoning behind this is
// that user errors are non-retryable without making changes to the request, and should be surfaced
// to the user first.
func batchRequestResponse(d *framework.FieldData, resp *logical.Response, req *logical.Request, successesInBatch, userErrorInBatch, internalErrorInBatch bool) (*logical.Response, error) {
switch {
case userErrorInBatch:
return logical.RespondWithStatusCode(resp, req, http.StatusBadRequest)
code := http.StatusBadRequest
if successesInBatch {
if codeRaw, ok := d.GetOk("partial_failure_response_code"); ok {
code = codeRaw.(int)
if code < 1 || code > 599 {
resp.AddWarning("invalid HTTP response code override from partial_failure_response_code, reverting to HTTP 400")
code = http.StatusBadRequest
}
}
}
return logical.RespondWithStatusCode(resp, req, code)
case internalErrorInBatch:
return logical.RespondWithStatusCode(resp, req, http.StatusInternalServerError)
}

4
changelog/17118.txt Normal file
View File

@ -0,0 +1,4 @@
```release-note:improvement
secrets/transit: Added a parameter to encrypt/decrypt batch operations to allow the caller to
override the HTTP response code in case of partial user-input failures.
```

View File

@ -579,6 +579,12 @@ will be returned.
all nonces are unique for a given context. Failing to do so will severely
impact the ciphertext's security.
- `partial_failure_response_code` `(int: 400)` Ordinarily, if a batch item fails
to encrypt due to a bad input, but other batch items succeed, the HTTP response
code is 400 (Bad Request). Some applications may want to treat partial failures
differently. Providing the parameter returns the given response code integer
instead of a 400 in this case. If all values fail HTTP 400 is still returned.
~>**NOTE:** All plaintext data **must be base64-encoded**. The reason for this
requirement is that Vault does not require that the plaintext is "text". It
could be a binary file such as a PDF or image. The easiest safe transport
@ -663,6 +669,11 @@ This endpoint decrypts the provided ciphertext using the named key.
}
]
```
- `partial_failure_response_code` `(int: 400)` Ordinarily, if a batch item fails
to encrypt due to a bad input, but other batch items succeed, the HTTP response
code is 400 (Bad Request). Some applications may want to treat partial failures
differently. Providing the parameter returns the given response code integer
instead of a 400 in this case. If all values fail HTTP 400 is still returned.
### Sample Payload