fix(security): remediate Medium-severity audit cluster (#17–#21)

AGENTS.md

@@ -33,6 +33,7 @@ Maintenance rules:
 
 - Use subagents liberally to keep main context window clean
 - Offload research, exploration, and parallel analysis to subagents
+- Always use adversarial agents to attempt to exploit the code that is being generated. The adversarial agents must report in detail about any findings
 - For complex problems, throw more compute at it via subagents
 - One task per subagent for focused execution
 

CHANGELOG.md

@@ -6,6 +6,58 @@ All notable changes to didcomm-dotnet are documented here. Format follows
 
 ## [Unreleased]
 
+### Security — Medium-severity audit remediation (`feat/security-medium-cluster`)
+
+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
+ships with negative and regression tests, and each was then put through an adversarial red-team pass
+which surfaced two further hardening items (folded in below) and two deeper residuals (filed as
+follow-ups). The full suite (600 tests) is green.
+
+- **Illegal envelope compositions rejected (#17, FR-ENV-04a).** `EnvelopeReader.Unpack` now tracks
+  each unwrapped layer with its authenticated/anonymous flag and enforces the legal DIDComm v2.1
+  receive grammar `anoncrypt? authcrypt? sign? plaintext` before any content/consistency processing.
+  Illegal nestings — sign-outside-encrypt (FR-ENV-05), `anoncrypt(anoncrypt)`, `authcrypt(authcrypt)`,
+  `authcrypt(anoncrypt)`, double-sign — are rejected with `MalformedMessageException`. The grammar
+  admits every spec Appendix C unpack vector, including `authcrypt(sign)` (FR-ENV-03) and
+  `anoncrypt(authcrypt(sign))` (Appendix C.3). The auth/anon flag is what distinguishes the legal
+  `anoncrypt(authcrypt)` from the illegal `anoncrypt(anoncrypt)` (indistinguishable at envelope-kind
+  level). PRD gains **FR-ENV-04a** documenting the receive-acceptance set.
+- **Controller-aware resolver authorization (#18, FR-CONSIST-06 / DD-01).** `NetDidKeyService`
+  `IsKeyAuthorizedAsync` now walks the raw resolved verification methods (not the curve-projected
+  JWKs, which dropped `controller`) and authorizes a `kid` only when its id-subject **and** its
+  `controller` resolve to the asserted DID — rejecting a key listed under the DID's relationship but
+  controlled by a different DID, and cross-DID embedded ids. did:key / did:peer keep authorizing
+  (controller == subject by construction).
+- **`from_prior` JWT parse errors normalized (#19).** `FromPriorValidator` wraps base64url decode and
+  all JSON reads in one boundary that surfaces malformed input (`FormatException`,
+  `InvalidOperationException`, `ArgumentException` on an empty segment, `JsonException`,
+  `KeyNotFoundException`) as the typed `ProtocolException` with a generic message — closing an
+  unhandled-exception path out of `UnpackAsync` and removing the header-vs-claims info leak.
+- **Receive endpoint response oracle closed (#20, #28, #33).** Both ASP.NET Core HTTP receive overloads
+  collapse every input- or handler-triggered failure to a single opaque `400` with an empty body,
+  logging the reason server-side only. This closes the verbatim-`CryptoException` decryption /
+  recipient-`kid` enumeration oracle (#20), the `Consistency`/`Resolution`/`UnsupportedDidMethod`
+  exceptions that previously escaped to `500` + dev stack-trace (#28), and the echoed handler-bug
+  detail (#33). The 202 success, 415, and 413 paths are unchanged; cancellation still propagates.
+  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**
+  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.
+  **Operator note:** a receiver-side handler bug (FR-THR-04 `InvalidOperationException`) now returns
+  `400` instead of `500` for response uniformity — it is logged at `Error` level, so alert on the
+  `DidComm.AspNetCore` error log rather than on a 5xx rate for handler faults.
+- **Thread-state store bounded (#21).** `InMemoryThreadStateStore` is now capped
+  (`DefaultMaxEntries = 10_000`, overridable via ctor) with approximate-LRU eviction, converting an
+  unbounded-growth memory-exhaustion DoS (attacker-chosen, unauthenticated `thid`/`pthid`) into a
+  few-MB bound. Secure-by-default — no DI wiring change. Red-team hardening: eviction is **single-flight**
+  (an `Interlocked` guard) so N concurrent inserters over the cap can't each run a full O(n log n)
+  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`)
 
 didcomm-dotnet no longer carries its own cryptography or JWE/JWS assembly. The entire `Crypto/`

docs/didcomm-dotnet_PRD.md

@@ -265,6 +265,7 @@ public sealed class DidComm {
 | FR-ENV-02 | MUST | Support exactly these legal envelope compositions for a single hop: `plaintext`, `signed(plaintext)`, `anoncrypt(plaintext)`, `authcrypt(plaintext)`, `anoncrypt(sign(plaintext))`, `anoncrypt(authcrypt(plaintext))`. | Each composition packs and unpacks. |
 | FR-ENV-03 | SHOULD NOT | Do not emit `authcrypt(sign(plaintext))`; MAY accept it on receive (subject to FR-CONSIST-05). | Library never produces this; can ingest it. |
 | FR-ENV-04 | MUST NOT | Do not emit any single-hop composition other than those in FR-ENV-02 (e.g. never `anoncrypt(authcrypt(sign))`). | No disallowed nesting produced. |
+| FR-ENV-04a | MUST | On **receive**, accept exactly the compositions matching the grammar `anoncrypt? authcrypt? sign? plaintext` (at most one anoncrypt outermost, then at most one authcrypt, then at most one signature innermost, then the plaintext). This is a superset of the FR-ENV-02 emit set that additionally admits the receive-only `authcrypt(sign(plaintext))` (FR-ENV-03) and the protect-sender-plus-sign `anoncrypt(authcrypt(sign(plaintext)))` (spec Appendix C.3). Reject any other layering — sign-outside-encrypt (FR-ENV-05), `anoncrypt(anoncrypt)`, `authcrypt(authcrypt)`, `authcrypt(anoncrypt)`, more than one signature — as malformed. | Every spec Appendix C unpack vector succeeds; illegal layer orderings are rejected with `MalformedMessageException` before any content/consistency processing. |
 | FR-ENV-05 | MUST | When signing+encrypting, sign first, then encrypt (nested JWM). | Sign-then-encrypt order enforced. |
 | FR-ENV-06 | MUST | Encrypted form is a JWE in **General JSON Serialization** (multi-recipient). | Output contains `protected`/`recipients`/`iv`/`ciphertext`/`tag`. |
 | FR-ENV-07 | SHOULD | Set envelope `typ` in the JWE/JWS protected header. | `typ` present in protected header. |

src/DidComm.AspNetCore/DidCommEndpointRouteBuilderExtensions.cs

@@ -27,9 +27,12 @@ public static class DidCommEndpointRouteBuilderExtensions
     /// <summary>
     /// Map an HTTP POST endpoint that accepts DIDComm envelopes, unpacks them via
     /// <see cref="DidCommClient.UnpackAsync"/>, and dispatches to <paramref name="onReceive"/>
-    /// (FR-TRN-07). Returns <c>202 Accepted</c> on success. Returns <c>415</c> for an unknown
-    /// content type, <c>413</c> when the body exceeds <see cref="DidCommOptions.MaxReceiveBytes"/>
-    /// (FR-API-06), <c>400</c> for malformed envelopes, and <c>500</c> for unexpected errors.
+    /// (FR-TRN-07). Returns <c>202 Accepted</c> on success, <c>415</c> for an unknown content type,
+    /// and <c>413</c> when the body exceeds <see cref="DidCommOptions.MaxReceiveBytes"/> (FR-API-06).
+    /// ANY envelope/processing failure — crypto, malformed structure, consistency, or DID resolution —
+    /// returns a uniform <c>400 Bad Request</c> with an empty body and the reason logged server-side
+    /// only, so the response cannot be used as a decryption / recipient-kid / resolution oracle
+    /// (FR-API-07, issues #20/#28/#33).
     /// </summary>
     /// <param name="endpoints">The ASP.NET Core endpoint route builder.</param>
     /// <param name="pattern">URL pattern (e.g. <c>"/didcomm"</c>).</param>
@@ -78,21 +81,46 @@ public static IEndpointConventionBuilder MapDidCommEndpoint(
                 return Results.StatusCode(StatusCodes.Status413PayloadTooLarge);
             }
 
+            var logger = httpContext.RequestServices.GetService<ILoggerFactory>()?.CreateLogger("DidComm.AspNetCore.Receive");
+
             UnpackResult unpacked;
             try
             {
                 unpacked = await client.UnpackAsync(body, httpContext.RequestAborted).ConfigureAwait(false);
             }
-            catch (MalformedMessageException ex)
+            catch (OperationCanceledException) when (httpContext.RequestAborted.IsCancellationRequested)
             {
-                return Results.Problem(detail: ex.Message, statusCode: StatusCodes.Status400BadRequest);
+                throw; // genuine client abort — propagate, do not mask as a rejection
             }
-            catch (CryptoException ex)
+            catch (Exception ex)
             {
-                return Results.Problem(detail: ex.Message, statusCode: StatusCodes.Status400BadRequest);
+                // Every unpack failure collapses to one opaque 400 so the peer can't distinguish them:
+                // crypto / malformed / consistency / resolution / did:web / missing-secret, AND a
+                // downstream DID-resolution timeout (TaskCanceledException : OperationCanceledException
+                // with the request token NOT cancelled, e.g. an attacker-controlled did:webvh host that
+                // hangs). Only a real client abort is rethrown above, so there is no 400-vs-500 oracle.
+                return NormalizedReceiveRejection(logger, ex);
+            }
+
+            // The message was received and unpacked; HTTP receive is one-way / fire-and-forget
+            // (FR-TRN-10). A fault in the consumer callback is the receiver's own concern — log it but
+            // still answer 202, so a throwing callback can't turn a successfully-unpacked message into a
+            // distinguishable 500. Otherwise an attacker could probe "did this envelope decrypt for us?"
+            // by inducing a callback throw — re-exposing the unpack-success oracle the uniform-400
+            // rejection closes (#20 red-team).
+            try
+            {
+                await onReceive(unpacked, httpContext.RequestAborted).ConfigureAwait(false);
+            }
+            catch (OperationCanceledException) when (httpContext.RequestAborted.IsCancellationRequested)
+            {
+                throw; // genuine client abort
+            }
+            catch (Exception ex)
+            {
+                logger?.LogError(ex, "DidComm receive: onReceive callback threw after a successful unpack; acknowledging anyway (one-way receive).");
             }
 
-            await onReceive(unpacked, httpContext.RequestAborted).ConfigureAwait(false);
             return Results.StatusCode(StatusCodes.Status202Accepted);
         });
     }
@@ -142,42 +170,26 @@ public static IEndpointConventionBuilder MapDidCommEndpoint(
                 return Results.StatusCode(StatusCodes.Status413PayloadTooLarge);
             }
 
-            UnpackResult unpacked;
-            try
-            {
-                unpacked = await client.UnpackAsync(body, httpContext.RequestAborted).ConfigureAwait(false);
-            }
-            catch (MalformedMessageException ex)
-            {
-                return Results.Problem(detail: ex.Message, statusCode: StatusCodes.Status400BadRequest);
-            }
-            catch (CryptoException ex)
-            {
-                return Results.Problem(detail: ex.Message, statusCode: StatusCodes.Status400BadRequest);
-            }
-
-            DispatchOutcome outcome;
+            // Unpack AND dispatch share one opaque exit: a handler bug (FR-THR-04 InvalidOperationException),
+            // an unpack rejection (crypto/malformed/consistency/resolution), and a downstream
+            // DID-resolution timeout (TaskCanceledException with the request token NOT cancelled) all
+            // return the same 400 with no body, so the status code itself is not an oracle (no
+            // 400-vs-500 partition). Only a genuine client abort is rethrown.
             try
             {
-                outcome = await dispatcher.DispatchAsync(unpacked, client, coreOptions, httpContext.RequestAborted).ConfigureAwait(false);
+                var unpacked = await client.UnpackAsync(body, httpContext.RequestAborted).ConfigureAwait(false);
+                var outcome = await dispatcher.DispatchAsync(unpacked, client, coreOptions, httpContext.RequestAborted).ConfigureAwait(false);
+                LogOutcome(logger, outcome, sameSocketDelivered: false);
+                return Results.StatusCode(StatusCodes.Status202Accepted);
             }
-            catch (InvalidOperationException ex)
+            catch (OperationCanceledException) when (httpContext.RequestAborted.IsCancellationRequested)
             {
-                // FR-THR-04 rule 2 violation by a handler (a bug): surface as 500 with a clear
-                // detail rather than letting it bubble out as an opaque unhandled exception.
-                logger?.LogWarning(ex, "MapDidCommEndpoint: handler bug (FR-THR-04 rule 2); responding 500.");
-                return Results.Problem(detail: ex.Message, statusCode: StatusCodes.Status500InternalServerError);
+                throw; // genuine client abort — propagate
             }
-            catch (Exception ex) when (ex is not OperationCanceledException)
+            catch (Exception ex)
             {
-                // Handlers are application code; an unexpected throw is a server error, not a
-                // protocol error. Map to 500 so the inbound peer learns we couldn't process it
-                // without leaking the inner exception type.
-                logger?.LogError(ex, "MapDidCommEndpoint: handler threw an unhandled exception; responding 500.");
-                return Results.Problem(detail: "Handler failed; see server logs.", statusCode: StatusCodes.Status500InternalServerError);
+                return NormalizedReceiveRejection(logger, ex);
             }
-            LogOutcome(logger, outcome, sameSocketDelivered: false);
-            return Results.StatusCode(StatusCodes.Status202Accepted);
         });
     }
 
@@ -494,6 +506,30 @@ private static void LogOutcome(ILogger? logger, DispatchOutcome outcome, bool sa
         }
     }
 
+    /// <summary>
+    /// FR-API-07 / FR-TRN-10: collapse every input- or handler-triggered failure on the HTTP receive
+    /// path to one opaque rejection — a bare <c>400</c> with no body — so a remote peer cannot use the
+    /// status code or response text as a decryption / recipient-kid / DID-resolution oracle (issues
+    /// #20/#28/#33). The detailed reason is logged server-side only. The WebSocket paths already take
+    /// this posture (log-and-discard, echo nothing); this brings the HTTP paths to parity.
+    /// </summary>
+    private static IResult NormalizedReceiveRejection(ILogger? logger, Exception ex)
+    {
+        if (ex is DidCommException)
+        {
+            // Expected, peer-triggered category (malformed / crypto / consistency / resolution /
+            // unsupported-method / secret-missing) — it's the peer's envelope, not our bug.
+            logger?.LogWarning(ex, "DidComm receive: rejecting inbound envelope ({Reason}).", ex.GetType().Name);
+        }
+        else
+        {
+            // A handler bug (FR-THR-04 InvalidOperationException) or any other unexpected throw.
+            logger?.LogError(ex, "DidComm receive: inbound processing failed ({Reason}).", ex.GetType().Name);
+        }
+
+        return Results.StatusCode(StatusCodes.Status400BadRequest);
+    }
+
     private static async Task ReceiveLoopAsync(
         System.Net.WebSockets.WebSocket socket,
         DidCommClient client,