diff --git a/.claude/settings.json b/.claude/settings.json new file mode 100644 index 0000000..5372c6a --- /dev/null +++ b/.claude/settings.json @@ -0,0 +1,40 @@ +{ + "permissions": { + "allow": [ + "Bash(mkdir -p /tmp/akwprobe_dir)", + "Bash(cp /tmp/akwprobe.cs /tmp/akwprobe_dir/Program.cs)", + "Bash(awk '/private static byte\\\\[\\\\] DeriveKeyCore/,/^ }$/' /Users/moises/Projects/didcomm-dotnet/src/DidComm.Core/Crypto/Kdf/Ecdh1PuKdf.cs)", + "Bash(awk '/RenderJwe|recipients = recipientArr|@protected|encrypted_key|header = new/' /Users/moises/Projects/didcomm-dotnet/src/DidComm.Core/Jose/Encryption/JweBuilder.cs)", + "Bash(awk '/private static string RenderJwe/,/^ }$/' /Users/moises/Projects/didcomm-dotnet/src/DidComm.Core/Jose/Encryption/JweBuilder.cs)", + "Bash(find / -name \"ConcatKdf.cs\" -path \"*NetDid*\")", + "Bash(find / -name \"ConcatKdf.cs\" -path \"*net-did*\")", + "Bash(xargs -I{} sh -c 'echo \"=== {} ===\"; cat \"{}\"')", + "Bash(xargs -I{} sh -c 'echo \"=== {} ===\"; cat {}')", + "Bash(echo \"\\(exit: $?\\)\")", + "Bash(xargs -I{} echo \".claude entries staged: {}\")", + "Bash(xargs -I{} echo \"{} packages.lock.json files\")", + "Bash(xargs -I{} echo \"{} lock files \\(want 0\\)\")", + "Bash(echo \"---exit:$?---\")", + "Bash(awk 'NR>=342 && NR<=421' /Users/moises/Projects/didcomm-dotnet/src/DidComm.Core/Facade/DidCommClient.cs)", + "Bash(/bin/ls -la /Users/moises/Projects/didcomm-dotnet/src/DidComm.Core/Jose/Encryption/)", + "Bash(/bin/ls -la /Users/moises/Projects/didcomm-dotnet/)", + "Bash(/bin/ls -la /Users/moises/Projects/didcomm-dotnet/src/)", + "Bash(xargs -I{} basename {})", + "Bash(/bin/ls -la /Users/moises/Projects/didcomm-dotnet/src/DidComm.Core/Composition/)", + "Bash(echo \"=== exit: $? \\(1 = no matches\\) ===\")", + "Bash(sed -n '1,40p' src/DidComm.Core/Threading/InMemoryThreadStateStore.cs)", + "Bash(sed -n '88,100p' src/DidComm.Core/Protocols/ProtocolDispatcher.cs)", + "Bash(echo \"exit: $?\")", + "Bash(awk '/FR-ENV-02/{p=1} p{print} /FR-ENV-05/{c++; if\\(c>=1 && /Acceptance|acceptance/\\) }' docs/didcomm-dotnet_PRD.md)", + "Bash(xargs sed -n '1,60p')", + "Bash(grep -rn \"AllowSynchronousIO\\\\|PreserveExecutionContext\\\\|ThrowOnUnhandled\\\\|TestServerOptions\" /usr/local/share/dotnet 2>/dev/null | head; echo \"=== check TestServer option name via reflection doc ===\"; grep -rn \"UseTestServer\\\\|TestServer\" tests/DidComm.InteropTests --include=\"*.cs\" | head)", + "Read(//usr/local/share/dotnet/**)", + "Bash(xargs sed -n '1,12p')", + "Bash(cd /Users/moises/Projects/didcomm-dotnet *)", + "Bash(ln -sfn /Users/moises/Projects/didcomm-dotnet/tests/DidComm.InteropTests/bin/Debug/net10.0/fixtures /tmp/rotpoc2/bin/Debug/net10.0/fixtures)" + ], + "additionalDirectories": [ + "/Users/moises/Projects/dataproofs-dotnet/src/DataProofsDotnet.Jose/Encryption" + ] + } +} diff --git a/CHANGELOG.md b/CHANGELOG.md index 692d3e5..8b565a1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,7 +6,28 @@ All notable changes to didcomm-dotnet are documented here. Format follows ## [Unreleased] -### Fixed — receive-path correctness (`feat/security-receive-correctness`) +### Fixed — rotation JWT compliance + +Resolves two Low-severity audit findings on the `from_prior` rotation path (GitHub issues #25, #26). +Both passed a PoC-backed adversarial red-team pass; full suite (625 tests) green. + +- **`FromPriorBuilder` now emits `exp`/`nbf` (#25, FR-ROT-05).** The builder previously serialized only + `{ iat, iss, sub }`, silently discarding any `FromPriorClaims.Exp`/`Nbf` — so every self-issued + rotation JWT was non-expiring and the validator-side freshness control (already enforced by + `DidCommClient.ValidateFromPriorFreshness`) could never fire on tokens this library minted. Claims are + now built via an insertion-ordered `JsonObject` (`exp, iat, iss, nbf, sub`), emitting `exp`/`nbf` only + when set (the no-expiry payload is byte-identical to before). A new + `BuildAsync(claims, signerJwk, TimeSpan lifetime)` overload sets `exp = iat + lifetime` (rejecting a + sub-second lifetime, which would floor to an already-expired token) so issuing a freshness-bounded + rotation JWT is one call. The PRD's rotation cookbook/API-matrix is realigned to the actual API + (`FromPriorBuilder.BuildAsync` + `MessageBuilder.WithFromPrior`); the previously-documented + `WithDidRotation` helper was never built and is noted as possible future DX. +- **`from_prior` validator rejects a `crit` protected header (#26, RFC 7515 §4.1.11).** The validator + hand-parses the JWS header and previously read only `alg`/`kid`, silently ignoring `crit` — unlike + `JwsParser`/`JweParser`, which fail closed. It now rejects any `crit` member with `ProtocolException`, + before signature verification, bringing the rotation path to parity. + +### Fixed — receive-path correctness Resolves three Low-severity audit findings on the unpack/JOSE receive surface (GitHub issues #22, #23, #24). Each ships with tests and an adversarial red-team pass; full suite (615 tests) green. @@ -33,7 +54,7 @@ Resolves three Low-severity audit findings on the unpack/JOSE receive surface (G both flags true) was already unreachable after the #17 composition gate; this makes the code match its contract by construction. -### Security — Medium-severity audit remediation (`feat/security-medium-cluster`) +### Security — Medium-severity audit remediation Remediates the five Medium-severity findings from the multi-agent security & compliance audit (GitHub issues #17–#21), plus two lower-severity defects on the same code path (#28, #33). Each fix @@ -70,7 +91,7 @@ follow-ups). The full suite (600 tests) is green. Red-team hardening: the inline-callback overload now also wraps `onReceive` so a consumer callback that throws after a successful unpack returns 202 (logged) instead of escaping as a distinguishable `500` — removing an unpack-success oracle the uniform-400 path would otherwise leave. (A residual - *timing* side-channel in the underlying decrypt path — held-vs-unheld recipient kid — is **not** + _timing_ side-channel in the underlying decrypt path — held-vs-unheld recipient kid — is **not** closed by this change and is tracked as a follow-up.) A downstream DID-resolution timeout (`TaskCanceledException` with the request token not cancelled — e.g. a hung `did:webvh` host) also collapses to the uniform 400 rather than escaping as a `500`; only a genuine client abort propagates. @@ -85,7 +106,7 @@ follow-ups). The full suite (600 tests) is green. snapshot-sort — closing a CPU-amplification DoS. (A residual cascade-guard-reset-via-eviction vector, costing the attacker ~`cap` messages per suppressed report, is tracked as a follow-up alongside #29.) -### Changed — delegate the envelope layer to DataProofsDotnet.Jose (`feat/delegate-jose-dataproofs`) +### Changed — delegate the envelope layer to DataProofsDotnet.Jose didcomm-dotnet no longer carries its own cryptography or JWE/JWS assembly. The entire `Crypto/` folder and almost all of `Jose/` were deleted; envelope build/parse is delegated to @@ -131,10 +152,10 @@ all DIDComm v2.1 Appendix C byte-equivalence vectors (anoncrypt/authcrypt/signed (method name **and** return type change). DataProofs signs through an async `ISigner`, so direct callers must `await` the new method. The primary `DidCommClient` pack/unpack facade is unchanged (it was already async); `ForwardWrapper.WrapAsync` is `internal`. -- **ES512 / P-521 *signing* dropped.** The old `KeyTypeMapper` mapped `P-521 → ES512` for JWS; the +- **ES512 / P-521 _signing_ dropped.** The old `KeyTypeMapper` mapped `P-521 → ES512` for JWS; the delegated signer scopes JWS to EdDSA / ES256 / ES384 / ES256K, so a P-521 signer JWK now raises `NotSupportedException`. **Not a DIDComm v2.1 regression** — signed messages require only - EdDSA / ES256 / ES256K (none were tested with P-521), and P-521 *key agreement* + EdDSA / ES256 / ES256K (none were tested with P-521), and P-521 _key agreement_ (anoncrypt/authcrypt) is unaffected and still tested. ### Notes diff --git a/docs/didcomm-dotnet_PRD.md b/docs/didcomm-dotnet_PRD.md index 4fab53e..e9bc59d 100644 --- a/docs/didcomm-dotnet_PRD.md +++ b/docs/didcomm-dotnet_PRD.md @@ -876,12 +876,22 @@ bool acked = received.Message.Acks.Contains(sentId); **N. DID rotation (from_prior — FR-ROT-*)** ```csharp -var rotated = Message.Builder(type).From("did:peer:alice2").To("did:peer:bob") - .WithDidRotation(newDid: "did:peer:alice2", oldDid: "did:peer:alice", - oldKid: "did:peer:alice#key-1") // signed via ISecretsResolver +// Mint the rotation JWT, signed by a key authorized under the PRIOR DID's `authentication` +// relationship (fetch oldSignerPrivateJwk from your ISecretsResolver). Pass a short lifetime so the +// token is freshness-bounded and cannot be replayed past the window (FR-ROT-05). +var fromPrior = await FromPriorBuilder.BuildAsync( + new FromPriorClaims(Sub: "did:peer:alice2", Iss: "did:peer:alice", + Iat: DateTimeOffset.UtcNow.ToUnixTimeSeconds()), + oldSignerPrivateJwk, lifetime: TimeSpan.FromMinutes(5)); + +var rotated = new MessageBuilder().WithType(type) + .WithFrom("did:peer:alice2").WithTo("did:peer:bob") + .WithFromPrior(fromPrior) // ride the JWT on a sender-authenticated envelope (FR-ROT-01/03) .Build(); -// On unpack: res.Metadata.FromPrior is populated and validated. +// On unpack: unpacked.FromPrior is populated and validated (FR-ROT-01..05). ``` +> `MessageBuilder.WithDidRotation(...)` is a possible future DX convenience; today the rotation JWT is +> minted explicitly via `FromPriorBuilder.BuildAsync` and attached with `WithFromPrior`. **O. Routing via a mediator (automatic from the recipient's routingKeys — FR-ROUTE-*)** ```csharp @@ -1018,7 +1028,7 @@ This matrix is the traceability artifact behind FR-DX-01/09. Each public API gro | Params/results | `PackEncryptedParams`, `PackSignedParams`, `UnpackParams`, `*Result`, `UnpackMetadata`, `SendOptions` | 02, 03 | | Message model | `Message`, `Message.Builder`, `Attachment` (+factories), `ContentEncryptionAlgorithm` | 02, 03 | | Threading/ACKs | `WithThid/WithPthid/WithPleaseAck/WithAck`, `Message.Empty` | 03, 07 | -| Rotation | `WithDidRotation`, `UnpackMetadata.FromPrior` | 03 | +| Rotation | `FromPriorBuilder.BuildAsync`, `MessageBuilder.WithFromPrior`, `UnpackResult.FromPrior` | 03 | | DI | `AddDidComm`, `DidCommBuilder.UseNetDidResolver/UseSecretsResolver/UseTransport/AddProtocol/Configure` | 02, 08 | | Resolution | `IDidKeyService`, `NetDidKeyService`, `UseNetDidResolver`, `UnsupportedDidMethodException` | 09 | | Secrets | `ISecretsResolver`, `Secret`, `NetDidKeyStoreSecretsResolver` | 08 | diff --git a/src/DidComm.Core/Protocols/Rotation/FromPriorBuilder.cs b/src/DidComm.Core/Protocols/Rotation/FromPriorBuilder.cs index 4d880bc..9b11aec 100644 --- a/src/DidComm.Core/Protocols/Rotation/FromPriorBuilder.cs +++ b/src/DidComm.Core/Protocols/Rotation/FromPriorBuilder.cs @@ -1,4 +1,4 @@ -using System.Text.Json; +using System.Text.Json.Nodes; using DidComm.Jose.Signing; using DpSig = DataProofsDotnet.Jose.Signing; @@ -29,14 +29,18 @@ public static async Task BuildAsync(FromPriorClaims claims, Jwk signerPr // JwsSignerFactory validates crv/d/kid and adapts the JWK into a NetCrypto-backed signer. var signer = JwsSignerFactory.FromPrivateJwk(signerPrivateJwk); - // Claims — key order: iat, iss, sub (lexicographic). Serialize rather than interpolate so - // an unusual value is JSON-escaped, not injected. - var claimsJson = JsonSerializer.Serialize(new - { - iat = claims.Iat, - iss = claims.Iss, - sub = claims.Sub, - }); + // Claims in lexicographic key order (exp, iat, iss, nbf, sub) so identical inputs produce + // byte-identical payloads across runs. exp/nbf are emitted only when present (RFC 7519 + // §4.1.4/§4.1.5) — a from_prior without them is non-expiring, so callers SHOULD set Exp to + // bound replay (FR-ROT-05); the lifetime overload below does that in one call. Built via + // JsonObject (insertion-ordered) rather than interpolation so values are JSON-escaped. + var claimsObj = new JsonObject(); + if (claims.Exp is long exp) claimsObj["exp"] = exp; + claimsObj["iat"] = claims.Iat; + claimsObj["iss"] = claims.Iss; + if (claims.Nbf is long nbf) claimsObj["nbf"] = nbf; + claimsObj["sub"] = claims.Sub; + var claimsJson = claimsObj.ToJsonString(); // Compact JWS with typ=JWT. DataProofs builds the {alg,kid,typ} protected header from the // signer and signs ASCII(b64u(header) "." b64u(payload)); the payload is these claim bytes. @@ -44,4 +48,28 @@ public static async Task BuildAsync(FromPriorClaims claims, Jwk signerPr .BuildCompactAsync(Encoding.UTF8.GetBytes(claimsJson), signer, typ: "JWT", detachedPayload: false, ct) .ConfigureAwait(false); } + + /// + /// Build a freshness-bounded from_prior JWT: sets exp = .Iat + + /// (overriding any ) so the + /// rotation token cannot be replayed past the window (FR-ROT-05). A short lifetime is recommended — + /// a from_prior only needs to ride until a message reaches the new DID (FR-ROT-04). + /// + /// Sub / Iss / Iat (and optional Nbf); Exp is computed from . + /// Private JWK; Kid MUST identify a key authorized under .Iss authentication. + /// Positive validity window added to Iat to form exp. + /// Cancellation token. + /// is not positive. + public static Task BuildAsync(FromPriorClaims claims, Jwk signerPrivateJwk, TimeSpan lifetime, CancellationToken ct = default) + { + ArgumentNullException.ThrowIfNull(claims); + // exp is second-granular, so a sub-second lifetime would floor to exp == iat — a token already + // expired at issue. Require at least one whole second (red-team: avoid the silent zero-window). + if (lifetime < TimeSpan.FromSeconds(1)) + throw new ArgumentOutOfRangeException(nameof(lifetime), lifetime, + "from_prior lifetime must be at least 1 second (exp is second-granular)."); + + var bounded = claims with { Exp = claims.Iat + (long)lifetime.TotalSeconds }; + return BuildAsync(bounded, signerPrivateJwk, ct); + } } diff --git a/src/DidComm.Core/Protocols/Rotation/FromPriorValidator.cs b/src/DidComm.Core/Protocols/Rotation/FromPriorValidator.cs index 5be26ba..30d43dc 100644 --- a/src/DidComm.Core/Protocols/Rotation/FromPriorValidator.cs +++ b/src/DidComm.Core/Protocols/Rotation/FromPriorValidator.cs @@ -74,6 +74,13 @@ internal static async Task ValidateAsync( alg = headerDoc.RootElement.GetProperty("alg").GetString(); kid = headerDoc.RootElement.GetProperty("kid").GetString(); + // RFC 7515 §4.1.11: fail closed on a 'crit' header — the library understands no from_prior + // crit extensions, so any are by definition unsupported. Mirrors JwsParser/JweParser crit + // rejection (#26). 'crit' is covered by the signature but was previously read and ignored. + // The check runs before signature verification, so a crit envelope is rejected as malformed. + if (headerDoc.RootElement.TryGetProperty("crit", out _)) + throw new ProtocolException("from_prior JWT marks an unsupported extension critical ('crit')."); + using var claimsDoc = JsonDocument.Parse(claimsJson, DidCommJson.StrictDocument); var iss = claimsDoc.RootElement.GetProperty("iss").GetString() ?? throw new ProtocolException("from_prior JWT 'iss' is missing or null."); diff --git a/tasks/todo20260617T033547Z.md b/tasks/todo20260617T033547Z.md new file mode 100644 index 0000000..638c411 --- /dev/null +++ b/tasks/todo20260617T033547Z.md @@ -0,0 +1,105 @@ +# Plan — Batch B: rotation JWT compliance (#25, #26) + +**Branch:** `feat/security-rotation-compliance` (off `main`) +**One PR** closing **#25, #26**. + +Both fixes live in the `from_prior` rotation files. The validator/enforcement side already works +(`FromPriorValidator` extracts `exp`/`nbf`; `DidCommClient.ValidateFromPriorFreshness` enforces them +with clock skew, FR-ROT-05) — so the gaps are narrow. + +--- + +## [ ] #25 — `FromPriorBuilder` discards `exp`/`nbf` (FR-ROT-05 dead on the issuing side) + +- **Root cause:** `FromPriorBuilder.BuildAsync` serializes only `{ iat, iss, sub }` + (`FromPriorBuilder.cs:34-39`), dropping `FromPriorClaims.Exp`/`Nbf` even when set — so every + self-issued rotation JWT is non-expiring. +- **Fix (core):** emit `exp`/`nbf` when present, preserving lexicographic key order + (`exp, iat, iss, nbf, sub`). Build the claims with a `JsonObject` (insertion-ordered) and add keys + conditionally, instead of the fixed anonymous object. `null` Exp/Nbf ⇒ key omitted (so the existing + `{iat,iss,sub}` round-trip is byte-identical — no regression). +- **Fix (ergonomics):** add a convenience overload + `BuildAsync(FromPriorClaims, Jwk, TimeSpan lifetime, CancellationToken)` that sets `exp = iat + lifetime` + so issuing a freshness-bounded token is one call. Document a recommended lifetime (the spec mandates + no specific TTL; suggest a short window, e.g. minutes-to-hours, since `from_prior` rides only until a + message reaches the new DID, FR-ROT-04). **Not** auto-defaulting on the plain overload (would surprise + existing callers; spec doesn't require a default value — it requires the builder to be *able* to emit). +- **Tests:** builder with `Exp`/`Nbf` set → decode JWT, assert payload carries `exp`/`nbf` in order; + round-trip validates; **end-to-end**: a token with `Exp` in the past → `ConsistencyException` + (FR-ROT-05) on `UnpackAsync`; `Nbf` in the future → rejected; the lifetime overload sets `exp` + correctly; the no-exp path still emits `{iat,iss,sub}` unchanged. + +### ⚠️ Decision: the PRD references a `WithDidRotation` helper that doesn't exist +`docs/didcomm-dotnet_PRD.md:880` (cookbook) and `:1021` (API matrix) show +`.WithDidRotation(newDid, oldDid, oldKid)`, but the real API is `FromPriorBuilder.BuildAsync` + +`MessageBuilder.WithFromPrior(string)`. No FR mandates `WithDidRotation`, and it fits awkwardly on the +**sync** `MessageBuilder` (signing is async and needs the private JWK, not just a kid). The suite is +green, so no test enforces it — it's a doc-only aspiration. +**My recommendation:** align the PRD example to the actual API (`BuildAsync` + `WithFromPrior`) and note +`WithDidRotation` as possible future DX sugar, rather than build an awkward sync helper for a Low fix. +_Tell me if you'd rather I build `WithDidRotation` instead — that's a larger DX addition._ + +--- + +## [ ] #26 — `from_prior` validator ignores a `crit` protected header (RFC 7515 §4.1.11) + +- **Root cause:** `FromPriorValidator` hand-parses the header reading only `alg`/`kid` + (`FromPriorValidator.cs:73-75`); it never inspects `crit`, so a `from_prior` marking an unsupported + extension critical is silently accepted — unlike `JwsParser`/`JweParser`, which fail closed. +- **Fix:** after reading `alg`/`kid`, reject any header carrying a `crit` member (the library + understands no `from_prior` crit extensions), mirroring the external `JwsParser` parity check — + `if (headerDoc.RootElement.TryGetProperty("crit", out _)) throw new ProtocolException("from_prior JWT marks an unsupported extension critical ('crit').")`. + Placed inside the existing parse `try`; `ProtocolException` is **not** in the catch filter, so it + propagates cleanly (same pattern as the `iss`/`sub` null guards). The check precedes signature + verification, so a negative test can splice `crit` without re-signing. +- **Scope note:** the issue suggests "ideally route from_prior through the shared `JwsParser`." That's a + larger refactor (the validator needs relationship-specific verification + claim extraction the shared + parser doesn't do); the minimal `crit` rejection achieves parity and is the issue's primary + recommendation. Routing-through-JwsParser left as a possible follow-up. +- **Test:** a valid `from_prior` with a `crit` member spliced into the header → `ProtocolException` + (reuse the existing `MutateHeader` helper; no re-sign needed). + +--- + +## Cross-cutting +- [ ] Branch `feat/security-rotation-compliance` off `main`. +- [ ] Implement both fixes (additive; the lifetime overload is the only new public surface). +- [ ] `dotnet build` (warnings-as-errors) + full `dotnet test` green, incl. new tests. +- [ ] **Adversarial red-team pass** on both fixes (AGENTS.md §2), remediate findings. +- [ ] Align the PRD §N rotation example to the real API (pending the decision above). +- [ ] Update `CHANGELOG.md`; one PR: `fix(security): rotation JWT compliance (#25, #26)`, + `Closes #25`, `closes #26`. + +## Verification standard +- #25: prove a self-issued token with `Exp` in the past is actually *rejected* end-to-end on unpack + (FR-ROT-05 now reachable on the issuing side), and the no-exp path is byte-unchanged. +- #26: prove a `crit`-bearing `from_prior` is rejected *before* signature verification. + +## Review section + +Both fixes implemented on `feat/security-rotation-compliance`. **625 tests green**, build clean. + +- **#25** — `FromPriorBuilder` emits `exp`/`nbf` via an ordered `JsonObject` (exact-payload tests pin the + order and the byte-identical no-exp path); added the `TimeSpan lifetime` overload. End-to-end test + proves a self-issued expired token is now rejected on unpack (FR-ROT-05 reachable on the issuing side). +- **#26** — `FromPriorValidator` rejects any `crit` header before signature verification (parity with + `JwsParser`); negative test splices `crit` without re-signing. +- **PRD** — realigned the rotation cookbook (§N) + API matrix to the real API; noted `WithDidRotation` + as future DX (per your decision). + +### Adversarial red-team pass (AGENTS.md §2) — both hold +Subagents ran this time with PoC harnesses against the compiled assemblies. +- **#25 holds** — lifetime overflow is unconstructible (`TimeSpan.MaxValue.TotalSeconds` ≈ 9.2e11 ≪ + `long` overflow), and a forced wrap fails *closed* at the receiver (negative `exp` → always expired); + no JSON injection (JsonObject escapes); round-trip type-correct; freshness boundary math clean + (negative skew tightens, never loosens). **Remediated** the one footgun it found: a sub-second + `lifetime` floored to `exp == iat` — the overload now rejects `< 1s`. +- **#26 holds** — crit rejection survives unicode-escaped/`\u`-partial/duplicate(→malformed)/empty/null/ + wrong-type variants; the only "accepted" inputs are non-normative member names (`Crit`, ` crit`, + nested) no JOSE peer honors as crit. Runs before signature verify; `ProtocolException` propagates. + Informational (not a gap): from_prior doesn't separately reject `b64`/`zip` like the envelope parser, + but those are unreachable-to-honor without `crit` (now rejected) — optional belt-and-suspenders only. + +**By-design residual (noted, not changed):** the plain `BuildAsync` still emits no `exp` by default +(spec makes `exp` optional); FR-ROT-03 sender-authentication gating caps the replay risk, and callers +bound it via the lifetime overload. diff --git a/tests/DidComm.InteropTests/Rotation/FromPriorRotationTests.cs b/tests/DidComm.InteropTests/Rotation/FromPriorRotationTests.cs index 2681ba4..9e4fe5c 100644 --- a/tests/DidComm.InteropTests/Rotation/FromPriorRotationTests.cs +++ b/tests/DidComm.InteropTests/Rotation/FromPriorRotationTests.cs @@ -271,4 +271,116 @@ public async Task DidCommClient_RejectsFromPriorOnAnoncrypt() await act.Should().ThrowAsync() .Where(e => e.Message.Contains("not authenticated")); } + + // ---- Issue #25: the builder must emit exp/nbf so the FR-ROT-05 freshness control is reachable on + // the issuing side; the no-expiry payload stays byte-identical. ---- + + [Fact] + public async Task Builder_emits_exp_and_nbf_in_lexicographic_order() + { + var claims = new FromPriorClaims(Sub: NewSenderDid, Iss: PriorDid, Iat: 1700000000, Exp: 1700003600, Nbf: 1699999900); + var jwt = await FromPriorBuilder.BuildAsync(claims, SignerPrivateJwk()); + + var payload = Encoding.UTF8.GetString(Base64Url.Decode(jwt.Split('.')[1])); + payload.Should().Be( + """{"exp":1700003600,"iat":1700000000,"iss":"did:example:alice","nbf":1699999900,"sub":"did:example:newAlice"}"""); + } + + [Fact] + public async Task Builder_without_exp_nbf_emits_only_iat_iss_sub_unchanged() + { + // Regression: the no-expiry path is byte-identical to the prior {iat,iss,sub} payload. + var jwt = await FromPriorBuilder.BuildAsync(SampleClaims(), SignerPrivateJwk()); + + var payload = Encoding.UTF8.GetString(Base64Url.Decode(jwt.Split('.')[1])); + payload.Should().Be("""{"iat":1700000000,"iss":"did:example:alice","sub":"did:example:newAlice"}"""); + } + + [Fact] + public async Task Builder_lifetime_overload_sets_exp_from_iat_plus_lifetime() + { + var claims = new FromPriorClaims(Sub: NewSenderDid, Iss: PriorDid, Iat: 1700000000); + var jwt = await FromPriorBuilder.BuildAsync(claims, SignerPrivateJwk(), lifetime: TimeSpan.FromMinutes(5)); + + var payload = JsonNode.Parse(Encoding.UTF8.GetString(Base64Url.Decode(jwt.Split('.')[1])))!.AsObject(); + ((long)payload["exp"]!).Should().Be(1700000000 + 300); + } + + [Theory] + [InlineData(0)] // zero + [InlineData(-5)] // negative + [InlineData(0.5)] // sub-second: floors to exp == iat (already-expired) → rejected (red-team) + public async Task Builder_lifetime_overload_rejects_lifetime_below_one_second(double seconds) + { + var act = async () => await FromPriorBuilder.BuildAsync(SampleClaims(), SignerPrivateJwk(), TimeSpan.FromSeconds(seconds)); + await act.Should().ThrowAsync(); + } + + [Fact] + public async Task DidCommClient_RejectsExpiredFromPrior_FrRot05() + { + // Issue #25 end-to-end: a self-issued from_prior with exp in the past is now actually rejected + // on unpack (FR-ROT-05) — previously unreachable because the builder never emitted exp. + var claims = new FromPriorClaims(Sub: "did:example:alice", Iss: PriorDid, Iat: 1700000000, Exp: 1700000100); // 2023, long past + var jwt = await FromPriorBuilder.BuildAsync(claims, SignerPrivateJwk()); + + var message = new MessageBuilder() + .WithType("http://example.com/protocols/lets_do_lunch/1.0/proposal") + .WithFrom("did:example:alice") + .WithTo("did:example:bob") + .WithFromPrior(jwt) + .WithBody(JsonNode.Parse("""{"a":"b"}""")!.AsObject()) + .Build(); + + var client = new DidCommClient(Actors.Value.AsSecretsResolver(), NewKeyService(), new DidCommOptions()); + var packed = (await client.PackEncryptedAsync(message, + new PackEncryptedOptions(Recipients: new[] { "did:example:bob" }, From: "did:example:alice"))).Message; + + var act = async () => await client.UnpackAsync(packed); + + await act.Should().ThrowAsync().Where(e => e.Message.Contains("FR-ROT-05")); + } + + [Fact] + public async Task DidCommClient_RejectsNotYetValidFromPrior_FrRot05() + { + // Issue #25 end-to-end, symmetric to the expired-Exp case: a from_prior whose nbf is in the + // future must be rejected on unpack (FR-ROT-05). The clock is pinned to iat so nbf = iat + 1 day + // is deterministically not-yet-valid (well past any skew) regardless of wall-clock time. + const long iat = 1700000000; + var pinnedNow = DateTimeOffset.FromUnixTimeSeconds(iat); + var claims = new FromPriorClaims(Sub: "did:example:alice", Iss: PriorDid, Iat: iat, Nbf: iat + 86400); + var jwt = await FromPriorBuilder.BuildAsync(claims, SignerPrivateJwk()); + + var message = new MessageBuilder() + .WithType("http://example.com/protocols/lets_do_lunch/1.0/proposal") + .WithFrom("did:example:alice") + .WithTo("did:example:bob") + .WithFromPrior(jwt) + .WithBody(JsonNode.Parse("""{"a":"b"}""")!.AsObject()) + .Build(); + + var options = new DidCommOptions { Clock = () => pinnedNow }; + var client = new DidCommClient(Actors.Value.AsSecretsResolver(), NewKeyService(), options); + var packed = (await client.PackEncryptedAsync(message, + new PackEncryptedOptions(Recipients: new[] { "did:example:bob" }, From: "did:example:alice"))).Message; + + var act = async () => await client.UnpackAsync(packed); + + await act.Should().ThrowAsync().Where(e => e.Message.Contains("not yet valid")); + } + + [Fact] + public async Task Validator_RejectsCritHeader_FrRot26() + { + // Issue #26: a from_prior whose protected header marks an extension critical must be rejected + // (RFC 7515 §4.1.11), mirroring JwsParser/JweParser. The check precedes signature verification, + // so splicing 'crit' (without re-signing) still triggers it. + var jwt = await FromPriorBuilder.BuildAsync(SampleClaims(), SignerPrivateJwk()); + var withCrit = MutateHeader(jwt, h => h["crit"] = new JsonArray("urn:example:unsupported")); + + var act = async () => await FromPriorValidator.ValidateAsync(withCrit, NewSenderDid, NewKeyService()); + + await act.Should().ThrowAsync().Where(e => e.Message.Contains("crit")); + } }