Skip to content

Pin harness attestation issuers in authority sdk#9

Open
jescalan wants to merge 1 commit intomainfrom
je.pin-harness-attestation
Open

Pin harness attestation issuers in authority sdk#9
jescalan wants to merge 1 commit intomainfrom
je.pin-harness-attestation

Conversation

@jescalan
Copy link
Copy Markdown
Contributor

Tightens harness attestation verification in the Authority SDK by replacing issuer-derived JWKS lookup with pinned trusted attestation issuers, so unknown iss values are rejected before any network fetch. It also aligns the spec and implementation by requiring attestation sub to match harness.id, comparing cnf.jwk by key material rather than raw object serialization, and adding regression tests for trusted, untrusted, and mismatched-attestation cases.

@jescalan jescalan requested a review from colinclerk March 18, 2026 20:20
@vercel
Copy link
Copy Markdown

vercel Bot commented Mar 18, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
agentpass Ready Ready Preview, Comment Mar 18, 2026 8:20pm

Request Review

@colinclerk
Copy link
Copy Markdown
Member

I'm not sure about this one. Would it prevent people from using new harnesses until the authority explicitly approves them?

I would think that's a per-authority decision, not a spec decision

@jescalan
Copy link
Copy Markdown
Contributor Author

@colinclerk ok, so this one was primarily the prerogative of codex tbh. it flagged this area as a security concern and i said go for it and see if we can patch it. i haven't really dug in deep on this one like i did with most of the other PRs here. that being said, here's codex's reply to the pushback. let me know if you'd like me to not delegate to AI here and really dig in, I definitely can. Just trying to context-manage my own brain a bit here 😅

What this change does is not require per-harness approval. It requires per-attestation-issuer trust. In the current model, a “new harness” from an already trusted issuer like a known vendor would still work without any extra approval, because the attestation sub carries the harness identity and the trust decision is on iss. What would require authority policy is a new attestation issuer, which is exactly the thing we should not trust implicitly.

That distinction matters because the security problem here was never “how do we identify individual harnesses,” it was “do we let an arbitrary iss claim tell us where to fetch keys from.” If the spec does not require some authority-local trust decision before dereferencing issuer-derived key material, then we are back in TOFU / SSRF territory. So I do think this belongs in the spec at the trust-model level. The per-authority part is already there in the wording: “trusted under Authority local policy.” The spec is not choosing which issuers to trust; it is saying the Authority must choose, rather than blindly trusting whatever iss arrives.

I do think this reply exposes a wording risk, though. The current text can be read as “every new harness must be pre-approved,” which is not the intent. If we want to address the reviewer cleanly, I’d say something like:

“This is intended to be issuer-level trust, not per-harness registration. New harness identities from an already trusted attestation issuer should continue to work without explicit approval. The normative requirement here is only that an Authority must not derive trust or network locations from an arbitrary iss; it must have a local policy for which attestation issuers, if any, it accepts.”

And if we want to tighten the spec later, I’d consider a sentence in Section 3.1.2 or 3.1.4 along the lines of: “This does not require prior registration of individual harness identities; the trust decision applies to attestation issuers.” That would probably defuse the review concern without backing away from the security property.

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.

2 participants