Skip to content

Commit

Permalink
keyring: Fix a panic when decrypting aead with empty RSA block. (#24383)
Browse files Browse the repository at this point in the history
Clusters that have gone through several upgrades have be found to
include keyring material which has an empty RSA block.

In more recent versions of Nomad, an empty RSA block is omitted
from being written to disk. This results in the panic not being
present. Older versions, however, did not have this struct tag
meaning we wrote an empty JSON block which is not accounted for
in the current version.
  • Loading branch information
jrasell authored Nov 7, 2024
1 parent 498b29b commit 316430b
Show file tree
Hide file tree
Showing 3 changed files with 65 additions and 1 deletion.
3 changes: 3 additions & 0 deletions .changelog/24383.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
```release-note:bug
keyring: Fixed a panic on server startup when decrypting AEAD key data with empty RSA block
```
2 changes: 1 addition & 1 deletion nomad/encrypter.go
Original file line number Diff line number Diff line change
Expand Up @@ -805,7 +805,7 @@ func (e *Encrypter) loadKeyFromStore(path string) (*structs.UnwrappedRootKey, er
// 1.7 an ed25519 key derived from the root key was used instead of an RSA
// key.
var rsaKey []byte
if kekWrapper.WrappedRSAKey != nil {
if kekWrapper.WrappedRSAKey != nil && len(kekWrapper.WrappedRSAKey.Ciphertext) > 0 {
rsaKey, err = wrapper.Decrypt(e.srv.shutdownCtx, kekWrapper.WrappedRSAKey)
if err != nil {
return nil, fmt.Errorf("%w (rsa key): %w", ErrDecryptFailed, err)
Expand Down
61 changes: 61 additions & 0 deletions nomad/encrypter_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,67 @@ func TestEncrypter_LoadSave(t *testing.T) {

}

// TestEncrypter_loadKeyFromStore_emptyRSA tests a panic seen by some
// operators where the aead key disk file content had an empty RSA block.
func TestEncrypter_loadKeyFromStore_emptyRSA(t *testing.T) {
ci.Parallel(t)

srv := &Server{
logger: testlog.HCLogger(t),
config: &Config{},
}

tmpDir := t.TempDir()

key, err := structs.NewUnwrappedRootKey(structs.EncryptionAlgorithmAES256GCM)
must.NoError(t, err)

encrypter, err := NewEncrypter(srv, tmpDir)
must.NoError(t, err)

wrappedKey, err := encrypter.encryptDEK(key, &structs.KEKProviderConfig{})
must.NotNil(t, wrappedKey)
must.NoError(t, err)

// Use an artisanally crafted key file.
kek, err := json.Marshal(wrappedKey.KeyEncryptionKey)
must.NoError(t, err)

wrappedDEKCipher, err := json.Marshal(wrappedKey.WrappedDataEncryptionKey.Ciphertext)
must.NoError(t, err)

testData := fmt.Sprintf(`
{
"Meta": {
"KeyID": %q,
"Algorithm": "aes256-gcm",
"CreateTime": 1730000000000000000,
"CreateIndex": 1555555,
"ModifyIndex": 1555555,
"State": "active",
"PublishTime": 0
},
"ProviderID": "aead",
"WrappedDEK": {
"ciphertext": %s,
"key_info": {
"key_id": %q
}
},
"WrappedRSAKey": {},
"KEK": %s
}
`, key.Meta.KeyID, wrappedDEKCipher, key.Meta.KeyID, kek)

path := filepath.Join(tmpDir, key.Meta.KeyID+".nks.json")
err = os.WriteFile(path, []byte(testData), 0o600)
must.NoError(t, err)

unwrappedKey, err := encrypter.loadKeyFromStore(path)
must.NoError(t, err)
must.NotNil(t, unwrappedKey)
}

// TestEncrypter_Restore exercises the entire reload of a keystore,
// including pairing metadata with key material
func TestEncrypter_Restore(t *testing.T) {
Expand Down

0 comments on commit 316430b

Please sign in to comment.