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 c1cf97adac
commit 5d8791631c
5 changed files with 86 additions and 19 deletions

View File

@@ -4,8 +4,6 @@ import (
"context" "context"
"encoding/base64" "encoding/base64"
"fmt" "fmt"
"net/http"
"github.com/hashicorp/vault/sdk/framework" "github.com/hashicorp/vault/sdk/framework"
"github.com/hashicorp/vault/sdk/helper/errutil" "github.com/hashicorp/vault/sdk/helper/errutil"
"github.com/hashicorp/vault/sdk/helper/keysutil" "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 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+.`, 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{ Callbacks: map[logical.Operation]framework.OperationFunc{
@@ -142,6 +148,7 @@ func (b *backend) pathDecryptWrite(ctx context.Context, req *logical.Request, d
p.Lock(false) p.Lock(false)
} }
successesInBatch := false
for i, item := range batchInputItems { for i, item := range batchInputItems {
if batchResponseItems[i].Error != "" { if batchResponseItems[i].Error != "" {
continue continue
@@ -158,6 +165,7 @@ func (b *backend) pathDecryptWrite(ctx context.Context, req *logical.Request, d
batchResponseItems[i].Error = err.Error() batchResponseItems[i].Error = err.Error()
continue continue
} }
successesInBatch = true
batchResponseItems[i].Plaintext = plaintext batchResponseItems[i].Plaintext = plaintext
} }
@@ -183,18 +191,7 @@ func (b *backend) pathDecryptWrite(ctx context.Context, req *logical.Request, d
p.Unlock() p.Unlock()
// Depending on the errors in the batch, different status codes should be returned. User errors return batchRequestResponse(d, resp, req, successesInBatch, userErrorInBatch, internalErrorInBatch)
// 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
} }
const pathDecryptHelpSyn = `Decrypt a ciphertext value using a named key` 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 want []DecryptBatchResponseItem
shouldErr bool shouldErr bool
wantHTTPStatus int wantHTTPStatus int
params map[string]interface{}
}{ }{
{ {
name: "nil-input", name: "nil-input",
@@ -182,6 +183,19 @@ func TestTransit_BatchDecryption_DerivedKey(t *testing.T) {
}, },
wantHTTPStatus: http.StatusBadRequest, 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", name: "batch-full-failure",
in: []interface{}{ in: []interface{}{
@@ -194,6 +208,20 @@ func TestTransit_BatchDecryption_DerivedKey(t *testing.T) {
}, },
wantHTTPStatus: http.StatusBadRequest, 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 { for _, tt := range tests {
@@ -206,6 +234,9 @@ func TestTransit_BatchDecryption_DerivedKey(t *testing.T) {
"batch_input": tt.in, "batch_input": tt.in,
}, },
} }
for k, v := range tt.params {
req.Data[k] = v
}
resp, err = b.HandleRequest(context.Background(), req) resp, err = b.HandleRequest(context.Background(), req)
didErr := err != nil || (resp != nil && resp.IsError()) 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 Must be 0 (for latest) or a value greater than or equal
to the min_encryption_version configured on the key.`, 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{ 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 // item fails, respectively mark the error in the response
// collection and continue to process other items. // collection and continue to process other items.
warnAboutNonceUsage := false warnAboutNonceUsage := false
successesInBatch := false
for i, item := range batchInputItems { for i, item := range batchInputItems {
if batchResponseItems[i].Error != "" { if batchResponseItems[i].Error != "" {
continue continue
@@ -402,6 +411,7 @@ func (b *backend) pathEncryptWrite(ctx context.Context, req *logical.Request, d
continue continue
} }
successesInBatch = true
keyVersion := item.KeyVersion keyVersion := item.KeyVersion
if keyVersion == 0 { if keyVersion == 0 {
keyVersion = p.LatestVersion keyVersion = p.LatestVersion
@@ -443,13 +453,27 @@ func (b *backend) pathEncryptWrite(ctx context.Context, req *logical.Request, d
p.Unlock() p.Unlock()
// Depending on the errors in the batch, different status codes should be returned. User errors return batchRequestResponse(d, resp, req, successesInBatch, userErrorInBatch, internalErrorInBatch)
// 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. // 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 { switch {
case userErrorInBatch: 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: case internalErrorInBatch:
return logical.RespondWithStatusCode(resp, req, http.StatusInternalServerError) 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 all nonces are unique for a given context. Failing to do so will severely
impact the ciphertext's security. 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 ~>**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 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 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 ### Sample Payload