Skip to content

Defense in depth: authcrypt unpack should assert non-empty skid (parity with the JWS branch) #52

Description

@moisesja

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 fromskid 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 fromskid 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.

Metadata

Metadata

Assignees

Labels

No labels
No labels

Projects

No projects

Milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions