Skip to content

security: a credential's proof must be made by a key of its issuer - #36

Merged
stormer78 merged 1 commit into
mainfrom
security/issuer-binding
Oct 21, 2026
Merged

stormer78 merged 1 commit into
mainfrom
security/issuer-binding

Conversation

@stormer78

Copy link
Copy Markdown
Contributor

Defect

DTGCredential::verify_proof_with_public_key verified the Data Integrity proof under the key the caller passed, but never checked that the proof's verificationMethod belongs to the credential's issuer. A caller that resolves the method the proof names and then trusts issuer() accepted a credential naming any issuer, signed by any key. verify_grant_with_public_key already made this check; the credential-level entry point did not.

Fix

One private helper, check_proof_from_issuer, now runs after the signature in both public verification entry points:

Entry point Before After
DTGCredential::verify_proof_with_public_key no binding binding
verify_grant_with_public_key binding (inline) same binding, shared helper

authority::verify_chain and delegation::verify_chain check no signatures themselves (by design, documented); they rely on each link having gone through verify_proof_with_public_key, so they gain the binding through it. No other public function verifies a proof.

A method belongs to the issuer when:

  • the DID before its # is exactly the issuer (did:x:a#key-1), or
  • it is the issuer itself, with no fragment, or
  • it is a bare #fragment, read against the issuer (previously refused by the grant path).

Comparison is verbatim (a DID URL with a query or path before the fragment is not the issuer); an empty issuer matches nothing. On the grant path the issuer may be a string or an object with an id.

Compatibility

  • Not API-breaking. DTGCredentialError::ProofNotFromIssuer already existed and the enum is #[non_exhaustive]. No version bump here; CHANGELOG entry under [Unreleased].
  • Behaviour change: a credential signed with a key whose id is not a method of the issuer no longer verifies. Two unit tests signed with randomly-named keys and were given issuer-bound key ids.

Tests

tests/issuer_binding.rs, each case against both entry points: another DID refused; a DID that merely extends the issuer's refused; a DID URL with a query refused; a method of the issuer accepted; the issuer itself as the method accepted; a relative #fragment accepted; object-form issuer bound by its id (accept and refuse).

Locally: cargo fmt --check, cargo clippy --all-targets --all-features -D warnings, cargo test --all-features, cargo test --no-default-features, cargo check on 1.95.0, cargo package, and both examples run.

`DTGCredential::verify_proof_with_public_key` verified the Data Integrity
proof under the key it was handed, but never checked that the proof's
`verificationMethod` belongs to the credential's `issuer`. A caller that
resolves the method the proof names and then trusts `issuer()` therefore
accepted a credential naming any issuer, signed by any key. Its sibling
`verify_grant_with_public_key` already made this check.

Both public verification entry points now run one shared check after the
signature and return `DTGCredentialError::ProofNotFromIssuer` on a
mismatch. A method belongs to the issuer when the DID before its `#` is
exactly the issuer, when it is the issuer itself, or when it is a bare
`#fragment`, read against the issuer. Comparison is verbatim, and an empty
issuer matches nothing. `authority::verify_chain` and
`delegation::verify_chain` check no signatures themselves and rely on each
link having been verified with `verify_proof_with_public_key`, so they gain
the binding through it.

Not a breaking API change: the error variant already existed and the enum
is `#[non_exhaustive]`. It is a behaviour change: credentials signed with a
key whose id is not a method of the issuer no longer verify.

Signed-off-by: Glenn Gore <glenn.g@affinidi.com>
@stormer78
stormer78 requested a review from a team as a code owner October 16, 2026 16:28
@stormer78
stormer78 merged commit 8f3ae05 into main Oct 21, 2026
7 checks passed
@stormer78
stormer78 deleted the security/issuer-binding branch October 21, 2026 04:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant