luxolus
/
open-vault
2
0
Fork 0
Code Issues 1 Pull Requests Releases Activity

backport of commit 7d4d8cb708de62167340fa84770f8237c7bfdd1e (#22900) 

Co-authored-by: Scott Miller <smiller@hashicorp.com>
This commit is contained in:
hc-github-team-secure-vault-core 2023-09-08 10:48:26 -04:00 committed by GitHub
parent d019802cc0
commit bdee24128a
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
7 changed files with 142 additions and 13 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

25
builtin/logical/transit/backend_test.go
View File

@ -1153,10 +1153,12 @@ func testConvergentEncryptionCommon(t *testing.T, ver int, keyType keysutil.KeyT
// Now test encrypting the same value twice
req.Data = map[string]interface{}{
"plaintext": "emlwIHphcA==", // "zip zap"
"nonce": "b25ldHdvdGhyZWVl", // "onetwothreee"
"plaintext": "emlwIHphcA==", // "zip zap"
"context": "pWZ6t/im3AORd0lVYE0zBdKpX6Bl3/SvFtoVTPWbdkzjG788XmMAnOlxandSdd7S",
}
if ver == 0 {
req.Data["nonce"] = "b25ldHdvdGhyZWVl" // "onetwothreee"
}
resp, err = b.HandleRequest(context.Background(), req)
if err != nil {
t.Fatal(err)
@ -1187,11 +1189,10 @@ func testConvergentEncryptionCommon(t *testing.T, ver int, keyType keysutil.KeyT
// For sanity, also check a different nonce value...
req.Data = map[string]interface{}{
"plaintext": "emlwIHphcA==", // "zip zap"
"nonce": "dHdvdGhyZWVmb3Vy", // "twothreefour"
"plaintext": "emlwIHphcA==", // "zip zap"
"context": "pWZ6t/im3AORd0lVYE0zBdKpX6Bl3/SvFtoVTPWbdkzjG788XmMAnOlxandSdd7S",
}
if ver < 2 {
if ver == 0 {
req.Data["nonce"] = "dHdvdGhyZWVmb3Vy" // "twothreefour"
} else {
req.Data["context"] = "pWZ6t/im3AORd0lVYE0zBdKpX6Bl3/SvFtoVTPWbdkzjG788XmMAnOldandSdd7S"
@ -1230,10 +1231,12 @@ func testConvergentEncryptionCommon(t *testing.T, ver int, keyType keysutil.KeyT
// ...and a different context value
req.Data = map[string]interface{}{
"plaintext": "emlwIHphcA==", // "zip zap"
"nonce": "dHdvdGhyZWVmb3Vy", // "twothreefour"
"plaintext": "emlwIHphcA==", // "zip zap"
"context": "qV4h9iQyvn+raODOer4JNAsOhkXBwdT4HZ677Ql4KLqXSU+Jk4C/fXBWbv6xkSYT",
}
if ver == 0 {
req.Data["nonce"] = "dHdvdGhyZWVmb3Vy" // "twothreefour"
}
resp, err = b.HandleRequest(context.Background(), req)
if err != nil {
t.Fatal(err)
@ -1345,9 +1348,11 @@ func testConvergentEncryptionCommon(t *testing.T, ver int, keyType keysutil.KeyT
// Finally, check operations on empty values
// First, check without setting a plaintext at all
req.Data = map[string]interface{}{
"nonce": "b25ldHdvdGhyZWVl", // "onetwothreee"
"context": "pWZ6t/im3AORd0lVYE0zBdKpX6Bl3/SvFtoVTPWbdkzjG788XmMAnOlxandSdd7S",
}
if ver == 0 {
req.Data["nonce"] = "dHdvdGhyZWVmb3Vy" // "twothreefour"
}
resp, err = b.HandleRequest(context.Background(), req)
if err == nil {
t.Fatal("expected error, got nil")
@ -1362,9 +1367,11 @@ func testConvergentEncryptionCommon(t *testing.T, ver int, keyType keysutil.KeyT
// Now set plaintext to empty
req.Data = map[string]interface{}{
"plaintext": "",
"nonce": "b25ldHdvdGhyZWVl", // "onetwothreee"
"context": "pWZ6t/im3AORd0lVYE0zBdKpX6Bl3/SvFtoVTPWbdkzjG788XmMAnOlxandSdd7S",
}
if ver == 0 {
req.Data["nonce"] = "dHdvdGhyZWVmb3Vy" // "twothreefour"
}
resp, err = b.HandleRequest(context.Background(), req)
if err != nil {
t.Fatal(err)

4
builtin/logical/transit/path_datakey.go
View File

@ -170,6 +170,10 @@ func (b *backend) pathDatakeyWrite(ctx context.Context, req *logical.Request, d
},
}
if len(nonce) > 0 && !nonceAllowed(p) {
return nil, ErrNonceNotAllowed
}
if constants.IsFIPS() && shouldWarnAboutNonceUsage(p, nonce) {
resp.AddWarning("A provided nonce value was used within FIPS mode, this violates FIPS 140 compliance.")
}

26
builtin/logical/transit/path_encrypt.go
View File

@ -468,6 +468,13 @@ func (b *backend) pathEncryptWrite(ctx context.Context, req *logical.Request, d
successesInBatch := false
for i, item := range batchInputItems {
if batchResponseItems[i].Error != "" {
userErrorInBatch = true
continue
}
if item.Nonce != "" && !nonceAllowed(p) {
userErrorInBatch = true
batchResponseItems[i].Error = ErrNonceNotAllowed.Error()
continue
}
@ -568,6 +575,25 @@ func (b *backend) pathEncryptWrite(ctx context.Context, req *logical.Request, d
return batchRequestResponse(d, resp, req, successesInBatch, userErrorInBatch, internalErrorInBatch)
}
func nonceAllowed(p *keysutil.Policy) bool {
var supportedKeyType bool
switch p.Type {
case keysutil.KeyType_MANAGED_KEY:
return true
case keysutil.KeyType_AES128_GCM96, keysutil.KeyType_AES256_GCM96, keysutil.KeyType_ChaCha20_Poly1305:
supportedKeyType = true
default:
supportedKeyType = false
}
if supportedKeyType && p.ConvergentEncryption && p.ConvergentVersion == 1 {
// We only use the user supplied nonce for v1 convergent encryption keys
return true
}
return false
}
// 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

81
builtin/logical/transit/path_encrypt_test.go
View File

@ -7,6 +7,7 @@ import (
"context"
"encoding/json"
"fmt"
"net/http"
"reflect"
"strings"
"testing"
@ -654,13 +655,26 @@ func TestTransit_BatchEncryptionCase12(t *testing.T) {
}
// Case13: Incorrect input for nonce when we aren't in convergent encryption should fail the operation
func TestTransit_BatchEncryptionCase13(t *testing.T) {
func TestTransit_EncryptionCase13(t *testing.T) {
var err error
b, s := createBackendWithStorage(t)
// Non-batch first
data := map[string]interface{}{"plaintext": "bXkgc2VjcmV0IGRhdGE=", "nonce": "R80hr9eNUIuFV52e"}
req := &logical.Request{
Operation: logical.CreateOperation,
Path: "encrypt/my-key",
Storage: s,
Data: data,
}
resp, err := b.HandleRequest(context.Background(), req)
if err == nil {
t.Fatal("expected invalid request")
}
batchInput := []interface{}{
map[string]interface{}{"plaintext": "bXkgc2VjcmV0IGRhdGE=", "nonce": "YmFkbm9uY2U="},
map[string]interface{}{"plaintext": "bXkgc2VjcmV0IGRhdGE=", "nonce": "R80hr9eNUIuFV52e"},
}
batchData := map[string]interface{}{
@ -672,10 +686,71 @@ func TestTransit_BatchEncryptionCase13(t *testing.T) {
Storage: s,
Data: batchData,
}
_, err = b.HandleRequest(context.Background(), batchReq)
resp, err = b.HandleRequest(context.Background(), batchReq)
if err != nil {
t.Fatal(err)
}
if v, ok := resp.Data["http_status_code"]; !ok || v.(int) != http.StatusBadRequest {
t.Fatal("expected request error")
}
}
// Case14: Incorrect input for nonce when we are in convergent version 3 should fail
func TestTransit_EncryptionCase14(t *testing.T) {
var err error
b, s := createBackendWithStorage(t)
cReq := &logical.Request{
Operation: logical.UpdateOperation,
Path: "keys/my-key",
Storage: s,
Data: map[string]interface{}{
"convergent_encryption": "true",
"derived": "true",
},
}
resp, err := b.HandleRequest(context.Background(), cReq)
if err != nil {
t.Fatal(err)
}
// Non-batch first
data := map[string]interface{}{"plaintext": "bXkgc2VjcmV0IGRhdGE=", "context": "SGVsbG8sIFdvcmxkCg==", "nonce": "R80hr9eNUIuFV52e"}
req := &logical.Request{
Operation: logical.CreateOperation,
Path: "encrypt/my-key",
Storage: s,
Data: data,
}
resp, err = b.HandleRequest(context.Background(), req)
if err == nil {
t.Fatal("expected invalid request")
}
batchInput := []interface{}{
data,
}
batchData := map[string]interface{}{
"batch_input": batchInput,
}
batchReq := &logical.Request{
Operation: logical.CreateOperation,
Path: "encrypt/my-key",
Storage: s,
Data: batchData,
}
resp, err = b.HandleRequest(context.Background(), batchReq)
if err != nil {
t.Fatal(err)
}
if v, ok := resp.Data["http_status_code"]; !ok || v.(int) != http.StatusBadRequest {
t.Fatal("expected request error")
}
}
// Test that the fast path function decodeBatchRequestItems behave like mapstructure.Decode() to decode []BatchRequestItem.

8
builtin/logical/transit/path_rewrap.go
View File

@ -6,6 +6,7 @@ package transit
import (
"context"
"encoding/base64"
"errors"
"fmt"
"github.com/hashicorp/vault/helper/constants"
@ -16,6 +17,8 @@ import (
"github.com/mitchellh/mapstructure"
)
var ErrNonceNotAllowed = errors.New("provided nonce not allowed for this key")
func (b *backend) pathRewrap() *framework.Path {
return &framework.Path{
Pattern: "rewrap/" + framework.GenericNameRegex("name"),
@ -152,6 +155,11 @@ func (b *backend) pathRewrapWrite(ctx context.Context, req *logical.Request, d *
continue
}
if item.Nonce != "" && !nonceAllowed(p) {
batchResponseItems[i].Error = ErrNonceNotAllowed.Error()
continue
}
plaintext, err := p.Decrypt(item.DecodedContext, item.DecodedNonce, item.Ciphertext)
if err != nil {
switch err.(type) {

3
changelog/22852.txt Normal file
View File

@ -0,0 +1,3 @@
```release-note:security
secrets/transit: fix a regression that was honoring nonces provided in non-convergent modes during encryption.
```

8
sdk/helper/keysutil/policy.go
View File

@ -2009,9 +2009,15 @@ func (p *Policy) EncryptWithFactory(ver int, context []byte, nonce []byte, value
encBytes := 32
hmacBytes := 0
if p.convergentVersion(ver) > 2 {
convergentVersion := p.convergentVersion(ver)
if convergentVersion > 2 {
deriveHMAC = true
hmacBytes = 32
if len(nonce) > 0 {
return "", errutil.UserError{Err: "nonce provided when not allowed"}
}
} else if len(nonce) > 0 && (!p.ConvergentEncryption || convergentVersion != 1) {
return "", errutil.UserError{Err: "nonce provided when not allowed"}
}
if p.Type == KeyType_AES128_GCM96 {
encBytes = 16