feat!: a VDC is not a bearer credential; upgrade ordering; digest wording - #26
Merged
Merged
Conversation
…ding Closes the two findings in #22 and #23. `delegation::verify_chain` now takes `presenter: &str` and requires the leaf to appoint it, refusing otherwise with `DelegationError::NotTheDelegate`. 0.8.0 did this for the VAC and left the VDC alone, and the asymmetry was the whole of the gap: `VerifiedDelegation.delegate` was returned, so a caller could compare it, but nothing in the signature obliged them to - exactly the shape the VAC was in before 0.8.0, where two consumers each found the omission independently and patched around it. That is Working Draft 02's Invocation Binding rule: a verifier MUST NOT accept a party as acting in the delegator's name unless that party demonstrates control of the verification method associated with `credentialSubject.id`. Delegation is the sharper case of the two - a captured VAC replays whatever it confers, while a captured VDC replays as somebody, and every act it carries is attributed to the principal. Only the leaf's delegate is asked for anything; the parties above it are not present, which is what keeps re-delegation working. Upgrade ordering is now stated in the README, for 0.7.0, 0.8.0 and this release alike: verifiers before issuers. A new credential reaching an old verifier is the direction that misleads - a 0.6 verifier compares a 0.7 digest against an `id`, finds them unequal, and reports a broken chain, printing both values with nothing to say they are different kinds of identifier. That cost a consumer an hour, and an old verifier cannot be taught to say otherwise, so the fix is ordering rather than a better message. The other direction was already safe and loud, failing with `InvalidDigest`, which names the WD01 `sha256:<hex>` form explicitly. `AuthorityError::BrokenLink` described its two fields as `id`s. Both have carried digests since 0.7.0, and that wording is plausibly part of why the skew reads as a mismatched chain to whoever goes looking. `DelegationError::BrokenLink` documented its fields not at all. Both now say `digestMultibase`. Version is 0.9.1 rather than 0.10.0 by explicit choice: 0.9.0 published hours earlier and has no known consumers. Cargo treats it as compatible with 0.9.0, so anyone on `dtg-credentials = "0.9"` picks it up on a routine `cargo update` and gets a compile error naming the new parameter. The changelog says so. 124 tests, up from 121. The `data_room` example presents the VDC as the scheduler and demonstrates the same VDC being refused for anyone else. Refs: #22 Refs: #23 Signed-off-by: Glenn Gore <glenn.g@affinidi.com>
🛡️ AI Agentic Security Code Review🔎 A manual security review is recommended before merging. Please contact the Security team for specifics and remediation guidance.
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #23 and #22, plus a stale-documentation fix found while checking them.
Important
API-breaking despite the patch version.
delegation::verify_chaintakes a newargument. 0.9.1 was chosen deliberately — 0.9.0 published hours earlier with no known
consumers — but Cargo treats
0.9.1as semver-compatible with0.9.0, so anyone ondtg-credentials = "0.9"picks this up on a routinecargo update. They get a compileerror naming the new parameter rather than a silent behaviour change. Flipping to
0.10.0is a one-line change inCargo.tomlplus the changelog heading if you'drather force the opt-in.
1. A VDC is not a bearer credential (#23) — breaking
delegation::verify_chainnow takespresenter: &strand requires the leaf to appointit, refusing with the new
DelegationError::NotTheDelegate. Nothing else about chainverification changed.
#21 did this for the VAC and left the VDC alone. The asymmetry was the whole gap:
VerifiedDelegation.delegatewas returned, so a caller could compare it, but nothingin the signature obliged them to — exactly the shape the VAC was in before #21, where
two consumers each found the omission independently and patched around it.
Delegation is the sharper of the two cases. A captured VAC replays whatever it confers;
a captured VDC replays as somebody, and every act it carries is attributed to the
principal.
Three tests cover it: a captured VDC refused for its captor, the principal refused for
its own delegation, and an intermediate delegate refused for a chain below it — that
last one pinning that only the leaf's delegate is asked for anything, which is what
keeps re-delegation working.
2. Upgrade ordering (#22)
New Upgrading section in the README: verifiers before issuers, with a table covering
0.7.0, 0.8.0 and 0.9.1.
One note on scope. #22 suggested a hint in
BrokenLinkwhennameddecodes as a digestand
presenteddoes not. That cannot help the reported case: the confusing message isemitted by the 0.6 verifier, and nothing shipped here changes what 0.6 prints. The
direction this crate does control was already correct — an old credential reaching a new
verifier fails with
InvalidDigest, which names the WD01sha256:<hex>form explicitly.So the ordering guidance is the whole of the fix, which is what the issue itself expected.
3.
BrokenLinkdocumented its fields as identifiersFound while checking #22, and not in either issue.
AuthorityError::BrokenLinkdescribednamedandpresentedas "theidthe link points at" / "theidof the credentialactually presented". Both have carried digests since 0.7.0.
That stale wording is plausibly part of why the skew in #22 reads as a mismatched chain
to whoever goes looking — the error prints two values and the docs told you they were
identifiers. Both now say
digestMultibase, and the variant points at the upgradeordering.
DelegationError::BrokenLinkdocumented its fields not at all, and now matches.One correction to #23
The issue notes the 0.7.0 changelog "already claims a VAC and a VDC are both non-bearer,
and that is only half true until this lands." In context (
CHANGELOG.md, 0.7.0) thatsentence sits inside a list of what 0.7.0 explicitly did not implement, describing
upstream PR #41 — it was accurate when written, and 0.8.0 doesn't overclaim either. The
gap was real; no changelog needed correcting, so none was.
Notes
cargo clippy --all-targetsandcargo fmt --checkclean.data_roomexample now presents the VDC as the scheduler, and demonstrates thesame VDC being refused for anyone else. Verified by running it.
and it is verifiers first for every breaking release so far. The VTI bump from 0.6
now crosses three of them.
Refs: #22
Refs: #23