From 03b923c9e1141fc500afc802d086ca60e19f981e Mon Sep 17 00:00:00 2001 From: Moises E Jaramillo Date: Wed, 24 Jun 2026 13:16:37 -0400 Subject: [PATCH 1/2] Fix VP verification for holder-less and credential-less presentations (#11) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit VCDM 2.0 makes `holder` and `verifiableCredential` optional, but the verifier rejected a presentation omitting either — so a standard DI-signed VP from another implementation (the W3C suite's eddsa-rdfc-2022 presentations carry no `holder`) failed to verify. The earlier "VP proof reported as NoProof" diagnosis was wrong; the engine verifies a valid suite VP fine when UseRdfcSuites() is registered. The real causes: - BindHolder failed when `holder` was absent. Now a signed presentation with no holder passes the binding check on possession alone (the proof verified + challenge/domain matched; no holder identity to bind). Safe: `holder` is in the proof's signed scope, so stripping a victim's holder invalidates the proof before this check runs — locked by a new regression test (Stripping_the_holder..._breaks_the_proof). - presentation_no_credentials is already gated on RequireAtLeastOneCredential; the conformance shim now sets it false (a VP may carry no credentials). Raises the W3C VCDM 2.0 conformance baseline 36 -> 43/59 (the 4.13 VP group now passes); harness baseline and docs/conformance.md updated. Existing M6/M7 security tests unchanged + green (forgery rejection holds when a holder IS present). Build 0-warning; 359 tests + conformance harness green. Fixes #11 Co-Authored-By: Claude Opus 4.8 (1M context) --- CHANGELOG.md | 17 +++++ docs/conformance.md | 5 +- src/Credentials.Core/Roles/DefaultVerifier.cs | 9 ++- .../W3cVcdm2SuiteTests.cs | 2 +- .../Credentials.Conformance.VcApi/Program.cs | 3 + .../M6PresentationTests.cs | 69 +++++++++++++++++++ 6 files changed, 99 insertions(+), 6 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ffab119..9955fe1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,23 @@ All notable changes to `credentials-dotnet` are documented here. The format is b ## [Unreleased] +### Fixed + +- **Verifiable Presentation verification for holder-less and credential-less presentations (#11).** VCDM 2.0 + makes both `holder` and `verifiableCredential` **optional**, but the verifier rejected a presentation that + omitted either — so a standard Data-Integrity-signed VP from another implementation (e.g. the W3C suite's + `eddsa-rdfc-2022` presentations, which carry no `holder`) failed to verify. The earlier "VP authentication + proof reported as `NoProof`" diagnosis was incorrect; the real causes were two over-strict checks: + - `BindHolder` failed when `holder` was absent. Now a signed presentation with no holder passes the binding + check on **possession alone** (the binding proof verified and the challenge/domain matched; there is no + holder identity to bind). This is not abusable: `holder` is inside the proof's signed scope, so stripping + a victim's `holder` invalidates the proof before the check runs — guarded by a new regression test. + - The empty-presentation rule (`presentation_no_credentials`) is correctly gated on the existing + `RequireAtLeastOneCredential` option; the conformance shim sets it `false` (a VP may legitimately carry no + credentials). + This raises the W3C VCDM 2.0 conformance baseline **36 → 43 / 59** (the `4.13-verifiable-presentations` + group now passes). See [docs/conformance.md](docs/conformance.md). + ### Added — Milestone M8c (Conformance + interop) — the M8 finale The last of three M8 PRs: empirical W3C VCDM 2.0 conformance + cross-implementation interop, closing diff --git a/docs/conformance.md b/docs/conformance.md index 74574c9..ac0b84a 100644 --- a/docs/conformance.md +++ b/docs/conformance.md @@ -10,10 +10,10 @@ interoperability suite) is run against a thin ASP.NET shim (`tests/Credentials.C exposes `POST /credentials/issue`, `/credentials/verify`, and `/presentations/verify` over the engine's `IIssuer`/`IVerifier`. The shim issues with `eddsa-rdfc-2022`; the suite injects the shim's own `did:key` as the credential issuer, so issuance satisfies the engine's issuer-binding. `Credentials.Conformance.Tests` -boots the shim on loopback, runs the suite, and asserts a passing **baseline of 36** so the conformance +boots the shim on loopback, runs the suite, and asserts a passing **baseline of 43** so the conformance level cannot regress; the full run is `conformance.yml` (PR + nightly). -**Current result: 36 / 59 passing.** The 23 not-yet-passing tests are not silent — they are known +**Current result: 43 / 59 passing.** The 16 not-yet-passing tests are not silent — they are known limitations, grouped: | Group | Why it does not pass | Status | @@ -22,7 +22,6 @@ limitations, grouped: | `relatedResource` integrity (wrong/missing/duplicate digest, non-object form) | `relatedResource` digest verification is not implemented. | Future feature. | | `name` / `description` language-value-object validation (extra properties) | The engine does not validate the §11.1 language/direction object shape of `name`/`description`. | Future hardening. | | A few issuer/credentialSchema/credentialStatus identifier-URL negatives | The structural validator does not yet reject every non-URL identifier the suite checks. | Incremental validator hardening. | -| Verifiable Presentation verification (the suite's `eddsa-rdfc-2022` authentication proof) | The engine's VP holder-binding verification reports the suite's VP authentication proof as not-found (`NoProof`); a VP-proof interop gap distinct from the (passing) credential path. | Tracked: [#11](https://github.com/moisesja/credentials-dotnet/issues/11). | This is reported as a tracked baseline rather than a "fully conformant" claim: the engine passes the structural / issue / verify core of the suite, and the gaps above are explicit and individually diff --git a/src/Credentials.Core/Roles/DefaultVerifier.cs b/src/Credentials.Core/Roles/DefaultVerifier.cs index e0f0e3f..4b51bda 100644 --- a/src/Credentials.Core/Roles/DefaultVerifier.cs +++ b/src/Credentials.Core/Roles/DefaultVerifier.cs @@ -270,8 +270,13 @@ private static CheckResult BindHolder(VerifiablePresentation vp, SecuringVerific var holderId = vp.Holder; if (string.IsNullOrEmpty(holderId)) { - return CheckResult.Failed(CheckKinds.HolderBinding, "holder_binding_missing", - "The presentation has no holder to bind the binding proof to.", "/holder"); + // VCDM 2.0: `holder` is OPTIONAL. A signed presentation with no holder still proves possession + // of the binding key and freshness (the binding proof verified and the challenge/domain + // matched); there is simply no holder identity to bind it to. This cannot be abused to strip a + // victim's `holder`: `holder` is inside the proof's signed scope, so removing it invalidates the + // proof before this check runs (the mechanism returns Invalid, not Verified). So a holder-less + // signed presentation is bound on possession alone. + return CheckResult.Passed(CheckKinds.HolderBinding); } if (result.VerificationMethods.Count == 0 diff --git a/tests/Credentials.Conformance.Tests/W3cVcdm2SuiteTests.cs b/tests/Credentials.Conformance.Tests/W3cVcdm2SuiteTests.cs index 94970cc..d67a10c 100644 --- a/tests/Credentials.Conformance.Tests/W3cVcdm2SuiteTests.cs +++ b/tests/Credentials.Conformance.Tests/W3cVcdm2SuiteTests.cs @@ -19,7 +19,7 @@ public sealed partial class W3cVcdm2SuiteTests { // The number of suite tests the engine currently passes. Raising this when the engine improves is // expected; a drop below it is a regression and fails the gate. - private const int PassingBaseline = 36; + private const int PassingBaseline = 43; [SkippableFact] [Trait("Category", "Conformance")] diff --git a/tests/Credentials.Conformance.VcApi/Program.cs b/tests/Credentials.Conformance.VcApi/Program.cs index 41d08ef..0cc1388 100644 --- a/tests/Credentials.Conformance.VcApi/Program.cs +++ b/tests/Credentials.Conformance.VcApi/Program.cs @@ -99,6 +99,9 @@ var result = await verifier.VerifyPresentationAsync(presentation, new PresentationVerificationOptions { RequireHolderBinding = false, + // VCDM 2.0 makes `verifiableCredential` optional, and the suite submits credential-less VPs — + // so don't treat an empty presentation as a structure failure here. + RequireAtLeastOneCredential = false, ExpectedChallenge = challenge, ExpectedDomain = domain, }); diff --git a/tests/Credentials.Extensions.DependencyInjection.Tests/M6PresentationTests.cs b/tests/Credentials.Extensions.DependencyInjection.Tests/M6PresentationTests.cs index 1338d95..18dca0b 100644 --- a/tests/Credentials.Extensions.DependencyInjection.Tests/M6PresentationTests.cs +++ b/tests/Credentials.Extensions.DependencyInjection.Tests/M6PresentationTests.cs @@ -90,6 +90,75 @@ public async Task Data_integrity_bound_presentation_round_trips() fromBytes.Decision.Should().Be(VerificationDecision.Accepted, fromBytes.ToString()); } + [Fact] + [FrTag("FR-041")] + public async Task Holder_less_signed_presentation_verifies_on_possession_alone() + { + // VCDM 2.0: `holder` is OPTIONAL. A presentation signed with no holder still proves possession of + // the binding key and freshness, so its binding check passes (W3C conformance; issue #11). + using var provider = BuildProvider(); + var issuer = provider.GetRequiredService(); + var holder = provider.GetRequiredService(); + var verifier = provider.GetRequiredService(); + + var (held, holderKey) = await HoldACredentialAsync(issuer, holder); + // Drop the holder before signing, then bind — the proof now covers a holder-less presentation. + var noHolder = JsonNode.Parse(BuildVp(holder, held, holderKey.Did).ToBytes())!.AsObject(); + noHolder.Remove("holder"); + var bound = await holder.BindWithDataIntegrityAsync( + VerifiablePresentation.Parse(noHolder.ToJsonString()), + new VpBindingRequest + { + HolderSigner = holderKey.Signer, + VerificationMethod = holderKey.VerificationMethod, + Challenge = Challenge, + Domain = Domain, + }); + + var result = await verifier.VerifyPresentationAsync( + bound, new PresentationVerificationOptions { ExpectedChallenge = Challenge, ExpectedDomain = Domain }); + + result.Check(CheckKinds.HolderBinding)!.Status.Should().Be(CheckStatus.Passed, result.ToString()); + result.Decision.Should().Be(VerificationDecision.Accepted, result.ToString()); + } + + [Fact] + [FrTag("FR-041")] + public async Task Stripping_the_holder_from_a_bound_presentation_breaks_the_proof() + { + // The holder-less Passed path must be unreachable by tampering: `holder` is in the proof's signed + // scope, so removing a victim's holder from a holder-BOUND presentation invalidates the proof — the + // binding fails as a proof failure, NOT the holder-less shortcut. + using var provider = BuildProvider(); + var issuer = provider.GetRequiredService(); + var holder = provider.GetRequiredService(); + var verifier = provider.GetRequiredService(); + + var (held, holderKey) = await HoldACredentialAsync(issuer, holder); + var bound = await holder.BindWithDataIntegrityAsync( + BuildVp(holder, held, holderKey.Did), + new VpBindingRequest + { + HolderSigner = holderKey.Signer, + VerificationMethod = holderKey.VerificationMethod, + Challenge = Challenge, + Domain = Domain, + }); + + var stripped = JsonNode.Parse(bound.ToBytes())!.AsObject(); + stripped.Remove("holder"); + + var result = await verifier.VerifyPresentationAsync( + VerifiablePresentation.Parse(stripped.ToJsonString()), + new PresentationVerificationOptions { ExpectedChallenge = Challenge, ExpectedDomain = Domain }); + + result.Decision.Should().Be(VerificationDecision.Rejected); + var binding = result.Check(CheckKinds.HolderBinding)!; + binding.Status.Should().Be(CheckStatus.Failed); + binding.Diagnostics.Should().NotContain(d => d.Code == "holder_binding_missing", + "the failure must be the broken proof, not the holder-less shortcut"); + } + [Fact] [FrTag("FR-033")] public async Task Jose_vp_jwt_bound_presentation_round_trips() From ed6cf6a49e71786677453f2cc1ace0793be8f712 Mon Sep 17 00:00:00 2001 From: Moises E Jaramillo Date: Wed, 24 Jun 2026 13:31:59 -0400 Subject: [PATCH 2/2] PR #12 review: document holder-binding scope, add a real runtime backstop MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses the (revised) PR #12 review: - BindHolder gains a runtime guard (fail-closed Indeterminate if the status isn't Verified) — the "real backstop" the reviewer endorsed, vs the withdrawn Debug.Assert (which compiles out of Release). The holder-less Passed path is now self-enforcing, not convention-only. - Document the SCOPE of holder binding on PresentationVerificationOptions.RequireHolderBinding: a passing binding proves possession + freshness, NOT that the presenter is the credential subject (no holder<-> subject linkage; pre-existing). New test Holder_binding_proves_possession_not_that_the_presenter_is_the_subject demonstrates a non-subject presenter -> Accepted, so downstream integrators don't read Accepted as "presented by the subject". - CHANGELOG: state explicitly that the holder-less acceptance is an engine-DEFAULT change (all callers, still replay-safe via RequireHolderBinding default true), while RequireAtLeastOneCredential stays true by default and is only relaxed in the conformance shim. Build 0-warning; 360 tests green (+1 semantics test). Co-Authored-By: Claude Opus 4.8 (1M context) --- CHANGELOG.md | 14 ++++-- src/Credentials.Core/Roles/DefaultVerifier.cs | 10 ++++ .../PresentationVerificationOptions.cs | 11 +++++ .../M6PresentationTests.cs | 49 +++++++++++++++++++ 4 files changed, 79 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9955fe1..f166c5a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,11 +15,15 @@ All notable changes to `credentials-dotnet` are documented here. The format is b proof reported as `NoProof`" diagnosis was incorrect; the real causes were two over-strict checks: - `BindHolder` failed when `holder` was absent. Now a signed presentation with no holder passes the binding check on **possession alone** (the binding proof verified and the challenge/domain matched; there is no - holder identity to bind). This is not abusable: `holder` is inside the proof's signed scope, so stripping - a victim's `holder` invalidates the proof before the check runs — guarded by a new regression test. - - The empty-presentation rule (`presentation_no_credentials`) is correctly gated on the existing - `RequireAtLeastOneCredential` option; the conformance shim sets it `false` (a VP may legitimately carry no - credentials). + holder identity to bind). This is an **engine-default behaviour change** — it applies to all callers under + default options (still replay-safe: `RequireHolderBinding` defaults `true`, forcing a challenge), and is the + intended interop fix. It is not abusable: `holder` is inside the proof's signed scope, so stripping a + victim's `holder` invalidates the proof before the check runs (guarded by a new regression test). The scope + of holder binding (possession + freshness, **not** that the presenter is the credential subject) is now + documented on `PresentationVerificationOptions.RequireHolderBinding` and covered by a test. + - The empty-presentation rule (`presentation_no_credentials`) is, by contrast, gated on the existing + `RequireAtLeastOneCredential` option, which is **unchanged at its `true` default**; only the conformance + shim sets it `false` (a VP may legitimately carry no credentials). This raises the W3C VCDM 2.0 conformance baseline **36 → 43 / 59** (the `4.13-verifiable-presentations` group now passes). See [docs/conformance.md](docs/conformance.md). diff --git a/src/Credentials.Core/Roles/DefaultVerifier.cs b/src/Credentials.Core/Roles/DefaultVerifier.cs index 4b51bda..5a0507d 100644 --- a/src/Credentials.Core/Roles/DefaultVerifier.cs +++ b/src/Credentials.Core/Roles/DefaultVerifier.cs @@ -267,6 +267,16 @@ private static CheckResult CheckPresentationFreshness(VerifiablePresentation vp, // presentation's `holder` — to bind a presentation as a victim holder an attacker needs the victim's key. private static CheckResult BindHolder(VerifiablePresentation vp, SecuringVerificationResult result) { + // Defence in depth: the holder-less Passed path below is sound only because the binding proof has + // already verified. Today BindHolder is reachable only from the Verified switch arm; this runtime + // guard (a real backstop — unlike a Debug.Assert, which compiles out of Release) keeps a future call + // path that passed a non-verified result from reaching the holder-less shortcut. It fails closed. + if (result.Status != SecuringVerificationStatus.Verified) + { + return CheckResult.Indeterminate(CheckKinds.HolderBinding, "binding_not_verified", + "The holder binding could not be confirmed."); + } + var holderId = vp.Holder; if (string.IsNullOrEmpty(holderId)) { diff --git a/src/Credentials.Core/Verification/PresentationVerificationOptions.cs b/src/Credentials.Core/Verification/PresentationVerificationOptions.cs index d4c6e47..8c99838 100644 --- a/src/Credentials.Core/Verification/PresentationVerificationOptions.cs +++ b/src/Credentials.Core/Verification/PresentationVerificationOptions.cs @@ -27,6 +27,17 @@ public sealed record PresentationVerificationOptions /// not carry. Per-contained-credential holder binding is additionally enabled whenever /// is supplied (see the verifier's contained-options derivation), so a /// verifier that names itself still enforces each child's KB-JWT without changing the credential-level default. + /// + /// SCOPE OF HOLDER BINDING. A passing holder-binding check proves only possession of the binding key + /// and freshness (the presentation's authentication proof verified and its challenge/domain matched + /// the verifier's). It deliberately does not prove that the presenter is the + /// credentialSubject of the contained credentials — the engine performs no holder↔subject linkage. + /// A party who obtains a credential can present it in a presentation signed with their own key and + /// the presentation composes to . Binding the presenter to the + /// credential subject (and any holder-identity policy) is the verifying application's responsibility; do not + /// read as "presented by the subject". (Per VCDM 2.0, holder + /// itself is optional: a signed presentation with no holder still passes binding on possession alone.) + /// /// public bool RequireHolderBinding { get; init; } = true; diff --git a/tests/Credentials.Extensions.DependencyInjection.Tests/M6PresentationTests.cs b/tests/Credentials.Extensions.DependencyInjection.Tests/M6PresentationTests.cs index 18dca0b..6f7f682 100644 --- a/tests/Credentials.Extensions.DependencyInjection.Tests/M6PresentationTests.cs +++ b/tests/Credentials.Extensions.DependencyInjection.Tests/M6PresentationTests.cs @@ -159,6 +159,55 @@ public async Task Stripping_the_holder_from_a_bound_presentation_breaks_the_proo "the failure must be the broken proof, not the holder-less shortcut"); } + [Fact] + [FrTag("FR-041")] + public async Task Holder_binding_proves_possession_not_that_the_presenter_is_the_subject() + { + // Documents the holder-binding scope (see PresentationVerificationOptions.RequireHolderBinding): a + // passing binding proves possession + freshness, NOT that the presenter is the credential subject. + // A party who holds a credential can present it in a VP signed with their OWN key and it Accepts; + // binding the presenter to credentialSubject.id is the verifying application's policy, not the engine's. + using var provider = BuildProvider(); + var issuer = provider.GetRequiredService(); + var holder = provider.GetRequiredService(); + var verifier = provider.GetRequiredService(); + + // A credential whose subject is someone OTHER than the presenter. + var issuerKey = TestKeys.New(KeyType.Ed25519); + const string subjectDid = "did:example:the-actual-subject"; + var credential = Credential.Build() + .WithIssuer(issuerKey.Did) + .AddSubject(new JsonObject { ["id"] = subjectDid, ["alumniOf"] = "Example University" }) + .Seal(); + var issued = await issuer.IssueAsync(credential, new DataIntegrityIssuanceRequest + { + Cryptosuite = "eddsa-jcs-2022", + Signer = issuerKey.Signer, + VerificationMethod = issuerKey.VerificationMethod, + }); + + // A DIFFERENT party (the presenter) wraps it in a presentation signed with THEIR key. + var presenterKey = TestKeys.New(KeyType.Ed25519); + presenterKey.Did.Should().NotBe(subjectDid); + var bound = await holder.BindWithDataIntegrityAsync( + BuildVp(holder, holder.Ingest(issued.Credential.ToBytes()), presenterKey.Did), + new VpBindingRequest + { + HolderSigner = presenterKey.Signer, + VerificationMethod = presenterKey.VerificationMethod, + Challenge = Challenge, + Domain = Domain, + }); + + var result = await verifier.VerifyPresentationAsync( + bound, new PresentationVerificationOptions { ExpectedChallenge = Challenge, ExpectedDomain = Domain }); + + // Possession + freshness verify even though presenter != subject; the engine does not link them. + result.Decision.Should().Be(VerificationDecision.Accepted, result.ToString()); + result.Check(CheckKinds.HolderBinding)!.Status.Should().Be(CheckStatus.Passed); + result.Credentials[0].Decision.Should().Be(VerificationDecision.Accepted); + } + [Fact] [FrTag("FR-033")] public async Task Jose_vp_jwt_bound_presentation_round_trips()