Skip to content
2 changes: 1 addition & 1 deletion rs/cycles_account_manager/src/cycles_account_manager.rs
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ const DAY: Duration = Duration::from_secs(SECONDS_PER_DAY as u64);

/// Maximum payload size of a management call to update_settings
/// overriding the canister's freezing threshold.
const MAX_DELAYED_INGRESS_COST_PAYLOAD_SIZE: usize = 352;
const MAX_DELAYED_INGRESS_COST_PAYLOAD_SIZE: usize = 366;

struct CyclesBurnedRate {
memory: CompoundCycles<Memory>,
Expand Down
37 changes: 32 additions & 5 deletions rs/execution_environment/src/canister_manager.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ use crate::as_round_instructions;
use crate::canister_settings::CanisterSettings;
use crate::execution::common::{
validate_controller, validate_controller_or_subnet_admin, validate_snapshot_visibility,
validate_subnet_admin,
validate_status_visibility, validate_subnet_admin,
};
use crate::execution::install_code::OriginalContext;
use crate::execution::{install::execute_install, upgrade::execute_upgrade};
Expand Down Expand Up @@ -182,11 +182,31 @@ impl CanisterManager {
}
}

// `CanisterStatus` is governed by the canister's status visibility
// settings: subnet admins always retain access; otherwise access is
// granted to the controllers plus any additional allowed viewers, or
// to everyone if the status is public.
Ok(Ic00Method::CanisterStatus) => {
match effective_canister_id {
Some(canister_id) => {
let canister = state.canister_state(&canister_id).ok_or_else(|| UserError::new(
ErrorCode::CanisterNotFound,
format!("Canister {canister_id} not found"),
))?;
let subnet_admins = state.get_own_subnet_admins();
validate_status_visibility(canister, subnet_admins, &sender.get()).map_err(|err| err.into())
},
None => Err(UserError::new(
ErrorCode::InvalidManagementPayload,
format!("Failed to decode payload for ic00 method: {method_name}"),
)),
}
},

// These methods are only valid if they are sent by the controller
// of the canister or a subnet admin. We assume that the canister
// always wants to accept such messages.
Ok(Ic00Method::CanisterStatus)
| Ok(Ic00Method::StartCanister)
Ok(Ic00Method::StartCanister)
| Ok(Ic00Method::UninstallCode)
| Ok(Ic00Method::StopCanister)
| Ok(Ic00Method::DeleteCanister)
Expand Down Expand Up @@ -359,6 +379,11 @@ impl CanisterManager {
canister.system_state.snapshot_visibility = snapshot_visibility.clone();
}

// Status visibility: apply.
if let Some(status_visibility) = settings.status_visibility() {
canister.system_state.status_visibility = status_visibility.clone();
}

