Skip to content

Commit

Permalink
feat: [IC-272] add log_visibility logic to checking fetch_canister_lo…
Browse files Browse the repository at this point in the history
…gs ingress message
  • Loading branch information
maksymar committed Jan 25, 2024
1 parent 5c60fea commit e841826
Show file tree
Hide file tree
Showing 5 changed files with 135 additions and 15 deletions.
31 changes: 24 additions & 7 deletions rs/execution_environment/src/canister_manager.rs
Expand Up @@ -17,8 +17,8 @@ use ic_cycles_account_manager::{CyclesAccountManager, ResourceSaturation};
use ic_error_types::{ErrorCode, RejectCode, UserError};
use ic_ic00_types::{
CanisterChangeDetails, CanisterChangeOrigin, CanisterInstallModeV2, CanisterStatusResultV2,
CanisterStatusType, InstallChunkedCodeArgs, InstallCodeArgsV2, Method as Ic00Method,
StoredChunksReply, UploadChunkReply,
CanisterStatusType, InstallChunkedCodeArgs, InstallCodeArgsV2, LogVisibility,
Method as Ic00Method, StoredChunksReply, UploadChunkReply,
};
use ic_interfaces::execution_environment::{
CanisterOutOfCyclesError, HypervisorError, IngressHistoryWriter, SubnetAvailableMemory,
Expand Down Expand Up @@ -531,11 +531,28 @@ impl CanisterManager {
}
},

// TODO(IC-272).
Ok(Ic00Method::FetchCanisterLogs) => Err(UserError::new(
ErrorCode::CanisterRejectedMessage,
format!("{} API is not yet implemented", Ic00Method::FetchCanisterLogs)
)),
Ok(Ic00Method::FetchCanisterLogs) => {
match effective_canister_id {
Some(canister_id) => {
let canister = state.canister_state(&canister_id).ok_or_else(|| UserError::new(
ErrorCode::CanisterNotFound,
format!("Canister {} not found", canister_id),
))?;
match canister.log_visibility() {
LogVisibility::Public => Ok(()),
LogVisibility::Controllers if canister.controllers().contains(&sender.get()) => Ok(()),
LogVisibility::Controllers => Err(UserError::new(
ErrorCode::CanisterRejectedMessage,
format!("Caller {} is not allowed to call ic00 method {}", sender, method_name),
)),
}
},
None => Err(UserError::new(
ErrorCode::InvalidManagementPayload,
format!("Failed to decode payload for ic00 method: {}", method_name),
)),
}
},

Ok(Ic00Method::ProvisionalCreateCanisterWithCycles)
| Ok(Ic00Method::BitcoinGetSuccessors)
Expand Down
83 changes: 80 additions & 3 deletions rs/execution_environment/src/execution_environment/tests.rs
Expand Up @@ -11,9 +11,9 @@ use ic_error_types::{ErrorCode, RejectCode, UserError};
use ic_ic00_types::{
self as ic00, BitcoinGetUtxosArgs, BitcoinNetwork, BoundedHttpHeaders, CanisterChange,
CanisterHttpRequestArgs, CanisterIdRecord, CanisterStatusResultV2, CanisterStatusType,
DerivationPath, EcdsaCurve, EcdsaKeyId, EmptyBlob, HttpMethod, LogVisibility, Method,
Payload as Ic00Payload, ProvisionalCreateCanisterWithCyclesArgs, ProvisionalTopUpCanisterArgs,
TransformContext, TransformFunc, IC_00,
DerivationPath, EcdsaCurve, EcdsaKeyId, EmptyBlob, FetchCanisterLogsRequest, HttpMethod,
LogVisibility, Method, Payload as Ic00Payload, ProvisionalCreateCanisterWithCyclesArgs,
ProvisionalTopUpCanisterArgs, TransformContext, TransformFunc, IC_00,
};
use ic_registry_routing_table::canister_id_into_u64;
use ic_registry_routing_table::CanisterIdRange;
Expand Down Expand Up @@ -3270,3 +3270,80 @@ fn test_canister_settings_log_visibility_set_to_public() {
LogVisibility::Public
);
}

#[test]
fn test_fetch_canister_logs_should_accept_ingress_message_log_visibility_public_succeeds() {
// Arrange.
let mut test = ExecutionTestBuilder::new().build();
let canister_id = test.universal_canister().unwrap();
let not_a_controller = user_test_id(42);
test.set_log_visibility(canister_id, LogVisibility::Public)
.unwrap();
// Act.
test.set_user_id(not_a_controller);
let result = test.should_accept_ingress_message(
test.state().metadata.own_subnet_id.into(),
Method::FetchCanisterLogs,
FetchCanisterLogsRequest {
canister_id: canister_id.into(),
}
.encode(),
);
// Assert.
assert_eq!(result, Ok(()));
}

