Dispose generated KeyPair in DidKeyMethod/Numalgo0Handler (ND-E8 follow-up)#99
Merged
Conversation
NetCrypto 1.2.0 made KeyPair IDisposable with deterministic zeroization. DidKeyMethod.CreateCoreAsync and Numalgo0Handler.Create now dispose the pair they generate and discard, instead of orphaning the pinned private key to the GC unwiped. PublicKey is defensively copied on get, so the DID built from the pre-disposal copy is unchanged. Issue97_* regression theories cover all 8 supported key types per method via a recording IKeyGenerator fake (fail-first proven: 8/8 fail per project without the fix). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…claim Review confirmed two Low findings: (1) the CHANGELOG "no behavior change" claim was false for custom IKeyGenerator implementations that retain the returned KeyPair — reworded to state the ownership contract; (2) the test fake recorded the public key via the same getter under test, so a defensive-copy regression would alias the snapshot and pass zeros-to-zeros — the fake now clones the recorded key and the theories assert the snapshot is non-zero. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
moisesja
force-pushed
the
claude/github-issue-97-b20117
branch
from
July 13, 2026 02:14
4c402a1 to
2789352
Compare
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
1 participant
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.
Fixes #97
What
NetCrypto 1.2.0 made
KeyPairIDisposable(deterministic zeroization of the pinned private-key copy). The two fresh-key create paths flagged in #97 generated aKeyPair, read only its public key, and orphaned the private half to the GC unwiped. Both now dispose the pair as soon as the public key is copied out:DidKeyMethod.CreateCoreAsync—src/NetDid.Method.Key/DidKeyMethod.csNumalgo0Handler.Create—src/NetDid.Method.Peer/Numalgo0Handler.csKeyPair.PublicKeydefensively copies on get (verified against the NetCrypto 1.2.0 docs and empirically), so created DIDs and documents are byte-identical tomain.ExistingKey(ISigner) andIKeyStorecustody paths are untouched. Grep-verified these are the only twoGenerate(...)call sites insrc/.Tests (fail-first)
16 new regression tests:
Issue97_Create_FreshKey_DisposesGeneratedKeyPairandIssue97_Numalgo0_FreshKey_DisposesGeneratedKeyPair, theories over all 8 supported key types per method. A recordingIKeyGeneratorfake (hand-rolled — NSubstitute can't interceptReadOnlySpanparams) captures the generated pair plus an independent clone of its public key; each test asserts the pair throwsObjectDisposedExceptionafter create, the snapshot is non-zero, and the resulting DID encodes the pre-disposal key. With the fix stashed, all 16 fail (8/8 per project); mutation-tested independently during review with the same result.Verification
git diff --checkcleanAdversarial review
Three independent red-team passes (lifecycle/use-after-dispose, scope completeness, contract/test validity). No defect confirmed in the change; empirical probes covered defensive-copy integrity, dispose idempotency on exception unwind, and a 20k-iteration torn-read race (0 torn reads). Two Low findings were fixed in the second commit: the CHANGELOG's "no behavior change" claim is now scoped to conforming
IKeyGeneratorimplementations (a generator that retains the returned pair now observes it disposed — permitted by NetCrypto's ownership contract), and the test snapshot no longer aliases the getter under test.Notes
NetDidVersionbump (release-PR concern); CHANGELOG entry under[Unreleased]→ Security.🤖 Generated with Claude Code