diff --git a/CHANGELOG.md b/CHANGELOG.md index a08f6ecb18b..dd5b34d64cb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,13 @@ ## Unreleased +**Breaking Changes**: + +- Stop accepting the deprecated Expect-CT, HPKP, and Expect-Staple security reports and remove their + event types and event schema fields. Such reports are now rejected at ingest with an `invalid` + outcome (`security_report_type`), including events which an older upstream Relay already classified + as `hpkp`, `expectct`, or `expectstaple`. ([#6230](https://github.com/getsentry/relay/pull/6230)) + **Features**: - Raise the size limit for the flags context to 128 KiB. ([#6310](https://github.com/getsentry/relay/pull/6310)) diff --git a/relay-base-schema/src/data_category.rs b/relay-base-schema/src/data_category.rs index aea4e4261c1..1e16a3dee49 100644 --- a/relay-base-schema/src/data_category.rs +++ b/relay-base-schema/src/data_category.rs @@ -28,7 +28,7 @@ pub enum DataCategory { /// /// SDK rate limiting behavior: apply to the entire envelope if it contains an item `transaction`. Transaction = 2, - /// Events with an event type of `csp`, `hpkp`, `expectct` and `expectstaple`. + /// Events with an event type of `csp`. /// /// SDK rate limiting behavior: ignore. Security = 3, @@ -400,9 +400,7 @@ impl From for DataCategory { match ty { EventType::Default | EventType::Error => Self::Error, EventType::Transaction => Self::Transaction, - EventType::Csp | EventType::Hpkp | EventType::ExpectCt | EventType::ExpectStaple => { - Self::Security - } + EventType::Csp => Self::Security, EventType::UserReportV2 => Self::UserReportV2, } } diff --git a/relay-base-schema/src/events.rs b/relay-base-schema/src/events.rs index 4639e39e19d..1c67585c2c8 100644 --- a/relay-base-schema/src/events.rs +++ b/relay-base-schema/src/events.rs @@ -16,9 +16,8 @@ use serde::{Deserialize, Serialize}; /// /// - **Error monitoring events** (`default`, `error`): Processed and grouped into unique issues /// based on their exception stack traces and error messages. -/// - **Security events** (`csp`, `hpkp`, `expectct`, `expectstaple`): Derived from Browser -/// security violation reports and grouped into unique issues based on the endpoint and -/// violation. SDKs do not send such events. +/// - **Security events** (`csp`): Derived from Browser security violation reports and grouped into +/// unique issues based on the endpoint and violation. SDKs do not send such events. /// - **Transaction events** (`transaction`): Contain operation spans and collected into traces for /// performance monitoring. #[derive( @@ -30,12 +29,6 @@ pub enum EventType { Error, /// A CSP violation payload. Csp, - /// An HPKP violation payload. - Hpkp, - /// An ExpectCT violation payload. - ExpectCt, - /// An ExpectStaple violation payload. - ExpectStaple, /// Performance monitoring transactions carrying spans. Transaction, /// User feedback payload. @@ -55,9 +48,6 @@ impl EventType { EventType::Default => "default", EventType::Error => "error", EventType::Csp => "csp", - EventType::Hpkp => "hpkp", - EventType::ExpectCt => "expectct", - EventType::ExpectStaple => "expectstaple", EventType::Transaction => "transaction", EventType::UserReportV2 => "feedback", } @@ -84,9 +74,6 @@ impl FromStr for EventType { "default" => EventType::Default, "error" => EventType::Error, "csp" => EventType::Csp, - "hpkp" => EventType::Hpkp, - "expectct" => EventType::ExpectCt, - "expectstaple" => EventType::ExpectStaple, "transaction" => EventType::Transaction, "feedback" => EventType::UserReportV2, _ => return Err(ParseEventTypeError), diff --git a/relay-cabi/include/relay.h b/relay-cabi/include/relay.h index 5f023f2625e..aa1d4e747c5 100644 --- a/relay-cabi/include/relay.h +++ b/relay-cabi/include/relay.h @@ -35,7 +35,7 @@ enum RelayDataCategory { */ RELAY_DATA_CATEGORY_TRANSACTION = 2, /** - * Events with an event type of `csp`, `hpkp`, `expectct` and `expectstaple`. + * Events with an event type of `csp`. * * SDK rate limiting behavior: ignore. */ diff --git a/relay-event-normalization/src/event.rs b/relay-event-normalization/src/event.rs index 1074cc0145b..0a039ae269a 100644 --- a/relay-event-normalization/src/event.rs +++ b/relay-event-normalization/src/event.rs @@ -418,9 +418,6 @@ fn normalize_security_report( fn is_security_report(event: &Event) -> bool { event.csp.value().is_some() - || event.expectct.value().is_some() - || event.expectstaple.value().is_some() - || event.hpkp.value().is_some() } /// Backfills IP addresses in various places. @@ -1222,8 +1219,11 @@ pub fn is_valid_platform(platform: &str) -> bool { VALID_PLATFORMS.contains(&platform) } -/// Infers the `EventType` from the event's interfaces. -fn infer_event_type(event: &Event) -> EventType { +/// Infers the [`EventType`] from the event's interfaces. +/// +/// This is the type normalization assigns. A declared type is only honoured for transactions and +/// user feedback. +pub fn infer_event_type(event: &Event) -> EventType { // The event type may be set explicitly when constructing the event items from specific // items. This is DEPRECATED, and each distinct event type may get its own base class. For // the time being, this is only implemented for transactions, so be specific: @@ -1246,12 +1246,6 @@ fn infer_event_type(event: &Event) -> EventType { EventType::Error } else if event.csp.value().is_some() { EventType::Csp - } else if event.hpkp.value().is_some() { - EventType::Hpkp - } else if event.expectct.value().is_some() { - EventType::ExpectCt - } else if event.expectstaple.value().is_some() { - EventType::ExpectStaple } else { EventType::Default } diff --git a/relay-event-normalization/src/lib.rs b/relay-event-normalization/src/lib.rs index 0e8e9946f0f..97c8c67d1bc 100644 --- a/relay-event-normalization/src/lib.rs +++ b/relay-event-normalization/src/lib.rs @@ -28,7 +28,8 @@ mod validation; pub use validation::{EventValidationConfig, validate_event, validate_standalone_span}; pub mod replay; pub use event::{ - NormalizationConfig, normalize_event, normalize_measurements, normalize_performance_score, + NormalizationConfig, infer_event_type, normalize_event, normalize_measurements, + normalize_performance_score, }; pub use normalize::breakdowns::*; pub use normalize::*; diff --git a/relay-event-schema/src/protocol/event.rs b/relay-event-schema/src/protocol/event.rs index 3781b773c55..fee96144012 100644 --- a/relay-event-schema/src/protocol/event.rs +++ b/relay-event-schema/src/protocol/event.rs @@ -12,10 +12,10 @@ use uuid::Uuid; use crate::processor::ProcessValue; use crate::protocol::{ AppContext, Breadcrumb, Breakdowns, BrowserContext, ClientSdkInfo, Contexts, Csp, DebugMeta, - DefaultContext, DeviceContext, EventType, Exception, ExpectCt, ExpectStaple, Fingerprint, - GpuContext, Hpkp, LenientString, Level, LogEntry, Measurements, Metrics, MonitorContext, - OsContext, ProfileContext, RelayInfo, Request, ResponseContext, RuntimeContext, Span, SpanId, - Stacktrace, Tags, TemplateInfo, Thread, Timestamp, TraceContext, TransactionInfo, User, Values, + DefaultContext, DeviceContext, EventType, Exception, Fingerprint, GpuContext, LenientString, + Level, LogEntry, Measurements, Metrics, MonitorContext, OsContext, ProfileContext, RelayInfo, + Request, ResponseContext, RuntimeContext, Span, SpanId, Stacktrace, Tags, TemplateInfo, Thread, + Timestamp, TraceContext, TransactionInfo, User, Values, }; /// Wrapper around a UUID with slightly different formatting. @@ -444,21 +444,6 @@ pub struct Event { #[metastructure(omit_from_schema)] // we only document error events for now pub csp: Annotated, - /// 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, - - /// 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, - - /// 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, - /// Spans for tracing. #[metastructure(max_bytes = 819200)] #[metastructure(omit_from_schema)] // we only document error events for now diff --git a/relay-event-schema/src/protocol/security_report.rs b/relay-event-schema/src/protocol/security_report.rs index 499600cf569..67e948f18c1 100644 --- a/relay-event-schema/src/protocol/security_report.rs +++ b/relay-event-schema/src/protocol/security_report.rs @@ -1,14 +1,13 @@ //! Contains definitions for the security report interfaces. //! -//! The security interfaces are CSP, HPKP, ExpectCT and ExpectStaple. +//! The security interface is CSP. use std::borrow::Cow; use std::collections::BTreeMap; use std::fmt::{self, Write}; -use chrono::{DateTime, Utc}; -use relay_protocol::{Annotated, Array, Empty, FromValue, IntoValue, Object, Value}; -use serde::de::{self, Error, IgnoredAny}; +use relay_protocol::{Annotated, Empty, FromValue, IntoValue, Object, Value}; +use serde::de::{self, IgnoredAny}; use serde::{Deserialize, Deserializer, Serialize}; use url::Url; @@ -607,557 +606,9 @@ impl Csp { } } -#[derive(Clone, Copy, Debug, PartialEq, Eq)] -enum ExpectCtStatus { - Unknown, - Valid, - Invalid, -} - -relay_common::derive_fromstr_and_display!(ExpectCtStatus, InvalidSecurityError, { - ExpectCtStatus::Unknown => "unknown", - ExpectCtStatus::Valid => "valid", - ExpectCtStatus::Invalid => "invalid", -}); - -relay_common::impl_str_serde!(ExpectCtStatus, "an expect-ct status"); - -#[derive(Clone, Copy, Debug, PartialEq, Eq)] -enum ExpectCtSource { - TlsExtension, - Ocsp, - Embedded, -} - -relay_common::derive_fromstr_and_display!(ExpectCtSource, InvalidSecurityError, { - ExpectCtSource::TlsExtension => "tls-extension", - ExpectCtSource::Ocsp => "ocsp", - ExpectCtSource::Embedded => "embedded", -}); - -relay_common::impl_str_serde!(ExpectCtSource, "an expect-ct source"); - -#[derive(Clone, Debug, PartialEq, Deserialize, Serialize)] -struct SingleCertificateTimestampRaw { - version: Option, - status: Option, - source: Option, - serialized_sct: Option, // NOT kebab-case! -} - -impl SingleCertificateTimestampRaw { - fn into_protocol(self) -> SingleCertificateTimestamp { - SingleCertificateTimestamp { - version: Annotated::from(self.version), - status: Annotated::from(self.status.map(|s| s.to_string())), - source: Annotated::from(self.source.map(|s| s.to_string())), - serialized_sct: Annotated::from(self.serialized_sct), - } - } -} - -#[derive(Clone, Debug, Default, PartialEq, Deserialize, Serialize)] -#[serde(rename_all = "kebab-case")] -struct ExpectCtRaw { - #[serde(with = "serde_date_time_3339")] - date_time: Option>, - hostname: String, - port: Option, - scheme: Option, - #[serde(with = "serde_date_time_3339")] - effective_expiration_date: Option>, - served_certificate_chain: Option>, - validated_certificate_chain: Option>, - scts: Option>, - failure_mode: Option, - test_report: Option, -} - -mod serde_date_time_3339 { - use serde::de::Visitor; - - use super::*; - - pub fn serialize(date_time: &Option>, serializer: S) -> Result - where - S: serde::Serializer, - { - match date_time { - None => serializer.serialize_none(), - Some(d) => serializer.serialize_str(&d.to_rfc3339()), - } - } - - pub fn deserialize<'de, D>(deserializer: D) -> Result>, D::Error> - where - D: serde::Deserializer<'de>, - { - struct DateTimeVisitor; - - impl Visitor<'_> for DateTimeVisitor { - type Value = Option>; - - fn expecting(&self, formatter: &mut fmt::Formatter) -> fmt::Result { - formatter.write_str("expected a date-time in RFC 3339 format") - } - - fn visit_str(self, s: &str) -> Result - where - E: serde::de::Error, - { - DateTime::parse_from_rfc3339(s) - .map(|d| Some(d.with_timezone(&Utc))) - .map_err(serde::de::Error::custom) - } - - fn visit_none(self) -> Result - where - E: Error, - { - Ok(None) - } - } - - deserializer.deserialize_any(DateTimeVisitor) - } -} - -impl ExpectCtRaw { - fn get_message(&self) -> String { - format!("Expect-CT failed for '{}'", self.hostname) - } - - fn into_protocol(self) -> ExpectCt { - ExpectCt { - date_time: Annotated::from(self.date_time.map(|d| d.to_rfc3339())), - hostname: Annotated::from(self.hostname), - port: Annotated::from(self.port), - scheme: Annotated::from(self.scheme), - effective_expiration_date: Annotated::from( - self.effective_expiration_date.map(|d| d.to_rfc3339()), - ), - served_certificate_chain: Annotated::new( - self.served_certificate_chain - .map(|s| s.into_iter().map(Annotated::from).collect()) - .unwrap_or_default(), - ), - - validated_certificate_chain: Annotated::new( - self.validated_certificate_chain - .map(|v| v.into_iter().map(Annotated::from).collect()) - .unwrap_or_default(), - ), - scts: Annotated::from(self.scts.map(|scts| { - scts.into_iter() - .map(|elm| Annotated::from(elm.into_protocol())) - .collect() - })), - failure_mode: Annotated::from(self.failure_mode), - test_report: Annotated::from(self.test_report), - } - } - - fn get_culprit(&self) -> String { - self.hostname.clone() - } - - fn get_tags(&self) -> Tags { - let mut tags = vec![Annotated::new(TagEntry( - Annotated::new("hostname".to_owned()), - Annotated::new(self.hostname.clone()), - ))]; - - if let Some(port) = self.port { - tags.push(Annotated::new(TagEntry( - Annotated::new("port".to_owned()), - Annotated::new(port.to_string()), - ))); - } - - Tags(PairList::from(tags)) - } - - fn get_request(&self) -> Request { - Request { - url: Annotated::from(self.hostname.clone()), - ..Request::default() - } - } -} - -#[derive(Clone, Debug, Default, PartialEq, Deserialize, Serialize)] -#[serde(rename_all = "kebab-case")] -struct ExpectCtReportRaw { - expect_ct_report: ExpectCtRaw, -} - -/// Object used in ExpectCt reports -/// -/// See . -#[derive(Clone, Debug, Default, PartialEq, Empty, FromValue, IntoValue, ProcessValue)] -pub struct SingleCertificateTimestamp { - pub version: Annotated, - pub status: Annotated, - pub source: Annotated, - pub serialized_sct: Annotated, -} - -/// Expect CT security report sent by user agent (browser). -/// -/// See -#[derive(Clone, Debug, Default, PartialEq, Empty, FromValue, IntoValue, ProcessValue)] -pub struct ExpectCt { - /// Date time in rfc3339 format YYYY-MM-DDTHH:MM:DD{.FFFFFF}(Z|+/-HH:MM) - /// UTC time that the UA observed the CT compliance failure - pub date_time: Annotated, - /// The hostname to which the UA made the original request that failed the CT compliance check. - pub hostname: Annotated, - pub port: Annotated, - pub scheme: Annotated, - /// Date time in rfc3339 format - pub effective_expiration_date: Annotated, - pub served_certificate_chain: Annotated>, - pub validated_certificate_chain: Annotated>, - pub scts: Annotated>, - pub failure_mode: Annotated, - pub test_report: Annotated, -} - -impl ExpectCt { - pub fn apply_to_event(data: &[u8], event: &mut Event) -> Result<(), serde_json::Error> { - let raw_report = serde_json::from_slice::(data)?; - let raw_expect_ct = raw_report.expect_ct_report; - - event.logentry = Annotated::new(LogEntry::from(raw_expect_ct.get_message())); - event.culprit = Annotated::new(raw_expect_ct.get_culprit()); - event.tags = Annotated::new(raw_expect_ct.get_tags()); - event.request = Annotated::new(raw_expect_ct.get_request()); - event.expectct = Annotated::new(raw_expect_ct.into_protocol()); - - Ok(()) - } -} - -/// Defines external, RFC-defined schema we accept, while `Hpkp` defines our own schema. -/// -/// See `Hpkp` for meaning of fields. -#[derive(Clone, Debug, Default, PartialEq, Deserialize, Serialize)] -#[serde(rename_all = "kebab-case")] -struct HpkpRaw { - #[serde(skip_serializing_if = "Option::is_none")] - date_time: Option>, - hostname: String, - #[serde(skip_serializing_if = "Option::is_none")] - port: Option, - #[serde(skip_serializing_if = "Option::is_none")] - effective_expiration_date: Option>, - #[serde(skip_serializing_if = "Option::is_none")] - include_subdomains: Option, - #[serde(skip_serializing_if = "Option::is_none")] - noted_hostname: Option, - #[serde(skip_serializing_if = "Option::is_none")] - served_certificate_chain: Option>, - #[serde(skip_serializing_if = "Option::is_none")] - validated_certificate_chain: Option>, - known_pins: Vec, - #[serde(flatten)] - other: BTreeMap, -} - -impl HpkpRaw { - fn get_message(&self) -> String { - format!( - "Public key pinning validation failed for '{}'", - self.hostname - ) - } - - fn into_protocol(self) -> Hpkp { - Hpkp { - date_time: Annotated::from(self.date_time.map(|d| d.to_rfc3339())), - hostname: Annotated::new(self.hostname), - port: Annotated::from(self.port), - effective_expiration_date: Annotated::from( - self.effective_expiration_date.map(|d| d.to_rfc3339()), - ), - include_subdomains: Annotated::from(self.include_subdomains), - noted_hostname: Annotated::from(self.noted_hostname), - served_certificate_chain: Annotated::from( - self.served_certificate_chain - .map(|chain| chain.into_iter().map(Annotated::from).collect()), - ), - validated_certificate_chain: Annotated::from( - self.validated_certificate_chain - .map(|chain| chain.into_iter().map(Annotated::from).collect()), - ), - known_pins: Annotated::new(self.known_pins.into_iter().map(Annotated::from).collect()), - other: self - .other - .into_iter() - .map(|(k, v)| (k, Annotated::from(v))) - .collect(), - } - } - - fn get_tags(&self) -> Tags { - let mut tags = vec![Annotated::new(TagEntry( - Annotated::new("hostname".to_owned()), - Annotated::new(self.hostname.clone()), - ))]; - - if let Some(port) = self.port { - tags.push(Annotated::new(TagEntry( - Annotated::new("port".to_owned()), - Annotated::new(port.to_string()), - ))); - } - - if let Some(include_subdomains) = self.include_subdomains { - tags.push(Annotated::new(TagEntry( - Annotated::new("include-subdomains".to_owned()), - Annotated::new(include_subdomains.to_string()), - ))); - } - - Tags(PairList::from(tags)) - } - - fn get_request(&self) -> Request { - Request { - url: Annotated::from(self.hostname.clone()), - ..Request::default() - } - } -} - -/// Schema as defined in RFC7469, Section 3 -#[derive(Clone, Debug, Default, PartialEq, Empty, FromValue, IntoValue, ProcessValue)] -pub struct Hpkp { - /// Indicates the time the UA observed the Pin Validation failure. - pub date_time: Annotated, - /// Hostname to which the UA made the original request that failed Pin Validation. - pub hostname: Annotated, - /// The port to which the UA made the original request that failed Pin Validation. - pub port: Annotated, - /// Effective Expiration Date for the noted pins. - pub effective_expiration_date: Annotated, - /// Indicates whether or not the UA has noted the includeSubDomains directive for the Known - /// Pinned Host. - pub include_subdomains: Annotated, - - /// Indicates the hostname that the UA noted when it noted the Known Pinned Host. This field - /// allows operators to understand why Pin Validation was performed for, e.g., foo.example.com - /// when the noted Known Pinned Host was example.com with includeSubDomains set. - pub noted_hostname: Annotated, - /// The certificate chain, as served by the Known Pinned Host during TLS session setup. It - /// is provided as an array of strings; each string pem1, ... pemN is the Privacy-Enhanced Mail - /// (PEM) representation of each X.509 certificate as described in [RFC7468]. - /// - /// [RFC7468]: https://tools.ietf.org/html/rfc7468 - pub served_certificate_chain: Annotated>, - /// The certificate chain, as constructed by the UA during certificate chain verification. - pub validated_certificate_chain: Annotated>, - - /// Pins that the UA has noted for the Known Pinned Host. - // TODO: regex this string for 'pin-sha256="ABC123"' syntax - #[metastructure(required = true)] - pub known_pins: Annotated>, - - #[metastructure(pii = "true", additional_properties)] - pub other: Object, -} - -impl Hpkp { - pub fn apply_to_event(data: &[u8], event: &mut Event) -> Result<(), serde_json::Error> { - let raw_hpkp = serde_json::from_slice::(data)?; - - event.logentry = Annotated::new(LogEntry::from(raw_hpkp.get_message())); - event.tags = Annotated::new(raw_hpkp.get_tags()); - event.request = Annotated::new(raw_hpkp.get_request()); - event.hpkp = Annotated::new(raw_hpkp.into_protocol()); - - Ok(()) - } -} - -/// Defines external, RFC-defined schema we accept, while `ExpectStaple` defines our own schema. -/// -/// See `ExpectStaple` for meaning of fields. -#[derive(Clone, Debug, Default, PartialEq, Deserialize, Serialize)] -#[serde(rename_all = "kebab-case")] -struct ExpectStapleReportRaw { - expect_staple_report: ExpectStapleRaw, -} - -#[derive(Clone, Copy, Debug, PartialEq, Eq)] -pub enum ExpectStapleResponseStatus { - Missing, - Provided, - ErrorResponse, - BadProducedAt, - NoMatchingResponse, - InvalidDate, - ParseResponseError, - ParseResponseDataError, -} - -relay_common::derive_fromstr_and_display!(ExpectStapleResponseStatus, InvalidSecurityError, { - ExpectStapleResponseStatus::Missing => "MISSING", - ExpectStapleResponseStatus::Provided => "PROVIDED", - ExpectStapleResponseStatus::ErrorResponse => "ERROR_RESPONSE", - ExpectStapleResponseStatus::BadProducedAt => "BAD_PRODUCED_AT", - ExpectStapleResponseStatus::NoMatchingResponse => "NO_MATCHING_RESPONSE", - ExpectStapleResponseStatus::InvalidDate => "INVALID_DATE", - ExpectStapleResponseStatus::ParseResponseError => "PARSE_RESPONSE_ERROR", - ExpectStapleResponseStatus::ParseResponseDataError => "PARSE_RESPONSE_DATA_ERROR", -}); - -relay_common::impl_str_serde!(ExpectStapleResponseStatus, "an expect-ct response status"); - -#[derive(Clone, Copy, Debug, PartialEq, Eq)] -pub enum ExpectStapleCertStatus { - Good, - Revoked, - Unknown, -} - -relay_common::derive_fromstr_and_display!(ExpectStapleCertStatus, InvalidSecurityError, { - ExpectStapleCertStatus::Good => "GOOD", - ExpectStapleCertStatus::Revoked => "REVOKED", - ExpectStapleCertStatus::Unknown => "UNKNOWN", -}); - -relay_common::impl_str_serde!(ExpectStapleCertStatus, "an expect-staple cert status"); - -/// Inner (useful) part of a Expect Stable report as sent by a user agent ( browser) -#[derive(Clone, Debug, Default, PartialEq, Deserialize, Serialize)] -#[serde(rename_all = "kebab-case")] -struct ExpectStapleRaw { - #[serde(skip_serializing_if = "Option::is_none")] - date_time: Option>, - hostname: String, - #[serde(skip_serializing_if = "Option::is_none")] - port: Option, - #[serde(skip_serializing_if = "Option::is_none")] - effective_expiration_date: Option>, - #[serde(skip_serializing_if = "Option::is_none")] - response_status: Option, - #[serde(skip_serializing_if = "Option::is_none")] - ocsp_response: Option, - #[serde(skip_serializing_if = "Option::is_none")] - cert_status: Option, - #[serde(skip_serializing_if = "Option::is_none")] - served_certificate_chain: Option>, - #[serde(skip_serializing_if = "Option::is_none")] - validated_certificate_chain: Option>, -} - -impl ExpectStapleRaw { - fn get_message(&self) -> String { - format!("Expect-Staple failed for '{}'", self.hostname) - } - - fn into_protocol(self) -> ExpectStaple { - ExpectStaple { - date_time: Annotated::from(self.date_time.map(|d| d.to_rfc3339())), - hostname: Annotated::from(self.hostname), - port: Annotated::from(self.port), - effective_expiration_date: Annotated::from( - self.effective_expiration_date.map(|d| d.to_rfc3339()), - ), - response_status: Annotated::from(self.response_status.map(|rs| rs.to_string())), - cert_status: Annotated::from(self.cert_status.map(|cs| cs.to_string())), - served_certificate_chain: Annotated::from( - self.served_certificate_chain - .map(|cert_chain| cert_chain.into_iter().map(Annotated::from).collect()), - ), - validated_certificate_chain: Annotated::from( - self.validated_certificate_chain - .map(|cert_chain| cert_chain.into_iter().map(Annotated::from).collect()), - ), - ocsp_response: Annotated::from(self.ocsp_response), - } - } - - fn get_culprit(&self) -> String { - self.hostname.clone() - } - - fn get_tags(&self) -> Tags { - let mut tags = vec![Annotated::new(TagEntry( - Annotated::new("hostname".to_owned()), - Annotated::new(self.hostname.clone()), - ))]; - - if let Some(port) = self.port { - tags.push(Annotated::new(TagEntry( - Annotated::new("port".to_owned()), - Annotated::new(port.to_string()), - ))); - } - - if let Some(response_status) = self.response_status { - tags.push(Annotated::new(TagEntry( - Annotated::new("response_status".to_owned()), - Annotated::new(response_status.to_string()), - ))); - } - - if let Some(cert_status) = self.cert_status { - tags.push(Annotated::new(TagEntry( - Annotated::new("cert_status".to_owned()), - Annotated::new(cert_status.to_string()), - ))); - } - - Tags(PairList::from(tags)) - } - - fn get_request(&self) -> Request { - Request { - url: Annotated::from(self.hostname.clone()), - ..Request::default() - } - } -} - -/// Represents an Expect Staple security report. -/// -/// See for specification. -#[derive(Clone, Debug, Default, PartialEq, Empty, FromValue, IntoValue, ProcessValue)] -pub struct ExpectStaple { - date_time: Annotated, - hostname: Annotated, - port: Annotated, - effective_expiration_date: Annotated, - response_status: Annotated, - cert_status: Annotated, - served_certificate_chain: Annotated>, - validated_certificate_chain: Annotated>, - ocsp_response: Annotated, -} - -impl ExpectStaple { - pub fn apply_to_event(data: &[u8], event: &mut Event) -> Result<(), serde_json::Error> { - let raw_report = serde_json::from_slice::(data)?; - let raw_expect_staple = raw_report.expect_staple_report; - - event.logentry = Annotated::new(LogEntry::from(raw_expect_staple.get_message())); - event.culprit = Annotated::new(raw_expect_staple.get_culprit()); - event.tags = Annotated::new(raw_expect_staple.get_tags()); - event.request = Annotated::new(raw_expect_staple.get_request()); - event.expectstaple = Annotated::new(raw_expect_staple.into_protocol()); - - Ok(()) - } -} - #[derive(Clone, Debug, PartialEq, Eq)] pub enum SecurityReportType { Csp, - ExpectCt, - ExpectStaple, - Hpkp, Unsupported, } @@ -1173,9 +624,6 @@ impl SecurityReportType { #[serde(rename = "type")] ty: Option, csp_report: Option, - known_pins: Option, - expect_staple_report: Option, - expect_ct_report: Option, } let helper: SecurityReport = serde_json::from_slice(data)?; @@ -1186,12 +634,6 @@ impl SecurityReportType { Some(SecurityReportType::Csp) } else if let Some(CspViolationType::Other) = helper.ty { Some(SecurityReportType::Unsupported) - } else if helper.known_pins.is_some() { - Some(SecurityReportType::Hpkp) - } else if helper.expect_staple_report.is_some() { - Some(SecurityReportType::ExpectStaple) - } else if helper.expect_ct_report.is_some() { - Some(SecurityReportType::ExpectCt) } else { None }) @@ -1786,209 +1228,6 @@ mod tests { insta::assert_debug_snapshot!(message.as_str().unwrap(), @r###""Blocked 'style' from 'notlocalhost:8000'""###); } - #[test] - fn test_expectct_basic() { - let json = r#"{ - "expect-ct-report": { - "date-time": "2014-04-06T13:00:50Z", - "hostname": "www.example.com", - "port": 443, - "scheme": "https", - "effective-expiration-date": "2014-05-01T12:40:50Z", - "served-certificate-chain": ["-----BEGIN CERTIFICATE-----\n-----END CERTIFICATE-----"], - "validated-certificate-chain": ["-----BEGIN CERTIFICATE-----\n-----END CERTIFICATE-----"], - "scts": [ - { - "version": 1, - "status": "invalid", - "source": "embedded", - "serialized_sct": "ABCD==" - } - ], - "failure-mode": "enforce", - "test-report": false - } - }"#; - - let mut event = Event::default(); - ExpectCt::apply_to_event(json.as_bytes(), &mut event).unwrap(); - assert_annotated_snapshot!(Annotated::new(event), @r###" - { - "culprit": "www.example.com", - "logentry": { - "formatted": "Expect-CT failed for 'www.example.com'" - }, - "request": { - "url": "www.example.com" - }, - "tags": [ - [ - "hostname", - "www.example.com" - ], - [ - "port", - "443" - ] - ], - "expectct": { - "date_time": "2014-04-06T13:00:50+00:00", - "hostname": "www.example.com", - "port": 443, - "scheme": "https", - "effective_expiration_date": "2014-05-01T12:40:50+00:00", - "served_certificate_chain": [ - "-----BEGIN CERTIFICATE-----\n-----END CERTIFICATE-----" - ], - "validated_certificate_chain": [ - "-----BEGIN CERTIFICATE-----\n-----END CERTIFICATE-----" - ], - "scts": [ - { - "version": 1, - "status": "invalid", - "source": "embedded", - "serialized_sct": "ABCD==" - } - ], - "failure_mode": "enforce", - "test_report": false - } - } - "###); - } - - #[test] - fn test_expectct_invalid() { - let json = r#"{ - "hostname": "www.example.com", - "date_time": "Not an RFC3339 datetime" - }"#; - - let mut event = Event::default(); - ExpectCt::apply_to_event(json.as_bytes(), &mut event) - .expect_err("date_time should fail to parse"); - } - - #[test] - fn test_expectstaple_basic() { - let json = r#"{ - "expect-staple-report": { - "date-time": "2014-04-06T13:00:50Z", - "hostname": "www.example.com", - "port": 443, - "response-status": "ERROR_RESPONSE", - "cert-status": "REVOKED", - "effective-expiration-date": "2014-05-01T12:40:50Z", - "served-certificate-chain": ["-----BEGIN CERTIFICATE-----\n-----END CERTIFICATE-----"], - "validated-certificate-chain": ["-----BEGIN CERTIFICATE-----\n-----END CERTIFICATE-----"] - } - }"#; - - let mut event = Event::default(); - ExpectStaple::apply_to_event(json.as_bytes(), &mut event).unwrap(); - assert_annotated_snapshot!(Annotated::new(event), @r###" - { - "culprit": "www.example.com", - "logentry": { - "formatted": "Expect-Staple failed for 'www.example.com'" - }, - "request": { - "url": "www.example.com" - }, - "tags": [ - [ - "hostname", - "www.example.com" - ], - [ - "port", - "443" - ], - [ - "response_status", - "ERROR_RESPONSE" - ], - [ - "cert_status", - "REVOKED" - ] - ], - "expectstaple": { - "date_time": "2014-04-06T13:00:50+00:00", - "hostname": "www.example.com", - "port": 443, - "effective_expiration_date": "2014-05-01T12:40:50+00:00", - "response_status": "ERROR_RESPONSE", - "cert_status": "REVOKED", - "served_certificate_chain": [ - "-----BEGIN CERTIFICATE-----\n-----END CERTIFICATE-----" - ], - "validated_certificate_chain": [ - "-----BEGIN CERTIFICATE-----\n-----END CERTIFICATE-----" - ] - } - } - "###); - } - - #[test] - fn test_hpkp_basic() { - let json = r#"{ - "date-time": "2014-04-06T13:00:50Z", - "hostname": "example.com", - "port": 443, - "effective-expiration-date": "2014-05-01T12:40:50Z", - "include-subdomains": false, - "served-certificate-chain": ["-----BEGIN CERTIFICATE-----\n-----END CERTIFICATE-----"], - "validated-certificate-chain": ["-----BEGIN CERTIFICATE-----\n-----END CERTIFICATE-----"], - "known-pins": ["pin-sha256=\"E9CZ9INDbd+2eRQozYqqbQ2yXLVKB9+xcprMF+44U1g=\""] - }"#; - - let mut event = Event::default(); - Hpkp::apply_to_event(json.as_bytes(), &mut event).unwrap(); - assert_annotated_snapshot!(Annotated::new(event), @r###" - { - "logentry": { - "formatted": "Public key pinning validation failed for 'example.com'" - }, - "request": { - "url": "example.com" - }, - "tags": [ - [ - "hostname", - "example.com" - ], - [ - "port", - "443" - ], - [ - "include-subdomains", - "false" - ] - ], - "hpkp": { - "date_time": "2014-04-06T13:00:50+00:00", - "hostname": "example.com", - "port": 443, - "effective_expiration_date": "2014-05-01T12:40:50+00:00", - "include_subdomains": false, - "served_certificate_chain": [ - "-----BEGIN CERTIFICATE-----\n-----END CERTIFICATE-----" - ], - "validated_certificate_chain": [ - "-----BEGIN CERTIFICATE-----\n-----END CERTIFICATE-----" - ], - "known_pins": [ - "pin-sha256=\"E9CZ9INDbd+2eRQozYqqbQ2yXLVKB9+xcprMF+44U1g=\"" - ] - } - } - "###); - } - #[test] fn test_security_report_type_deserializer_recognizes_csp_reports() { let csp_report_text = r#"{ @@ -2030,7 +1269,8 @@ mod tests { } #[test] - fn test_security_report_type_deserializer_recognizes_expect_ct_reports() { + fn test_security_report_type_deserializer_rejects_expect_ct_reports() { + // Expect-CT is no longer a supported report type: the classifier must not recognize it. let expect_ct_report_text = r#"{ "expect-ct-report": { "date-time": "2014-04-06T13:00:50Z", @@ -2055,11 +1295,12 @@ mod tests { }"#; let report_type = SecurityReportType::from_json(expect_ct_report_text.as_bytes()).unwrap(); - assert_eq!(report_type, Some(SecurityReportType::ExpectCt)); + assert_eq!(report_type, None); } #[test] - fn test_security_report_type_deserializer_recognizes_expect_staple_reports() { + fn test_security_report_type_deserializer_rejects_expect_staple_reports() { + // Expect-Staple is no longer a supported report type: the classifier must not recognize it. let expect_staple_report_text = r#"{ "expect-staple-report": { "date-time": "2014-04-06T13:00:50Z", @@ -2074,11 +1315,12 @@ mod tests { }"#; let report_type = SecurityReportType::from_json(expect_staple_report_text.as_bytes()).unwrap(); - assert_eq!(report_type, Some(SecurityReportType::ExpectStaple)); + assert_eq!(report_type, None); } #[test] - fn test_security_report_type_deserializer_recognizes_hpkp_reports() { + fn test_security_report_type_deserializer_rejects_hpkp_reports() { + // HPKP is no longer a supported report type: the classifier must not recognize it. let hpkp_report_text = r#"{ "date-time": "2014-04-06T13:00:50Z", "hostname": "www.example.com", @@ -2098,7 +1340,7 @@ mod tests { }"#; let report_type = SecurityReportType::from_json(hpkp_report_text.as_bytes()).unwrap(); - assert_eq!(report_type, Some(SecurityReportType::Hpkp)); + assert_eq!(report_type, None); } #[test] diff --git a/relay-pii/src/processor.rs b/relay-pii/src/processor.rs index 0dcd7368181..61c681665bb 100644 --- a/relay-pii/src/processor.rs +++ b/relay-pii/src/processor.rs @@ -657,7 +657,7 @@ mod tests { "username": "hey man 73.133.27.120", // should be stripped despite not being "known ip field" "ip_address": "is this an ip address? 73.133.27.120", // <-------- }, - "hpkp":"invalid data my ip address is 74.133.27.120 and my credit card number is 4571234567890111 ", + "extra":"invalid data my ip address is 74.133.27.120 and my credit card number is 4571234567890111 ", }) .into(), ); diff --git a/relay-pii/src/snapshots/relay_pii__processor__tests__does_not_scrub_if_no_graphql.snap b/relay-pii/src/snapshots/relay_pii__processor__tests__does_not_scrub_if_no_graphql.snap index 68a1ba04fbf..584332d9f68 100644 --- a/relay-pii/src/snapshots/relay_pii__processor__tests__does_not_scrub_if_no_graphql.snap +++ b/relay-pii/src/snapshots/relay_pii__processor__tests__does_not_scrub_if_no_graphql.snap @@ -100,9 +100,6 @@ Event { grouping_config: ~, checksum: ~, csp: ~, - hpkp: ~, - expectct: ~, - expectstaple: ~, spans: ~, measurements: ~, breakdowns: ~, diff --git a/relay-pii/src/snapshots/relay_pii__processor__tests__scrub_graphql_response_data_with_variables.snap b/relay-pii/src/snapshots/relay_pii__processor__tests__scrub_graphql_response_data_with_variables.snap index c6e8f570640..001ae1c8b22 100644 --- a/relay-pii/src/snapshots/relay_pii__processor__tests__scrub_graphql_response_data_with_variables.snap +++ b/relay-pii/src/snapshots/relay_pii__processor__tests__scrub_graphql_response_data_with_variables.snap @@ -100,9 +100,6 @@ Event { grouping_config: ~, checksum: ~, csp: ~, - hpkp: ~, - expectct: ~, - expectstaple: ~, spans: ~, measurements: ~, breakdowns: ~, diff --git a/relay-pii/src/snapshots/relay_pii__processor__tests__scrub_graphql_response_data_without_variables.snap b/relay-pii/src/snapshots/relay_pii__processor__tests__scrub_graphql_response_data_without_variables.snap index 817bca3bb77..45166219a7e 100644 --- a/relay-pii/src/snapshots/relay_pii__processor__tests__scrub_graphql_response_data_without_variables.snap +++ b/relay-pii/src/snapshots/relay_pii__processor__tests__scrub_graphql_response_data_without_variables.snap @@ -81,9 +81,6 @@ Event { grouping_config: ~, checksum: ~, csp: ~, - hpkp: ~, - expectct: ~, - expectstaple: ~, spans: ~, measurements: ~, breakdowns: ~, diff --git a/relay-pii/src/snapshots/relay_pii__processor__tests__scrub_original_value.snap b/relay-pii/src/snapshots/relay_pii__processor__tests__scrub_original_value.snap index dfa3ca20d29..1422fa955d0 100644 --- a/relay-pii/src/snapshots/relay_pii__processor__tests__scrub_original_value.snap +++ b/relay-pii/src/snapshots/relay_pii__processor__tests__scrub_original_value.snap @@ -85,24 +85,14 @@ Event { template: ~, threads: ~, tags: ~, - extra: ~, - debug_meta: ~, - client_sdk: ~, - ingest_path: ~, - errors: ~, - key_id: ~, - project: ~, - grouping_config: ~, - checksum: ~, - csp: ~, - hpkp: Meta { + extra: Meta { remarks: [], errors: [ Error { kind: InvalidData, data: { "reason": String( - "expected hpkp", + "expected an object", ), }, }, @@ -114,8 +104,15 @@ Event { ), ), }, - expectct: ~, - expectstaple: ~, + debug_meta: ~, + client_sdk: ~, + ingest_path: ~, + errors: ~, + key_id: ~, + project: ~, + grouping_config: ~, + checksum: ~, + csp: ~, spans: ~, measurements: ~, breakdowns: ~, diff --git a/relay-pii/src/snapshots/relay_pii__processor__tests__sentry_user.snap b/relay-pii/src/snapshots/relay_pii__processor__tests__sentry_user.snap index dcb3886df0e..3b09002bab6 100644 --- a/relay-pii/src/snapshots/relay_pii__processor__tests__sentry_user.snap +++ b/relay-pii/src/snapshots/relay_pii__processor__tests__sentry_user.snap @@ -98,9 +98,6 @@ Event { grouping_config: ~, checksum: ~, csp: ~, - hpkp: ~, - expectct: ~, - expectstaple: ~, spans: ~, measurements: ~, breakdowns: ~, diff --git a/relay-server/src/endpoints/common.rs b/relay-server/src/endpoints/common.rs index 599b6b07e40..50944da26c7 100644 --- a/relay-server/src/endpoints/common.rs +++ b/relay-server/src/endpoints/common.rs @@ -702,13 +702,13 @@ mod tests { #[test] fn test_minimal_event_type() { - let json = r#"{"type": "expectct"}"#; + let json = r#"{"type": "csp"}"#; let minimal = minimal_event_from_json(json.as_ref()).unwrap(); assert_eq!( minimal, MinimalEvent { id: None, - ty: EventType::ExpectCt, + ty: EventType::Csp, } ); } diff --git a/relay-server/src/endpoints/security_report.rs b/relay-server/src/endpoints/security_report.rs index 0d502f9ab71..73028f35161 100644 --- a/relay-server/src/endpoints/security_report.rs +++ b/relay-server/src/endpoints/security_report.rs @@ -81,9 +81,6 @@ fn is_security_mime(mime: Mime) -> bool { (ty, subty, suffix), ("application", "json", None) | ("application", "csp-report", None) - | ("application", "expect-ct-report", None) - | ("application", "expect-ct-report", Some("json")) - | ("application", "expect-staple-report", None) | ("application", "reports", Some("json")) ) } diff --git a/relay-server/src/envelope/item.rs b/relay-server/src/envelope/item.rs index b60a76ad8c4..959e8c1ae0a 100644 --- a/relay-server/src/envelope/item.rs +++ b/relay-server/src/envelope/item.rs @@ -821,9 +821,7 @@ impl ItemType { EventType::Default | EventType::Error => ItemType::Event, EventType::Transaction => ItemType::Transaction, EventType::UserReportV2 => ItemType::UserReportV2, - EventType::Csp | EventType::Hpkp | EventType::ExpectCt | EventType::ExpectStaple => { - ItemType::Security - } + EventType::Csp => ItemType::Security, } } diff --git a/relay-server/src/processing/errors/errors/mod.rs b/relay-server/src/processing/errors/errors/mod.rs index 29e8f8e1938..a4476548376 100644 --- a/relay-server/src/processing/errors/errors/mod.rs +++ b/relay-server/src/processing/errors/errors/mod.rs @@ -43,7 +43,6 @@ pub struct Context<'a> { #[cfg(test)] impl Context<'static> { /// Returns a [`Context`] with default values for testing. - #[cfg_attr(not(feature = "processing"), expect(unused))] pub fn for_test() -> Self { use std::sync::LazyLock; diff --git a/relay-server/src/processing/errors/errors/raw_security.rs b/relay-server/src/processing/errors/errors/raw_security.rs index f486be84bd5..140a65020db 100644 --- a/relay-server/src/processing/errors/errors/raw_security.rs +++ b/relay-server/src/processing/errors/errors/raw_security.rs @@ -1,7 +1,5 @@ use relay_base_schema::events::EventType; -use relay_event_schema::protocol::{ - Csp, Event, ExpectCt, ExpectStaple, Hpkp, LenientString, Metrics, SecurityReportType, -}; +use relay_event_schema::protocol::{Csp, Event, LenientString, Metrics, SecurityReportType}; use relay_protocol::Annotated; use relay_quotas::DataCategory; @@ -12,6 +10,7 @@ use crate::processing::ForwardContext; use crate::processing::errors::Result; use crate::processing::errors::errors::{Context, Expansion, SentryError, utils}; use crate::services::processor::ProcessingError; +use crate::utils::DebugBytes; #[derive(Debug)] pub struct RawSecurity; @@ -83,22 +82,14 @@ fn event_from_security_report( let (apply_result, event_type) = match report_type { SecurityReportType::Csp => (Csp::apply_to_event(data, &mut event), EventType::Csp), - SecurityReportType::ExpectCt => ( - ExpectCt::apply_to_event(data, &mut event), - EventType::ExpectCt, - ), - SecurityReportType::ExpectStaple => ( - ExpectStaple::apply_to_event(data, &mut event), - EventType::ExpectStaple, - ), - SecurityReportType::Hpkp => (Hpkp::apply_to_event(data, &mut event), EventType::Hpkp), SecurityReportType::Unsupported => return Err(ProcessingError::UnsupportedSecurityType), }; if let Err(json_error) = apply_result { - // logged in extract_event + // The payload is attacker controlled and as large as `max_event_size`. + let payload = format!("{:?}", DebugBytes(data)); relay_log::configure_scope(|scope| { - scope.set_extra("payload", String::from_utf8_lossy(data).into()); + scope.set_extra("payload", payload.into()); }); return Err(ProcessingError::InvalidSecurityReport(json_error)); diff --git a/relay-server/src/processing/errors/errors/security.rs b/relay-server/src/processing/errors/errors/security.rs index f99af353dd7..989a8fd11e5 100644 --- a/relay-server/src/processing/errors/errors/security.rs +++ b/relay-server/src/processing/errors/errors/security.rs @@ -1,3 +1,5 @@ +use relay_base_schema::events::EventType; +use relay_event_normalization::infer_event_type; use relay_quotas::DataCategory; use crate::envelope::{Item, ItemType}; @@ -5,6 +7,7 @@ use crate::managed::{Counted, Quantities, RecordKeeper}; use crate::processing::ForwardContext; use crate::processing::errors::Result; use crate::processing::errors::errors::{Context, Expansion, SentryError, utils}; +use crate::services::processor::ProcessingError; #[derive(Debug)] pub struct Security; @@ -19,10 +22,24 @@ impl SentryError for Security { return Ok(None); }; + let payload = ev.payload(); let mut metrics = Default::default(); + let mut event = utils::event_from_json_payload(ev, None, &mut metrics, ctx)?; + + // Normalization honours a declared `transaction` or `feedback` type. Discard it, a + // security item must not turn into an event of a different data category. + if let Some(event) = event.value_mut() { + event.ty.set_value(None); + } + + // CSP is the only remaining security report. Older Relays may still forward the removed + // `hpkp`, `expectct` and `expectstaple` types, which no longer parse into one. + if event.value().map(infer_event_type) != Some(EventType::Csp) { + return Err(ProcessingError::InvalidSecurityType(payload).into()); + } Ok(Some(Expansion { - event: Box::new(utils::event_from_json_payload(ev, None, &mut metrics, ctx)?), + event: Box::new(event), attachments: utils::take_items_of_type(items, ItemType::Attachment), user_reports: utils::take_items_of_type(items, ItemType::UserReport), error: Self, @@ -54,3 +71,62 @@ impl Counted for Security { Default::default() } } + +#[cfg(test)] +mod tests { + use super::*; + + use crate::envelope::ContentType; + use crate::processing::errors::Error; + + fn expand(payload: &str) -> Result> { + let mut item = Item::new(ItemType::Security); + item.set_payload(ContentType::Json, payload.to_owned()); + + Ok(Security::try_expand(&mut vec![item], Context::for_test())? + .expect("a security item is expanded by `Security`")) + } + + fn assert_invalid_security_type(payload: &str) { + let error = expand(payload).expect_err("expected the item to be rejected"); + assert!( + matches!( + error, + Error::ProcessingFailed(ProcessingError::InvalidSecurityType(_)) + ), + "{payload}: {error:?}" + ); + } + + #[test] + fn test_csp_report() { + let expansion = + expand(r#"{"type":"csp","csp":{"effective_directive":"style-src"}}"#).unwrap(); + + assert_eq!(expansion.error.event_category(), DataCategory::Security); + } + + #[test] + fn test_declared_event_type_ignored() { + // A security item must not be able to declare itself a transaction. + let expansion = + expand(r#"{"type":"transaction","csp":{"effective_directive":"style-src"}}"#).unwrap(); + + assert_eq!(expansion.error.event_category(), DataCategory::Security); + assert!(expansion.event.value().unwrap().ty.value().is_none()); + } + + #[test] + fn test_removed_report_types_rejected() { + for ty in ["hpkp", "expectct", "expectstaple"] { + assert_invalid_security_type(&format!( + r#"{{"type":"{ty}","{ty}":{{"hostname":"example.com"}}}}"# + )); + } + } + + #[test] + fn test_non_security_event_rejected() { + assert_invalid_security_type(r#"{"message":"not a security report"}"#); + } +} diff --git a/tests/integration/fixtures/security_report/expect_ct.input.json b/tests/integration/fixtures/security_report/expect_ct.input.json deleted file mode 100644 index 566ffd6ccc1..00000000000 --- a/tests/integration/fixtures/security_report/expect_ct.input.json +++ /dev/null @@ -1,22 +0,0 @@ -{ - "expect-ct-report": { - "date-time": "2014-04-06T13:00:50Z", - "hostname": "www.example.com", - "port": 443, - "effective-expiration-date": "2014-05-01T12:40:50Z", - "served-certificate-chain": [ - "-----BEGIN CERTIFICATE-----\nABC\n-----END CERTIFICATE-----" - ], - "validated-certificate-chain": [ - "-----BEGIN CERTIFICATE-----\nCDE\n-----END CERTIFICATE-----" - ], - "scts": [ - { - "version": 1, - "status": "invalid", - "source": "embedded", - "serialized_sct": "ABCD==" - } - ] - } -} diff --git a/tests/integration/fixtures/security_report/expect_ct.no_processing.output.json b/tests/integration/fixtures/security_report/expect_ct.no_processing.output.json deleted file mode 100644 index 15c99040563..00000000000 --- a/tests/integration/fixtures/security_report/expect_ct.no_processing.output.json +++ /dev/null @@ -1,76 +0,0 @@ -{ - "type": "expectct", - "contexts": { - "browser": { - "browser": "Chrome 74.0.3729", - "name": "Chrome", - "type": "browser", - "version": "74.0.3729" - }, - "client_os": { - "os": "Windows >=10", - "name": "Windows", - "type": "os", - "version": ">=10" - } - }, - "culprit": "www.example.com", - "level": "error", - "logentry": { - "formatted": "Expect-CT failed for 'www.example.com'" - }, - "logger": "csp", - "platform": "other", - "project": 42, - "release": "01d5c3165d9fbc5c8bdcf9550a1d6793a80fc02b", - "environment": "production", - "grouping_config": { - "enhancements": "eJybzDhxY05qemJypZWRgaGlroGxrqHRBABbEwcC", - "id": "legacy:2019-03-12" - }, - "key_id": "123", - "request": { - "headers": [ - [ - "User-Agent", - "Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/74.0.3729.169 Safari/537.36" - ] - ], - "url": "www.example.com" - }, - "tags": [ - [ - "hostname", - "www.example.com" - ], - [ - "port", - "443" - ] - ], - "expectct": { - "date_time": "2014-04-06T13:00:50+00:00", - "hostname": "www.example.com", - "port": 443, - "effective_expiration_date": "2014-05-01T12:40:50+00:00", - "served_certificate_chain": [ - "-----BEGIN CERTIFICATE-----\nABC\n-----END CERTIFICATE-----" - ], - "validated_certificate_chain": [ - "-----BEGIN CERTIFICATE-----\nCDE\n-----END CERTIFICATE-----" - ], - "scts": [ - { - "version": 1, - "status": "invalid", - "source": "embedded", - "serialized_sct": "ABCD==" - } - ] - }, - "user": { - "ip_address": "127.0.0.1", - "sentry_user": "ip:127.0.0.1" - }, - "version": "7" -} diff --git a/tests/integration/fixtures/security_report/expect_staple.input.json b/tests/integration/fixtures/security_report/expect_staple.input.json deleted file mode 100644 index 047e973052e..00000000000 --- a/tests/integration/fixtures/security_report/expect_staple.input.json +++ /dev/null @@ -1,16 +0,0 @@ -{ - "expect-staple-report": { - "date-time": "2014-04-06T13:00:50Z", - "hostname": "www.example.com", - "port": 443, - "response-status": "ERROR_RESPONSE", - "cert-status": "REVOKED", - "effective-expiration-date": "2014-05-01T12:40:50Z", - "served-certificate-chain": [ - "-----BEGIN CERTIFICATE-----\nABC\n-----END CERTIFICATE-----" - ], - "validated-certificate-chain": [ - "-----BEGIN CERTIFICATE-----\nCDE\n-----END CERTIFICATE-----" - ] - } -} diff --git a/tests/integration/fixtures/security_report/expect_staple.no_processing.output.json b/tests/integration/fixtures/security_report/expect_staple.no_processing.output.json deleted file mode 100644 index 98f6efcaa82..00000000000 --- a/tests/integration/fixtures/security_report/expect_staple.no_processing.output.json +++ /dev/null @@ -1,78 +0,0 @@ -{ - "type": "expectstaple", - "contexts": { - "browser": { - "browser": "Chrome 74.0.3729", - "name": "Chrome", - "type": "browser", - "version": "74.0.3729" - }, - "client_os": { - "os": "Windows >=10", - "name": "Windows", - "type": "os", - "version": ">=10" - } - }, - "culprit": "www.example.com", - "level": "error", - "logentry": { - "formatted": "Expect-Staple failed for 'www.example.com'" - }, - "logger": "csp", - "platform": "other", - "project": 42, - "release": "01d5c3165d9fbc5c8bdcf9550a1d6793a80fc02b", - "environment": "production", - "grouping_config": { - "enhancements": "eJybzDhxY05qemJypZWRgaGlroGxrqHRBABbEwcC", - "id": "legacy:2019-03-12" - }, - "key_id": "123", - "request": { - "headers": [ - [ - "User-Agent", - "Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/74.0.3729.169 Safari/537.36" - ] - ], - "url": "www.example.com" - }, - "tags": [ - [ - "hostname", - "www.example.com" - ], - [ - "port", - "443" - ], - [ - "response_status", - "ERROR_RESPONSE" - ], - [ - "cert_status", - "REVOKED" - ] - ], - "expectstaple": { - "date_time": "2014-04-06T13:00:50+00:00", - "hostname": "www.example.com", - "port": 443, - "effective_expiration_date": "2014-05-01T12:40:50+00:00", - "response_status": "ERROR_RESPONSE", - "cert_status": "REVOKED", - "served_certificate_chain": [ - "-----BEGIN CERTIFICATE-----\nABC\n-----END CERTIFICATE-----" - ], - "validated_certificate_chain": [ - "-----BEGIN CERTIFICATE-----\nCDE\n-----END CERTIFICATE-----" - ] - }, - "user": { - "ip_address": "127.0.0.1", - "sentry_user": "ip:127.0.0.1" - }, - "version": "7" -} diff --git a/tests/integration/fixtures/security_report/hpkp.input.json b/tests/integration/fixtures/security_report/hpkp.input.json deleted file mode 100644 index bc1c76f1d83..00000000000 --- a/tests/integration/fixtures/security_report/hpkp.input.json +++ /dev/null @@ -1,17 +0,0 @@ -{ - "date-time": "2014-04-06T13:00:50Z", - "hostname": "www.example.com", - "port": 443, - "effective-expiration-date": "2014-05-01T12:40:50Z", - "include-subdomains": false, - "served-certificate-chain": [ - "-----BEGIN CERTIFICATE-----\n MIIEBDCCAuygBQUAMEIxCzAJBgNVBAYTAlVT\n -----END CERTIFICATE-----" - ], - "validated-certificate-chain": [ - "-----BEGIN CERTIFICATE-----\n MIIEBDCCAuygAwIBAgIDCzAJBgNVBAYTAlVT\n -----END CERTIFICATE-----" - ], - "known-pins": [ - "pin-sha256=\"d6qzRu9zOECb90Uez27xWltNsj0e1Md7GkYYkVoZWmM=\"", - "pin-sha256=\"E9CZ9INDbd+2eRQozYqqbQ2yXLVKB9+xcprMF+44U1g=\"" - ] -} diff --git a/tests/integration/fixtures/security_report/hpkp.no_processing.output.json b/tests/integration/fixtures/security_report/hpkp.no_processing.output.json deleted file mode 100644 index 7cc4f1e57bd..00000000000 --- a/tests/integration/fixtures/security_report/hpkp.no_processing.output.json +++ /dev/null @@ -1,76 +0,0 @@ -{ - "type": "hpkp", - "contexts": { - "browser": { - "browser": "Chrome 74.0.3729", - "name": "Chrome", - "type": "browser", - "version": "74.0.3729" - }, - "client_os": { - "os": "Windows >=10", - "name": "Windows", - "type": "os", - "version": ">=10" - } - }, - "level": "error", - "logentry": { - "formatted": "Public key pinning validation failed for 'www.example.com'" - }, - "logger": "csp", - "platform": "other", - "project": 42, - "release": "01d5c3165d9fbc5c8bdcf9550a1d6793a80fc02b", - "environment": "production", - "grouping_config": { - "enhancements": "eJybzDhxY05qemJypZWRgaGlroGxrqHRBABbEwcC", - "id": "legacy:2019-03-12" - }, - "key_id": "123", - "request": { - "headers": [ - [ - "User-Agent", - "Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/74.0.3729.169 Safari/537.36" - ] - ], - "url": "www.example.com" - }, - "tags": [ - [ - "hostname", - "www.example.com" - ], - [ - "port", - "443" - ], - [ - "include-subdomains", - "false" - ] - ], - "hpkp": { - "date_time": "2014-04-06T13:00:50+00:00", - "hostname": "www.example.com", - "port": 443, - "effective_expiration_date": "2014-05-01T12:40:50+00:00", - "include_subdomains": false, - "served_certificate_chain": [ - "-----BEGIN CERTIFICATE-----\n MIIEBDCCAuygBQUAMEIxCzAJBgNVBAYTAlVT\n -----END CERTIFICATE-----" - ], - "validated_certificate_chain": [ - "-----BEGIN CERTIFICATE-----\n MIIEBDCCAuygAwIBAgIDCzAJBgNVBAYTAlVT\n -----END CERTIFICATE-----" - ], - "known_pins": [ - "pin-sha256=\"d6qzRu9zOECb90Uez27xWltNsj0e1Md7GkYYkVoZWmM=\"", - "pin-sha256=\"E9CZ9INDbd+2eRQozYqqbQ2yXLVKB9+xcprMF+44U1g=\"" - ] - }, - "user": { - "ip_address": "127.0.0.1", - "sentry_user": "ip:127.0.0.1" - }, - "version": "7" -} diff --git a/tests/integration/test_security_report.py b/tests/integration/test_security_report.py index 8b9ead2c5a7..9dfddc141d7 100644 --- a/tests/integration/test_security_report.py +++ b/tests/integration/test_security_report.py @@ -3,6 +3,10 @@ import pytest from requests.exceptions import HTTPError +from sentry_relay.consts import DataCategory +from sentry_sdk.envelope import Envelope, Item, PayloadRef + +from .asserts import time_within_delta CSP_IGNORED_FIELDS = ( "event_id", @@ -10,9 +14,6 @@ "received", "ingest_path", ) -EXPECT_CT_IGNORED_FIELDS = ("event_id", "ingest_path") -EXPECT_STAPLE_IGNORED_FIELDS = ("event_id", "ingest_path") -HPKP_IGNORED_FIELDS = ("event_id", "ingest_path") def get_security_report(envelope): @@ -238,18 +239,12 @@ def test_deprication_reports_with_processing( ("csp_chrome", CSP_IGNORED_FIELDS), ("csp_chrome_blocked_asset", CSP_IGNORED_FIELDS), ("csp_firefox_blocked_asset", CSP_IGNORED_FIELDS), - ("expect_ct", EXPECT_CT_IGNORED_FIELDS), - ("expect_staple", EXPECT_STAPLE_IGNORED_FIELDS), - ("hpkp", HPKP_IGNORED_FIELDS), ], ids=( "csp", "csp_chrome", "csp_chrome_blocked_asset", "csp_firefox_blocked_asset", - "expect_ct", - "expect_staple", - "hpkp", ), ) def test_security_report(mini_sentry, relay, test_case, json_fixture_provider): @@ -285,6 +280,154 @@ def test_security_report(mini_sentry, relay, test_case, json_fixture_provider): assert event == expected_evt +@pytest.mark.parametrize( + "payload", + [ + { + "expect-ct-report": { + "hostname": "www.example.com", + "port": 443, + "effective-expiration-date": "2014-05-01T12:40:50Z", + } + }, + { + "expect-staple-report": { + "hostname": "www.example.com", + "port": 443, + "response-status": "ERROR_RESPONSE", + "cert-status": "REVOKED", + } + }, + { + "hostname": "www.example.com", + "port": 443, + "known-pins": ['pin-sha256="d6qzRu9zOECb90Uez27xWltNsj0e1Md7GkYYkVoZWmM="'], + }, + ], + ids=("expect_ct", "expect_staple", "hpkp"), +) +def test_security_report_rejects_deprecated_types(mini_sentry, relay, payload): + """ + Expect-CT, Expect-Staple and HPKP are no longer supported. Relay must reject such + reports with an ``invalid`` outcome and forward nothing upstream. + """ + proj_id = 42 + relay = relay(mini_sentry, {"outcomes": {"emit_outcomes": True}}) + mini_sentry.add_full_project_config(proj_id) + + resp = relay.send_security_report( + project_id=proj_id, + content_type="application/json", + payload=payload, + release="01d5c3165d9fbc5c8bdcf9550a1d6793a80fc02b", + environment="production", + ) + assert resp.status_code == 200 + + assert mini_sentry.get_outcomes(n=1) == [ + { + "category": DataCategory.SECURITY.value, + "outcome": 3, # Invalid + "reason": "security_report_type", + "quantity": 1, + "timestamp": time_within_delta(), + } + ] + + assert mini_sentry.captured_envelopes.empty() + + +def test_security_report_rejects_dedicated_content_types(mini_sentry, relay): + """ + The dedicated ``application/expect-ct-report`` and ``application/expect-staple-report`` + content types are no longer accepted and are rejected at the endpoint. + """ + proj_id = 42 + relay = relay(mini_sentry) + mini_sentry.add_full_project_config(proj_id) + + for content_type in ( + "application/expect-ct-report", + "application/expect-staple-report", + ): + with pytest.raises(HTTPError) as excinfo: + relay.send_security_report( + project_id=proj_id, + content_type=content_type, + payload={"expect-ct-report": {"hostname": "www.example.com"}}, + release="01d5c3165d9fbc5c8bdcf9550a1d6793a80fc02b", + environment="production", + ) + assert excinfo.value.response.status_code == 415 + + assert mini_sentry.captured_envelopes.empty() + + +def test_security_report_forwards_classified_events(mini_sentry, relay): + """A ``security`` item from an upstream Relay is forwarded as a security event.""" + proj_id = 42 + relay = relay(mini_sentry, {"outcomes": {"emit_outcomes": True}}) + mini_sentry.add_full_project_config(proj_id) + + envelope = Envelope(headers={"event_id": "cbf6960622e14a45abc1f03b2055b186"}) + envelope.add_item( + Item( + payload=PayloadRef( + json={ + "type": "csp", + "csp": { + "blocked_uri": "http://evilhackerscripts.com", + "document_uri": "https://example.com/foo/bar", + "effective_directive": "default-src", + }, + } + ), + type="security", + ) + ) + relay.send_envelope(proj_id, envelope) + + envelope = mini_sentry.get_captured_envelope() + assert [item.type for item in envelope.items] == ["security"] + assert get_security_report(envelope)["csp"]["effective_directive"] == "default-src" + + assert mini_sentry.captured_outcomes.empty() + + +@pytest.mark.parametrize("event_type", ["hpkp", "expectct", "expectstaple"]) +def test_security_report_rejects_deprecated_events(mini_sentry, relay, event_type): + """Reports an older upstream Relay already classified are rejected, not turned into errors.""" + proj_id = 42 + relay = relay(mini_sentry, {"outcomes": {"emit_outcomes": True}}) + mini_sentry.add_full_project_config(proj_id) + + envelope = Envelope(headers={"event_id": "cbf6960622e14a45abc1f03b2055b186"}) + envelope.add_item( + Item( + payload=PayloadRef( + json={ + "type": event_type, + event_type: {"hostname": "www.example.com", "port": 443}, + } + ), + type="security", + ) + ) + relay.send_envelope(proj_id, envelope) + + assert mini_sentry.get_outcomes(n=1) == [ + { + "category": DataCategory.SECURITY.value, + "outcome": 3, # Invalid + "reason": "security_report_type", + "quantity": 1, + "timestamp": time_within_delta(), + } + ] + + assert mini_sentry.captured_envelopes.empty() + + def split_header(header_val): return [x.strip() for x in header_val.split(",")]