flashbots / flashbots/attested-tls

Verifier fetches collateral and verifies quotes the policy will always reject

Open
#87 1 comment 0 reactions 0 assignees View on GitHub
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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.