Issue-time validation: well-formed windows, bounded JSON depth, and _for grant constructors (0.10.0) - #28
Merged
Conversation
`new_vac`, `new_vdc`, `attenuate`, `redelegate` and both edge-answering constructors accepted a `validUntil` at or before `validFrom`, and returned a credential that is never valid. A chain built from one was caught at verification, because `verify_chain` checks every link against an instant; a standalone credential was caught nowhere, and `sign` put a proof on it. Every constructor that returns a `Result` now checks the window before anything else and refuses with the new `DTGCredentialError::InvalidValidityWindow`. The constructors that return `Self` cannot refuse, so the new `DTGCredential::validate` checks their output instead, and `sign` calls it before signing. A caller that signs with another backend can call it directly. Both ends are compared at whole seconds, because that is all the wire form carries: a window narrower than a second serializes as an empty one. A `validFrom` in the past is still accepted. Backdating is how a re-issued credential keeps the date the original took effect, so only the ordering of the two ends is checked, never either end against the clock. Signed-off-by: Glenn Gore <glenn.g@affinidi.com>
A VEC's `endorsement`, `credentialStatus`, and the unmodelled top-level members in `DTGCommon::extra` are held as open `serde_json::Value`s. Digesting, signing and verifying clone, serialize and canonicalize them recursively, so a value nested a few thousand levels deep overflowed the stack and aborted the process rather than returning an error. `MAX_JSON_DEPTH` (64, counted from the top of the credential) now bounds them, and `DTGCredentialError::JsonTooDeep` reports a document that crosses it. The check walks with an explicit stack, and runs before anything recursive touches the value: - `digest_multibase_json`, and the deprecated `digest_json`, on the document before `proof` is stripped from a clone of it. That covers every constructor that digests a grant or parent it was handed. - `attenuate_from_json` and `redelegate_from_json`, before a member is cloned out of the parent. - `DTGCredential::digest_multibase` and `digest`, on the open members, before the credential is cloned. - `DTGCredential::validate`, which `sign` already calls and `verify_proof_with_public_key` now calls as well. 64 is half of serde_json's default parser limit, so anything this library signs parses back in a stock verifier, and a credential received over the wire is already bounded by that limit before it gets here. The bound cannot protect a caller that builds a deeper value itself, because dropping a `Value` recurses too; the parser remains the real boundary, and the constant's documentation says so. `new_vec` and `with_credential_status` gain a Security section. Both embed what they are given verbatim, and a consumer must verify the proof and the issuer's standing before relying on any of it. The new `tests/json_bounds.rs` runs its deep cases on a 256 KiB stack. With the check disabled, each of them aborts with a stack overflow. Signed-off-by: Glenn Gore <glenn.g@affinidi.com>
…s proof
`new_member_vmc` and `new_delegate_vdc` read the member or delegate off the
grant they are handed, and check neither who is answering it nor whether
anybody signed it. The digest binds the answer to the grant, so the result is
binding evidence, but nothing in either signature obliged a caller to confirm
that the grant names the party doing the answering - the shape
`verify_chain` was in before 0.8.0 and 0.9.1 put the presenter in its
signature. Nor was the answer held to the grant's expiry.
Two new constructors take the party the caller expects:
- `DTGCredential::new_member_vmc_for(grant, member, valid_from, valid_until)`
- `DTGCredential::new_delegate_vdc_for(grant, delegate, valid_from, valid_until)`
Each refuses a grant whose `credentialSubject.id` is not that party, with
`DTGCredentialError::NotTheGrantSubject { expected, found }`, and an answer
whose `validUntil` is later than the grant's, or absent against a grant that
expires, with `OutlivesGrant`. A grant `validUntil` that cannot be read is
refused rather than treated as absent. The parties are still read off the
grant: the new parameter is compared with it, not substituted for it.
The old constructors are deprecated in favour of these and otherwise keep
their behaviour, so nothing breaks; a build with `-D warnings` names the
replacement.
Behind `affinidi-signing`, `verify_grant_with_public_key(grant, public_key, at)`
verifies a grant in its wire form before it is answered: the proof, over the
document without its `proof`; that the proof's verification method belongs to
the grant's `issuer` (`ProofNotFromIssuer` otherwise); and that the window is
well formed and contains `at` (`NotValidAt`). `MalformedCredential` covers a
grant with no `issuer` or `validFrom`, or with an unreadable timestamp or
proof.
`acknowledges`, `accepts` and both new constructors gain a Security section:
a binding is not a verified edge. The README samples and the `data_room`
example now verify the grant, then answer it with the new constructors.
Signed-off-by: Glenn Gore <glenn.g@affinidi.com>
DTGCredentialError is now #[non_exhaustive]: it gains seven variants in this release, and future variants will not be breaking. With new_member_vmc and new_delegate_vdc deprecated, a -D warnings build changes on upgrade, so this ships as 0.10.0 rather than 0.9.2 and a cargo update on "0.9" does not pick it up silently. Signed-off-by: Glenn Gore <glenn.g@affinidi.com>
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.
Summary
Four commits. No function signatures change, and nothing changes on the wire for a well-formed credential. The release is 0.10.0 because
DTGCredentialErrorbecomes#[non_exhaustive]and two constructors are deprecated (see Compatibility).DTGCredentialError::InvalidValidityWindow { valid_from, valid_until }.create::check_windowruns first innew_vac,new_vdc,new_member_vmc(_for),new_delegate_vdc(_for),attenuate_innerandredelegate_inner. It refusesvalid_until <= valid_from, compared at whole seconds because that is the wire precision.DTGCredential::validate(), called at the top ofsign().validFromstays legal.pub const MAX_JSON_DEPTH: usize = 64, counted from the top of the document, andDTGCredentialError::JsonTooDeep { max }.digest_multibase_jsonanddigest_json, beforeprooflessclones;attenuate_from_jsonandredelegate_from_json, before a member is cloned out of the parent;DTGCredential::digest_multibaseanddigest, onendorsement,card,credentialStatusandextra, before the credential is cloned;validate(), which bothsign()andverify_proof_with_public_key()now call. This coverswith_credential_status/set_credential_status.# Securitydocs added onnew_vecandwith_credential_status.DTGCredential::new_member_vmc_for(grant, member, valid_from, valid_until)andDTGCredential::new_delegate_vdc_for(grant, delegate, valid_from, valid_until).NotTheGrantSubject { expected, found }whencredentialSubject.idis not the given party.OutlivesGrant { valid_until, grant_valid_until }when the answer ends after the grant, or has no end against a grant that expires.validUntilthat is present but unreadable is refused.new_member_vmcandnew_delegate_vdcare now#[deprecated(since = "0.10.0")], pointing at the_forforms. Apart from the window check in commit 1, their behaviour is unchanged.verify_grant_with_public_key(grant, public_key, at)is behindaffinidi-signing. It runs these checks in order:proofpresent (NotSigned);DataIntegrity);issuer(ProofNotFromIssuer);at(InvalidValidityWindow/NotValidAt).Missing or unreadable members return
MalformedCredential.# Securitysections added onacknowledges,acceptsand both new constructors.Internal tests, the README samples and
examples/data_room.rsnow use the new API. The example verifies each grant before answering it.DTGCredentialErrormarked#[non_exhaustive].Compatibility
DTGCredentialErroris now#[non_exhaustive]and gains seven variants:InvalidValidityWindow,JsonTooDeep,NotTheGrantSubject,OutlivesGrant,ProofNotFromIssuer,NotValidAtandMalformedCredential. An exhaustivematchdownstream needs a wildcard arm. Future variants will not be breaking.dtg-credentials = "0.9"do not pick this up withcargo update; the bump is deliberate. That matters because of the deprecations below.new_member_vmcandnew_delegate_vdcstill compile.-D warningswill flag their call sites, and that includesclippy --all-targets.Resultconstructor,validate(),sign()andverify_proof_with_public_key()._from_jsonderivations,validate(),sign()andverify_proof_with_public_key().Callers of the deprecated functions in
verifiable-trust-infrastructureChecked against origin/main
5a6d4923and not edited here. Both call sites are inside#[cfg(test)]modules:vtc-service/src/members/inbound_vmc.rs:398:DTGCredential::new_member_vmc(&grant_wire, valid_from, None)(test module starts at line 371).vtc-service/src/routes/relationships.rs:2140:DTGCredential::new_member_vmc(&grant_json, m.joined_at, None)(test module starts at line 1966).There are no callers of
new_delegate_vdc, and VTI does not match onDTGCredentialErrorvariants.VTI's CI runs
cargo clippy --workspace --all-targets -- -D warnings. The change that bumps VTI to 0.10.0 therefore needs to move both call sites tonew_member_vmc_forin the same commit.Tests
161 tests with
--all-features, up from 124. 146 with--no-default-features.tests/membership_edge.rs(new, 14 tests).NotTheGrantSubject, and the comparison is exact.OutlivesGrant, whether it ends later or has no end.InvalidValidityWindow.#[allow(deprecated)].affinidi-signing:NotSigned;ProofNotFromIssuer;NotValidAt.tests/delegation_chain.rs.new_delegate_vdc_for.NotTheGrantSubject,OutlivesGrant, and an unsigned grant returningNotSigned(feature-gated).InvalidValidityWindowfornew_vdc,redelegate,redelegate_from_jsonand the acceptance.tests/authority_chain.rs.new_vac,attenuateandattenuate_from_json.tests/json_bounds.rs(new, 13 tests).MAX_JSON_DEPTH - 3passes, and exactly at the bound passes; one level over returnsJsonTooDeep.credentialStatusandextraare refused.digest_multibase_json,digest_multibase/validate,attenuate_from_jsonandredelegate_from_json. These run on astd::thread::Builderthread with a 256 KiB stack.affinidi-signing,signandverify_proof_with_public_keyrefuse over-deep members.src/lib.rsunit tests.validate()refuses inverted and empty windows and accepts backdated or open-ended ones.new_member_vmcrefuses an inverted window.sign()refuses an inverted window and leaves no proof.Regression check. With the iterative guard temporarily made to always return
false, each of the three small-stack tests aborts withthread 'small-stack' has overflowed its stack(SIGABRT). The guard was then restored.Verification
Run locally on macOS arm64 with the repository's toolchain; results for the final commit are in the PR conversation.
cargo fmt --all --checkcargo clippy --all-targets --all-features -- -D warningscargo test --all-featuresandcargo test --no-default-featurescargo package --lockedcargo run --example data_room: the example verifies both grants, then completes the membership and delegation edges.Open questions
OutlivesGrantenforces it in the new constructors only.MAX_JSON_DEPTH = 64as proposed. There is no byte-size cap; that is a policy call for the owners.