fix: check read_state timestamps (#521)

CHANGELOG.md

@@ -8,6 +8,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## Unreleased
 
+* Timestamps are now being checked in `Agent::verify`. If you were using it with old certificates, increase the expiry timeout to continue to verify them.
 * Added ECDSA and Bitcoin functions to `MgmtMethod`. There are no new wrappers in `ManagementCanister` because only canisters can call these functions.
 * Added `FetchCanisterLogs` function to `MgmtMethod` and a corresponding wrapper to `ManagementCanister`.
 

ic-agent/src/agent/builder.rs

@@ -82,7 +82,7 @@ impl AgentBuilder {
 
     /// Provides a _default_ ingress expiry. This is the delta that will be applied
     /// at the time an update or query is made. The default expiry cannot be a
-    /// fixed system time.
+    /// fixed system time. This is also used when checking certificate timestamps.
     ///
     /// The timestamp corresponding to this duration may be rounded in order to reduce
     /// cache misses. The current implementation rounds to the nearest minute if the

ic-agent/src/agent/mod.rs

@@ -27,7 +27,7 @@ mod agent_test;
 use crate::{
     agent::response_authentication::{
         extract_der, lookup_canister_info, lookup_canister_metadata, lookup_request_status,
-        lookup_subnet, lookup_subnet_metrics, lookup_value,
+        lookup_subnet, lookup_subnet_metrics, lookup_time, lookup_value,
     },
     export::Principal,
     identity::Identity,
@@ -210,11 +210,10 @@ pub enum PollResult {
 /// #     )
 /// # }
 /// #
-/// # const URL: &'static str = concat!("http://localhost:", env!("IC_REF_PORT"));
-/// #
 /// async fn create_a_canister() -> Result<Principal, Box<dyn std::error::Error>> {
+/// # let url = format!("http://localhost:{}", option_env!("IC_REF_PORT").unwrap_or("4943"));
 ///   let agent = Agent::builder()
-///     .with_url(URL)
+///     .with_url(url)
 ///     .with_identity(create_identity())
 ///     .build()?;
 ///
@@ -853,6 +852,8 @@ impl Agent {
         ic_verify_bls_signature::verify_bls_signature(sig, &msg, &key)
             .map_err(|_| AgentError::CertificateVerificationFailed())?;
 
+        self.verify_cert_timestamp(cert)?;
+
         Ok(())
     }
 
@@ -877,6 +878,18 @@ impl Agent {
             .map_err(|_| AgentError::CertificateVerificationFailed())
     }
 
+    fn verify_cert_timestamp(&self, cert: &Certificate) -> Result<(), AgentError> {
+        let time = lookup_time(cert)?;
+        if (OffsetDateTime::now_utc()
+            - OffsetDateTime::from_unix_timestamp_nanos(time.into()).unwrap())
+            > self.ingress_expiry
+        {
+            Err(AgentError::CertificateOutdated(self.ingress_expiry))
+        } else {
+            Ok(())
+        }
+    }
+
     fn check_delegation(
         &self,
         delegation: &Option<Delegation>,
@@ -1097,21 +1110,14 @@ impl Agent {
         let cert = self
             .read_state_raw(vec![vec!["subnet".into()]], *canister)
             .await?;
-        let time = leb128::read::unsigned(&mut lookup_value(&cert.tree, [b"time".as_ref()])?)?;
-        if (OffsetDateTime::now_utc()
-            - OffsetDateTime::from_unix_timestamp_nanos(time as _).unwrap())
-            > self.ingress_expiry
-        {
-            Err(AgentError::CertificateOutdated(self.ingress_expiry))
-        } else {
-            let (subnet_id, subnet) = lookup_subnet(&cert, &self.root_key.read().unwrap())?;
-            let subnet = Arc::new(subnet);
-            self.subnet_key_cache
-                .lock()
-                .unwrap()
-                .insert_subnet(subnet_id, subnet.clone());
-            Ok(subnet)
-        }
+
+        let (subnet_id, subnet) = lookup_subnet(&cert, &self.root_key.read().unwrap())?;
+        let subnet = Arc::new(subnet);
+        self.subnet_key_cache
+            .lock()
+            .unwrap()
+            .insert_subnet(subnet_id, subnet.clone());
+        Ok(subnet)
     }
 }
 

ic-agent/src/agent/response_authentication.rs

@@ -33,6 +33,13 @@ pub fn extract_der(buf: Vec<u8>) -> Result<Vec<u8>, AgentError> {
     Ok(key.to_vec())
 }
 
+pub(crate) fn lookup_time<Storage: AsRef<[u8]>>(
+    certificate: &Certificate<Storage>,
+) -> Result<u64, AgentError> {
+    let mut time = lookup_value(&certificate.tree, ["time".as_bytes()])?;
+    Ok(leb128::read::unsigned(&mut time)?)
+}
+
 pub(crate) fn lookup_canister_info<Storage: AsRef<[u8]>>(
     certificate: Certificate<Storage>,
     canister_id: Principal,

ic-agent/src/lib.rs

@@ -53,11 +53,10 @@
 //! #     )
 //! # }
 //! #
-//! # const URL: &'static str = concat!("http://localhost:", env!("IC_REF_PORT"));
-//! #
 //! async fn create_a_canister() -> Result<Principal, Box<dyn std::error::Error>> {
+//! # let url = format!("http://localhost:{}", option_env!("IC_REF_PORT").unwrap_or("4943"));
 //!   let agent = Agent::builder()
-//!     .with_url(URL)
+//!     .with_url(url)
 //!     .with_identity(create_identity())
 //!     .build()?;
 //!   // Only do the following call when not contacting the IC main net (e.g. a local emulator).

ic-utils/src/call.rs

@@ -78,11 +78,11 @@ where
     /// #     )
     /// # }
     /// #
-    /// # const URL: &'static str = concat!("http://localhost:", env!("IC_REF_PORT"));
+    /// # let url = format!("http://localhost:{}", option_env!("IC_REF_PORT").unwrap_or("4943"));
     /// #
     /// # let effective_id = Principal::from_text("rwlgt-iiaaa-aaaaa-aaaaa-cai").unwrap();
     ///   let agent = Agent::builder()
-    ///     .with_url(URL)
+    ///     .with_url(url)
     ///     .with_identity(create_identity())
     ///     .build()?;
     ///   agent.fetch_root_key().await?;

ic-utils/src/canister.rs

@@ -410,7 +410,7 @@ mod tests {
                 .expect("Could not read the key pair."),
         );
 
-        let port = std::env::var("IC_REF_PORT").unwrap_or_else(|_| "8001".into());
+        let port = std::env::var("IC_REF_PORT").unwrap_or_else(|_| "4943".into());
 
         let agent = ic_agent::Agent::builder()
             .with_transport(ReqwestTransport::create(format!("http://localhost:{port}")).unwrap())

ref-tests/src/utils.rs

@@ -94,7 +94,7 @@ Sks4xGbA/ZbazsrMl4v446U5UIVxCGGaKw==
 }
 
 pub async fn create_agent(identity: impl Identity + 'static) -> Result<Agent, String> {
-    let port_env = std::env::var("IC_REF_PORT").unwrap_or_else(|_| "8001".into());
+    let port_env = std::env::var("IC_REF_PORT").unwrap_or_else(|_| "4943".into());
     let port = port_env
         .parse::<u32>()
         .expect("Could not parse the IC_REF_PORT environment variable as an integer.");

ref-tests/tests/ic-ref.rs

@@ -1227,6 +1227,15 @@ mod extras {
                 "wrong error: {result:?}"
             );
 
+            ic00.stop_canister(&specified_id)
+                .call_and_wait()
+                .await
+                .unwrap();
+            ic00.delete_canister(&specified_id)
+                .call_and_wait()
+                .await
+                .unwrap();
+
             Ok(())
         })
     }