#[test]
fn test_fetch_canister_logs_should_accept_ingress_message_log_visibility_invalid_controller_fails()
{
// Arrange.
let mut test = ExecutionTestBuilder::new().build();
let canister_id = test.universal_canister().unwrap();
let not_a_controller = user_test_id(42);
test.set_log_visibility(canister_id, LogVisibility::Controllers)
.unwrap();
// Act.
test.set_user_id(not_a_controller);
let result = test.should_accept_ingress_message(
test.state().metadata.own_subnet_id.into(),
Method::FetchCanisterLogs,
FetchCanisterLogsRequest {
canister_id: canister_id.into(),
}
.encode(),
);
// Assert.
assert_eq!(
result,
Err(UserError::new(
ErrorCode::CanisterRejectedMessage,
format!(
"Caller {not_a_controller} is not allowed to call ic00 method fetch_canister_logs"
),
))
);
}

#[test]
fn test_fetch_canister_logs_should_accept_ingress_message_log_visibility_valid_controller_succeeds()
{
// Arrange.
let mut test = ExecutionTestBuilder::new().build();
let canister_id = test.universal_canister().unwrap();
let controller = user_test_id(42);
test.set_log_visibility(canister_id, LogVisibility::Controllers)
.unwrap();
test.set_controller(canister_id, controller.get()).unwrap();
// Act.
test.set_user_id(controller);
let result = test.should_accept_ingress_message(
test.state().metadata.own_subnet_id.into(),
Method::FetchCanisterLogs,
FetchCanisterLogsRequest {
canister_id: canister_id.into(),
}
.encode(),
);
// Assert.
assert_eq!(result, Ok(()));
}
6 changes: 5 additions & 1 deletion rs/replicated_state/src/canister_state.rs
Expand Up @@ -8,7 +8,7 @@ use crate::canister_state::queues::CanisterOutputQueuesIterator;
use crate::canister_state::system_state::{CanisterStatus, ExecutionTask, SystemState};
use crate::{InputQueueType, StateError};
pub use execution_state::{EmbedderCache, ExecutionState, ExportedFunctions, Global};
use ic_ic00_types::CanisterStatusType;
use ic_ic00_types::{CanisterStatusType, LogVisibility};
use ic_registry_subnet_type::SubnetType;
use ic_types::batch::TotalQueryStats;
use ic_types::methods::SystemMethod;
Expand Down Expand Up @@ -154,6 +154,10 @@ impl CanisterState {
&self.system_state.controllers
}

pub fn log_visibility(&self) -> LogVisibility {
self.system_state.log_visibility
}

/// Returns the difference in time since the canister was last charged for resource allocations.
pub fn duration_since_last_allocation_charge(&self, current_time: Time) -> Duration {
debug_assert!(
Expand Down
19 changes: 19 additions & 0 deletions rs/types/ic00_types/src/lib.rs
Expand Up @@ -2295,6 +2295,25 @@ pub struct NodeMetricsHistoryResponse {

impl Payload<'_> for NodeMetricsHistoryResponse {}

/// `CandidType` for `FetchCanisterLogsRequest`
/// ```text
/// record {
/// canister_id: principal;
/// }
/// ```
#[derive(Default, Clone, CandidType, Deserialize, Debug)]
pub struct FetchCanisterLogsRequest {
pub canister_id: PrincipalId,
}

impl Payload<'_> for FetchCanisterLogsRequest {}

impl FetchCanisterLogsRequest {
pub fn get_canister_id(&self) -> CanisterId {
CanisterId::unchecked_from_principal(self.canister_id)
}
}

/// Struct used for encoding/decoding
/// `(record {
/// canister_id: principal;
Expand Down
11 changes: 7 additions & 4 deletions rs/types/types/src/messages/ingress_messages.rs
Expand Up @@ -11,9 +11,9 @@ use crate::{
};
use ic_error_types::{ErrorCode, UserError};
use ic_ic00_types::{
CanisterIdRecord, CanisterInfoRequest, ClearChunkStoreArgs, InstallChunkedCodeArgs,
InstallCodeArgsV2, Method, Payload, StoredChunksArgs, UpdateSettingsArgs, UploadChunkArgs,
IC_00,
CanisterIdRecord, CanisterInfoRequest, ClearChunkStoreArgs, FetchCanisterLogsRequest,
InstallChunkedCodeArgs, InstallCodeArgsV2, Method, Payload, StoredChunksArgs,
UpdateSettingsArgs, UploadChunkArgs, IC_00,
};
use ic_protobuf::{
log::ingress_message_log_entry::v1::IngressMessageLogEntry,
Expand Down Expand Up @@ -511,7 +511,10 @@ pub fn extract_effective_canister_id(
Ok(record) => Ok(Some(record.get_canister_id())),
Err(err) => Err(ParseIngressError::InvalidSubnetPayload(err.to_string())),
},
Ok(Method::FetchCanisterLogs) => Err(ParseIngressError::UnknownSubnetMethod), // TODO(IC-272).
Ok(Method::FetchCanisterLogs) => match FetchCanisterLogsRequest::decode(ingress.arg()) {
Ok(record) => Ok(Some(record.get_canister_id())),
Err(err) => Err(ParseIngressError::InvalidSubnetPayload(err.to_string())),
},
Ok(Method::DeleteChunks)
| Ok(Method::TakeCanisterSnapshot)
| Ok(Method::LoadCanisterSnapshot)
Expand Down

0 comments on commit e841826

Please sign in to comment.