From 66246e9d016ee98ba5b13d0a5f36b7d5d6a426ee Mon Sep 17 00:00:00 2001
From: Adam Spofford <93943719+adamspofford-dfinity@users.noreply.github.com>
Date: Thu, 29 Feb 2024 10:48:58 -0800
Subject: [PATCH] fix: Include certificate time checks for read_subnet_state
 too (#522)

---
 CHANGELOG.md              |  2 +-
 ic-agent/src/agent/mod.rs | 30 ++++++++++++++++++++++++------
 2 files changed, 25 insertions(+), 7 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index cd078bb1..3ac9d977 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -8,7 +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.
+* Timestamps are now being checked in `Agent::verify` and `Agent::verify_for_subnet`. 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`.
 
diff --git a/ic-agent/src/agent/mod.rs b/ic-agent/src/agent/mod.rs
index 6a7d6b88..ecf8794b 100644
--- a/ic-agent/src/agent/mod.rs
+++ b/ic-agent/src/agent/mod.rs
@@ -838,6 +838,16 @@ impl Agent {
         &self,
         cert: &Certificate,
         effective_canister_id: Principal,
+    ) -> Result<(), AgentError> {
+        self.verify_cert(cert, effective_canister_id)?;
+        self.verify_cert_timestamp(cert)?;
+        Ok(())
+    }
+
+    fn verify_cert(
+        &self,
+        cert: &Certificate,
+        effective_canister_id: Principal,
     ) -> Result<(), AgentError> {
         let sig = &cert.signature;
 
@@ -851,9 +861,6 @@ impl Agent {
 
         ic_verify_bls_signature::verify_bls_signature(sig, &msg, &key)
             .map_err(|_| AgentError::CertificateVerificationFailed())?;
-
-        self.verify_cert_timestamp(cert)?;
-
         Ok(())
     }
 
@@ -863,6 +870,16 @@ impl Agent {
         &self,
         cert: &Certificate,
         subnet_id: Principal,
+    ) -> Result<(), AgentError> {
+        self.verify_cert_for_subnet(cert, subnet_id)?;
+        self.verify_cert_timestamp(cert)?;
+        Ok(())
+    }
+
+    fn verify_cert_for_subnet(
+        &self,
+        cert: &Certificate,
+        subnet_id: Principal,
     ) -> Result<(), AgentError> {
         let sig = &cert.signature;
 
@@ -875,7 +892,8 @@ impl Agent {
         let key = extract_der(der_key)?;
 
         ic_verify_bls_signature::verify_bls_signature(sig, &msg, &key)
-            .map_err(|_| AgentError::CertificateVerificationFailed())
+            .map_err(|_| AgentError::CertificateVerificationFailed())?;
+        Ok(())
     }
 
     fn verify_cert_timestamp(&self, cert: &Certificate) -> Result<(), AgentError> {
@@ -903,7 +921,7 @@ impl Agent {
                 if cert.delegation.is_some() {
                     return Err(AgentError::CertificateHasTooManyDelegations);
                 }
-                self.verify(&cert, effective_canister_id)?;
+                self.verify_cert(&cert, effective_canister_id)?;
                 let canister_range_lookup = [
                     "subnet".as_bytes(),
                     delegation.subnet_id.as_ref(),
@@ -940,7 +958,7 @@ impl Agent {
                 if cert.delegation.is_some() {
                     return Err(AgentError::CertificateHasTooManyDelegations);
                 }
-                self.verify_for_subnet(&cert, subnet_id)?;
+                self.verify_cert_for_subnet(&cert, subnet_id)?;
                 let public_key_path = [
                     "subnet".as_bytes(),
                     delegation.subnet_id.as_ref(),