Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ref(envelope): Introduce an item type for security reports #617

Merged
merged 4 commits into from
Jun 9, 2020
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
79 changes: 45 additions & 34 deletions relay-general/src/protocol/security_report.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ use serde::{Deserialize, Serialize};
use url::Url;

use crate::protocol::{
Event, HeaderName, HeaderValue, Headers, LogEntry, PairList, Request, TagEntry, Tags,
Event, EventType, HeaderName, HeaderValue, Headers, LogEntry, PairList, Request, TagEntry, Tags,
};
use crate::types::{Annotated, Array, Object, Value};

Expand Down Expand Up @@ -490,6 +490,7 @@ impl Csp {
.effective_directive()
.map_err(serde::de::Error::custom)?;

event.ty = Annotated::new(EventType::Csp);
event.logentry = Annotated::new(LogEntry::from(raw_csp.get_message(effective_directive)));
event.culprit = Annotated::new(raw_csp.get_culprit());
event.tags = Annotated::new(raw_csp.get_tags(effective_directive));
Expand Down Expand Up @@ -744,6 +745,7 @@ impl ExpectCt {
let raw_report = serde_json::from_slice::<ExpectCtReportRaw>(data)?;
let raw_expect_ct = raw_report.expect_ct_report;

event.ty = Annotated::new(EventType::ExpectCT);
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());
Expand Down Expand Up @@ -885,6 +887,7 @@ impl Hpkp {
pub fn apply_to_event(data: &[u8], event: &mut Event) -> Result<(), serde_json::Error> {
let raw_hpkp = serde_json::from_slice::<HpkpRaw>(data)?;

event.ty = Annotated::new(EventType::Hpkp);
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());
Expand Down Expand Up @@ -1094,6 +1097,7 @@ impl ExpectStaple {
let raw_report = serde_json::from_slice::<ExpectStapleReportRaw>(data)?;
let raw_expect_staple = raw_report.expect_staple_report;

event.ty = Annotated::new(EventType::ExpectStaple);
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());
Expand Down Expand Up @@ -1202,6 +1206,7 @@ mod tests {

assert_annotated_snapshot!(Annotated::new(event), @r###"
{
"type": "csp",
"culprit": "style-src cdn.example.com",
"logentry": {
"formatted": "Blocked 'style' from 'example.com'"
Expand Down Expand Up @@ -1243,6 +1248,7 @@ mod tests {

assert_annotated_snapshot!(Annotated::new(event), @r###"
{
"type": "csp",
"culprit": "",
"logentry": {
"formatted": "Blocked unsafe (eval() or inline) 'script'"
Expand Down Expand Up @@ -1286,39 +1292,40 @@ mod tests {
Csp::apply_to_event(json.as_bytes(), &mut event).unwrap();

assert_annotated_snapshot!(Annotated::new(event), @r###"
⋮{
⋮ "culprit": "default-src self",
⋮ "logentry": {
⋮ "formatted": "Blocked 'default-src' from 'evilhackerscripts.com'"
⋮ },
⋮ "request": {
⋮ "url": "https://example.com/foo/bar",
⋮ "headers": [
⋮ [
⋮ "Referer",
⋮ "https://www.google.com/"
⋮ ]
⋮ ]
⋮ },
⋮ "tags": [
⋮ [
⋮ "effective-directive",
⋮ "default-src"
⋮ ],
⋮ [
⋮ "blocked-uri",
⋮ "http://evilhackerscripts.com"
⋮ ]
⋮ ],
⋮ "csp": {
⋮ "effective_directive": "default-src",
⋮ "blocked_uri": "http://evilhackerscripts.com",
⋮ "document_uri": "https://example.com/foo/bar",
⋮ "original_policy": "default-src self; report-uri /csp-hotline.php",
⋮ "referrer": "https://www.google.com/",
⋮ "violated_directive": "default-src self"
⋮ }
⋮}
{
"type": "csp",
"culprit": "default-src self",
"logentry": {
"formatted": "Blocked 'default-src' from 'evilhackerscripts.com'"
},
"request": {
"url": "https://example.com/foo/bar",
"headers": [
[
"Referer",
"https://www.google.com/"
]
]
},
"tags": [
[
"effective-directive",
"default-src"
],
[
"blocked-uri",
"http://evilhackerscripts.com"
]
],
"csp": {
"effective_directive": "default-src",
"blocked_uri": "http://evilhackerscripts.com",
"document_uri": "https://example.com/foo/bar",
"original_policy": "default-src self; report-uri /csp-hotline.php",
"referrer": "https://www.google.com/",
"violated_directive": "default-src self"
}
}
"###);
}

Expand Down Expand Up @@ -1346,6 +1353,7 @@ mod tests {

assert_annotated_snapshot!(Annotated::new(event), @r###"
{
"type": "csp",
"culprit": "script-src",
"logentry": {
"formatted": "Blocked 'script' from 'baddomain.com'"
Expand Down Expand Up @@ -1698,6 +1706,7 @@ mod tests {
ExpectCt::apply_to_event(json.as_bytes(), &mut event).unwrap();
assert_annotated_snapshot!(Annotated::new(event), @r###"
{
"type": "expectct",
"culprit": "www.example.com",
"logentry": {
"formatted": "Expect-CT failed for 'www.example.com'"
Expand Down Expand Up @@ -1770,6 +1779,7 @@ mod tests {
ExpectStaple::apply_to_event(json.as_bytes(), &mut event).unwrap();
assert_annotated_snapshot!(Annotated::new(event), @r###"
{
"type": "expectstaple",
"culprit": "www.example.com",
"logentry": {
"formatted": "Expect-Staple failed for 'www.example.com'"
Expand Down Expand Up @@ -1830,6 +1840,7 @@ mod tests {
Hpkp::apply_to_event(json.as_bytes(), &mut event).unwrap();
assert_annotated_snapshot!(Annotated::new(event), @r###"
{
"type": "hpkp",
"logentry": {
"formatted": "Public key pinning validation failed for 'example.com'"
},
Expand Down
18 changes: 8 additions & 10 deletions relay-server/src/actors/events.rs
Original file line number Diff line number Diff line change
Expand Up @@ -428,7 +428,8 @@ impl EventProcessor {
// `process_event`.
let event_item = envelope.take_item_by(|item| item.ty() == ItemType::Event);
let transaction_item = envelope.take_item_by(|item| item.ty() == ItemType::Transaction);
let security_item = envelope.take_item_by(|item| item.ty() == ItemType::SecurityReport);
let security_item = envelope.take_item_by(|item| item.ty() == ItemType::Security);
let raw_security_item = envelope.take_item_by(|item| item.ty() == ItemType::RawSecurity);
let form_item = envelope.take_item_by(|item| item.ty() == ItemType::FormData);
let attachment_item = envelope
.take_item_by(|item| item.attachment_type() == Some(AttachmentType::EventPayload));
Expand All @@ -437,10 +438,10 @@ impl EventProcessor {
let breadcrumbs_item2 = envelope
.take_item_by(|item| item.attachment_type() == Some(AttachmentType::Breadcrumbs));

if let Some(item) = event_item {
if let Some(item) = event_item.or(security_item) {
log::trace!("processing json event");
return Ok(metric!(timer(RelayTimers::EventProcessingDeserialize), {
// Event items can never include transactions, so kill the event type and let
// Event items can never include transactions, so retain the event type and let
// inference deal with this during store normalization.
self.event_from_json_payload(item, None)?
}));
Expand All @@ -455,7 +456,7 @@ impl EventProcessor {
}));
}

if let Some(item) = security_item {
if let Some(item) = raw_security_item {
log::trace!("processing security report");
return Ok(self.event_from_security_report(item)?);
}
Expand Down Expand Up @@ -616,8 +617,9 @@ impl EventProcessor {
// These should always be removed by `extract_event`:
ItemType::Event => true,
ItemType::Transaction => true,
ItemType::Security => true,
ItemType::FormData => true,
ItemType::SecurityReport => true,
ItemType::RawSecurity => true,

// These should be removed conditionally:
ItemType::UnrealReport => self.config.processing_enabled(),
Expand Down Expand Up @@ -798,12 +800,8 @@ impl EventProcessor {
event.to_json().map_err(ProcessingError::SerializeFailed)?
});

let item_type = match event_type {
Some(EventType::Transaction) => ItemType::Transaction,
_ => ItemType::Event,
};

// Add the normalized event back to the envelope. All the other items are attachments.
let item_type = ItemType::from_event_type(event_type.unwrap_or_default());
let mut event_item = Item::new(item_type);
event_item.set_payload(ContentType::Json, data);
envelope.add_item(event_item);
Expand Down
8 changes: 6 additions & 2 deletions relay-server/src/actors/store.rs
Original file line number Diff line number Diff line change
Expand Up @@ -420,8 +420,12 @@ impl Handler<StoreEnvelope> for StoreForwarder {

let retention = envelope.retention();
let event_id = envelope.event_id();
let event_item = envelope
.get_item_by(|item| matches!(item.ty(), ItemType::Event | ItemType::Transaction));
let event_item = envelope.get_item_by(|item| {
matches!(
item.ty(),
ItemType::Event | ItemType::Transaction | ItemType::Security
)
});

let topic = if envelope.get_item_by(is_slow_item).is_some() {
KafkaTopic::Attachments
Expand Down
3 changes: 2 additions & 1 deletion relay-server/src/endpoints/common.rs
Original file line number Diff line number Diff line change
Expand Up @@ -334,7 +334,8 @@ fn check_envelope_size_limits(config: &Config, envelope: &Envelope) -> bool {
match item.ty() {
ItemType::Event
| ItemType::Transaction
| ItemType::SecurityReport
| ItemType::Security
| ItemType::RawSecurity
| ItemType::FormData => event_size += item.len(),
ItemType::Attachment | ItemType::UnrealReport => {
if item.len() > config.max_attachment_size() {
Expand Down
2 changes: 1 addition & 1 deletion relay-server/src/endpoints/security_report.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ fn extract_envelope(
return Err(BadStoreRequest::EmptyBody);
}

let mut report_item = Item::new(ItemType::SecurityReport);
let mut report_item = Item::new(ItemType::RawSecurity);
report_item.set_payload(ContentType::Json, data);

if let Some(sentry_release) = params.sentry_release {
Expand Down
8 changes: 2 additions & 6 deletions relay-server/src/endpoints/store.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ use bytes::{Bytes, BytesMut};
use futures::Future;
use serde::Serialize;

use relay_general::protocol::{EventId, EventType};
use relay_general::protocol::EventId;

use crate::body::StoreBody;
use crate::endpoints::common::{self, BadStoreRequest};
Expand Down Expand Up @@ -56,11 +56,7 @@ fn parse_event(

// Old SDKs used to send transactions to the store endpoint with an explicit `Transaction` event
// type. The processing queue expects those in an explicit item.
let item_type = match minimal.ty {
EventType::Transaction => ItemType::Transaction,
_ => ItemType::Event,
};

let item_type = ItemType::from_event_type(minimal.ty);
let mut event_item = Item::new(item_type);
event_item.set_payload(content_type, data);

Expand Down
28 changes: 23 additions & 5 deletions relay-server/src/envelope.rs
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ use failure::Fail;
use serde::{de::DeserializeOwned, Deserialize, Serialize};
use smallvec::SmallVec;

use relay_general::protocol::EventId;
use relay_general::protocol::{EventId, EventType};
use relay_general::types::Value;

use crate::constants::DEFAULT_EVENT_RETENTION;
Expand Down Expand Up @@ -77,12 +77,14 @@ pub enum ItemType {
Event,
/// Transaction event payload encoded in JSON.
Transaction,
/// Security report event payload encoded in JSON.
Security,
/// Raw payload of an arbitrary attachment.
Attachment,
/// Multipart form data collected into a stream of JSON tuples.
FormData,
/// Security report as sent by the browser in JSON.
SecurityReport,
RawSecurity,
/// Raw compressed UE4 crash report.
UnrealReport,
/// User feedback encoded as JSON.
Expand All @@ -91,14 +93,28 @@ pub enum ItemType {
Session,
}

impl ItemType {
/// Returns the event item type corresponding to the given `EventType`.
pub fn from_event_type(event_type: EventType) -> Self {
match event_type {
EventType::Default | EventType::Error => ItemType::Event,
EventType::Transaction => ItemType::Transaction,
EventType::Csp | EventType::Hpkp | EventType::ExpectCT | EventType::ExpectStaple => {
ItemType::Security
}
}
}
}

impl fmt::Display for ItemType {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
match *self {
Self::Event => write!(f, "event"),
Self::Transaction => write!(f, "transaction"),
Self::Security => write!(f, "security report"),
Self::Attachment => write!(f, "attachment"),
Self::FormData => write!(f, "form data"),
Self::SecurityReport => write!(f, "security report"),
Self::RawSecurity => write!(f, "raw security report"),
Self::UnrealReport => write!(f, "unreal report"),
Self::UserReport => write!(f, "user feedback"),
Self::Session => write!(f, "session"),
Expand Down Expand Up @@ -440,7 +456,8 @@ impl Item {
// These items are direct event types.
ItemType::Event
| ItemType::Transaction
| ItemType::SecurityReport
| ItemType::Security
| ItemType::RawSecurity
| ItemType::UnrealReport => true,

// Attachments are only event items if they are crash reports or if they carry partial
Expand Down Expand Up @@ -472,9 +489,10 @@ impl Item {
match self.ty() {
ItemType::Event => true,
ItemType::Transaction => true,
ItemType::Security => true,
ItemType::Attachment => true,
ItemType::FormData => true,
ItemType::SecurityReport => true,
ItemType::RawSecurity => true,
ItemType::UnrealReport => true,
ItemType::UserReport => true,
ItemType::Session => false,
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
{
"type": "csp",
"csp": {
"blocked_uri": "http://evilhackerscripts.com",
"document_uri": "https://example.com/foo/bar",
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
{
"type": "csp",
"csp": {
"blocked_uri": "http://localhost:8000/lol.css",
"document_uri": "http://localhost:8000/",
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
{
"type": "csp",
"csp": {
"blocked_uri": "http://notlocalhost:8000/lol.css",
"document_uri": "http://notlocalhost:8000/",
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
{
"type": "csp",
"csp": {
"blocked_uri": "http://notlocalhost:8000/lol.css",
"document_uri": "http://notlocalhost:8000/",
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
{
"type": "expectct",
"culprit": "www.example.com",
"logentry": {
"formatted": "Expect-CT failed for 'www.example.com'"
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
{
"type": "expectstaple",
"culprit": "www.example.com",
"logentry": {
"formatted": "Expect-Staple failed for 'www.example.com'"
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
{
"type": "hpkp",
"logentry": {
"formatted": "Public key pinning validation failed for 'www.example.com'"
},
Expand Down
Loading