ref(security): Stop accepting Expect-CT, HPKP, and Expect-Staple reports - #6230
ref(security): Stop accepting Expect-CT, HPKP, and Expect-Staple reports#6230mrduncan wants to merge 3 commits into
Conversation
HPKP, Expect-CT, and Expect-Staple are dead in all shipping browsers, so these report types can no longer be produced. Relay now rejects them at ingest: the security report classifier no longer recognizes them, and the dedicated expect-ct-report / expect-staple-report content types are refused at the endpoint. The interface types, EventType variants, and Event fields are intentionally retained for a later full-removal PR that coordinates with the Sentry backend. Refs getsentry/sentry#119638 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
jjbayer
left a comment
There was a problem hiding this comment.
Looks reasonable to me. We'll have to remove the sentry-side relay_integration tests in order for CI to pass.
To fully stop ingestion we'll also have to remove it from the /// HPKP (security) reports.
#[metastructure(pii = "true", legacy_alias = "sentry.interfaces.Hpkp")]
#[metastructure(omit_from_schema)] // we only document error events for now
pub hpkp: Annotated<Hpkp>,
/// ExpectCT (security) reports.
#[metastructure(pii = "true", legacy_alias = "sentry.interfaces.ExpectCT")]
#[metastructure(omit_from_schema)] // we only document error events for now
pub expectct: Annotated<ExpectCt>,
/// ExpectStaple (security) reports.
#[metastructure(pii = "true", legacy_alias = "sentry.interfaces.ExpectStaple")]
#[metastructure(omit_from_schema)] // we only document error events for now
pub expectstaple: Annotated<ExpectStaple>,Can be done in a follow-up, but maybe easier to do it in one sweep. Without making these changes, outdated customer hosted Relays and custom clients can still send these reports. |
…ts (#119685) Relay is dropping ingest support for Expect-CT, HPKP, and Expect-Staple security reports in getsentry/relay#6230 — these report types are dead in all shipping browsers (HPKP removed from Chrome 72 / Firefox 72; Expect-CT removed from Chromium 107; Expect-Staple never shipped broadly) and can no longer be produced. This unblocks removing support from Relay, which uses these integration tests. Refs getsentry/relay#6230 Refs #119638 Co-authored-by: Claude <noreply@anthropic.com>
Follow-up to 9fbaceb, which stopped accepting these reports at ingest but deliberately kept the protocol surface for a coordinated removal. This does that removal: the EventType variants, the hpkp / expectct / expectstaple fields on the Event schema, and the now-unreachable interface types and their raw parsers. Ingestion behavior is unchanged -- reports are already rejected at the endpoint with an `invalid` outcome. What changes is that an event which an older upstream Relay already classified as one of these types is now forwarded as a `default` event, so it counts against the error quota rather than the security quota. getsentry/sentry#121028 removed the Sentry-side tests that exercised these types through Relay's normalizer, so this no longer breaks Sentry CI. Removing the dead Sentry source (eventtypes, interfaces, grouping strategies) is a follow-up that must land after Sentry bumps its sentry-relay dependency. Refs getsentry/sentry#119638 Refs getsentry/sentry#121028 Co-Authored-By: Claude <noreply@anthropic.com>
| @@ -83,15 +81,6 @@ fn event_from_security_report( | |||
|
|
|||
| let (apply_result, event_type) = match report_type { | |||
There was a problem hiding this comment.
Security report JSON parsed without depth limit via unbounded serde_json deserialization
Attacker-controlled payload bytes reach serde_json::from_slice through an untagged enum (CspVariant) with no caller-side nesting or depth guard, allowing a deeply-nested JSON payload to overflow the stack during parsing. The existing ~1 MiB byte-size cap does not prevent the attack because a 1 MiB payload of nested objects or arrays can contain ~100 k–500 k nesting levels.
Evidence
item.payload()inrelay-server/src/processing/errors/errors/raw_security.rsreceives untrusted request bytes already bounded tomax_event_size()(~1 MiB by default) viaDefaultBodyLimitandcheck_envelope_size_limits.- Those bytes are passed to
Csp::apply_to_event(data, &mut event), which callsserde_json::from_slice::<CspVariant>(data)atrelay-event-schema/src/protocol/security_report.rs:585. CspVariantis an untagged enum (#[serde(untagged)]), soserdebuffers the entire JSON value into a recursiveContenttree before matching variants.- A 1 MiB payload of nested objects or arrays can reach ~100 k–500 k nesting levels, enough to overflow the native stack during recursive deserialization, causing an uncatchable process-wide abort.
- No caller-side JSON depth limit is applied; by contrast, MessagePack deserialization in the same codebase (
relay-server/src/utils/rmp.rs) caps nesting atMAX_DEPTHto protect against the same class of attack.
Identified by Warden · wrdn-dos-review · FJM-GJW
HPKP, Expect-CT, and Expect-Staple are dead in all shipping browsers, so these report types can no longer be produced. Relay now rejects them at ingest: the security report classifier no longer recognizes them, and the dedicated expect-ct-report / expect-staple-report content types are refused at the endpoint.
This is the ~simplest change so that we stop accepting these reports and keep this PR manageable. The remaining cleanup should be straightforward as a follow up.
Planned removal: August 17th.
Refs getsentry/sentry#119638