From f222cb893e53f1a631a1d018409196de0af609be Mon Sep 17 00:00:00 2001
From: Thomas Eizinger <thomas@eizinger.io>
Date: Fri, 28 Feb 2025 11:16:28 +1100
Subject: [PATCH] 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.
---
 rust/connlib/tunnel/src/client/resource.rs | 29 +++++++++++--
 rust/connlib/tunnel/src/messages/client.rs | 48 ++++++++++++++++++++--
 2 files changed, 71 insertions(+), 6 deletions(-)

diff --git a/rust/connlib/tunnel/src/client/resource.rs b/rust/connlib/tunnel/src/client/resource.rs
index 5f30be0f2..ccdf3a60e 100644
--- a/rust/connlib/tunnel/src/client/resource.rs
+++ b/rust/connlib/tunnel/src/client/resource.rs
@@ -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,
diff --git a/rust/connlib/tunnel/src/messages/client.rs b/rust/connlib/tunnel/src/messages/client.rs
index 97293d6bf..8ced372d9 100644
--- a/rust/connlib/tunnel/src/messages/client.rs
+++ b/rust/connlib/tunnel/src/messages/client.rs
@@ -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#"