aws / aws/aws-encryption-sdk-javascript

Hierarchical keyring: concurrent decrypts corrupt shared branch-key material

Ouverte
#1,691 0 commentaires 0 réactions 0 personnes assignées Voir sur GitHub
Langage dominant
TypeScript
Étoiles
260
Forks
68
Merge moyen
22 h 19 min
PR mergées (30 j)
2

Description

### 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`](https://github.com/aws/aws-encryption-sdk-javascript/blob/a05029577aa41713bab481ef5992bfbb4067b32c/modules/cache-material/src/get_local_cryptographic_materials_cache.ts#L38-L40) calls `zeroUnencryptedDataKey()` whenever an entry leaves the cache, including on an overwrite.
- [`zeroUnencryptedDataKey()`](https://github.com/aws/aws-encryption-sdk-javascript/blob/a05029577aa41713bab481ef5992bfbb4067b32c/modules/material-management/src/cryptographic_material.ts#L211-L213) runs `this._branchKey.fill(0)`, wiping the buffer in place.
- [`branchKey()`](https://github.com/aws/aws-encryption-sdk-javascript/blob/a05029577aa41713bab481ef5992bfbb4067b32c/modules/material-management/src/cryptographic_material.ts#L204-L206) 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
```

```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 #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 (#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.

Guide de contribution

Ouvrir le guide de contribution

Piste de recherche

Commencez par branchKey() et zeroUnencryptedDataKey() dans modules/material-management/src/cryptographic_material.ts, puis examinez dispose dans modules/cache-material/src/get_local_cryptographic_materials_cache.ts. Exécutez le repro.js fourni pour observer la mise à zéro du buffer partagé après une réécriture ou une éviction du cache. Le travail est terminé lorsqu’une clé de branche conservée pendant le déchiffrement reste inchangée pendant l’éviction du cache et que les déchiffrements concurrents n’échouent plus lors de l’authentification.

Rédigé par le modèle d'indexation à partir du texte de l'issue.

Évaluation

Stack technique
node.js, typescript
Domaine
cryptography
Type d'issue
Bug
Difficulté
4/5
Temps estimé
3-5 jours
Activité
Calme
Clarté
Clairement spécifiée
Accessibilité débutants
68/100

Recevez les nouvelles issues par e-mail

Un résumé court des issues GitHub adaptées aux débutants.