Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Warn user supplying nonce values in FIPS mode for transit encryption requests #13366

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion builtin/logical/transit/path_datakey.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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)
}
Expand Down
38 changes: 38 additions & 0 deletions builtin/logical/transit/path_encrypt.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
"encoding/base64"
"encoding/json"
"fmt"
"github.com/hashicorp/vault/helper/constants"
"net/http"
"reflect"

Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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")
}
Expand All @@ -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`

Expand Down
80 changes: 80 additions & 0 deletions builtin/logical/transit/path_encrypt_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package transit
import (
"context"
"encoding/json"
"github.com/hashicorp/vault/sdk/helper/keysutil"
"reflect"
"strings"
"testing"
Expand Down Expand Up @@ -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)
}
})
}
}
}
11 changes: 10 additions & 1 deletion builtin/logical/transit/path_rewrap.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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
Expand All @@ -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) {
Expand Down Expand Up @@ -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
}
Expand Down
8 changes: 8 additions & 0 deletions helper/constants/fips.go
Original file line number Diff line number Diff line change
@@ -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
}