Add defensive key material zeroization to prevent memory leaks#4
Merged
brody-0125 merged 4 commits intomainfrom Apr 16, 2026
Merged
Add defensive key material zeroization to prevent memory leaks#4brody-0125 merged 4 commits intomainfrom
brody-0125 merged 4 commits intomainfrom
Conversation
Raw Ed25519 seed bytes and the ecdsa-sd-2023 HMAC key were held in byte[] arrays and JCA PrivateKey / SecretKeySpec objects that were never cleared, leaving recoverable key material on the heap after GC and exposing it to heap dumps, core dumps, and swap files. Add a package-private KeyWipe helper exposing zero(byte[]) and tryDestroy(Destroyable), and wrap the affected signing paths in try-finally so sensitive buffers and key objects are wiped on both normal return and exception: - CredentialSigner.signEd25519Raw: zero the decoded seed, destroy the Ed25519 PrivateKey. - CredentialSigner.signEcdsaP256Raw: destroy the extracted ECPrivateKey. - SelectiveDisclosure.createBaseProof: zero the HMAC-SHA256 key after it has been copied into the CBOR proof value. - SelectiveDisclosure.replaceBlankNodesWithHmac: destroy the SecretKeySpec after the Mac loop. - SelectiveDisclosure.signEcdsaP256: destroy the extracted ECPrivateKey. Add KeyWipeTest covering null-safety, byte zeroing, already-destroyed skip, and DestroyFailedException swallowing. https://claude.ai/code/session_01Tjyxx8f3ZhfXdNEFMP1avf
Review feedback: - SelectiveDisclosure.createBaseProof extracts the JCA ECPrivateKey once before the loop and reuses it for both the per-quad signatures and the base signature, instead of re-running toECPrivateKey() (which invokes KeyFactory.generatePrivate internally) on every iteration. Destruction is hoisted to the outer finally along with hmacKey zeroization. - signEcdsaP256 now takes the pre-extracted ECPrivateKey and no longer owns the destroy lifecycle. - KeyWipe.tryDestroy narrows the catch to DestroyFailedException so real runtime bugs are not silently swallowed. - Drop the stale "Re-initialize for next use" comment in replaceBlankNodesWithHmac and tighten the CBOR finally-safety note. - KeyWipeTest uses assertFalse instead of assertTrue(!x). https://claude.ai/code/session_01Tjyxx8f3ZhfXdNEFMP1avf
Conflicts in CredentialSigner.signEcdsaP256Raw and SelectiveDisclosure.signEcdsaP256 came from PR #3 (low-S canonicalization) and PR #5 (ecdsa-sd-2023 derivation) landing on main alongside this branch's key-zeroization rewrite of the same methods. Resolution: - CredentialSigner.signEcdsaP256Raw: keep ecPrivateKey hoisted out of the inner try (needed by our finally{tryDestroy}) and add the byte[] p1363 staging variable from main so normalizeToLowS can run on the result before return. - SelectiveDisclosure.signEcdsaP256: keep our pre-extracted ECPrivateKey parameter shape (avoids re-running toECPrivateKey per quad) and adopt main's algorithm-name constants (CredentialSigner.ALGO_ECDSA_*), P256_COMPONENT_LEN, and final normalizeToLowS step. The outer JOSE try/catch is dropped because the JOSE call is now in the caller. Drive-by: EcdsaInternalsTest.selectiveDisclosureBaseSignatureIsAlwaysLowS was broken on plain main — PR #5 changed createBaseProof to W3C-required multibase-base64url-no-pad ('u' prefix) and updated the CBOR base-proof tag low byte from 0x02 to 0x00, but PR #3's test still called decodeBase58Btc and asserted 0x02. Switch the test to decodeBase64UrlNoPad and the 0x00 tag so this branch's CI is green. https://claude.ai/code/session_01Tjyxx8f3ZhfXdNEFMP1avf
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This PR introduces defensive zeroization of sensitive cryptographic key material to prevent recovery via heap dumps, core dumps, or cold-boot attacks. A new
KeyWipeutility class provides safe helpers for clearing byte arrays and destroyingDestroyablekey objects, which are now integrated throughout the credential signing code paths.Key Changes
New
KeyWipeutility class: Provides two static helper methods:zero(byte[]): Safely overwrites byte arrays with zeros (null-safe)tryDestroy(Destroyable): Safely destroysDestroyableobjects, skipping already-destroyed instances and swallowingDestroyFailedException(null-safe)SelectiveDisclosure.createBaseProof():ECPrivateKeyonce at the start and reuses it for all signatures (performance optimization)SelectiveDisclosure.replaceBlankNodesWithHmac():SecretKeySpeccreation outside the try block for proper cleanupSelectiveDisclosure.signEcdsaP256():ECPrivateKeyinstead ofECKey(removes redundant key extraction)JOSEExceptionCredentialSigner.signEd25519Raw():CredentialSigner.signEcdsaP256Raw():Implementation Details
All zeroization is performed in finally blocks to ensure cleanup even when exceptions occur. The
KeyWipe.tryDestroy()method gracefully handles JDK implementations that don't fully support theDestroyableinterface by catching and ignoringDestroyFailedException.Comprehensive unit tests verify that
KeyWipecorrectly handles null inputs, overwrites all bytes, respects already-destroyed state, and properly swallows exceptions.https://claude.ai/code/session_01Tjyxx8f3ZhfXdNEFMP1avf