// Wasm memory threshold: apply.
if let Some(wasm_memory_threshold) = settings.wasm_memory_threshold() {
canister.system_state.wasm_memory_threshold = wasm_memory_threshold;
Expand Down Expand Up @@ -1071,10 +1096,10 @@ impl CanisterManager {
ready_for_migration: bool,
subnet_admins: Option<BTreeSet<PrincipalId>>,
) -> Result<CanisterStatusResultV2, CanisterManagerError> {
// Skip the controller check if the canister itself is requesting its
// Skip the visibility check if the canister itself is requesting its
// own status, as the canister is considered in the same trust domain.
if sender != canister.canister_id().get() {
validate_controller_or_subnet_admin(canister, subnet_admins, &sender)?
validate_status_visibility(canister, subnet_admins, &sender)?
}

let controller = canister.system_state.controller();
Expand Down Expand Up @@ -1105,6 +1130,7 @@ impl CanisterManager {
canister.system_state.minimum_incoming_canister_call_cycles;
let log_visibility = canister.system_state.log_visibility.clone();
let snapshot_visibility = canister.system_state.snapshot_visibility.clone();
let status_visibility = canister.system_state.status_visibility.clone();
let log_memory_limit = canister.log_memory_limit().get();
let wasm_memory_limit = canister.system_state.wasm_memory_limit;
let wasm_memory_threshold = canister.system_state.wasm_memory_threshold;
Expand Down Expand Up @@ -1137,6 +1163,7 @@ impl CanisterManager {
minimum_incoming_canister_call_cycles.get(),
log_visibility,
snapshot_visibility,
status_visibility,
log_memory_limit,
self.cycles_account_manager
.idle_cycles_burned_rate(
Expand Down
11 changes: 4 additions & 7 deletions rs/execution_environment/src/canister_manager/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1391,9 +1391,9 @@ fn get_canister_status_with_incorrect_controller_fails() {

let err = test.canister_status(canister_id).unwrap_err();

assert_eq!(err.code(), ErrorCode::CanisterInvalidController);
assert_eq!(err.code(), ErrorCode::CanisterStatusAccessDenied);
assert!(err.description().contains(&format!(
"Only the controllers of the canister {canister_id} can control it"
"Caller {test_user} is not allowed to read the canister status"
)));
}

Expand Down Expand Up @@ -8603,12 +8603,9 @@ fn non_controller_and_non_subnet_admin_cannot_perform_subnet_admin_actions_on_ca

// ...or status cannot be checked...
let err = test.canister_status(canister_id).unwrap_err();
assert_eq!(
err.code(),
ErrorCode::CanisterInvalidControllerOrSubnetAdmin
);
assert_eq!(err.code(), ErrorCode::CanisterStatusAccessDenied);
assert!(err.description().contains(&format!(
"Only the controllers of the canister {canister_id} or subnet admins can perform certain actions"
"Caller {test_user} is not allowed to read the canister status"
)));
// ...or canister metrics cannot be retrieved...
let err = test.canister_metrics(canister_id).unwrap_err();
Expand Down
17 changes: 17 additions & 0 deletions rs/execution_environment/src/canister_manager/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -501,6 +501,9 @@ pub(crate) enum CanisterManagerError {
caller: PrincipalId,
method_name: String,
},
CanisterStatusAccessDenied {
caller: PrincipalId,
},
FetchCanisterLogsNotEnoughCycles {
sent: Cycles,
required: Cycles,
Expand Down Expand Up @@ -761,6 +764,11 @@ impl AsErrorHelp for CanisterManagerError {
.to_string(),
doc_link: doc_ref("invalid-controller"),
},
CanisterManagerError::CanisterStatusAccessDenied { .. } => ErrorHelp::UserError {
suggestion: "Execute this call from a principal with canister status read access."
.to_string(),
doc_link: "".to_string(),
},
CanisterManagerError::FetchCanisterLogsAccessDenied { .. } => ErrorHelp::UserError {
suggestion: "Execute this call from a controller of the target canister or \
a principal with log read access."
Expand Down Expand Up @@ -1201,6 +1209,15 @@ impl From<CanisterManagerError> for UserError {
ErrorCode::CanisterRejectedMessage,
format!("Caller {caller} is not allowed to call {method_name}"),
),
CanisterStatusAccessDenied { caller } => Self::new(
// `CanisterStatusAccessDenied` is a dedicated error code that is
// mapped to the same reject code (`CanisterError`) as the
// `CanisterInvalidController` error code that governed access to
// `canister_status` before the status visibility feature was
// introduced.
ErrorCode::CanisterStatusAccessDenied,
format!("Caller {caller} is not allowed to read the canister status"),
),
CanisterLogMemoryLimitIsTooHigh { bytes, limit } => Self::new(
ErrorCode::CanisterRejectedMessage,
format!(
Expand Down
30 changes: 30 additions & 0 deletions rs/execution_environment/src/canister_settings.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ use ic_base_types::{EnvironmentVariables, NumBytes, NumSeconds};
use ic_error_types::{ErrorCode, UserError};
use ic_management_canister_types_private::{
BoundedAllowedViewers, CanisterSettingsArgs, LogVisibilityV2, SnapshotVisibility,
StatusVisibility,
};
use ic_types::{ComputeAllocation, InvalidComputeAllocationError, MemoryAllocation, PrincipalId};
use ic_types_cycles::Cycles;
Expand Down Expand Up @@ -29,12 +30,14 @@ pub(crate) struct CanisterSettings {
pub(crate) minimum_incoming_canister_call_cycles: Option<Cycles>,
pub(crate) log_visibility: Option<LogVisibilityV2>,
pub(crate) snapshot_visibility: Option<SnapshotVisibility>,
pub(crate) status_visibility: Option<StatusVisibility>,
pub(crate) log_memory_limit: Option<NumBytes>,
pub(crate) wasm_memory_limit: Option<NumBytes>,
pub(crate) environment_variables: Option<EnvironmentVariables>,
}

impl CanisterSettings {
#[allow(clippy::too_many_arguments)]
pub fn new(
controllers: Option<Vec<PrincipalId>>,
compute_allocation: Option<ComputeAllocation>,
Expand All @@ -45,6 +48,7 @@ impl CanisterSettings {
minimum_incoming_canister_call_cycles: Option<Cycles>,
log_visibility: Option<LogVisibilityV2>,
snapshot_visibility: Option<SnapshotVisibility>,
status_visibility: Option<StatusVisibility>,
log_memory_limit: Option<NumBytes>,
wasm_memory_limit: Option<NumBytes>,
environment_variables: Option<EnvironmentVariables>,
Expand All @@ -59,6 +63,7 @@ impl CanisterSettings {
minimum_incoming_canister_call_cycles,
log_visibility,
snapshot_visibility,
status_visibility,
log_memory_limit,
wasm_memory_limit,
environment_variables,
Expand Down Expand Up @@ -101,6 +106,10 @@ impl CanisterSettings {
self.snapshot_visibility.as_ref()
}

pub fn status_visibility(&self) -> Option<&StatusVisibility> {
self.status_visibility.as_ref()
}

pub fn log_memory_limit(&self) -> Option<NumBytes> {
self.log_memory_limit
}
Expand Down Expand Up @@ -223,6 +232,7 @@ impl TryFrom<CanisterSettingsArgs> for CanisterSettings {
minimum_incoming_canister_call_cycles,
input.log_visibility,
input.snapshot_visibility,
input.status_visibility,
log_memory_limit,
wasm_memory_limit,
environment_variables,
Expand Down Expand Up @@ -251,6 +261,7 @@ pub(crate) struct CanisterSettingsBuilder {
minimum_incoming_canister_call_cycles: Option<Cycles>,
log_visibility: Option<LogVisibilityV2>,
snapshot_visibility: Option<SnapshotVisibility>,
status_visibility: Option<StatusVisibility>,
log_memory_limit: Option<NumBytes>,
wasm_memory_limit: Option<NumBytes>,
environment_variables: Option<EnvironmentVariables>,
Expand All @@ -269,6 +280,7 @@ impl CanisterSettingsBuilder {
minimum_incoming_canister_call_cycles: None,
log_visibility: None,
snapshot_visibility: None,
status_visibility: None,
log_memory_limit: None,
wasm_memory_limit: None,
environment_variables: None,
Expand All @@ -286,6 +298,7 @@ impl CanisterSettingsBuilder {
minimum_incoming_canister_call_cycles: self.minimum_incoming_canister_call_cycles,
log_visibility: self.log_visibility,
snapshot_visibility: self.snapshot_visibility,
status_visibility: self.status_visibility,
log_memory_limit: self.log_memory_limit,
wasm_memory_limit: self.wasm_memory_limit,
environment_variables: self.environment_variables,
Expand Down Expand Up @@ -358,6 +371,13 @@ impl CanisterSettingsBuilder {
}
}

pub fn with_status_visibility(self, status_visibility: StatusVisibility) -> Self {
Self {
status_visibility: Some(status_visibility),
..self
}
}

pub fn with_log_memory_limit(self, log_memory_limit: NumBytes) -> Self {
Self {
log_memory_limit: Some(log_memory_limit),
Expand Down Expand Up @@ -487,6 +507,16 @@ impl<'a> From<&'a SnapshotVisibility> for VisibilitySettings<'a> {
}
}

impl<'a> From<&'a StatusVisibility> for VisibilitySettings<'a> {
fn from(v: &'a StatusVisibility) -> Self {
match v {
StatusVisibility::Public => Self::Public,
StatusVisibility::Controllers => Self::Controllers,
StatusVisibility::AllowedViewers(principals) => Self::AllowedViewers(principals),
}
}
}

impl VisibilitySettings<'_> {
pub(crate) fn has_access(
&self,
Expand Down
26 changes: 26 additions & 0 deletions rs/execution_environment/src/execution/common.rs
Original file line number Diff line number Diff line change
Expand Up @@ -376,6 +376,32 @@ pub(crate) fn validate_snapshot_visibility(
Ok(())
}

/// Validates that the `caller` is allowed to read the status of the `canister`
/// according to its canister status visibility settings.
///
/// Subnet admins always retain access; otherwise access is governed by the
/// status visibility setting, which grants access to the controllers plus any
/// additional allowed viewers (or everyone, if the status is public).
pub(crate) fn validate_status_visibility(
canister: &CanisterState,
subnet_admins: Option<BTreeSet<PrincipalId>>,
caller: &PrincipalId,
) -> Result<(), CanisterManagerError> {
// Subnet admins always retain access to the canister status.
if let Some(subnet_admins) = &subnet_admins
&& subnet_admins.contains(caller)
{
return Ok(());
}
// Otherwise, access is governed by the status visibility setting.
if crate::canister_settings::VisibilitySettings::from(canister.status_visibility())
.has_access(caller, canister.controllers())
{
return Ok(());
}
Err(CanisterManagerError::CanisterStatusAccessDenied { caller: *caller })
}

pub(crate) fn validate_subnet_admin(
subnet_admins: &BTreeSet<PrincipalId>,
sender: &PrincipalId,
Expand Down
7 changes: 2 additions & 5 deletions rs/execution_environment/src/execution_environment/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3057,7 +3057,7 @@ fn management_message_with_invalid_sender_is_not_accepted_without_subnet_admins(
let err = test
.should_accept_ingress_message(IC_00, "canister_status", Encode!(&arg).unwrap())
.unwrap_err();
assert_eq!(ErrorCode::CanisterInvalidController, err.code());
assert_eq!(ErrorCode::CanisterStatusAccessDenied, err.code());
}

#[test]
Expand All @@ -3077,10 +3077,7 @@ fn management_message_with_invalid_sender_is_not_accepted_with_subnet_admins() {
let err = test
.should_accept_ingress_message(IC_00, "canister_status", Encode!(&arg).unwrap())
.unwrap_err();
assert_eq!(
ErrorCode::CanisterInvalidControllerOrSubnetAdmin,
err.code()
);
assert_eq!(ErrorCode::CanisterStatusAccessDenied, err.code());
}

#[test]
Expand Down
Loading
Loading