fix(security): rotation JWT compliance (#25, #26)

.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"
+    ]
+  }
+}

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

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 |

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,19 +29,47 @@ public static async Task<string> 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.
         return await DpSig.JwsBuilder
             .BuildCompactAsync(Encoding.UTF8.GetBytes(claimsJson), signer, typ: "JWT", detachedPayload: false, ct)
             .ConfigureAwait(false);
     }
+
+    /// <summary>
+    /// Build a freshness-bounded from_prior JWT: sets <c>exp = <paramref name="claims"/>.Iat +
+    /// <paramref name="lifetime"/></c> (overriding any <see cref="FromPriorClaims.Exp"/>) 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).
+    /// </summary>
+    /// <param name="claims">Sub / Iss / Iat (and optional Nbf); Exp is computed from <paramref name="lifetime"/>.</param>
+    /// <param name="signerPrivateJwk">Private JWK; <c>Kid</c> MUST identify a key authorized under <paramref name="claims"/>.Iss <c>authentication</c>.</param>
+    /// <param name="lifetime">Positive validity window added to <c>Iat</c> to form <c>exp</c>.</param>
+    /// <param name="ct">Cancellation token.</param>
+    /// <exception cref="ArgumentOutOfRangeException"><paramref name="lifetime"/> is not positive.</exception>
+    public static Task<string> 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);
+    }
 }

src/DidComm.Core/Protocols/Rotation/FromPriorValidator.cs

@@ -74,6 +74,13 @@ internal static async Task<FromPriorClaims> 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.");