Limit delegations to depth 1 (#500)

dfinity · Dec 11, 2023 · 4a8abe8 · 4a8abe8
1 parent e7b4cdf
commit 4a8abe8
Show file tree

Hide file tree

Showing 4 changed files with 63 additions and 3 deletions.
diff --git a/ic-agent/src/agent/agent_error.rs b/ic-agent/src/agent/agent_error.rs
@@ -110,6 +110,10 @@ pub enum AgentError {
     #[error("Certificate is stale (over {0:?}). Is the computer's clock synchronized?")]
     CertificateOutdated(Duration),
 
+    /// The certificate contained more than one delegation.
+    #[error("The certificate contained more than one delegation")]
+    CertificateHasTooManyDelegations,
+
     /// The query response did not contain any node signatures.
     #[error("Query response did not contain any node signatures")]
     MissingSignature,

diff --git a/ic-agent/src/agent/agent_test.rs b/ic-agent/src/agent/agent_test.rs
@@ -5,10 +5,10 @@ use self::mock::{assert_mock, assert_single_mock, mock, mock_additional};
 use crate::{
     agent::{http_transport::ReqwestTransport, Status},
     export::Principal,
-    Agent, AgentError,
+    Agent, AgentError, Certificate,
 };
 use candid::{Encode, Nat};
-use ic_certification::Label;
+use ic_certification::{Delegation, Label};
 use ic_transport_types::{NodeSignature, QueryResponse, RejectCode, RejectResponse, ReplyResponse};
 use std::{collections::BTreeMap, time::Duration};
 #[cfg(all(target_family = "wasm", feature = "wasm-bindgen"))]
@@ -488,6 +488,56 @@ async fn no_cert() {
     assert_mock(read_mock).await;
 }
 
+const RESP_WITH_SUBNET_KEY: &[u8] = include_bytes!("agent_test/with_subnet_key.bin");
+
+#[cfg_attr(not(target_family = "wasm"), tokio::test)]
+#[cfg_attr(target_family = "wasm", wasm_bindgen_test)]
+async fn too_many_delegations() {
+    // Use the certificate as its own delegation, and repeat the process the specified number of times
+    fn self_delegate_cert(subnet_id: Vec<u8>, cert: &Certificate, depth: u32) -> Certificate {
+        let mut current = cert.clone();
+        for _ in 0..depth {
+            current = Certificate {
+                tree: current.tree.clone(),
+                signature: current.signature.clone(),
+                delegation: Some(Delegation {
+                    subnet_id: subnet_id.clone(),
+                    certificate: serde_cbor::to_vec(&current).unwrap(),
+                }),
+            }
+        }
+        current
+    }
+
+    let canister_id_str = "rdmx6-jaaaa-aaaaa-aaadq-cai";
+    let canister_id = Principal::from_text(canister_id_str).unwrap();
+    let subnet_id = Vec::from(
+        Principal::from_text("uzr34-akd3s-xrdag-3ql62-ocgoh-ld2ao-tamcv-54e7j-krwgb-2gm4z-oqe")
+            .unwrap()
+            .as_slice(),
+    );
+
+    let (_read_mock, url) = mock(
+        "POST",
+        format!("/api/v2/canister/{}/read_state", canister_id_str).as_str(),
+        200,
+        RESP_WITH_SUBNET_KEY.into(),
+        Some("application/cbor"),
+    )
+    .await;
+    let path_label = Label::from_bytes("subnet".as_bytes());
+    let agent = make_untimed_agent(&url);
+    let cert = agent
+        .read_state_raw(vec![vec![path_label]], canister_id)
+        .await
+        .expect("read state failed");
+    let new_cert = self_delegate_cert(subnet_id, &cert, 1);
+    assert!(matches!(
+        agent.verify(&new_cert, canister_id).unwrap_err(),
+        AgentError::CertificateHasTooManyDelegations
+    ));
+}
+
 #[cfg(not(target_family = "wasm"))]
 mod mock {
 

diff --git a/ic-agent/src/agent/agent_test/with_subnet_key.bin b/ic-agent/src/agent/agent_test/with_subnet_key.bin
diff --git a/ic-agent/src/agent/mod.rs b/ic-agent/src/agent/mod.rs
@@ -813,6 +813,9 @@ impl Agent {
             Some(delegation) => {
                 let cert: Certificate = serde_cbor::from_slice(&delegation.certificate)
                     .map_err(AgentError::InvalidCborData)?;
+                if cert.delegation.is_some() {
+                    return Err(AgentError::CertificateHasTooManyDelegations);
+                }
                 self.verify(&cert, effective_canister_id)?;
                 let canister_range_lookup = [
                     "subnet".as_bytes(),
@@ -845,8 +848,11 @@ impl Agent {
         match delegation {
             None => Ok(self.read_root_key()),
             Some(delegation) => {
-                let cert = serde_cbor::from_slice(&delegation.certificate)
+                let cert: Certificate = serde_cbor::from_slice(&delegation.certificate)
                     .map_err(AgentError::InvalidCborData)?;
+                if cert.delegation.is_some() {
+                    return Err(AgentError::CertificateHasTooManyDelegations);
+                }
                 self.verify_for_subnet(&cert, subnet_id)?;
                 let public_key_path = [
                     "subnet".as_bytes(),