flashbots / flashbots/attested-tls
Verifier fetches collateral and verifies quotes the policy will always reject
- Dominant language
- Rust
- Stars
- 5
- Forks
- 3
- Avg merge
- 4d 1h
- Merged PRs (30d)
- 8
Description
Found this while I was reviewing #70 because the no-cache mode is what makes the fetch happen on every message.
It's in the verify path, not the construction API which is why I'm creating this separate issue however.
Have not reviewed it myself, so if Claude hallucinated this feel free to close.
## Claude Analysis
The policy check runs after verification, so a peer can make the verifier do
the full DCAP work — including an outbound collateral fetch — for an
attestation type the policy never accepts.
### What happens
`verify_attestation` matches on the attestation type carried by the incoming
message. `has_remote_attestation()` is consulted only in the
`AttestationType::None` arm (`crates/attestation/src/lib.rs:475`). The
`AzureTdx` and `DcapTdx | GcpTdx` arms verify unconditionally, and the policy
check happens afterwards at `crates/attestation/src/lib.rs:527`:
```rust
let measurements = match attestation_type {
AttestationType::None => {
if self.has_remote_attestation() { ... } // only checked here
}
AttestationType::DcapTdx | AttestationType::GcpTdx => {
let (measurements, quote) = dcap::verify_dcap_attestation(...).await?;
...
}
};
// Do a measurement / attestation type policy check
self.measurement_policy.check_measurement_with_gcp_cache(...)?;
```
So both of these do the full work before rejecting:
- A verifier built with `expect_none()` that receives a DCAP quote.
- A policy accepting only `AzureTdx` that receives a `DcapTdx` quote.
`verify_attestation_sync` has the same shape (`lib.rs:551` and `lib.rs:608`).
### Impact
Per connection, for a quote the policy can never accept, an unauthenticated
peer gets the verifier to run full DCAP signature verification and — on a
cache miss — an outbound HTTPS fetch to the PCCS endpoint or Intel PCS. The
rejection still happens, so this is wasted work rather than a soundness
problem. It matters most with no in-process cache, where every such message
is a fresh fetch.
### Suggested fix
`MeasurementRecord` already carries the type
(`crates/attestation/src/measurements.rs:323`), so the policy can answer this
directly:
```rust
impl MeasurementPolicy {
/// Whether any accepted record covers this attestation type
pub fn accepts_attestation_type(&self, attestation_type: AttestationType) -> bool {
self.accepted_measurements.iter().any(|r| r.attestation_type == attestation_type)
}
}
```
Then reject at the top of both verify functions, before the match:
```rust
if !self.measurement_policy.accepts_attestation_type(attestation_type) {
return Err(AttestationError::AttestationTypeNotAccepted);
}
```
That also subsumes the existing `has_remote_attestation()` check in the
`None` arm, which could then go away.
Contributor guide
No contributing guide indexed for this repository
Research direction
Start in crates/attestation/src/lib.rs at verify_attestation and verify_attestation_sync, then read MeasurementPolicy and MeasurementRecord in crates/attestation/src/measurements.rs. Trace how attestation types are matched and when the policy check runs. Done means unsupported types are rejected before DCAP verification or collateral fetching in both paths, without changing accepted verification behavior.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- rust
- Domain
- security
- Issue type
- Bug
- Difficulty
- 3/5
- Estimated time
- 1-2 days
- Activity status
- Active
- Clarity
- Mostly clear
- Newbie friendliness
- 68/100