feat: add POST /v1/enclave-assignment and an attestation-verifying client - #17
Conversation
Implements the spec §6 assignment endpoint the authenticator calls immediately before sealing a match payload. The response carries the attestation document and nothing else. The document already holds the enclave's identity (module_id) and its own expiry (the leaf certificate's notAfter), and the client must verify it before trusting either, so echoing those as unsigned JSON fields would restate signed data and add a mismatch case to reconcile. The host therefore relays opaque bytes it neither parses nor verifies. Failures are classified rather than collapsed into 503: timeouts are 504, transport failures and enclave-not-ready are 503, and a match-path error -- which cannot arise from an attestation request -- is 500 rather than being folded into retryable unavailability. Each failure logs a dependency name and failure class so triage does not parse Debug output. The signing-key attestation is no longer publicly reachable; it belongs to the Key Registry flow. The enclave keeps GetEnclaveKeysRequest for it. Caching is deliberately not implemented yet, so every request still costs an NSM attestation. The TODO moves to the new route with the constraint that this must not carry production traffic uncapped. Also lifts the enclave-client stub out of the matches tests into a shared api/src/test_support.rs, so both routes drive the same double. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Adds client/verifier-client with the first real AWS Nitro attestation verification in this repo. Until now nothing verified anything: pontifex's parse_raw_attestation_doc extracts the COSE payload with the verification key set to None, and the e2e harness relies on it. Ported from worldcoin/bedrock (bedrock/src/nitro_enclave), MIT (c) Tools for Humanity, which is the implementation shipping in World App -- the authenticator that will call POST /v1/enclave-assignment. Porting rather than reimplementing means the enclave is checked by the same logic on both sides of the protocol instead of by two independent readings of the AWS spec. The two public crates that look official were rejected: aws-nitro-enclaves-attestation is a private individual's alpha that depends on vendored openssl, and nitro_attest names a repository and GitHub org (aws-nitro-enclaves/nitro-attest) that do not exist. Divergences from bedrock, all deliberate: - no uniffi surface and no global get_config() coupling; policy is an argument - `now` is a parameter rather than a cfg(test)-only bypass flag, so tests exercise the production path and expired fixtures stay usable - expected PCRs come from the caller, since our PCR0 is not known until the enclave image is built - all-zero measurements (a --debug-mode enclave, whose memory the parent instance can read) are rejected unless the caller opts in - no sealing here; the caller seals to the verified key The trust anchor is confirmed three ways: the DER downloaded from AWS, the fingerprint AWS publishes, and bedrock's vendored copy all hash to 641a0321... Note AWS publishes the fingerprint of the certificate, not of the zip, so the certificate is what we pin -- a test asserts it. 16 tests, mostly negative, over a real attestation document captured from a live enclave. rejects_a_corrupted_cose_signature is the load bearing one: the document still parses and still chains to the AWS root, and is rejected solely because one signature byte flipped. deny.toml gains ISC (webpki, which validates the chain) and CDLA-Permissive-2.0 (webpki-roots, the Mozilla CA bundle behind reqwest's TLS), both following bedrock's own deny.toml. No openssl enters the lock. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Fetches POST /v1/enclave-assignment and returns the enclave's public key only if the attestation document it carries verifies. Bounded by construction: a connect timeout, a total request timeout, and a 64 kB cap on the response body, with the timeout as the backstop for a host that omits Content-Length. There is deliberately no retry -- the endpoint costs an NSM attestation per call, and the spec already has the authenticator re-assigning when a match fails, so retrying here would multiply load on the exact endpoint least able to absorb it. The CLI refuses to run without at least EXPECTED_PCR0: a verifier with no pinned measurements checks that a document came from some enclave, not from ours, and silently accepting that would defeat the point. It warns loudly when zeroed (debug-mode) measurements are allowed. Five tests drive the client against a stub host over a real TCP socket, so the JSON contract, the base64 hop and the verifier are exercised together: a real attestation document verifies end to end, an unverifiable one is rejected, a 503 surfaces to the caller, and a response missing the attestation field is rejected. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The harness called pontifex's parse_raw_attestation_doc, which extracts the COSE payload with the verification key set to None. It was therefore sealing to whatever key the document claimed, with nothing establishing the document came from an enclave at all -- the comment on the helper conceded as much. It now runs the real verifier. The PCR policy moves into verifier_client::policy so the CLI and the harness read it the same way rather than duplicating the parsing. It fails closed when nothing is pinned: an empty policy would accept any genuine Nitro enclave, including somebody else's. README documents the new route, the assignment response shape, and why a debug-mode enclave needs an explicit opt-in. Also fixes the run instructions, which omitted the ENCLAVE_CID and ENCLAVE_PORT the API panics without. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
Warning Review the following alerts detected in dependencies. According to your organization's Security Policy, it is recommended to resolve "Warn" alerts. Learn more about Socket for GitHub.
|
Comments were carrying rationale better suited to the PR description. Removes tests that duplicated stronger neighbours: a tampered-payload case superseded by the corrupted-signature one, a truncated-document case covered by the parse guards, a base64 entry-point case covered by the HTTP tests, and two that asserted serde behaviour or a string check. Two survivors get stricter. rejects_a_stale_document and rejects_a_document_timestamped_in_the_future each accepted two error types, so neither proved what its name claimed -- the future-timestamp case was in fact failing the chain check, an hour being well outside the certificate window. Both now pin one error and use offsets that reach the check under test. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The e2e harness already exercises the same path on a Nitro host, so the binary only duplicated it with a narrower scope. Removing it makes verifier-client a pure library and drops anyhow and tracing-subscriber from its dependencies; tokio moves to dev-dependencies, since only the tests use it directly. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The client is going to grow operations beyond assignment, and the previous shape did not admit them: it stored a prebuilt assignment_url and took its arguments positionally, so a second endpoint meant another URL field and another parameter. It now stores the base URL and builds paths per request, behind a shared post_json helper so status handling and the response size cap cannot diverge between operations. Bounds move into ClientConfig, which gives callers a way to override them without widening the constructor each time. ClientError variants carry the path they refer to, so a failure names the operation once several exist. Renames the module http -> client, since it is now the client rather than transport plumbing, and re-exports Client, ClientConfig and ClientError at the crate root to avoid a verifier_client::client::Client stutter. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Configuration was split across a ClientConfig with public fields, a separate verifier argument, and env-var reading in two places. That matched nothing else in the protocol and left the host URL configurable only in code. It now follows world_id_primitives::Config: one struct covering everything, private fields with accessors, a validating constructor, and from_json. Config is stored on the Client as a public field, mirroring Authenticator. Nothing is read from the environment -- world-id-protocol reads none, and the e2e harness now names a config file instead of setting five semantic variables. Validation moves to config construction: the host URL is parsed into a url::Url up front, and a policy that pins no measurements is rejected outright rather than at first use. The harness now takes its encryption key over HTTP through the host rather than over vsock, so the assignment route and the client are exercised together instead of only against a stub. The signing key is not part of an assignment, so it still comes over vsock. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Net -150 lines, no behaviour change. The hand-rolled COSE Sig_structure is replaced by coset's tbs_data, which produces byte-identical output -- proven by the real-document test still verifying and the corrupted-signature test still failing. That drops ~30 lines re-implementing RFC 8152 4.4 and one place the spec could drift. It is the one deliberate divergence from bedrock, which builds the structure by hand. failure_class moves onto EnclaveClientError, so /v1/matches emits the same dependency and failure_class fields as the assignment route -- identical enclave failures were being triaged differently depending on which route hit them. The assignment route's status map also stops using a catch-all arm, so a new EnclaveError variant is now a compile error there as it already was in matches. post_json is inlined. It had one caller and pinned method, body and content-type in a shape neither announced follow-up fits -- matches posts octet-stream, a signing-key lookup is a GET -- so it would have been reworked rather than reused. Its `path` field disappears from four error variants, where it was always the same constant. Config gains the builder the crate already uses one module over, so the security toggle stops being a bare positional bool and the timeouts become reachable without a JSON round trip. Three accessors with no non-test caller are gone, as is Client's public config field, which was derived into http and verifier and so could silently desync. Also: CodeUntrusted drops a pcr_index that every construction site set to 0; the trust anchor is Cow rather than a per-verifier Vec clone; the stub client swaps four constructors for Default plus struct literals; three PCR tests that fell through the same branch fold into one table; and the config test stops asserting url and hex crate behaviour. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
An empty set of measurements pins nothing, and `all()` over an empty
iterator is vacuously true, so a configuration like
"allowed_pcr_configs": [[], [ ...real... ]]
returned Ok for any genuine Nitro enclave -- the empty entry matched
first and short-circuited the real one. Config::validate only rejected
the case where *every* entry was empty, so this shape passed validation
and then defeated it.
Found while diffing this port against bedrock, which has the same
vacuous-truth behaviour. It is more reachable here: bedrock builds its
configurations from compile-time constants, whereas this crate loads
them from a hand-written JSON file where an empty array is easy to
write.
Fixed at both layers, since EnclaveAttestationVerifier is public API and
usable without Config: the verifier skips empty configurations, and
Config rejects any config file containing one.
The regression test pairs an empty configuration with a deliberately
non-matching one, so acceptance can only come from the vacuous path.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The crate denies missing_docs, so failure_class without one fails the lint job. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
768 lines of the nitro module are derived from worldcoin/bedrock, which is MIT and requires the copyright and permission notice be included in all copies or substantial portions. Only a doc-comment reference was present, which credits the source but does not satisfy that. Adds client/verifier-client/NOTICE with bedrock's notice scoped to the four derived files, and points each module doc at it so the obligation is discoverable from the code rather than only from the repository root. Also records the AWS Nitro root certificate as redistributed unmodified. cargo-deny cannot catch this: it checks the licences of dependencies resolved through Cargo, not source copied into the tree. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…outer
Addresses three review comments together, since they turn out to be the
same change.
Error handling moves into api/src/error.rs, following the AppError
pattern from world-chat-backend. Both per-route status_for functions are
replaced by AppError::enclave_assignment and AppError::enclave_match.
The mapping stays per route because the same enclave error means
different things depending on what was asked -- DecryptFailed is a
client error on /v1/matches and a host bug on assignment -- so a blanket
From impl would have to pick one and be wrong for the other.
IntoResponse now owns the logging and the response body, so handlers no
longer log and no longer return bare status codes with empty bodies.
Clients get {"allowRetry": bool, "error": {"code", "message"}}, which
moves us toward the structured errors spec section 6 asks for.
EnclaveClientError::failure_class is deleted. The error code already is
the machine-readable failure class, and maintaining two taxonomies would
let them drift.
Route tests move to api/tests/, with the stub in tests/common/mod.rs and
src/test_support.rs deleted, so the production crate no longer ships
test scaffolding. Tests now drive requests through routes::handler()
rather than calling handler functions directly, which covers the path
and method a route is registered under. That gap was real: this PR
changed both, and nothing verified either. Two new tests would have
failed before -- assignment_is_not_reachable_by_get and
the_old_enclave_keys_route_is_gone.
It also caught a wrong assumption in my own test, which asserted
liveImageHash where the wire format is live_image_hash. Asserting
against the Rust struct could not have caught that.
Mapping tests stay as unit tests in error.rs, since they are pure
functions that need no stub.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Three things in this workspace are called a client, and two of them sit within fifteen lines of each other in the e2e harness: pontifex's vsock client talking to the enclave, and this one talking to the host over HTTP. A bare `Client` gave no signal which side of the trust boundary it was on, which is what a reviewer tripped over. The crate name made it worse -- verifier_client::Client is effectively client::Client -- and the host crate already uses EnclaveClient for the opposite direction, host to enclave. Also documents on the type that verification is not optional. It owns an EnclaveAttestationVerifier and returns nothing it has not verified, which the old name hid entirely on a security-critical path. ClientError keeps its name. It is the only error type in the crate, so it is unambiguous, and FaceVerifierClientError reads badly. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Four comments described history a future reader has no access to: how another codebase built the same structure, which route this one replaced, and why one test approach was chosen over another. Each is rewritten to state the property that holds now. Renames the_old_enclave_keys_route_is_gone, whose name only made sense to someone who had seen the diff, to describe the invariant instead. The remaining bedrock references are licence attribution pointing at client/verifier-client/NOTICE and stay. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
| let response = state | ||
| .enclave_client() | ||
| .get_enclave_keys() | ||
| .await | ||
| .map_err(|error| AppError::enclave_assignment(&error))?; |
There was a problem hiding this comment.
| let response = state | |
| .enclave_client() | |
| .get_enclave_keys() | |
| .await | |
| .map_err(|error| AppError::enclave_assignment(&error))?; | |
| let response = state | |
| .enclave_client() | |
| .get_enclave_keys() | |
| .await?; |
nit, there is a rust trait (forgetting which) you can implement that would allow you to drop this from and just rely on ? (This would abstract all the error logic in the error.rs and errors would propagate almost like magic to AppError)
There was a problem hiding this comment.
this a personal preference, feel free to skip
There was a problem hiding this comment.
nit. In the futre lets have different test file for each route, makes it more readable and move send() to common utils
| Ok(public_key.into_vec()) | ||
| /// Loads the client configuration named by `VERIFIER_CONFIG`. Schema is in the README. | ||
| fn load_config() -> Result<Config> { | ||
| let path = env::var("VERIFIER_CONFIG") |
There was a problem hiding this comment.
nit. Should we have an example of how the VERIFIER_CONFIG should look like here?
2 participants
Implements spec §6
POST /v1/enclave-assignmentand adds the first real Nitro attestation verification in this repo.Host
Replaces
GET /v1/enclave/keys, which was not in the spec. The response is the attestation document and nothing else:{ "attestation": "base64" }module_idand the certificate'snotAfterare already in the document, and clients must verify it before trusting either, so echoing them as unsigned JSON would restate signed data and add a mismatch case. The host relays opaque bytes it never parses. The spec has been updated to match.Errors are classified instead of collapsing into 503. Timeouts are 504, transport and not-ready are 503, and a match-path error that cannot occur here is 500.
failure_classlives onEnclaveClientError, so/v1/matchesnow emits the same structured fields rather than triaging identical enclave failures differently.The signing-key attestation is no longer publicly reachable. It belongs to the Key Registry flow.
Client
New
client/verifier-client. The verifier is ported fromworldcoin/bedrock'snitro_enclavemodule (MIT, Tools for Humanity), which ships in World App, the authenticator that calls this endpoint. Both sides therefore run the same logic rather than two readings of the AWS spec.Divergences from bedrock:
nowis a parameter, not acfg(test)bypass of the certificate time checkSig_structureusescoset::tbs_datainstead of being built by handAWS Nitro Root G1 is vendored and matches the certificate fingerprint AWS publishes. A test asserts it.
Configuration follows
world_id_primitives::Config. One struct covers the host URL, the measurement policy and the request bounds, with private fields and accessors, a validating constructor, andfrom_json. Nothing is read from the environment. A configuration that pins no measurements is rejected at construction, because with nothing pinned verification only proves a document came from some enclave.e2e
The harness used
parse_raw_attestation_doc, which extracts the COSE payload with the verification key set toNone, so it sealed to whatever key the document claimed. It now verifies, and takes its encryption key over HTTP through the host, so the assignment route and the client are exercised together rather than the client only ever meeting a stub. The signing key is not part of an assignment, so it still comes over vsock.Not included
TimeoutLayeror body limit.api/src/server.rsstill applies onlyTraceLayer.shared/api-typescrate would fix that and gets more worthwhile with each operation added.Test plan
cargo fmt --all -- --check cargo clippy --locked --all-targets --all-features -- cargo test --locked --all cargo deny check bans licenses sources14 verifier and config tests over a real attestation document captured from a live enclave, mostly negative: wrong root, expired chain, corrupted signature, mismatched PCRs, empty PCR configuration, debug-mode measurements. 3 client tests drive the HTTP path against a stub host over a real socket.
deny.tomlgains ISC (webpki) and CDLA-Permissive-2.0 (webpki-roots), matching bedrock's config. No openssl in the lock.On a Nitro host, with a client config naming the host and the PCR0 from
scripts/build-eif.sh(schema in the README):The second must fail closed with a deliberately wrong PCR0.