fix(connlib): be more lenient in deserialising resources (#8289)

At present, `connlib` can process a resource list gracefully that
handles unknown resource types. If a known type fails to match the
schema however, we fail to deserialise the entire list.

To reduce the blast radius of potential bugs here, we accept everything
that is valid JSON as the "value" of a resource. Only when processing
the individual items will we attempt to deserialise it into the expected
model, skipping any resources that cannot be deserialised.
This commit is contained in:
Thomas Eizinger
2025-02-28 11:16:28 +11:00
committed by GitHub
parent 315d99f723
commit f222cb893e
2 changed files with 71 additions and 6 deletions

View File

@@ -8,6 +8,7 @@ use connlib_model::{
};
use ip_network::{IpNetwork, Ipv4Network, Ipv6Network};
use itertools::Itertools as _;
use serde::Deserialize;
use crate::messages::client::{
ResourceDescription, ResourceDescriptionCidr, ResourceDescriptionDns,
@@ -68,9 +69,31 @@ pub struct InternetResource {
impl Resource {
pub fn from_description(resource: ResourceDescription) -> Option<Self> {
match resource {
ResourceDescription::Dns(i) => Some(Resource::Dns(DnsResource::from_description(i))),
ResourceDescription::Cidr(i) => Some(Resource::Cidr(CidrResource::from_description(i))),
ResourceDescription::Internet(i) => {
ResourceDescription::Dns(json) => {
let i = ResourceDescriptionDns::deserialize(&json)
.inspect_err(
|e| tracing::warn!(%json, "Failed to deserialise `ResourceDescriptionDns`: {e}"),
)
.ok()?;
Some(Resource::Dns(DnsResource::from_description(i)))
}
ResourceDescription::Cidr(json) => {
let i = ResourceDescriptionCidr::deserialize(&json)
.inspect_err(|e| {
tracing::warn!(%json, "Failed to deserialise `ResourceDescriptionCidr`: {e}")
})
.ok()?;
Some(Resource::Cidr(CidrResource::from_description(i)))
}
ResourceDescription::Internet(json) => {
let i = ResourceDescriptionInternet::deserialize(&json)
.inspect_err(|e| {
tracing::warn!(%json, "Failed to deserialise `ResourceDescriptionInternet`: {e}")
})
.ok()?;
Some(Resource::Internet(InternetResource::from_description(i)))
}
ResourceDescription::Unknown => None,

View File

@@ -62,9 +62,9 @@ pub struct ResourceDescriptionInternet {
#[derive(Debug, Deserialize)]
#[serde(tag = "type", rename_all = "snake_case")]
pub enum ResourceDescription {
Dns(ResourceDescriptionDns),
Cidr(ResourceDescriptionCidr),
Internet(ResourceDescriptionInternet),
Dns(serde_json::Value),
Cidr(serde_json::Value),
Internet(serde_json::Value),
#[serde(other)]
Unknown, // Important for forwards-compatibility with future resource types.
}
@@ -365,6 +365,48 @@ mod tests {
assert!(matches!(message, IngressMessages::Init(_)));
}
#[test]
fn can_deserialize_bad_resources() {
let json = r#"{
"event": "init",
"payload": {
"interface": {
"ipv4": "100.72.112.111",
"ipv6": "fd00:2021:1111::13:efb9",
"upstream_dns": []
},
"resources": [
{
"address_foobar": "172.172.0.0/16",
"id": "73037362-715d-4a83-a749-f18eadd970e6",
"type": "cidr"
},
{
"i_am_not_a_valid_field": null,
"type": "cidr"
},
{
"what": "gitlab.mycorp.com",
"type": "dns"
},
{
"address": "github.mycorp.com",
"id": "03000143-e25e-45c7-aafb-144990e57dce",
"name": "github.mycorp.com",
"gateway_groups": [{"name": "test", "id": "bf56f32d-7b2c-4f5d-a784-788977d014a4"}],
"type": "dns"
}
]
},
"ref": null,
"topic": "client"
}"#;
let message = serde_json::from_str::<IngressMessages>(json).unwrap();
assert!(matches!(message, IngressMessages::Init(_)));
}
#[test]
fn can_deserialize_relay_presence() {
let json = r#"