From d5acfe3a8d93ef80b20a55728ae5da036cfea5de Mon Sep 17 00:00:00 2001 From: Steven Clark Date: Wed, 8 Dec 2021 14:37:25 -0500 Subject: [PATCH] Warn user supplying nonce values in FIPS mode for transit encryption requests (#13366) * Warn user supplying nonce values in FIPS mode for transit encryption requests - Send back a warning within the response if an end-user supplies nonce values that we use within the various transit encrypt apis. - We do not send a warning if an end-user supplies a nonce value but we don't use it. - Affected api methods are encrypt, rewrap and datakey - The warning is only sent when we are operating in FIPS mode. --- builtin/logical/transit/path_datakey.go | 6 +- builtin/logical/transit/path_encrypt.go | 38 ++++++++++ builtin/logical/transit/path_encrypt_test.go | 80 ++++++++++++++++++++ builtin/logical/transit/path_rewrap.go | 11 ++- helper/constants/fips.go | 8 ++ 5 files changed, 141 insertions(+), 2 deletions(-) create mode 100644 helper/constants/fips.go diff --git a/builtin/logical/transit/path_datakey.go b/builtin/logical/transit/path_datakey.go index 9e9ef2c173..a9c1f426d9 100644 --- a/builtin/logical/transit/path_datakey.go +++ b/builtin/logical/transit/path_datakey.go @@ -5,7 +5,7 @@ import ( "crypto/rand" "encoding/base64" "fmt" - + "github.com/hashicorp/vault/helper/constants" "github.com/hashicorp/vault/sdk/framework" "github.com/hashicorp/vault/sdk/helper/errutil" "github.com/hashicorp/vault/sdk/helper/keysutil" @@ -159,6 +159,10 @@ func (b *backend) pathDatakeyWrite(ctx context.Context, req *logical.Request, d }, } + if constants.IsFIPS() && shouldWarnAboutNonceUsage(p, nonce) { + resp.AddWarning("A provided nonce value was used within FIPS mode, this violates FIPS 140 compliance.") + } + if plaintextAllowed { resp.Data["plaintext"] = base64.StdEncoding.EncodeToString(newKey) } diff --git a/builtin/logical/transit/path_encrypt.go b/builtin/logical/transit/path_encrypt.go index deb564b129..ac683dadd1 100644 --- a/builtin/logical/transit/path_encrypt.go +++ b/builtin/logical/transit/path_encrypt.go @@ -5,6 +5,7 @@ import ( "encoding/base64" "encoding/json" "fmt" + "github.com/hashicorp/vault/helper/constants" "net/http" "reflect" @@ -357,11 +358,16 @@ func (b *backend) pathEncryptWrite(ctx context.Context, req *logical.Request, d // Process batch request items. If encryption of any request // item fails, respectively mark the error in the response // collection and continue to process other items. + warnAboutNonceUsage := false for i, item := range batchInputItems { if batchResponseItems[i].Error != "" { continue } + if !warnAboutNonceUsage && shouldWarnAboutNonceUsage(p, item.DecodedNonce) { + warnAboutNonceUsage = true + } + ciphertext, err := p.Encrypt(item.KeyVersion, item.DecodedContext, item.DecodedNonce, item.Plaintext) if err != nil { switch err.(type) { @@ -411,6 +417,10 @@ func (b *backend) pathEncryptWrite(ctx context.Context, req *logical.Request, d } } + if constants.IsFIPS() && warnAboutNonceUsage { + resp.AddWarning("A provided nonce value was used within FIPS mode, this violates FIPS 140 compliance.") + } + if req.Operation == logical.CreateOperation && !upserted { resp.AddWarning("Attempted creation of the key during the encrypt operation, but it was created beforehand") } @@ -431,6 +441,34 @@ func (b *backend) pathEncryptWrite(ctx context.Context, req *logical.Request, d return resp, nil } +// shouldWarnAboutNonceUsage attempts to determine if we will use a provided nonce or not. Ideally this +// would be information returned through p.Encrypt but that would require an SDK api change and this is +// transit specific +func shouldWarnAboutNonceUsage(p *keysutil.Policy, userSuppliedNonce []byte) bool { + if len(userSuppliedNonce) == 0 { + return false + } + + var supportedKeyType bool + switch p.Type { + 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 + } + + if supportedKeyType && !p.ConvergentEncryption { + return true + } + + return false +} + const pathEncryptHelpSyn = `Encrypt a plaintext value or a batch of plaintext blocks using a named key` diff --git a/builtin/logical/transit/path_encrypt_test.go b/builtin/logical/transit/path_encrypt_test.go index 1842e069a5..227ff4cf49 100644 --- a/builtin/logical/transit/path_encrypt_test.go +++ b/builtin/logical/transit/path_encrypt_test.go @@ -3,6 +3,7 @@ package transit import ( "context" "encoding/json" + "github.com/hashicorp/vault/sdk/helper/keysutil" "reflect" "strings" "testing" @@ -729,3 +730,82 @@ func TestTransit_decodeBatchRequestItems(t *testing.T) { }) } } + +func TestShouldWarnAboutNonceUsage(t *testing.T) { + tests := []struct { + name string + keyTypes []keysutil.KeyType + nonce []byte + convergentEncryption bool + convergentVersion int + expected bool + }{ + { + name: "-NoConvergent-WithNonce", + keyTypes: []keysutil.KeyType{keysutil.KeyType_AES256_GCM96, keysutil.KeyType_AES128_GCM96, keysutil.KeyType_ChaCha20_Poly1305}, + nonce: []byte("testnonce"), + convergentEncryption: false, + convergentVersion: -1, + expected: true, + }, + { + name: "-NoConvergent-NoNonce", + keyTypes: []keysutil.KeyType{keysutil.KeyType_AES256_GCM96, keysutil.KeyType_AES128_GCM96, keysutil.KeyType_ChaCha20_Poly1305}, + nonce: []byte{}, + convergentEncryption: false, + convergentVersion: -1, + expected: false, + }, + { + name: "-Convergentv1-WithNonce", + keyTypes: []keysutil.KeyType{keysutil.KeyType_AES256_GCM96, keysutil.KeyType_AES128_GCM96, keysutil.KeyType_ChaCha20_Poly1305}, + nonce: []byte("testnonce"), + convergentEncryption: true, + convergentVersion: 1, + expected: true, + }, + { + name: "-Convergentv2-WithNonce", + keyTypes: []keysutil.KeyType{keysutil.KeyType_AES256_GCM96, keysutil.KeyType_AES128_GCM96, keysutil.KeyType_ChaCha20_Poly1305}, + nonce: []byte("testnonce"), + convergentEncryption: true, + convergentVersion: 2, + expected: false, + }, + { + name: "-Convergentv3-WithNonce", + keyTypes: []keysutil.KeyType{keysutil.KeyType_AES256_GCM96, keysutil.KeyType_AES128_GCM96, keysutil.KeyType_ChaCha20_Poly1305}, + nonce: []byte("testnonce"), + convergentEncryption: true, + convergentVersion: 3, + expected: false, + }, + { + name: "-NoConvergent-WithNonce", + keyTypes: []keysutil.KeyType{keysutil.KeyType_RSA2048, keysutil.KeyType_RSA4096}, + nonce: []byte("testnonce"), + convergentEncryption: false, + convergentVersion: -1, + expected: false, + }, + } + + for _, tt := range tests { + for _, keyType := range tt.keyTypes { + testName := keyType.String() + tt.name + t.Run(testName, func(t *testing.T) { + p := keysutil.Policy{ + ConvergentEncryption: tt.convergentEncryption, + ConvergentVersion: tt.convergentVersion, + Type: keyType, + } + + actual := shouldWarnAboutNonceUsage(&p, tt.nonce) + + if actual != tt.expected { + t.Errorf("Expected actual '%v' but got '%v'", tt.expected, actual) + } + }) + } + } +} diff --git a/builtin/logical/transit/path_rewrap.go b/builtin/logical/transit/path_rewrap.go index c32fddc999..04c38d2f8e 100644 --- a/builtin/logical/transit/path_rewrap.go +++ b/builtin/logical/transit/path_rewrap.go @@ -4,7 +4,7 @@ import ( "context" "encoding/base64" "fmt" - + "github.com/hashicorp/vault/helper/constants" "github.com/hashicorp/vault/sdk/framework" "github.com/hashicorp/vault/sdk/helper/errutil" "github.com/hashicorp/vault/sdk/helper/keysutil" @@ -128,6 +128,7 @@ func (b *backend) pathRewrapWrite(ctx context.Context, req *logical.Request, d * p.Lock(false) } + warnAboutNonceUsage := false for i, item := range batchInputItems { if batchResponseItems[i].Error != "" { continue @@ -145,6 +146,10 @@ func (b *backend) pathRewrapWrite(ctx context.Context, req *logical.Request, d * } } + if !warnAboutNonceUsage && shouldWarnAboutNonceUsage(p, item.DecodedNonce) { + warnAboutNonceUsage = true + } + ciphertext, err := p.Encrypt(item.KeyVersion, item.DecodedContext, item.DecodedNonce, plaintext) if err != nil { switch err.(type) { @@ -190,6 +195,10 @@ func (b *backend) pathRewrapWrite(ctx context.Context, req *logical.Request, d * } } + if constants.IsFIPS() && warnAboutNonceUsage { + resp.AddWarning("A provided nonce value was used within FIPS mode, this violates FIPS 140 compliance.") + } + p.Unlock() return resp, nil } diff --git a/helper/constants/fips.go b/helper/constants/fips.go new file mode 100644 index 0000000000..4b8a99991c --- /dev/null +++ b/helper/constants/fips.go @@ -0,0 +1,8 @@ +// +build !fips_140_3 + +package constants + +// IsFIPS returns true if Vault is operating in a FIPS-140-{2,3} mode. +func IsFIPS() bool { + return false +}