Make the error and http code clearer when supplying wrong unseal key (#17836)
* Fix typos * Return http 400 when wrong unseal key is supplied * Add changelog * Add test cases and change one more return case to http 400 The new case is triggered when key length is within valid range [16, 32], but it has uneven bytes, causing crypto/aes to return invalid key size. * remove expected in unit tests * include error in the new error reason * add multikey and autoseal test cases * return invalid key for few more code paths
This commit is contained in:
nsimons
GitHub
parent
2a387b1d3a
commit
ce90a6fa38
|
|
@ -0,0 +1,3 @@
|
|||
```release-note:bug
|
||||
core: trying to unseal with the wrong key now returns HTTP 400
|
||||
```
|
||||
|
|
@ -1,11 +1,14 @@
|
|||
package http
|
||||
|
||||
import (
|
||||
"context"
|
||||
"encoding/base64"
|
||||
"encoding/hex"
|
||||
"encoding/json"
|
||||
"fmt"
|
||||
"net/http"
|
||||
"strconv"
|
||||
"strings"
|
||||
"testing"
|
||||
|
||||
"github.com/go-test/deep"
|
||||
|
|
@ -13,6 +16,7 @@ import (
|
|||
"github.com/hashicorp/vault/sdk/logical"
|
||||
"github.com/hashicorp/vault/sdk/version"
|
||||
"github.com/hashicorp/vault/vault"
|
||||
"github.com/hashicorp/vault/vault/seal"
|
||||
)
|
||||
|
||||
func TestSysSealStatus(t *testing.T) {
|
||||
|
|
@ -229,16 +233,173 @@ func TestSysUnseal(t *testing.T) {
|
|||
}
|
||||
}
|
||||
|
||||
func TestSysUnseal_badKey(t *testing.T) {
|
||||
core := vault.TestCore(t)
|
||||
vault.TestCoreInit(t, core)
|
||||
func subtestBadSingleKey(t *testing.T, seal vault.Seal) {
|
||||
core := vault.TestCoreWithSeal(t, seal, false)
|
||||
_, err := core.Initialize(context.Background(), &vault.InitParams{
|
||||
BarrierConfig: &vault.SealConfig{
|
||||
SecretShares: 1,
|
||||
SecretThreshold: 1,
|
||||
},
|
||||
RecoveryConfig: &vault.SealConfig{
|
||||
SecretShares: 1,
|
||||
SecretThreshold: 1,
|
||||
},
|
||||
})
|
||||
if err != nil {
|
||||
t.Fatalf("err: %s", err)
|
||||
}
|
||||
|
||||
ln, addr := TestServer(t, core)
|
||||
defer ln.Close()
|
||||
|
||||
resp := testHttpPut(t, "", addr+"/v1/sys/unseal", map[string]interface{}{
|
||||
"key": "0123",
|
||||
testCases := []struct {
|
||||
description string
|
||||
key string
|
||||
}{
|
||||
// hex key tests
|
||||
// hexadecimal strings have 2 symbols per byte; size(0xAA) == 1 byte
|
||||
{
|
||||
"short hex key",
|
||||
strings.Repeat("AA", 8),
|
||||
},
|
||||
{
|
||||
"long hex key",
|
||||
strings.Repeat("AA", 34),
|
||||
},
|
||||
{
|
||||
"uneven hex key byte length",
|
||||
strings.Repeat("AA", 33),
|
||||
},
|
||||
{
|
||||
"valid hex key but wrong cluster",
|
||||
"4482691dd3a710723c4f77c4920ee21b96c226bf4829fa6eb8e8262c180ae933",
|
||||
},
|
||||
|
||||
// base64 key tests
|
||||
// base64 strings have min. 1 character per byte; size("m") == 1 byte
|
||||
{
|
||||
"short b64 key",
|
||||
base64.StdEncoding.EncodeToString([]byte(strings.Repeat("m", 8))),
|
||||
},
|
||||
{
|
||||
"long b64 key",
|
||||
base64.StdEncoding.EncodeToString([]byte(strings.Repeat("m", 34))),
|
||||
},
|
||||
{
|
||||
"uneven b64 key byte length",
|
||||
base64.StdEncoding.EncodeToString([]byte(strings.Repeat("m", 33))),
|
||||
},
|
||||
{
|
||||
"valid b64 key but wrong cluster",
|
||||
"RIJpHdOnEHI8T3fEkg7iG5bCJr9IKfpuuOgmLBgK6TM=",
|
||||
},
|
||||
|
||||
// other key tests
|
||||
{
|
||||
"empty key",
|
||||
"",
|
||||
},
|
||||
{
|
||||
"key with bad format",
|
||||
"ThisKeyIsNeitherB64NorHex",
|
||||
},
|
||||
}
|
||||
|
||||
for _, tc := range testCases {
|
||||
t.Run(tc.description, func(t *testing.T) {
|
||||
resp := testHttpPut(t, "", addr+"/v1/sys/unseal", map[string]interface{}{
|
||||
"key": tc.key,
|
||||
})
|
||||
|
||||
testResponseStatus(t, resp, 400)
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func subtestBadMultiKey(t *testing.T, seal vault.Seal) {
|
||||
numKeys := 3
|
||||
|
||||
core := vault.TestCoreWithSeal(t, seal, false)
|
||||
_, err := core.Initialize(context.Background(), &vault.InitParams{
|
||||
BarrierConfig: &vault.SealConfig{
|
||||
SecretShares: numKeys,
|
||||
SecretThreshold: numKeys,
|
||||
},
|
||||
RecoveryConfig: &vault.SealConfig{
|
||||
SecretShares: numKeys,
|
||||
SecretThreshold: numKeys,
|
||||
},
|
||||
})
|
||||
testResponseStatus(t, resp, 400)
|
||||
if err != nil {
|
||||
t.Fatalf("err: %s", err)
|
||||
}
|
||||
ln, addr := TestServer(t, core)
|
||||
defer ln.Close()
|
||||
|
||||
testCases := []struct {
|
||||
description string
|
||||
keys []string
|
||||
}{
|
||||
{
|
||||
"all unseal keys from another cluster",
|
||||
[]string{
|
||||
"b189d98fdec3a15bed9b1cce5088f82b92896696b788c07bdf03c73da08279a5e8",
|
||||
"0fa98232f034177d8d9c2824899a2ac1e55dc6799348533e10510b856aef99f61a",
|
||||
"5344f5caa852f9ba1967d9623ed286a45ea7c4a529522d25f05d29ff44f17930ac",
|
||||
},
|
||||
},
|
||||
{
|
||||
"mixing unseal keys from different cluster, different share config",
|
||||
[]string{
|
||||
"b189d98fdec3a15bed9b1cce5088f82b92896696b788c07bdf03c73da08279a5e8",
|
||||
"0fa98232f034177d8d9c2824899a2ac1e55dc6799348533e10510b856aef99f61a",
|
||||
"e04ea3020838c2050c4a169d7ba4d30e034eec8e83e8bed9461bf2646ee412c0",
|
||||
},
|
||||
},
|
||||
{
|
||||
"mixing unseal keys from different clusters, similar share config",
|
||||
[]string{
|
||||
"b189d98fdec3a15bed9b1cce5088f82b92896696b788c07bdf03c73da08279a5e8",
|
||||
"0fa98232f034177d8d9c2824899a2ac1e55dc6799348533e10510b856aef99f61a",
|
||||
"413f80521b393aa6c4e42e9a3a3ab7f00c2002b2c3bf1e273fc6f363f35f2a378b",
|
||||
},
|
||||
},
|
||||
}
|
||||
|
||||
for _, tc := range testCases {
|
||||
t.Run(tc.description, func(t *testing.T) {
|
||||
for i, key := range tc.keys {
|
||||
resp := testHttpPut(t, "", addr+"/v1/sys/unseal", map[string]interface{}{
|
||||
"key": key,
|
||||
})
|
||||
|
||||
if i == numKeys-1 {
|
||||
// last key
|
||||
testResponseStatus(t, resp, 400)
|
||||
} else {
|
||||
// unseal in progress
|
||||
testResponseStatus(t, resp, 200)
|
||||
}
|
||||
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestSysUnseal_BadKeyNewShamir(t *testing.T) {
|
||||
seal := vault.NewTestSeal(t,
|
||||
&seal.TestSealOpts{StoredKeys: seal.StoredKeysSupportedShamirRoot})
|
||||
|
||||
subtestBadSingleKey(t, seal)
|
||||
subtestBadMultiKey(t, seal)
|
||||
}
|
||||
|
||||
func TestSysUnseal_BadKeyAutoUnseal(t *testing.T) {
|
||||
seal := vault.NewTestSeal(t,
|
||||
&seal.TestSealOpts{StoredKeys: seal.StoredKeysSupportedGeneric})
|
||||
|
||||
subtestBadSingleKey(t, seal)
|
||||
subtestBadMultiKey(t, seal)
|
||||
}
|
||||
|
||||
func TestSysUnseal_Reset(t *testing.T) {
|
||||
|
|
|
|||
|
|
@ -1305,13 +1305,13 @@ func (c *Core) Unseal(key []byte) (bool, error) {
|
|||
}
|
||||
|
||||
// unseal takes a key fragment and attempts to use it to unseal Vault.
|
||||
// Vault may remain unsealed afterwards even when no error is returned,
|
||||
// Vault may remain sealed afterwards even when no error is returned,
|
||||
// depending on whether enough key fragments were provided to meet the
|
||||
// target threshold.
|
||||
//
|
||||
// The provided key should be a recovery key fragment if the seal
|
||||
// is an autoseal, or a regular seal key fragment for shamir. In
|
||||
// migration scenarios "seal" in the preceding sentance refers to
|
||||
// migration scenarios "seal" in the preceding sentence refers to
|
||||
// the migration seal in c.migrationInfo.seal.
|
||||
//
|
||||
// We use getUnsealKey to work out if we have enough fragments,
|
||||
|
|
@ -1555,13 +1555,13 @@ func (c *Core) getUnsealKey(ctx context.Context, seal Seal) ([]byte, error) {
|
|||
} else {
|
||||
unsealKey, err = shamir.Combine(c.unlockInfo.Parts)
|
||||
if err != nil {
|
||||
return nil, fmt.Errorf("failed to compute combined key: %w", err)
|
||||
return nil, &ErrInvalidKey{fmt.Sprintf("failed to compute combined key: %v", err)}
|
||||
}
|
||||
}
|
||||
|
||||
if seal.RecoveryKeySupported() {
|
||||
if err := seal.VerifyRecoveryKey(ctx, unsealKey); err != nil {
|
||||
return nil, err
|
||||
return nil, &ErrInvalidKey{fmt.Sprintf("failed to verify recovery key: %v", err)}
|
||||
}
|
||||
}
|
||||
|
||||
|
|
@ -2794,7 +2794,7 @@ func (c *Core) unsealKeyToMasterKey(ctx context.Context, seal Seal, combinedKey
|
|||
|
||||
err := seal.GetAccess().Wrapper.(*aeadwrapper.ShamirWrapper).SetAesGcmKeyBytes(combinedKey)
|
||||
if err != nil {
|
||||
return nil, fmt.Errorf("failed to setup unseal key: %w", err)
|
||||
return nil, &ErrInvalidKey{fmt.Sprintf("failed to setup unseal key: %v", err)}
|
||||
}
|
||||
storedKeys, err := seal.GetStoredKeys(ctx)
|
||||
if storedKeys == nil && err == nil && allowMissing {
|
||||
|
|
|
|||
|
|
@ -7,6 +7,7 @@ import (
|
|||
"encoding/json"
|
||||
"errors"
|
||||
"fmt"
|
||||
"strings"
|
||||
"sync/atomic"
|
||||
|
||||
"github.com/hashicorp/vault/sdk/helper/jsonutil"
|
||||
|
|
@ -484,6 +485,9 @@ func readStoredKeys(ctx context.Context, storage physical.Backend, encryptor *se
|
|||
|
||||
pt, err := encryptor.Decrypt(ctx, blobInfo, nil)
|
||||
if err != nil {
|
||||
if strings.Contains(err.Error(), "message authentication failed") {
|
||||
return nil, &ErrInvalidKey{Reason: fmt.Sprintf("failed to decrypt keys from storage: %v", err)}
|
||||
}
|
||||
return nil, &ErrDecrypt{Err: fmt.Errorf("failed to decrypt keys from storage: %w", err)}
|
||||
}
|
||||
|
||||
|
|
|
|||
Loading…
Reference in New Issue