Skip to content

fix(delegation): require holder binding before acting on a chain - #97

Merged
imran-siddique merged 1 commit into
agentrust-io:mainfrom
zohebk8s:security/holder-binding
Aug 11, 2026
Merged

fix(delegation): require holder binding before acting on a chain#97
imran-siddique merged 1 commit into
agentrust-io:mainfrom
zohebk8s:security/holder-binding

Conversation

@zohebk8s

Copy link
Copy Markdown
Contributor

fix(delegation): require holder binding before acting on a chain

The gap

A delegation chain states what authority exists. Nothing in the inbound path required the presenter to demonstrate any relationship to it, so possession of the chain was sufficient to exercise it.

subject is an Ed25519 public key whose private half the delegate must hold in order to issue child credentials. At runtime it was only ever compared as a string, for continuity:

if cred.issuer != prev.subject:
    raise BrokenDelegationLink(...)

grep subject src/ca2a_runtime/peer.py returned nothing. PeerRequest had no field that could carry a proof.

CREDENTIAL_REPLAY does not cover it: that catches a duplicate credential_id inside one chain, not replay of a whole valid chain by a different party. Verbatim replay leaves no tamper evidence, since every signature verifies honestly and no check fails, and the provenance record names chain[-1].subject rather than the caller, so the audit trail attributes the call to the wrong party.

On aaf8762, over transport.server in software mode, unmodified. Mallory holds no key material, only a copy of a chain issued to Bob:

STEP 1  Bob makes a legitimate delegated call
  accepted            : True
  granted_capability  : write
  record.subject      : 13a7c8c962c2091f...  (= Bob)

STEP 2  Mallory obtains the chain. She holds NO private key.
  captured 1 credential(s), 400 bytes of JSON
  Mallory's key material: (none)

STEP 3  Mallory replays it verbatim to Carol
  accepted            : True
  granted_capability  : write
  record.subject      : 13a7c8c962c2091f...  (= Bob)

RESULT: EXPLOITED.

400 bytes of published JSON, no keys, and the record names Bob.

What appraisal establishes, and what this adds

Since #89 and #90 the callee learns what the caller is running: a measurement and an X25519 channel key, fresh against a challenge it issued. That is one of the two things a callee needs to know about a caller.

The other is whose authority it holds, and that is what this adds. The two live in separate key hierarchies: appraisal binds an X25519 channel key, delegation names an Ed25519 subject, and nothing joined them. So a caller can appraise at hardware assurance and still be exercising a chain issued to somebody else. On a default config neither question is asked, since require_caller_attestation is REQUIRE_NONE.

The proof commits to the offer's channel key, so where both are present the two answers provably refer to the same party. test_hold_006_an_attested_caller_cannot_use_another_partys_chain pins the distinction.

The fix

The callee challenges the key that was already in the credential. A caller answers with an Ed25519 signature under chain[-1].subject over the RFC 8785 canonical form of a body committing to your channel key, the challenge, the leaf credential_id and subject, the requested capability, the record_id, the sealed-payload digest, and the caller's own offered channel key.

That last field is the join: with both mechanisms present, the attested runtime and the delegated principal are provably the same party, which neither established alone. It is committed even when null, so an offer cannot be stripped from a proof made while attesting, nor bolted onto one made without.

Three orderings carry the property:

  1. Chain first, so the leaf subject is a key someone was genuinely delegated rather than one the caller asserted.
  2. Holder binding before the scope intersection, so a caller that proved nothing never reaches policy evaluation. Unlike an appraisal refusal it emits no provenance record: a caller with no shown relationship to the credential should not receive a signed statement about it, and a denial naming a subject it may have nothing to do with attributes the refusal to the wrong party.
  3. A proof is recorded only after it verifies, so a party holding none of the keys can neither fill the replay cache nor insert a signature to lock the real delegate out of its own proof.

HOLDER_PROOF_INVALID is 401, not 403: the chain may carry the authority requested, but the caller has not shown it is the party that authority was delegated to. RFC 7800 cnf semantics, the same pattern ca2a_verify.dag already applies to TRACE records, applied to the credential that gates authority.

Required by default, unlike caller_offer, and the asymmetry is deliberate: attesting a runtime is a capability not every caller has, whereas holding the key you were delegated is what being the delegate means.

Same tree, patch applied, every route Mallory has:

  (a) Bob, legitimate              -> HTTP 200  accepted=True
  (b) bare captured chain          -> HTTP 401  HOLDER_PROOF_INVALID
  (c) replay of Bob's own request  -> HTTP 401  HOLDER_PROOF_INVALID   (Bob's first send: 200)
  (d) proof forged with own key    -> HTTP 401  HOLDER_PROOF_INVALID

