Hierarchical keyring: concurrent decrypts corrupt shared branch-key material

[brenank]

Problem:

Seen on @aws-crypto/client-node@5.0.2.

When a branch-key cache entry is evicted, the cache wipes its key buffer by filling it with zeros. But branchKey() returns that buffer directly instead of a copy, so every caller shares one buffer. If a decrypt is still using it when another decrypt evicts the entry, its key gets zeroed mid-use, so it derives the wrong wrapping key and the unwrap fails gcm auth.

This only happens on a cold cache under concurrency. Retrying, or decrypting one at a time, works.

Three functions in the branch-key path line up to cause it:

• dispose calls zeroUnencryptedDataKey() whenever an entry leaves the cache, including on an overwrite.
• zeroUnencryptedDataKey() runs this._branchKey.fill(0), wiping the buffer in place.
• branchKey() returns this._branchKey directly, so a reader shares the buffer the cache will wipe.

Any eviction while a decrypt still holds that buffer corrupts it: an overwrite from a concurrent cold-miss, a ttl expiry, or a tail eviction.

Error: Unable to decrypt data key Error #1
  Error: Unsupported state or unable to authenticate data
      at Decipheriv.final (node:internal/crypto/cipher)
      at unwrapEncryptedDataKey (...)
      at KmsHierarchicalKeyRingNode._onDecrypt (...)

The overwrite is the usual trigger under concurrency. The cache is only written after the branch-key fetch (DynamoDB and KMS) finishes, and nothing dedupes in-flight fetches. So a burst of decrypts on a cold cache all miss, all fetch, and all write the same cache key. Each write after the first overwrites a live entry and zeros a buffer another decrypt is still using.

Impact: concurrent decrypts through one shared hierarchical keyring on a cold cache intermittently fail to unwrap valid ciphertext.

Reproduction:

npm i @aws-crypto/cache-material@5.0.2 @aws-crypto/material-management@5.0.2
node repro.js

// repro.js
// The branch-key cache hands out its key buffer by reference and zeros it on
// eviction. Under concurrency one decrypt reads that buffer, then a second
// decrypt's cold-miss overwrites the same entry and zeros the buffer the first
// is about to derive from. Modeled deterministically against the cache alone.
const { getLocalCryptographicMaterialsCache } = require('@aws-crypto/cache-material')
const { NodeBranchKeyMaterial } = require('@aws-crypto/material-management')

const CACHE_KEY = 'branch-id:version'
const branchKeyMaterial = (fill) =>
  new NodeBranchKeyMaterial(Buffer.alloc(32, fill), 'branch-id', '22222222-2222-4222-8222-222222222222', {})
const first4 = (buf) => Buffer.from(buf.slice(0, 4)).toString('hex')

const cache = getLocalCryptographicMaterialsCache(100)

// decrypt #1 cold-misses, populates the entry, and reads the branch key by
// reference to derive its wrapping key (what the unwrap path does).
cache.putBranchKeyMaterial(CACHE_KEY, branchKeyMaterial(0xaa))
const decrypt1BranchKey = cache.getBranchKeyMaterial(CACHE_KEY).response.branchKey()

// decrypt #2 also cold-missed; its fetch resolves and populates the SAME entry,
// evicting #1's material -> dispose() -> zeroUnencryptedDataKey().
cache.putBranchKeyMaterial(CACHE_KEY, branchKeyMaterial(0xbb))

// decrypt #1 has not finished; it now derives from the buffer it read earlier.
console.log('decrypt #1 branch key, first 4 bytes:')
console.log('  expected: aaaaaaaa (the key it read)')
console.log(`  actual:   ${first4(decrypt1BranchKey)} (zeroed by #2, so #1 derives the wrong wrapping key and gcm auth fails)`)

Actual output:

decrypt #1 branch key, first 4 bytes:
  expected: aaaaaaaa (the key it read)
  actual:   00000000 (zeroed by #2, so #1 derives the wrong wrapping key and gcm auth fails)

Solution:

Stop a reader from ever sharing a Buffer the cache can zero. Either:

• Return a copy from branchKey() (or copy in the unwrap read path), so eviction can't mutate a reference a decrypt is still using. Same shape as the Raw AES Keyring zeros out passed in unwrappedMasterKey #970 copy fix, different path.
• And/or don't zero while readers are outstanding (refcount, or skip zeroing on overwrite).

Workaround:

Decrypt one at a time when the calls share a keyring. The bug needs two decrypts running at once, so if only one runs at a time, nothing can wipe the buffer it is using. The tradeoff is you lose the speedup of decrypting in parallel, though you also avoid the duplicate keystore fetches.

Out of scope:

• Not a duplicate of the cold-cache stampede (Hierarchical Keyring: cold-cache stampede — N concurrent decrypts → N DynamoDB/KMS calls #1663), though they share the same trigger. The single-flight fix proposed there would stop the common case (many decrypts of the same key racing). It won't stop a different branch key from evicting and zeroing this one while a decrypt still holds it, which happens once the cache is at capacity, so this needs its own change.