JWE constant-work decrypt (#12) + async IEcdhKey seam (#13) → v1.1.0

[moisesja]

Bundles the two milestone-1.1.0 issues. Targets v1.1.0 (bumps DataProofsVersion 1.0.1 → 1.1.0).

#12 — Timing side-channel: recipient-key enumeration oracle (bug)

JweParser.Parse fast-failed before any ECDH when no recipient kid matched a held private key, so held-vs-unheld was observable as a response-time difference (the root cause of downstream didcomm-dotnet#35).

Fix (internal only, no public API change):

• New DecoyKeyCache — a per-process cached decoy ECDH key per curve (caching is load-bearing; a per-call keygen would itself re-introduce a timing signal).
• SelectRecipientOrDecoy routes the non-decryptable case through the same ECDH → AES-KW unwrap on the envelope's epk work curve. The predicate is "is a held private key present whose curve matches epk?" — which also closes a residual held-but-wrong-curve leak the issue's own suggested fix missed.
• Uniform failure: every post-selection unwrap/AEAD failure throws one JoseCryptoException, same message, no recipient kid, no inner cause — closing the exception-type/message channel (found by adversarial review: a length-corrupted captured envelope let a held key reach the AEAD stage while an unheld key died at unwrap).
• Always-on (secure default). The success path of a curve-matching held key is unchanged and pays nothing extra.

Scope caveat (documented): this makes the parser's own path constant-work for a fixed (alg, enc, epk-curve). A fully constant-time decrypt additionally requires the caller's IJweRecipientKeyResolver (FindPresent/TryGet) to be timing-independent — that contract stays with the caller.

#13 — Async IEcdhKey seam for opaque (HSM/keystore) ECDH keys (feature)

Additive seam so JWE ECDH can run on a private key that never exposes its scalar (HSM, KMS, keychain, NetCrypto.IKeyStore), keeping DataProofsDotnet.Jose DID-agnostic.

• Public IEcdhKey (Crv + DeriveAsync → raw Z) and RawEcdhKey.
• Async overloads JweParser.ParseAsync/ParseCompactAsync and JweBuilder.BuildEcdh1PuA256KwAsync.
• Async KDF cores in EcdhEsKdf/Ecdh1PuKdf share a new FinishConcatKdf with the frozen sync cores.
• The existing synchronous JWE API is unchanged and the draft-04 / RFC 7518 conformance vectors are byte-for-byte identical.

Note: #13 is an enhancement (not a bug); bundled per request.

Verification

• 818/818 tests pass solution-wide (Jose 337, incl. 6 Timing side-channel: JweParser fast-fails before ECDH when no recipient kid is held → recipient-key enumeration #12 + 16 Add an async IEcdhKey seam so JWE ECDH can use an opaque (keystore/HSM) private key #13 + uniform-failure / public-only-key regressions). Release build clean (0 warnings with TreatWarningsAsErrors → PublicAPI + BannedSymbols analyzers pass).
• Two adversarial review passes. Pass 1 found the exception-channel leak (Medium) → fixed + regression test. Pass 2 verified full closure (12/12 probes: held == unheld across timing and exception, on sync/compact/async/1PU) and no regression (zeroization intact; broad catch can't swallow null args; structural pre-selection errors keep descriptive messages).
• CHANGELOG [1.1.0], PRD FR-14, and README updated.

Closes #12, #13.

🤖 Generated with Claude Code

[moisesja]

Review — PR #14 (v1.1.0: JWE constant-work + async IEcdhKey)

This is a well-engineered PR. Both issues are closed correctly and the two adversarial review passes are evident in the design. No blocking issues found.

---

What looks good

Issue #12 (timing side-channel)

• The decoy predicate keys on the envelope's epk curve, not on a held key's curve — the correct predicate. The original kid-only approach still leaked on the held-but-wrong-curve case; this closes it.
• DecoyKeyCache uses Lazy<T>(ExecutionAndPublication) for a correct per-process singleton under concurrent first-calls. Per-call key generation would re-introduce the timing oracle; caching is correctly explained as load-bearing.
• SelectRecipientOrDecoy scans the full recipient list without early-out, preventing a selection-index timing signal.
• DecryptFailureMessage = "JWE could not be decrypted." is applied at both the AES-KW unwrap and AEAD stages — same exception type, same message, no inner cause, no recipient kid. The adversarial exception-channel finding is fully addressed.
• IsUsableForEnvelope checks !string.IsNullOrEmpty(key.D), so a public-only JWK under the right kid takes the decoy path rather than producing a distinguishable "missing 'd'" failure. Good catch from pass 2.
• The scope caveat (caller's IJweRecipientKeyResolver timing is outside the guarantee) is documented in XML docs, CHANGELOG, and the task file — the right places.

Issue #13 (async IEcdhKey seam)

• IEcdhKey is deliberately minimal: Crv + DeriveAsync. The structural surface test (IEcdhKey_surface_exposes_no_private_scalar) is a nice compile-time+reflection guard.
• The DeriveAsync XML doc noting "may be invoked more than once" (ECDH-1PU derives Ze then Zs) sets the contract for HSM/KMS implementations correctly.
• FinishConcatKdf extraction ensures sync and async cores share identical post-ECDH assembly — the parity guarantee is structural, not just empirical.
• RawEcdhKey.DeriveAsync completes synchronously and checks ct.ThrowIfCancellationRequested() before the computation.
• UnwrapAnyAndDecrypt scans all recipients, zeroizes losing candidates, and zeroizes the KEK in finally. Correct.

Tests

• Issue12ConstantWorkTests uses a call-count proxy (CountingCryptoProvider.DeriveSharedSecretCalls) rather than wall-clock timing — the right approach for a CI-stable side-channel regression test.
• Covers: wrong-curve key → decoy path, public-only key → decoy path, length-corrupted envelope → same exception for held and unheld, multi-recipient with a non-first held key, and the real opaque key not invoked on curve-mismatch in the async path. Comprehensive.

---

Minor observations (not blocking)

1. RawEcdhKey private bytes are not zeroized on disposal. The constructor copies privateKey.ToArray(), which lives until GC. For the stated use-case (in-process key bytes) this is fine, but a future IDisposable path or a doc note would help callers understand the security boundary. Not a regression — the existing sync API has the same characteristic.

2. FinishConcatKdf does not zeroize the concatenated z. In both EcdhEsKdf and Ecdh1PuKdf, ze/zs are zeroized but the concatenation z = ze‖zs is not cleared after the Concat KDF. Not a regression from the pre-refactor code, but worth tracking for a cleanup pass.

3. DecoyForEnvelope X25519 fallback. The placeholder returned for an absent/unsupported epk curve is never used for ECDH because the structural throw fires first. A Debug.Assert to make that invariant machine-checkable would be a small improvement.

---

These are all low-severity and fine to track as future work. The security change (constant-work + uniform failure) is solid, the async seam is correct, the 818-test suite is comprehensive, and the public API surface is properly registered. Ready to merge. ✅

---

Generated by Claude Code

[moisesja]

Thanks for the review. Evaluated the three minor observations; no code changes warranted:

1. RawEcdhKey zeroization / IDisposable — the copy's lifetime is already documented in the type's XML doc ("the copy lives for the object's lifetime"). IDisposable adds marginal security value here: the caller already holds the original d bytes passed into the constructor, so zeroing the internal copy doesn't remove the scalar from memory — at the cost of public-API surface and disposed-state handling. Leaving as-is (consistent with the existing sync API and the non-blocking rating).

2. FinishConcatKdf does not zeroize z — this one doesn't hold: both EcdhEsKdf.FinishConcatKdf and Ecdh1PuKdf.FinishConcatKdf zeroize the concatenated z in a finally (finally { CryptographicOperations.ZeroMemory(z); }), after zeroizing ze/zs. Matches the pre-refactor behavior — no gap.

3. Debug.Assert on the X25519 placeholder — the "never used for ECDH" invariant is enforced downstream in DecryptForRecipient (the RequireEpk / curve-mismatch throw fires before any ECDH), not at the DecoyForEnvelope site, so there's no clean place for the assert there. The site already carries a comment explaining the invariant.

Merging as-is.