feat: ICapableKeyStore custody surface — release 1.6.0 (closes #26)

[moisesja]

Closes #26. Milestone 1.6.0.

What this is

IKeyStore says "sign with the key behind this alias". That is enough for an in-memory store and not enough for a custody backend, where the consumer also needs to know what the backend can do before first use, which tenant it may touch, which key instance an alias currently holds, how to retry a mutation that may or may not have been applied, and which encoding a signature comes back in. Without that, every consumer forks its own signer surface downstream — which is what the issue's dependency gate found when it audited 1.4.0 and 1.5.0.

ICapableKeyStore : IKeyStore, IKeyStoreCapabilityProvider adds exactly that, additively. IKeyStore gains no member, so every existing store implementation, ISigner, KeyStoreSigner, and InMemoryKeyStore caller is source- and binary-compatible. No export operation anywhere, no KDF, no protocol semantics, and no router/profile/policy machinery — NetCrypto gets the self-description that makes routing possible, not the routing.

Scope: the full issue in one release (rather than the staged 1.6.0/1.7.0 option), so the downstream custody port audits once.

Discovery	IKeyStoreCapabilityProvider, KeyStoreCapabilitySet, KeyStoreCapability, KeyStoreOperation
Identity	KeyStoreNamespaceId, KeyInstanceId, KeyOperationId; StoredKeyInfo gains an optional InstanceId
Idempotency	request records, KeyMutationResult/KeyDeleteResult, and the KeyMutationOutcome receipt taxonomy
Operations	SignAsync(KeySignRequest), SignBbsAsync, DeriveSharedSecretAsync(KeyAgreementRequest), KeyStoreAlgorithms
Import	TransferableKeyMaterial — no private-key read, format, or export surface
Errors	KeyStoreException / KeyStoreError with RetryAfter
Reference impl	CapableInMemoryKeyStore over a shared InMemoryKeyStoreBackend, a sibling of the untouched InMemoryKeyStore

Two design notes worth a reviewer's attention. The algorithm identifier binds the observable encoding (es256-der vs es256-p1363), which is what finally makes the existing EcdsaSignatureFormat distinction reachable for a key that never leaves its store. And the backend is a separate object because the two properties it underwrites are only meaningful across instances: namespace isolation is a claim about two stores over one backend, and receipt durability is a claim about a receipt outliving the store that wrote it.

What the gates found

Both AGENTS.md gates ran before this was declared done, against a first implementation that looked correct. They found four integrity gaps — all fixed here, each with a regression test proven genuine by reverting the guard and confirming the test fails at its intended assertion:

• Provider output was length-checked but never verified against the key it claims to speak for. A hostile ICryptoProvider could return a well-formed signature made under a different key — or, for DER, any bytes at all. Now verified through an internal DefaultCryptoProvider, not the injected one, so a provider cannot both forge a signature and bless it. (NFR-6.2; generalizes what KeyStoreSigner already does on the recoverable path.)
• Unpaired surrogates collided in the mutation fingerprint. Encoding.UTF8 uses replacement fallback, so "\ud800", "\udc00" and U+FFFD all encode to EF BF BD. Two different requests under one operation id were judged an honest replay: the second got Replayed = true and a receipt naming an alias the caller never asked for. Now rejected at the boundary with a parameter name; well-formed surrogate pairs still work.
• Import never proved the public key belonged to the private key, so StoredKeyInfo.PublicKey — the identity downstream DID/VC code publishes — was attacker-chosen, and a 1-byte "P-256 key pair" could cross the custody boundary and be published as real.
• Monitor is reentrant, so a provider callback walked straight through the backend lock; a nested delete then zeroized the pinned buffer an in-flight private-key borrow was still reading.

Plus a BBS bound that was measured on the enumerator and enforced on the indexer, with expressions bypassing every constructor check (validation moved onto the init accessors), and several error-attribution fixes. Full triage — including the findings I argued down with reasoning, and the long list of attacks that found nothing — is in tasks/todo20260812-issue26-capable-keystore.md.

This also fixes a pre-existing NFR-3 leak in InMemoryKeyStore.DeriveSharedSecretAsync: an off-curve or low-order peer point surfaced as a platform CryptographicException instead of the parameter-named ArgumentException its contract promised. Invalid-input behavior only.

Verification

dotnet build NetCrypto.sln -c Release -warnaserror        → 0 errors, 0 warnings
dotnet test  NetCrypto.sln -c Release                     → 1195 passed, 5 failed
dotnet test  --filter "Category!=NativeFFI"  (no native)  → 1157 passed, 0 failed
all 11 samples, native present and absent                 → every one exits 0
tools/ApiCoverageCheck                                    → OK (exemption list still empty)

The 5 failures are BbsUnavailableTests, which assert BBS is absent and fail on any machine where the native library is built. Verified pre-existing — the same 5 fail on the base commit with this change stashed, and all 5 pass on the no-native leg they were written for.

Release state

Directory.Build.props → 1.6.0, CHANGELOG [1.6.0] closed with its compare link, and PublicAPI.Unshipped.txt (215 entries) promoted into PublicAPI.Shipped.txt per the PRD §8 per-release hygiene rule.

Deliberately not done: no merge, no v1.6.0 tag, no NuGet publish. release.yml fires on v*, so the irreversible tail is yours.

🤖 Generated with Claude Code

[moisesja]

Review — LGTM, with a couple of non-blocking notes

I read the security-critical seams end to end (CapableInMemoryKeyStore, TransferableKeyMaterial, KeyStoreIdentifiers, KeyStoreMutations, InMemoryKeyStoreBackend, KeyStoreException, the request records) rather than skimming, since this is custody surface. The design holds up well and the hard parts are handled correctly:

• Provider trust boundary (NFR-6.2): the return-path check on SignAsync verifies through a private DefaultCryptoProvider, not the injected one, so a hostile provider can't both forge and bless a signature. RouteToProvider also keeps every backend/platform exception type from escaping the contract while preserving the original as InnerException. Good.
• Import integrity: deriving the public key from the private scalar and rejecting a mismatched caller-supplied public key closes the "attacker-chosen StoredKeyInfo.PublicKey" hole cleanly, and the Consume/Discard split means a replay never reads the secret.
• Idempotency fingerprint: length-prefixed fields + SHA-256 + strict UTF-8 (rejecting unpaired surrogates at the boundary) genuinely closes the surrogate-collision replay. The init-accessor validation on the identifier structs also closes the with-expression bypass.
• Reentrancy: the OperationScope thread-static guard against a provider callback re-entering the reentrant Monitor and zeroizing a pinned buffer mid-borrow is a subtle one to have caught.
• CI green (ubuntu/macos + no-native leg), gates run pre-merge, regression tests proven genuine by revert.

Two things worth a look — neither blocking:

1. Two sign paths with different output guarantees on one object. The request-based SignAsync(KeySignRequest) runs RequireSpeaksForTheAdvertisedKey, but the inherited legacy SignAsync(string, ReadOnlyMemory<byte>) on the same CapableInMemoryKeyStore does not verify its output against the advertised key. This is consistent with the unchanged InMemoryKeyStore and is documented as "unchanged," so it's a deliberate seam — but a consumer that reaches for the legacy overload on a capable store silently gets the weaker integrity contract. Worth a one-line XML-doc note on the legacy overload pointing callers at the request-based path when they want the return-path check.

2. Minor asymmetry in the legacy generate path. GenerateAsync(string, KeyType) calls _keyGenerator.Generate before OperationScope.Enter(), whereas the request-based GenerateAsync generates inside the scope — so a reentrant key generator is caught on one path but not the other. Cosmetic (a reentrant generator is already a contract violation), just noting the inconsistency.

On scope: this is a large PR — the full issue in one release (~7k lines). The single-PR-per-release call is explained and reasonable given the downstream custody port audits once, and holding back the tag/publish for the maintainer is the right move. Approving.

---

Generated by Claude Code

[moisesja]

Requesting changes. This is not ready to merge.

The existing “LGTM” comment says the provider trust boundary and exception portability hold. They do not. I reproduced four concrete failures against commit 34272d1 with an isolated executable harness:

CAPABILITY_WITH: operation=Generate; algorithm=es256-p1363
BBS_SELF_CERTIFICATION: accepted=True; length=80
IMPORT_EXCEPTION: type=System.DllNotFoundException; consumed=True
BBS_COUNT: type=System.OutOfMemoryException

1. BBS output self-certifies through the same provider (CapableInMemoryKeyStore.cs:488-495). bbs is the injected provider that just produced the signature. A provider whose Sign returns 80 bytes of 0xAB and whose Verify returns true is accepted. The current BBS hardening test misses this because its fake lies only in Sign and delegates Verify to the real provider. The non-BBS path correctly uses a private DefaultCryptoProvider; BBS needs equivalent trust separation, plus a regression where the provider lies in both methods.

2. Import leaks backend/platform exceptions (CapableInMemoryKeyStore.cs:279-288). The call to IKeyGenerator.FromPrivateKey maps only ArgumentException. A generator throwing DllNotFoundException—the same failure used by the generate-path test—escapes directly after the transfer material has been consumed. That contradicts the public contract: “No backend exception type escapes a capable-store member.” Preserve invalid-key input as ArgumentException(nameof(request)), but map backend failures to KeyStoreException(Unavailable) and add an import-specific regression.

3. KeyStoreCapability does not preserve its advertised invariant under with (KeyStoreCapabilities.cs:90-100). Starting with a valid Sign capability, valid with { Operation = KeyStoreOperation.Generate } succeeds while retaining Algorithm == es256-p1363, even though Generate requires a null algorithm. The Operation setter validates only enum membership; it never revalidates Algorithm. This directly contradicts the comment claiming with cannot create an impossible capability. Fix the type so every publicly constructible state satisfies the cross-property invariant, and test single-property with mutations in both directions.

4. BBS has no finite message-count bound and allocates before its byte bound (CapableInMemoryKeyStore.cs:441-468). MaxInputBytes sums payload bytes only, so arbitrarily many empty messages remain “within” 1 MiB. The implementation allocates new byte[count][] before checking the total. An IReadOnlyList reporting Count == int.MaxValue produces OutOfMemoryException immediately; the provider is never reached. Enforce a finite message-count/resource limit before allocation/iteration and include message-count/encoding overhead in the advertised bound.

The test suite is large, but it is missing exactly these boundary cases. Add regression tests that fail on this commit and pass after the fixes.

The commit message is clear. The scope is not: 7,115 added lines, 36 files, four custody features, a version bump, and release bookkeeping in one commit is too much security-sensitive surface for one review unit. The issue itself proposed staging core custody separately from transfer+BBS; these missed defects are evidence that staging was the better choice. At minimum, fix the blockers and explain why this should remain one release PR instead of being split.

[moisesja]

Both reviews addressed in f218afe

All four blocking findings from the requesting review were verified by reproduction — each has a regression test that fails on 34272d1 and passes after the fix, proven genuine by reverting the guard:

1. BBS self-certification — confirmed; my earlier hardening fake lied only in Sign, which is exactly the check a provider lying in both halves defeats. Verification now runs through an internal DefaultBbsCryptoProvider whenever the native suite is loadable, matching the ECDSA path's trust separation (ABbsProviderThatLiesInBothSignAndVerify_CannotBlessItsOwnForgery). The fallback for a managed third-party BBS implementation on a no-native platform asks the producing provider and is documented as weaker rather than implied away — PRD FR-7b rule 11 updated accordingly.
2. Import exception leak — confirmed; only ArgumentException from FromPrivateKey was mapped. Backend failures during ingestion now surface as KeyStoreException(Unavailable) with the original as InnerException; the material stays consumed, since the store has seen the secret (AnImportWhoseGeneratorFailsWithABackendError_SurfacesAsUnavailable).
3. with cross-field hole on KeyStoreCapability — confirmed; per-field init validation missed pair consistency. The Operation accessor now re-validates the operation–algorithm pairing. One documented consequence: with cannot change operation and algorithm across the algorithm-bearing divide in either order — construct a new capability instead (WithExpressions_CannotProduceAnInconsistentOperationAlgorithmPair).
4. BBS count OOM — confirmed, and a lying negative Count would additionally have thrown OverflowException. SignBbsAsync now bounds the untrusted count (MaxBbsMessageCount = 4096) before any allocation; absurd counts are parameter-named argument faults (AMessageListReportingAnAbsurdCount_IsRejectedBeforeAnyAllocation). PRD rule 9 extended: a byte bound cannot limit a count of zero-byte messages, so the count needs its own.

Both notes from the approving comment are also in: the legacy SignAsync docs now state it carries no return-path identity check and point at SignAsync(KeySignRequest), and the legacy GenerateAsync enters the reentrancy scope before invoking the generator, with a test proving a reentrant generator is refused on that path too (AReentrantKeyGenerator_IsRefusedOnTheLegacyGeneratePathToo).

On scope ("should have been staged"): the single-release delivery was an explicit maintainer decision at plan approval — the downstream custody port gates on the full surface and audits once, and the issue itself only mildly preferred staging. The missed defects argue for the adversarial cycle these reviews provided, which the two subtle members got — inside this release — rather than for a second release boundary. Recorded in tasks/todo20260812-issue26-capable-keystore.md §6 with the full assessment table.

Verification after the fixes: -warnaserror clean; 1202 passed + the 5 known environment-dependent BbsUnavailableTests failures (native library present locally); no-native leg fully green at 1163; all 11 samples exit 0 on both legs.

🤖 Generated with Claude Code

[moisesja]

Re-review of b847152: still requesting changes.

What is actually fixed:

• KeyStoreCapability now rejects the reported cross-field with mutations.
• int.MaxValue/negative BBS counts now fail as ArgumentException("request") before allocation.
• DllNotFoundException during import is now wrapped as KeyStoreException(Unavailable).
• The BBS forgery is rejected when the native DefaultBbsCryptoProvider is loadable.
• The two earlier non-blocking legacy-path notes were addressed.

The scope rationale is also answered. I am not asking you to split the PR at this point.

Three blockers remain.

1. BBS still self-certifies in the supported no-native configuration (CapableInMemoryKeyStore.cs:488-499). The fallback is the same injected provider that produced the signature. In an isolated output with no libzkryptium_ffi, DefaultBbsCryptoProvider.IsAvailable == false; an injected provider with IsAvailable == true, Sign => 80 * 0xAB, and Verify => true still returns the fabricated signature successfully:

   BBS_SELF_CERTIFICATION: FAILED accepted=True; length=80
   

   Copying the native library into the same output changes the result to KeyStoreException(Unavailable), so the behavior is conclusively configuration-dependent. Updating the PRD to call this a “residual weakness” does not close NFR-6.2. No-native is an explicitly supported deployment mode, and this store advertises BBS there when the injected provider says it is available. Either require a genuinely independent verifier for advertised BBS, or do not advertise/perform BBS through this integrity-promising surface when no independent verifier exists. Add a no-native regression; the new test is marked NativeFFI, so it cannot catch this branch.

2. Backend exception portability remains type-dependent (CapableInMemoryKeyStore.cs:203-212, 275-286, 1101-1102). The import fix excludes every ObjectDisposedException, including one thrown by the injected generator from inside FromPrivateKey. A live transfer then leaks raw ODE after its secret has been consumed:

   IMPORT_ODE: type=System.ObjectDisposedException; consumed=True
   

   Request-based generation has the same structural problem. With valid caller input, an injected generator's backend failures surface as:

   backend=ObjectDisposedException; surfaced=ObjectDisposedException
   backend=OperationCanceledException; surfaced=OperationCanceledException
   backend=ArgumentException; surfaced=ArgumentException
   

   Generate passes a validated enum and gives the generator no cancellation token; these exceptions are backend failures, not caller faults or truthful cancellation. Likewise, import must distinguish an ODE raised by TransferableKeyMaterial from an ODE raised inside the generator instead of blanket-exempting the type. Normalize based on where the exception originated and actual store/token state, not merely its class. Add regressions for ODE, fabricated OCE, and backend-internal ArgumentException.

3. The BBS byte bound is still enforced after the full snapshot allocation (CapableInMemoryKeyStore.cs:453-468). The count guard closes one OOM path but not the underlying resource-bound bug. Every message is copied before total is computed. A 4,096-element list returning fresh 300 KB messages should exceed the 1 MiB cap after four reads; current code continues through 20 reads and leaks the sentinel from read 21:

   BBS_BYTE_BOUND: type=SentinelException; indexerReads=21
   

   The independent pass also measured a one-message 32 MiB request allocating ~33.5 MB before rejection, and a Count == 1 list with no index 0 leaking IndexOutOfRangeException. Read each item once into a local ReadOnlyMemory<byte>, incrementally reject from its length before copying it, validate the header before copying, and map inconsistent-list shape faults to ArgumentException("request"). Add a regression that asserts the indexer is not read after the cumulative advertised byte limit is crossed—not merely that the provider was not called.

Focused verification otherwise passes: Release build is clean with zero warnings/errors, and 157 key-store/capability/import/BBS tests pass. These are boundary omissions in tests, not general test-suite instability.

[moisesja]

Fixed the remaining review blockers in 0b629e3.

Specific changes:

• Removed the no-native same-provider BBS verification fallback. BBS is advertised only when both the producer and independent verifier are available.
• Gave the BBS producer a separate deep copy. A hostile producer could mutate shared byte-array elements and make the independent verifier certify altered input.
• Normalized generator-originated ODE, fabricated OCE, internal ArgumentException, native-load faults, and null/wrong-type/disposed/malformed outputs to KeyStoreException(Unavailable). Only the documented privateKey caller fault maps to ArgumentException(request).
• Independently re-derived generated/imported private keys before commit, rejecting same-type mismatched public/private halves.
• Enforced BBS count, header, and cumulative byte bounds before excess copying, stopped at first overflow, and normalized inconsistent list shape.

Regression proof: five original regressions fail at their intended assertions on b847152; both mismatched-pair tests also fail there; the BBS mutation test fails when its deep-copy fix is reverted. All pass with the fixes restored.

Verification:

• Release warnings-as-errors build: passed
• hardening: 53/53
• input-validation/fuzz: 325/325
• native-present: 1221/1221
• isolated no-native: 1178/1178
• independent adversarial harness: 30/30 native and 25/25 no-native, zero remaining confirmed defects
• BBS sample, API coverage, and diff check: passed

The capable-store sample still hits a local macOS Apple Crypto P-256 module-load failure, but the identical failure reproduces at pre-fix b847152; this patch did not introduce it.

Please re-review the current head.