(b) is the reported gap. (c) is the one worth reading: Mallory captures Bob's complete request, valid proof included, and replays it byte for byte inside the challenge window. Refused, because the proof was already honoured once. client.send_task also gains a required holder_key, so the caller side cannot send a task without the delegated key at all.

What changed since the diff you have

That diff was against 776f517 and no longer applies: #89 created src/ca2a_runtime/challenge.py, which it also created, and #93 reformatted the tree. I re-verified the finding on aaf8762 unmodified first, then reworked the fix to fit the mutual-attestation architecture rather than sit beside it.

In the diff you have Here
Shipped its own challenge.py Reuses yours; creates no such file
Consumed the nonce for single use Remembers the proof instead, since a stateless challenge cannot be consumed
No caller_channel_key in the proof Commits to it, which is the join above and was not possible before #90
Added GET /ca2a/challenge to transport/server.py Nothing, your handshake already issues one

The approach is unchanged. Those four are the deltas.

Two decisions for you

A store, where you chose stateless. challenge.py notes a challenge store "is the option this one was chosen over". Because a stateless challenge cannot be consumed, single use here comes from remembering the proof: ProofReplayCache, on by default in PeerNode, TTL matching your challenge TTL. Without it a complete request can be replayed inside its window.

Bounded, and the bound is stated rather than hidden: past capacity the oldest entry is evicted, so a flood degrades to the challenge window rather than causing an outage. seen_proofs=None opts out, which a multi-instance deployment wants alongside a shared secret or sticky routing.

If you would rather keep the whole path stateless, say so and I will drop it and document the window instead.

The escape hatch. require_holder_proof=False exists because offline replay of recorded evidence has no live caller to challenge; the conformance action-evidence replay uses it, with a comment saying why. It is also a footgun. It could leave the live path entirely by giving offline replay its own entry point, but that reshapes public API rather than fixing a vulnerability, so not here. Say the word for a follow-up.

Validation

411 passed, 3 skipped     (baseline on aaf8762: 373 passed, 3 skipped)
ruff check src/ tests/            All checks passed!
ruff format --check src/ tests/   82 files already formatted
mypy src/ca2a_runtime/ src/ca2a_verify/   Success, 42 source files
bandit -r src/ -c pyproject.toml  clean

Six claim experiments and three example demos still exit 0. 21 files, +1511 / -119.

tests/unit/test_holder_binding.py carries 32 of the 38, grouped as the attack, the join, single use, the scope of a proof, and the fail-closed wiring. tests/conformance/ adds HOLD-001 through HOLD-006, documented in tests/conformance/README.md. Profile requirement is P-4a.

Breaking on the inbound path, deliberately: opt-in would leave the default insecure. client.send_task gains a required holder_key. At 0.1.0a1 with no adopters, and with the extension binding still unconfirmed under #18, this is the cheapest point to make that break.

Not addressed

This binds who, not how long. No validity window and no revocation path, so a compromised delegate's authority cannot be withdrawn, and that interacts with P-4's offline-verification requirement since an offline verifier cannot learn a credential was revoked. Needs a position on the trade-off rather than an implementation.

The hardware attestation path is untested for lack of a confidential VM. I believe it is unaffected, since appraisal establishes runtime rather than credential-holding in either mode, but I have not run it.

A delegation chain stated what authority existed, never who was holding it. The
inbound path verified signatures, continuity, attenuation, depth, replay and the
caller's own attestation, then granted, without ever requiring the caller to
demonstrate a relationship to the chain it presented. `PeerRequest` had no field
that could carry such a proof, and `subject` -- an Ed25519 public key whose
private half a delegate must hold in order to issue child credentials -- was only
ever compared as a string for continuity. `grep subject peer.py` returned nothing.

That would be a bounded risk if chains were secret, but offline verifiability is
a design goal: chains are handed to auditors, embedded in provenance DAGs, and
shipped in examples/. The credential intended for publication was the credential
that granted authority. A chain lifted from any of those and replayed verbatim
was accepted, and the provenance record emitted afterwards named the legitimate
subject, so the audit trail attributed the call to the wrong party.

Verbatim replay leaves nothing to detect. Every signature verifies honestly
because the credential is genuine; no field is malformed and no check fails.
CREDENTIAL_REPLAY does not cover it either, catching only a duplicate
credential_id inside one chain rather than replay of a whole valid chain by a
different party. Mutation, scope widening, cross-chain splicing, record
reparenting and ciphertext tampering all remain correctly rejected. This was
never tampering, which is why nothing caught it.

Caller attestation does not close it. An appraised caller_offer establishes what
the caller is running: a measurement and an X25519 channel key, fresh against a
callee-issued challenge. It says nothing about the Ed25519 delegation subject, so
a caller can appraise at hardware assurance and still exercise a chain issued to
somebody else. require_caller_attestation defaults to REQUIRE_NONE besides.

