Fix VP verification for holder-less and credential-less presentations (#11)

CHANGELOG.md

@@ -6,6 +6,27 @@ 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 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).
+
 ### Added — Milestone M8c (Conformance + interop) — the M8 finale
 
 The last of three M8 PRs: empirical W3C VCDM 2.0 conformance + cross-implementation interop, closing

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

src/Credentials.Core/Roles/DefaultVerifier.cs

@@ -267,11 +267,26 @@ 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))
         {
-            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

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
     /// <see cref="ExpectedAudience"/> 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.
+    /// <para>
+    /// SCOPE OF HOLDER BINDING. A passing holder-binding check proves only <em>possession of the binding key</em>
+    /// and <em>freshness</em> (the presentation's authentication proof verified and its challenge/domain matched
+    /// the verifier's). It deliberately does <strong>not</strong> prove that the presenter is the
+    /// <c>credentialSubject</c> 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 <em>own</em> key and
+    /// the presentation composes to <see cref="VerificationDecision.Accepted"/>. Binding the presenter to the
+    /// credential subject (and any holder-identity policy) is the verifying application's responsibility; do not
+    /// read <see cref="VerificationDecision.Accepted"/> as "presented by the subject". (Per VCDM 2.0, <c>holder</c>
+    /// itself is optional: a signed presentation with no <c>holder</c> still passes binding on possession alone.)
+    /// </para>
     /// </remarks>
     public bool RequireHolderBinding { get; init; } = true;
 

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")]

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,
         });

tests/Credentials.Extensions.DependencyInjection.Tests/M6PresentationTests.cs

@@ -90,6 +90,124 @@ 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<IIssuer>();
+        var holder = provider.GetRequiredService<IHolder>();
+        var verifier = provider.GetRequiredService<IVerifier>();
+
+        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<IIssuer>();
+        var holder = provider.GetRequiredService<IHolder>();
+        var verifier = provider.GetRequiredService<IVerifier>();
+
+        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-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<IIssuer>();
+        var holder = provider.GetRequiredService<IHolder>();
+        var verifier = provider.GetRequiredService<IVerifier>();
+
+        // 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()