fix(jose): adversarial hardening pass — close #6

[moisesja]

Summary

Closes #6. Issue #6 asked for JWS/JWE enveloping building blocks; investigation found the JOSE layer (DataProofsDotnet.Jose: JWS/JWE/JWT/JWK/SD-JWT/SD-JWT VC/VC-JOSE) already implemented and passing — the issue was stale. Per the chosen path, this is a deep adversarial security/correctness hardening pass of that surface, fixing every confirmed weakness and pinning each with a regression test.

Method

A multi-agent workflow reviewed 9 dimensions (JWS bypass, JWE key-mgmt, ECDH-1PU, multi-recipient, JWK, SD-JWT/KB-JWT/VC, JWT, encoding/JSON, fail-closed/hygiene). Every finding was then put through 3 independent refute-by-default skeptics (exploitability / spec-correctness / already-mitigated lenses) and kept only on a ≥2/3 majority. Result: 11 findings verified, 5 confirmed, 6 refuted (correctly — by-design or already-guarded).

Confirmed findings fixed (each with a regression test)

Sev	Finding	Fix
HIGH	SdJwtReconstructor walked the disclosed payload by unbounded recursion; a chained recursive-disclosure presentation (RFC 9901 §6.3) rooted in an issuer-signed _sd digest could drive an uncatchable StackOverflowException that terminates the host process — the strongest fail-closed violation.	Depth-bound reconstruction (64) → MalformedJoseException (DISCLOSURE_INVALID).
MEDIUM	A malformed/unsupported cnf (or issuer) key made CompactJwt.Verify escape as an uncaught NotSupportedException/crypto exception (reachable via an attacker-influenced cnf on the KB path).	Fail closed as MalformedJoseException, which both call sites already handle.
LOW	JWS reported SignerKid from the unauthenticated unprotected header.	Report the verified signer only from the integrity-protected header; the unprotected kid stays a resolution hint.
LOW	JWT verifier ignored typ (RFC 8725 §3.11 token confusion).	Opt-in JwtValidationOptions.ExpectedType (case-insensitive, application/-prefix tolerant) + surfaced JwsParseResult.Typ.
LOW	Base64Url.Decode accepted whitespace/padding, contradicting the no-pad contract.	Strict base64url-alphabet validation.

Refuted (no change, with reasons): un-zeroed sender key on a 1PU build-throw, A256KW recipient-set commitment (by design), vct#integrity not validated (optional), padding-only base64 (subsumed by the strict fix), deterministic-number canonicalization (headers carry no floats).

API additions (opt-in, non-breaking)

• JwtValidationOptions.ExpectedType — pin the JWS typ on JwtHandler.Verify.
• JwsParseResult.Typ — the integrity-protected typ header.

Both are in PublicAPI.Unshipped.txt and exercised by Samples.JwtAndJwk (AC-9 gate green).

Verification

• dotnet build DataProofsDotnet.sln → 0 warnings, 0 errors.
• dotnet test DataProofsDotnet.sln → 781 passed, 0 failed (Jose 311, +14 new hardening regressions; no regressions elsewhere).
• samples-coverage gate green (530 members across 6 packages, 0 uncovered); AC-6 hygiene + AC-7 API discipline green.
• All crypto remains routed through NetCrypto; documented intentional algorithm omissions preserved; no new algorithms added.

🤖 Generated with Claude Code

[moisesja]

Automated review — PR #9 fix(jose): adversarial hardening pass — close #6

Solid hardening pass. The methodology (9 dimensions → 3 independent skeptics → majority-vote) is rigorous, each confirmed finding is paired with a regression test, the API additions are opt-in and non-breaking, and the PR description doubles as a useful security-audit trail. A few minor observations:

---

Base64Url.Decode — behavioral breaking change for lenient callers

The strict alphabet loop is the correct fix for JOSE contexts, but it changes the runtime behavior of an existing public API: any out-of-repo caller that previously passed =-padded or whitespace-containing input now gets a FormatException with no compile-time warning. The CHANGELOG frames this correctly as a correctness fix, but it is worth calling out explicitly so upgraders know to scan their own Base64Url call sites. A one-line note in the CHANGELOG ### Changed section (in addition to ### Security) would make the upgrade path clear.

---

Off-by-one on the depth guard (cosmetic)

private const int MaxReconstructionDepth = 64;
// …
if (depth > MaxReconstructionDepth)   // fires at depth 65 — allows 65 frames, not 64

The constant says 64 and the comment references the JsonNode.Parse default MaxDepth = 64, but > instead of >= means 65 frames are actually permitted. No real-world SD-JWT comes near either number and the test uses 200 links, so this is purely cosmetic. If you ever want the semantics to match the name exactly, swap to >= MaxReconstructionDepth.

---

samples-coverage-report.md contains an absolute local path

Samples directory: `/Users/moises/Projects/dataproofs-dotnet/samples`

Not a security issue, but the committed report will show a stale path whenever it is regenerated in CI or on another machine. Minor — worth addressing the next time the report is regenerated from a portable root.

---

Everything else looks correct

• CompactJwt exception filter correctly passes through MalformedJoseException and OperationCanceledException before wrapping anything else.
• TypeMatches correctly normalises the application/ prefix (case-insensitive, per RFC 8725 §3.11).
• SignerKid now sourced from header.Kid (integrity-protected) rather than sig.Kid (unprotected) — closes the real identity-confusion risk.
• 781/0 test result and samples-coverage gate green are good final confirmators.

None of the above block merge — the security fixes are sound.

---

Generated by Claude Code

[moisesja]

Also folded in (per discussion): retired the ac-5 didcomm parity-diff gate (commit 7cd923c).

Root cause of the ac-5 CI failure was not a stale SHA — the pinned commit is main's HEAD on moisesja/didcomm-dotnet (even tagged). It was a bug in tasks/parity-diff (RunGit set both WorkingDirectory and git -C to the repo path, so CI's relative --didcomm-repo .parity-src/didcomm-dotnet was applied twice → fatal: cannot change to '…' → spurious "commit not found"). A one-line Path.GetFullPath would have fixed it.

But since didcomm-dotnet is being refactored, the source-diff pin is becoming meaningless, so rather than fix the tool we removed the gate:

• Dropped the ac-5 job from .github/workflows/ci.yml (+ stale cross-references).
• Deleted the tasks/parity-diff tool (removed from the solution) and tests/.../Parity/PARITY.md.
• Marked AC-5 retired in the PRD §9 / Phase B note.

Kept the ported parity test suite (tests/.../Parity/* — real JWS/JWE/JWK/KDF/AEAD coverage); it still runs under ac-3 and build-test. JOSE conformance now rests on AC-3 (RFC vectors + oracle cross-verification).

Full solution: build 0/0, 792 tests pass (no test removed).