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

fix(dynamic-config): use matches_any_origin to scrub HTTP hosts in spans #3939

Merged
Merged
Show file tree
Hide file tree
Changes from 5 commits
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
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,10 @@

## Unreleased

**Bug Fixes**:

- Use custom `Vec<Host>` deserialize method for parsing dynamic config ([#3939](https://github.com/getsentry/relay/pull/3939))
aldy505 marked this conversation as resolved.
Show resolved Hide resolved

**Internal**:

- Bumped `sentry-native` submodule to v0.7.8. ([#3940](https://github.com/getsentry/relay/pull/3940))
Expand Down
65 changes: 64 additions & 1 deletion relay-dynamic-config/src/global.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
use std::collections::btree_map::Entry;
use std::collections::HashMap;
use std::fmt;
use std::fs::File;
use std::io::BufReader;
use std::path::Path;
Expand Down Expand Up @@ -220,7 +221,7 @@ pub struct Options {
/// At this point, it doesn't accept IP addresses in CIDR format.. yet.
#[serde(
rename = "relay.span-normalization.allowed_hosts",
deserialize_with = "default_on_error",
deserialize_with = "deserialize_host",
skip_serializing_if = "Vec::is_empty"
)]
pub http_span_allowed_hosts: Vec<Host>,
Expand Down Expand Up @@ -372,6 +373,39 @@ where
}
}

fn deserialize_host<'de, D>(deserializer: D) -> Result<Vec<Host>, D::Error>
where
D: serde::de::Deserializer<'de>,
{
// Stolen from https://users.rust-lang.org/t/need-help-with-serde-deserialize-with/18374
struct HostVisitor;

impl<'de> de::Visitor<'de> for HostVisitor {
type Value = Vec<Host>;

fn expecting(&self, formatter: &mut fmt::Formatter) -> fmt::Result {
formatter.write_str("a vector containing string of host data")
}

fn visit_seq<A>(self, mut seq: A) -> Result<Self::Value, A::Error>
where
A: de::SeqAccess<'de>,
{
let mut values = Vec::<Host>::new();
while let Some(value) = seq.next_element()? {
let this = Host::parse(value);
if let Ok(t) = this {
values.push(t)
}
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This visitor still fails if either one of the elements in the list has the wrong type or even the entire object is of the wrong type (more likely).

We can keep this fallible and not ignore invalid hosts (it's not user config, they should be always valid) and instead handle the entire error of the visitor at the deserialize.deserialize_* call.


Ok(values)
}
}

deserializer.deserialize_any(HostVisitor)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
deserializer.deserialize_any(HostVisitor)
deserializer.deserialize_seq(HostVisitor)

Can handle the error here.

}

fn is_ok_and_empty(value: &ErrorBoundary<MetricExtractionGroups>) -> bool {
matches!(
value,
Expand Down Expand Up @@ -548,4 +582,33 @@ mod tests {
assert_eq!(o.metric_bucket_set_encodings, original);
assert_eq!(o.metric_bucket_dist_encodings, original);
}

#[test]
fn test_http_span_allowed_hosts_deserialization() {
let input = r###"{
"relay.span-normalization.allowed_hosts": [
"foo.bar.internal",
"baz.qux.internal",
"192.168.1.1",
"[fd45:7aa3:7ae4::]",
"",
"[fdec:3625:8ec3::/48]",
"./foo/bar",
"fdec:3625:8ec3::/48",
"127.0.0.0.0.1",
"127.0.0.1:100000000000"
]
}"###;

let options: Options = serde_json::from_str(input).unwrap();
assert_eq!(
options.http_span_allowed_hosts,
vec![
Host::parse("foo.bar.internal").unwrap(),
Host::parse("baz.qux.internal").unwrap(),
Host::parse("192.168.1.1").unwrap(),
Host::parse("[fd45:7aa3:7ae4::]").unwrap(),
]
);
}
}
Loading