Summary
Surfaced by the adversarial review during #49/#50 (PR #51). Pre-existing, not introduced by that PR; not exploitable today. Filing so the defense-in-depth gap is tracked.
The FR-CONSIST-01/03 from↔skid binding — what makes an authenticated envelope's from trustworthy — runs in EnvelopeReader only when senderKid != null (src/DidComm.Core/.../EnvelopeReader.cs:126). That non-null is guaranteed solely by DataProofs' IsAuthenticated ⟺ non-empty skid contract (JweParser.cs), and the authcrypt branch does not defensively assert it.
Contrast the JWS branch, which explicitly fail-closes on an empty signer kid (EnvelopeReader.cs:216-218). The authcrypt branch has no equivalent guard.
Why it matters
If that upstream DataProofs invariant ever regressed (an IsAuthenticated == true result with a null/empty skid), the from↔skid consistency check would be skipped, letting message.From be attacker-set on an envelope the rest of the stack treats as authenticated. Every downstream consumer that trusts Authenticated + From would then over-trust it — including the new DiscoverFeaturesClient correlation (which has since added its own belt-and-suspenders key-id gate, but the envelope layer is the right place to enforce this once for everyone).
Proposed fix
In the authcrypt unpack branch, assert a non-empty skid whenever the envelope is treated as authenticated (mirroring the JWS branch's empty-signer-kid guard), and reject with a ConsistencyException otherwise. Small, contained, defense-in-depth; affects all authcrypt envelopes, not just disclose.
Scope
Independent of #49/#50 — this is envelope-layer hardening, deliberately kept out of PR #51.
Summary
Surfaced by the adversarial review during #49/#50 (PR #51). Pre-existing, not introduced by that PR; not exploitable today. Filing so the defense-in-depth gap is tracked.
The FR-CONSIST-01/03
from↔skidbinding — what makes an authenticated envelope'sfromtrustworthy — runs inEnvelopeReaderonly whensenderKid != null(src/DidComm.Core/.../EnvelopeReader.cs:126). That non-null is guaranteed solely by DataProofs'IsAuthenticated ⟺ non-empty skidcontract (JweParser.cs), and the authcrypt branch does not defensively assert it.Contrast the JWS branch, which explicitly fail-closes on an empty signer
kid(EnvelopeReader.cs:216-218). The authcrypt branch has no equivalent guard.Why it matters
If that upstream DataProofs invariant ever regressed (an
IsAuthenticated == trueresult with a null/emptyskid), thefrom↔skidconsistency check would be skipped, lettingmessage.Frombe attacker-set on an envelope the rest of the stack treats as authenticated. Every downstream consumer that trustsAuthenticated + Fromwould then over-trust it — including the newDiscoverFeaturesClientcorrelation (which has since added its own belt-and-suspenders key-id gate, but the envelope layer is the right place to enforce this once for everyone).Proposed fix
In the authcrypt unpack branch, assert a non-empty
skidwhenever the envelope is treated as authenticated (mirroring the JWS branch's empty-signer-kid guard), and reject with aConsistencyExceptionotherwise. Small, contained, defense-in-depth; affects all authcrypt envelopes, not justdisclose.Scope
Independent of #49/#50 — this is envelope-layer hardening, deliberately kept out of PR #51.