The fix uses the key that was already there. A callee challenges it and the
caller answers with an Ed25519 signature under chain[-1].subject over the RFC
8785 canonical form of a body committing to the callee's channel key, the
challenge, the leaf credential_id and subject, the requested capability, the
record_id, the sealed payload digest, and the caller's own offered channel key.

That last field is the join. With both mechanisms present the attested runtime
and the delegated principal are provably the same party, which neither
established alone. It is committed even when null, so an offer cannot be stripped
from a proof made while attesting, nor bolted onto one made without. A proof
therefore does not transfer between peers, calls, capabilities, records or
payloads.

Three orderings carry the property. The chain is verified first, so the leaf
subject is a key someone was genuinely delegated rather than one the caller
asserted. Holder binding runs before the scope intersection, so a caller that has
proved nothing never reaches policy evaluation; unlike an appraisal refusal it
emits no provenance record, because a caller with no shown relationship to the
credential should not receive a signed statement about it and a denial naming a
subject it may have nothing to do with attributes the refusal to the wrong party.
And a proof is recorded only after it verifies, so a party holding none of the
keys can neither fill the replay cache nor insert a signature to lock the real
delegate out of its own proof.

Single use comes from remembering the proof rather than the challenge, because
ca2a_runtime.challenge is stateless by design and so cannot be consumed. Without
that, a complete request including its valid proof could be replayed inside the
challenge window. ProofReplayCache does it, on by default in PeerNode with a TTL
matching its own challenge TTL, and bounded: past capacity the oldest entry is
evicted, so a flood degrades the property to the challenge window rather than
causing an outage. seen_proofs=None opts out, which a multi-instance deployment
wants alongside a shared secret or sticky routing. No second challenge mechanism
is introduced and challenge.py is untouched.

HOLDER_PROOF_INVALID is 401, not 403: the chain may carry the authority
requested, but the caller has not shown it is the party that authority was
delegated to. This is RFC 7800 cnf semantics, the confirmation pattern
ca2a_verify.dag already applies to TRACE records, applied to the credential that
gates authority.

Required by default, unlike caller attestation, and the asymmetry is deliberate:
attesting a runtime is a capability not every caller has, whereas holding the key
you were delegated is not optional, it is what being the delegate means.
require_holder_proof=False reproduces the old behaviour for offline replay of
recorded evidence, where no live caller exists to challenge, and must not be used
on a live peer path. client.send_task gains a required holder_key.

Adds profile requirement P-4a, conformance HOLD-001 through HOLD-006, and an
adversary and defence row in the threat model.

411 passed, 3 skipped (was 373 passed, 3 skipped). ruff check, ruff format
--check, mypy strict and bandit all clean over src/ and tests/. Six claim
experiments and three example demos still exit 0.

Signed-off-by: Mohammed Zoheb Shaik <zoheb.shaik7@gmail.com>
@github-actions

github-actions Bot commented Aug 11, 2026

Copy link
Copy Markdown
Contributor

🟡 Contributor Check: MEDIUM

Check Result
Profile MEDIUM
Credential LOW
Overall MEDIUM

Automated check by AgenTrust Contributor Check.

@github-actions github-actions Bot added the needs-review:MEDIUM Contributor check flagged MEDIUM risk label Aug 11, 2026

@imran-siddique imran-siddique left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approving. The finding is real, the fix is the right shape, and the reasoning about ordering is correct: chain before proof, proof before the scope intersection, and a proof recorded only after it verifies.

Verified locally: 427 passed, 3 skipped, with ruff, ruff format, mypy strict and bandit clean.

On your two questions.

The store. Drop it. Keep the path stateless and document the challenge window as the bound. ProofReplayCache._expire also rescans every entry on every call: 34us per record() at 500 entries, 1.2ms at 8k, 6.4ms at the 100,000 default, and filling toward that bound is quadratic. That makes the cache a denial of service well before its own bound, so the graceful degradation it claims does not hold. Removing it settles both questions at once.

The escape hatch. Leave it as is. A separate entry point for offline replay is a follow-up, not part of a vulnerability fix.

Merging now and tracking three follow-ups: remove the cache, stop verifying the chain twice (peer.py:402, then again inside effective_scope), and settle why parent_record_hash is the one request field the proof does not commit to.

Thank you for the disclosure, and for the patience on the reporting process.

@codecov-commenter

Copy link
Copy Markdown

⚠️ Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@imran-siddique
imran-siddique merged commit cbfeb40 into agentrust-io:main Aug 11, 2026
12 of 13 checks passed
@imran-siddique

Copy link
Copy Markdown
Member

Merged. The three follow-ups are #104 (remove the cache, keep the path stateless), #105 (chain verified twice per request), #106 (the parent_record_hash question, which is yours to answer). #104 is the one that matters and you are welcome to take it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

needs-review:MEDIUM Contributor check flagged MEDIUM risk

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants