From 12a8ef1cfd051a20ea679716a943d9269adf09c7 Mon Sep 17 00:00:00 2001 From: Scott Miller Date: Tue, 13 Sep 2022 11:51:09 -0600 Subject: [PATCH] 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 --- builtin/logical/transit/path_decrypt.go | 25 +++++++------- builtin/logical/transit/path_decrypt_test.go | 31 ++++++++++++++++++ builtin/logical/transit/path_encrypt.go | 34 +++++++++++++++++--- changelog/17118.txt | 4 +++ website/content/api-docs/secret/transit.mdx | 11 +++++++ 5 files changed, 86 insertions(+), 19 deletions(-) create mode 100644 changelog/17118.txt diff --git a/builtin/logical/transit/path_decrypt.go b/builtin/logical/transit/path_decrypt.go index 7a5d53e54..6b7cef805 100644 --- a/builtin/logical/transit/path_decrypt.go +++ b/builtin/logical/transit/path_decrypt.go @@ -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` diff --git a/builtin/logical/transit/path_decrypt_test.go b/builtin/logical/transit/path_decrypt_test.go index c52cd6cf0..9a6b21aa1 100644 --- a/builtin/logical/transit/path_decrypt_test.go +++ b/builtin/logical/transit/path_decrypt_test.go @@ -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()) diff --git a/builtin/logical/transit/path_encrypt.go b/builtin/logical/transit/path_encrypt.go index aa5bb503b..3e0c72037 100644 --- a/builtin/logical/transit/path_encrypt.go +++ b/builtin/logical/transit/path_encrypt.go @@ -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) } diff --git a/changelog/17118.txt b/changelog/17118.txt new file mode 100644 index 000000000..76c7748f0 --- /dev/null +++ b/changelog/17118.txt @@ -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. +``` \ No newline at end of file diff --git a/website/content/api-docs/secret/transit.mdx b/website/content/api-docs/secret/transit.mdx index cc103bcb0..f572114da 100644 --- a/website/content/api-docs/secret/transit.mdx +++ b/website/content/api-docs/secret/transit.mdx @@ -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