diff --git a/rs/boundary_node/rate_limits/canister/canister.rs b/rs/boundary_node/rate_limits/canister/canister.rs index 27f596da9a3..e1a0e183da8 100644 --- a/rs/boundary_node/rate_limits/canister/canister.rs +++ b/rs/boundary_node/rate_limits/canister/canister.rs @@ -22,7 +22,8 @@ use rate_limits_api::{ const REGISTRY_CANISTER_ID: &str = "rwlgt-iiaaa-aaaaa-aaaaa-cai"; const REGISTRY_CANISTER_METHOD: &str = "get_api_boundary_node_ids"; -const CANISTER_UPDATE_METHODS: [&str; 2] = ["add_config", "disclose_rules"]; +const UPDATE_METHODS: [&str; 2] = ["add_config", "disclose_rules"]; +const REPLICATED_QUERY_METHOD: &str = "get_config"; #[inspect_message] fn inspect_message() { @@ -30,26 +31,33 @@ fn inspect_message() { let caller_id: Principal = ic_cdk::api::caller(); let called_method = ic_cdk::api::call::method_name(); - // If the called method is not an update method, accept the message. - if !CANISTER_UPDATE_METHODS.contains(&called_method.as_str()) { - ic_cdk::api::call::accept_message(); - } else { - // For the update methods: - // - Check if the canister's authorized principal is set - // - Check caller_id matches the authorized principal - with_canister_state(|state| { - if let Some(authorized_principal) = state.get_authorized_principal() { - if caller_id == authorized_principal { - ic_cdk::api::call::accept_message(); - } else { - ic_cdk::api::trap("inspect_message_failed: unauthorized caller"); - } - } else { - ic_cdk::api::trap( - "inspect_message_failed: authorized principal for canister is not set", - ); - } + let (has_full_access, has_full_read_access) = with_canister_state(|state| { + let authorized_principal = state.get_authorized_principal().unwrap_or_else(|| { + ic_cdk::api::trap( + "message_inspection_failed: authorized principal for canister is not set", + ) }); + ( + caller_id == authorized_principal, + state.is_api_boundary_node_principal(&caller_id), + ) + }); + + if called_method == REPLICATED_QUERY_METHOD { + if has_full_access || has_full_read_access { + ic_cdk::api::call::accept_message(); + } + } else if UPDATE_METHODS.contains(&called_method.as_str()) { + if has_full_access { + ic_cdk::api::call::accept_message(); + } else { + ic_cdk::api::trap("message_inspection_failed: unauthorized caller"); + } + } else { + // All others calls are rejected + ic_cdk::api::trap( + "message_inspection_failed: method call is prohibited in the current context", + ); } } diff --git a/rs/boundary_node/rate_limits/canister/state.rs b/rs/boundary_node/rate_limits/canister/state.rs index 9423de12283..d185acded7b 100644 --- a/rs/boundary_node/rate_limits/canister/state.rs +++ b/rs/boundary_node/rate_limits/canister/state.rs @@ -1,12 +1,13 @@ use candid::Principal; use mockall::automock; +use std::collections::HashSet; use crate::{ add_config::{INIT_SCHEMA_VERSION, INIT_VERSION}, storage::{ LocalRef, StableMap, StorableConfig, StorableIncidentId, StorableIncidentMetadata, StorablePrincipal, StorableRuleId, StorableRuleMetadata, StorableVersion, - AUTHORIZED_PRINCIPAL, CONFIGS, INCIDENTS, RULES, + API_BOUNDARY_NODE_PRINCIPALS, AUTHORIZED_PRINCIPAL, CONFIGS, INCIDENTS, RULES, }, types::{IncidentId, InputConfig, InputRule, RuleId, Timestamp, Version}, }; @@ -31,6 +32,7 @@ pub trait CanisterApi { incident_id: IncidentId, rule_ids: StorableIncidentMetadata, ) -> Option; + fn is_api_boundary_node_principal(&self, principal: &Principal) -> bool; } #[derive(Clone)] @@ -39,6 +41,7 @@ pub struct CanisterState { rules: LocalRef>, incidents: LocalRef>, authorized_principal: LocalRef>, + api_boundary_node_principals: LocalRef>, } impl CanisterState { @@ -48,6 +51,7 @@ impl CanisterState { rules: &RULES, incidents: &INCIDENTS, authorized_principal: &AUTHORIZED_PRINCIPAL, + api_boundary_node_principals: &API_BOUNDARY_NODE_PRINCIPALS, } } } @@ -128,6 +132,11 @@ impl CanisterApi for CanisterState { .insert(StorableIncidentId(incident_id.0), rule_ids) }) } + + fn is_api_boundary_node_principal(&self, principal: &Principal) -> bool { + self.api_boundary_node_principals + .with(|cell| cell.borrow().contains(principal)) + } } pub fn init_version_and_config(time: Timestamp, canister_api: impl CanisterApi) { diff --git a/rs/boundary_node/rate_limits/canister_client/src/main.rs b/rs/boundary_node/rate_limits/canister_client/src/main.rs index 6d6aa5de7be..95d8a927ad9 100644 --- a/rs/boundary_node/rate_limits/canister_client/src/main.rs +++ b/rs/boundary_node/rate_limits/canister_client/src/main.rs @@ -236,9 +236,9 @@ async fn read_config(agent: &Agent, version: Version, canister_id: Principal) -> let args = Encode!(&Some(version)).unwrap(); let response = agent - .update(&canister_id, "get_config") + .query(&canister_id, "get_config") .with_arg(args) - .call_and_wait() + .call() .await .expect("update call failed"); @@ -276,9 +276,9 @@ async fn read_rule(agent: &Agent, rule_id: &RuleId, canister_id: Principal) { let args = Encode!(rule_id).unwrap(); let response = agent - .update(&canister_id, "get_rule_by_id") + .query(&canister_id, "get_rule_by_id") .with_arg(args) - .call_and_wait() + .call() .await .expect("update call failed"); diff --git a/rs/boundary_node/rate_limits/integration_tests/src/pocket_ic_helpers.rs b/rs/boundary_node/rate_limits/integration_tests/src/pocket_ic_helpers.rs index ce8389b3e3b..b3b83f9b0b3 100644 --- a/rs/boundary_node/rate_limits/integration_tests/src/pocket_ic_helpers.rs +++ b/rs/boundary_node/rate_limits/integration_tests/src/pocket_ic_helpers.rs @@ -96,18 +96,26 @@ pub async fn get_installed_wasm_hash(pocket_ic: &PocketIc, canister_id: Principa pub async fn canister_call( pocket_ic: &PocketIc, method: &str, + method_type: &str, canister_id: Principal, sender: Principal, payload: Vec, ) -> Result { - let result = pocket_ic - .update_call(canister_id, sender, method, payload) - .await - .map_err(|err| err.to_string())?; + let result = match method_type { + "query" => pocket_ic + .query_call(canister_id, sender, method, payload) + .await + .map_err(|err| err.to_string())?, + "update" => pocket_ic + .update_call(canister_id, sender, method, payload) + .await + .map_err(|err| err.to_string())?, + _ => panic!("{method_type} is not allowed"), + }; let result = match result { WasmResult::Reply(result) => result, - WasmResult::Reject(s) => panic!("Call to add_config failed: {:#?}", s), + WasmResult::Reject(s) => panic!("Call to {method} failed: {:#?}", s), }; let decoded: R = Decode!(&result, R).unwrap(); diff --git a/rs/boundary_node/rate_limits/integration_tests/tests/rate_limit_canister_tests.rs b/rs/boundary_node/rate_limits/integration_tests/tests/rate_limit_canister_tests.rs index a9ce34ef8f9..975b348fd20 100644 --- a/rs/boundary_node/rate_limits/integration_tests/tests/rate_limit_canister_tests.rs +++ b/rs/boundary_node/rate_limits/integration_tests/tests/rate_limit_canister_tests.rs @@ -31,6 +31,7 @@ async fn main() { let response: GetConfigResponse = canister_call( &pocket_ic, "get_config", + "query", canister_id, Principal::anonymous(), input_version, @@ -52,6 +53,7 @@ async fn main() { let response: AddConfigResponse = canister_call( &pocket_ic, "add_config", + "update", canister_id, Principal::anonymous(), input_config.clone(), @@ -60,12 +62,13 @@ async fn main() { let err_msg = response.unwrap_err(); - assert!(err_msg.contains("inspect_message_failed: unauthorized caller")); + assert!(err_msg.contains("message_inspection_failed: unauthorized caller")); // Try add config using authorized principal as sender, assert success let response: AddConfigResponse = canister_call( &pocket_ic, "add_config", + "update", canister_id, authorized_principal, input_config, @@ -81,6 +84,7 @@ async fn main() { let response: GetConfigResponse = canister_call( &pocket_ic, "get_config", + "query", canister_id, Principal::anonymous(), input_version, diff --git a/rs/tests/boundary_nodes/rate_limit_canister_test.rs b/rs/tests/boundary_nodes/rate_limit_canister_test.rs index 8f8721deed9..e2b0dab11ca 100644 --- a/rs/tests/boundary_nodes/rate_limit_canister_test.rs +++ b/rs/tests/boundary_nodes/rate_limit_canister_test.rs @@ -325,9 +325,9 @@ async fn read_config( let args = Encode!(&Some(version)).unwrap(); let response = agent - .update(&canister_id, "get_config") + .query(&canister_id, "get_config") .with_arg(args) - .call_and_wait() + .call() .await .expect("update call failed"); @@ -375,9 +375,9 @@ async fn read_rule( let args = Encode!(rule_id).unwrap(); let response = agent - .update(&canister_id, "get_rule_by_id") + .query(&canister_id, "get_rule_by_id") .with_arg(args) - .call_and_wait() + .call() .await .expect("update call failed");