fix(azure): bound attestation evidence payload size#52
Open
samlaf wants to merge 1 commit into
Open
Conversation
Reject serialized Azure attestation evidence larger than 128 KiB on every verification entry point, before deserializing untrusted bytes into owned strings and byte vectors. This includes get_measurements, which parses evidence without verifying it. Enforce the same bound after generation so a node fails fast instead of emitting evidence that verifiers would reject. Transports are expected to bound reads at the network edge before the payload is ever buffered; this is the library's own fail-closed limit so verification stays safe regardless of which transport delivered the evidence. On the attested-TLS path the evidence travels inside the peer certificate, which rustls already caps at 64 KiB, so this check is a backstop there rather than the primary defense. TODO at the network edges, in follow-ups: - explicitly bound reads on attested-TLS connections rather than relying on rustls' internal (pub(crate), unconfigurable) certificate size cap, if rustls exposes a way to do so - cap attestation_provider_client's HTTP response read, which currently buffers the response unbounded (reqwest .bytes()) before decoding and verification
Collaborator
|
Re:
Created #53 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Partially fixes #50
Reject serialized Azure attestation evidence larger than 128 KiB on every verification entry point, before deserializing untrusted bytes into owned strings and byte vectors. This includes get_measurements, which parses evidence without verifying it. Enforce the same bound after generation so a node fails fast instead of emitting evidence that verifiers would reject.
Realized this isn't enough, and the real DDoS mitigation should happen at the network boundary. Transports should bound reads at the network edge before the payload is ever buffered; this is the library's own fail-closed limit so verification stays safe regardless of which transport delivered the evidence. On the attested-TLS path the evidence travels inside the peer certificate, which rustls already caps at 64 KiB, so this check is a backstop there rather than the primary defense.
TODO (follow-ups):
These touch parts of the library that I don't know anything about (I've only worked with Azure). Also unsure if generic bounds would work for both azure and gcp, as gcp probably has smaller attestations I would guess? In any case probably worth creating a separate issue for anyone who has more context than me.