From daf72aa42790144c3a0ca9c17bb19b1c5bce66c6 Mon Sep 17 00:00:00 2001 From: Alexander Scheel Date: Wed, 31 May 2023 14:04:08 -0400 Subject: [PATCH] Fix transit import/export of hmac-only keys (#20864) * Fix export of HMAC typed keys When initially implemented, exporting HMAC keys resulted in returning the unused, internal HMACKey value rather than the main Key value that is used for HMAC operations. This is a breaking change. Signed-off-by: Alexander Scheel * Consistently handle HMAC keys in keysutil When generating HMAC-typed keys, set HMACKey = Key consistently, to allow users of HMAC-typed keys to use them backwards compatibly. Notably, this could discard the (unused) HMACKey field set today. Signed-off-by: Alexander Scheel * Add test proving export of HMAC keys work Signed-off-by: Alexander Scheel * Add changelog entry Signed-off-by: Alexander Scheel --------- Signed-off-by: Alexander Scheel --- builtin/logical/transit/path_backup_test.go | 4 ++++ builtin/logical/transit/path_byok_test.go | 6 +++++- builtin/logical/transit/path_export.go | 6 +++++- builtin/logical/transit/path_export_test.go | 4 ++++ changelog/20864.txt | 5 +++++ sdk/helper/keysutil/policy.go | 12 +++++++++++- 6 files changed, 34 insertions(+), 3 deletions(-) create mode 100644 changelog/20864.txt diff --git a/builtin/logical/transit/path_backup_test.go b/builtin/logical/transit/path_backup_test.go index ed9a7477d6..d17a9895ff 100644 --- a/builtin/logical/transit/path_backup_test.go +++ b/builtin/logical/transit/path_backup_test.go @@ -39,6 +39,7 @@ func TestTransit_BackupRestore(t *testing.T) { testBackupRestore(t, "rsa-2048", "hmac-verify") testBackupRestore(t, "rsa-3072", "hmac-verify") testBackupRestore(t, "rsa-4096", "hmac-verify") + testBackupRestore(t, "hmac", "hmac-verify") } func testBackupRestore(t *testing.T, keyType, feature string) { @@ -57,6 +58,9 @@ func testBackupRestore(t *testing.T, keyType, feature string) { "exportable": true, }, } + if keyType == "hmac" { + keyReq.Data["key_size"] = 32 + } resp, err = b.HandleRequest(context.Background(), keyReq) if err != nil || (resp != nil && resp.IsError()) { t.Fatalf("resp: %#v\nerr: %v", resp, err) diff --git a/builtin/logical/transit/path_byok_test.go b/builtin/logical/transit/path_byok_test.go index 7fc0c9946d..a05a719ea4 100644 --- a/builtin/logical/transit/path_byok_test.go +++ b/builtin/logical/transit/path_byok_test.go @@ -28,7 +28,8 @@ func TestTransit_BYOKExportImport(t *testing.T) { testBYOKExportImport(t, "rsa-3072", "sign-verify") testBYOKExportImport(t, "rsa-4096", "sign-verify") - // Unlike backup, we don't support importing HMAC keys here. + // Test HMAC sign/verify after a restore for supported keys. + testBYOKExportImport(t, "hmac", "hmac-verify") } func testBYOKExportImport(t *testing.T, keyType, feature string) { @@ -47,6 +48,9 @@ func testBYOKExportImport(t *testing.T, keyType, feature string) { "exportable": true, }, } + if keyType == "hmac" { + keyReq.Data["key_size"] = 32 + } resp, err = b.HandleRequest(context.Background(), keyReq) if err != nil || (resp != nil && resp.IsError()) { t.Fatalf("resp: %#v\nerr: %v", resp, err) diff --git a/builtin/logical/transit/path_export.go b/builtin/logical/transit/path_export.go index 7465c7be54..9239aee94b 100644 --- a/builtin/logical/transit/path_export.go +++ b/builtin/logical/transit/path_export.go @@ -162,7 +162,11 @@ func getExportKey(policy *keysutil.Policy, key *keysutil.KeyEntry, exportType st switch exportType { case exportTypeHMACKey: - return strings.TrimSpace(base64.StdEncoding.EncodeToString(key.HMACKey)), nil + src := key.HMACKey + if policy.Type == keysutil.KeyType_HMAC { + src = key.Key + } + return strings.TrimSpace(base64.StdEncoding.EncodeToString(src)), nil case exportTypeEncryptionKey: switch policy.Type { diff --git a/builtin/logical/transit/path_export_test.go b/builtin/logical/transit/path_export_test.go index df9cd8e558..cd25383f2b 100644 --- a/builtin/logical/transit/path_export_test.go +++ b/builtin/logical/transit/path_export_test.go @@ -28,6 +28,7 @@ func TestTransit_Export_KeyVersion_ExportsCorrectVersion(t *testing.T) { verifyExportsCorrectVersion(t, "hmac-key", "ecdsa-p384") verifyExportsCorrectVersion(t, "hmac-key", "ecdsa-p521") verifyExportsCorrectVersion(t, "hmac-key", "ed25519") + verifyExportsCorrectVersion(t, "hmac-key", "hmac") } func verifyExportsCorrectVersion(t *testing.T, exportType, keyType string) { @@ -43,6 +44,9 @@ func verifyExportsCorrectVersion(t *testing.T, exportType, keyType string) { "exportable": true, "type": keyType, } + if keyType == "hmac" { + req.Data["key_size"] = 32 + } _, err := b.HandleRequest(context.Background(), req) if err != nil { t.Fatal(err) diff --git a/changelog/20864.txt b/changelog/20864.txt new file mode 100644 index 0000000000..7193c6b81f --- /dev/null +++ b/changelog/20864.txt @@ -0,0 +1,5 @@ +```release-note:bug +secrets/transit: Fix export of HMAC-only key, correctly exporting the key used for sign operations. For consumers of the previously incorrect key, use the plaintext export to retrieve these incorrect keys and import them as new versions. +secrets/transit: Fix bug related to shorter dedicated HMAC key sizing. +sdk/helper/keysutil: New HMAC type policies will have HMACKey equal to Key and be copied over on import. +``` diff --git a/sdk/helper/keysutil/policy.go b/sdk/helper/keysutil/policy.go index 99eadeac1f..869733b3e9 100644 --- a/sdk/helper/keysutil/policy.go +++ b/sdk/helper/keysutil/policy.go @@ -767,6 +767,10 @@ func (p *Policy) Upgrade(ctx context.Context, storage logical.Storage, randReade entry.HMACKey = hmacKey p.Keys[strconv.Itoa(p.LatestVersion)] = entry persistNeeded = true + + if p.Type == KeyType_HMAC { + entry.HMACKey = entry.Key + } } if persistNeeded { @@ -1497,6 +1501,7 @@ func (p *Policy) ImportPublicOrPrivate(ctx context.Context, storage logical.Stor entry.Key = key if p.Type == KeyType_HMAC { p.KeySize = len(key) + entry.HMACKey = key } } else { var parsedKey any @@ -1613,7 +1618,7 @@ func (p *Policy) RotateInMemory(randReader io.Reader) (retErr error) { if p.Type == KeyType_AES128_GCM96 { numBytes = 16 } else if p.Type == KeyType_HMAC { - numBytes := p.KeySize + numBytes = p.KeySize if numBytes < HmacMinKeySize || numBytes > HmacMaxKeySize { return fmt.Errorf("invalid key size for HMAC key, must be between %d and %d bytes", HmacMinKeySize, HmacMaxKeySize) } @@ -1624,6 +1629,11 @@ func (p *Policy) RotateInMemory(randReader io.Reader) (retErr error) { } entry.Key = newKey + if p.Type == KeyType_HMAC { + // To avoid causing problems, ensure HMACKey = Key. + entry.HMACKey = newKey + } + case KeyType_ECDSA_P256, KeyType_ECDSA_P384, KeyType_ECDSA_P521: var curve elliptic.Curve switch p.Type {