Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(PocketIC): PocketIC HTTP handler uses PocketIC time for ingress expiry checks #4206

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 5 additions & 3 deletions packages/pocket-ic/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

## Unreleased

### Removed
- The module `management_canister` used to contain interface types of the IC management canister. Those types have since been published on crates.io as `ic-management-canister-types`, so PocketIC can depend on that and remove the redundant types.



## 7.0.0 - 2025-02-26

### Added
Expand All @@ -30,9 +35,6 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
nor the environment variable `POCKET_IC_BIN` provide the path to the PocketIC server binary.
- The current working directory is ignored when looking for the PocketIC server binary.

### Removed
- The module `management_canister` used to contain interface types of the IC management canister. Those types have since been published on crates.io as `ic-management-canister-types`, so PocketIC can depend on that and remove the redundant types.

## 6.0.0 - 2024-11-13

### Added
Expand Down
93 changes: 91 additions & 2 deletions packages/pocket-ic/tests/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,12 +8,12 @@ use ic_management_canister_types::{
SchnorrKeyId as SchnorrPublicKeyArgsKeyId, SchnorrPublicKeyResult,
};
use ic_transport_types::Envelope;
use ic_transport_types::EnvelopeContent::ReadState;
use ic_transport_types::EnvelopeContent::{Call, ReadState};
use pocket_ic::common::rest::{BlockmakerConfigs, RawSubnetBlockmaker, TickConfigs};
use pocket_ic::{
common::rest::{
BlobCompression, CanisterHttpReply, CanisterHttpResponse, MockCanisterHttpResponse,
RawEffectivePrincipal, SubnetKind,
RawEffectivePrincipal, RawMessageId, SubnetKind,
},
query_candid, update_candid, DefaultEffectiveCanisterIdError, ErrorCode, IngressStatusResult,
PocketIc, PocketIcBuilder, RejectCode,
Expand Down Expand Up @@ -2095,6 +2095,95 @@ fn read_state_request_status(
.unwrap()
}

#[test]
fn call_ingress_expiry() {
let pic = PocketIcBuilder::new()
.with_nns_subnet()
.with_application_subnet()
.build();
let canister_id = pic.create_canister();
pic.add_cycles(canister_id, INIT_CYCLES);
pic.install_canister(canister_id, test_canister_wasm(), vec![], None);

// submit an update call via /api/v2/canister/.../call using an ingress expiry in the future
let unix_time_secs = 2272143600; // Wed Jan 01 2042 00:00:00 GMT+0100
let time = SystemTime::UNIX_EPOCH + std::time::Duration::from_secs(unix_time_secs);
pic.set_certified_time(time);
let ingress_expiry = pic
.get_time()
.duration_since(std::time::SystemTime::UNIX_EPOCH)
.unwrap()
.as_nanos() as u64
+ 240_000_000_000;
let (resp, msg_id) = call_request(&pic, ingress_expiry, canister_id);
assert_eq!(resp.status(), reqwest::StatusCode::ACCEPTED);

// execute a round on the PocketIC instance to process that update call
pic.tick();

// check the update call status
let raw_message_id = RawMessageId {
effective_principal: RawEffectivePrincipal::CanisterId(canister_id.as_slice().to_vec()),
message_id: msg_id.to_vec(),
};
let reply = pic.ingress_status(raw_message_id).unwrap().unwrap();
let principal = Decode!(&reply, String).unwrap();
assert_eq!(principal, canister_id.to_string());

// use an invalid ingress expiry
let ingress_expiry = SystemTime::now()
.duration_since(std::time::SystemTime::UNIX_EPOCH)
.unwrap()
.as_nanos() as u64
+ 240_000_000_000;
let (resp, _msg_id) = call_request(&pic, ingress_expiry, canister_id);
assert_eq!(resp.status(), reqwest::StatusCode::BAD_REQUEST);
let err = String::from_utf8(resp.bytes().unwrap().to_vec()).unwrap();
assert!(
err.contains("Invalid request expiry: Specified ingress_expiry not within expected range")
);
}

fn call_request(
pic: &PocketIc,
ingress_expiry: u64,
canister_id: Principal,
) -> (reqwest::blocking::Response, [u8; 32]) {
let content = Call {
nonce: None,
ingress_expiry,
sender: Principal::anonymous(),
canister_id,
method_name: "whoami".to_string(),
arg: Encode!(&()).unwrap(),
};
let envelope = Envelope {
content: std::borrow::Cow::Borrowed(&content),
sender_pubkey: None,
sender_sig: None,
sender_delegation: None,
};

let mut serialized_bytes = Vec::new();
let mut serializer = serde_cbor::Serializer::new(&mut serialized_bytes);
serializer.self_describe().unwrap();
envelope.serialize(&mut serializer).unwrap();

let endpoint = format!(
"instances/{}/api/v2/canister/{}/call",
pic.instance_id(),
canister_id.to_text()
);
let client = reqwest::blocking::Client::new();
let resp = client
.post(pic.get_server_url().join(&endpoint).unwrap())
.header(reqwest::header::CONTENT_TYPE, "application/cbor")
.body(serialized_bytes)
.send()
.unwrap();
(resp, *content.to_request_id())
}

#[test]
fn await_call_no_ticks() {
let mut pic = PocketIcBuilder::new()
Expand Down
18 changes: 16 additions & 2 deletions rs/http_endpoints/public/src/call.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ use hyper::StatusCode;
use ic_crypto_interfaces_sig_verification::IngressSigVerifier;
use ic_error_types::UserError;
use ic_interfaces::ingress_pool::IngressPoolThrottler;
use ic_interfaces::time_source::{SysTimeSource, TimeSource};
use ic_interfaces_registry::RegistryClient;
use ic_logger::{error, warn, ReplicaLogger};
use ic_registry_client_helpers::{
Expand All @@ -27,7 +28,6 @@ use ic_types::{
messages::{
HttpCallContent, HttpRequestEnvelope, MessageId, SignedIngress, SignedIngressContent,
},
time::current_time,
CanisterId, CountBytes, NodeId, RegistryVersion, SubnetId,
};
use ic_validator::HttpRequestVerifier;
Expand All @@ -42,6 +42,7 @@ pub struct IngressValidatorBuilder {
node_id: NodeId,
subnet_id: SubnetId,
malicious_flags: Option<MaliciousFlags>,
time_source: Option<Arc<dyn TimeSource>>,
ingress_verifier: Arc<dyn IngressSigVerifier + Send + Sync>,
registry_client: Arc<dyn RegistryClient>,
ingress_filter: Arc<Mutex<IngressFilterService>>,
Expand All @@ -65,6 +66,7 @@ impl IngressValidatorBuilder {
node_id,
subnet_id,
malicious_flags: None,
time_source: None,
ingress_verifier,
registry_client,
ingress_filter,
Expand All @@ -78,13 +80,19 @@ impl IngressValidatorBuilder {
self
}

pub fn with_time_source(mut self, time_source: Arc<dyn TimeSource>) -> Self {
self.time_source = Some(time_source);
self
}

pub fn build(self) -> IngressValidator {
let log = self.log;
IngressValidator {
log: log.clone(),
node_id: self.node_id,
subnet_id: self.subnet_id,
registry_client: self.registry_client.clone(),
time_source: self.time_source.unwrap_or(Arc::new(SysTimeSource::new())),
validator: build_validator(self.ingress_verifier, self.malicious_flags),
ingress_filter: self.ingress_filter,
ingress_throttler: self.ingress_throttler,
Expand Down Expand Up @@ -164,6 +172,7 @@ pub struct IngressValidator {
node_id: NodeId,
subnet_id: SubnetId,
registry_client: Arc<dyn RegistryClient>,
time_source: Arc<dyn TimeSource>,
validator: Arc<dyn HttpRequestVerifier<SignedIngressContent, RegistryRootOfTrustProvider>>,
ingress_filter: Arc<Mutex<IngressFilterService>>,
ingress_throttler: Arc<RwLock<dyn IngressPoolThrottler + Send + Sync>>,
Expand All @@ -185,6 +194,7 @@ impl IngressValidator {
node_id,
subnet_id,
registry_client,
time_source,
validator,
ingress_filter,
ingress_throttler,
Expand Down Expand Up @@ -243,7 +253,11 @@ impl IngressValidator {
let request_c = msg.as_ref().clone();

tokio::task::spawn_blocking(move || {
validator.validate_request(&request_c, current_time(), &root_of_trust_provider)
validator.validate_request(
&request_c,
time_source.get_relative_time(),
&root_of_trust_provider,
)
})
.await
.map_err(|_| HttpError {
Expand Down
18 changes: 16 additions & 2 deletions rs/http_endpoints/public/src/query.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ use ic_error_types::{ErrorCode, RejectCode};
use ic_interfaces::{
crypto::BasicSigner,
execution_environment::{QueryExecutionError, QueryExecutionService},
time_source::{SysTimeSource, TimeSource},
};
use ic_interfaces_registry::RegistryClient;
use ic_logger::{error, ReplicaLogger};
Expand All @@ -31,7 +32,6 @@ use ic_types::{
HttpQueryResponseReply, HttpRequest, HttpRequestEnvelope, HttpSignedQueryResponse,
NodeSignature, Query, QueryResponseHash,
},
time::current_time,
CanisterId, NodeId,
};
use ic_validator::HttpRequestVerifier;
Expand All @@ -50,6 +50,7 @@ pub struct QueryService {
signer: Arc<dyn BasicSigner<QueryResponseHash> + Send + Sync>,
health_status: Arc<AtomicCell<ReplicaHealthStatus>>,
delegation_from_nns: Arc<OnceCell<CertificateDelegation>>,
time_source: Arc<dyn TimeSource>,
validator: Arc<dyn HttpRequestVerifier<Query, RegistryRootOfTrustProvider>>,
registry_client: Arc<dyn RegistryClient>,
query_execution_service: Arc<Mutex<QueryExecutionService>>,
Expand All @@ -62,6 +63,7 @@ pub struct QueryServiceBuilder {
health_status: Option<Arc<AtomicCell<ReplicaHealthStatus>>>,
malicious_flags: Option<MaliciousFlags>,
delegation_from_nns: Arc<OnceCell<CertificateDelegation>>,
time_source: Option<Arc<dyn TimeSource>>,
ingress_verifier: Arc<dyn IngressSigVerifier + Send + Sync>,
registry_client: Arc<dyn RegistryClient>,
query_execution_service: QueryExecutionService,
Expand Down Expand Up @@ -90,6 +92,7 @@ impl QueryServiceBuilder {
health_status: None,
malicious_flags: None,
delegation_from_nns,
time_source: None,
ingress_verifier,
registry_client,
query_execution_service,
Expand All @@ -101,6 +104,11 @@ impl QueryServiceBuilder {
self
}

pub fn with_time_source(mut self, time_source: Arc<dyn TimeSource>) -> Self {
self.time_source = Some(time_source);
self
}

pub fn with_health_status(
mut self,
health_status: Arc<AtomicCell<ReplicaHealthStatus>>,
Expand All @@ -119,6 +127,7 @@ impl QueryServiceBuilder {
.health_status
.unwrap_or_else(|| Arc::new(AtomicCell::new(ReplicaHealthStatus::Healthy))),
delegation_from_nns: self.delegation_from_nns,
time_source: self.time_source.unwrap_or(Arc::new(SysTimeSource::new())),
validator: build_validator(self.ingress_verifier, self.malicious_flags),
registry_client: self.registry_client,
query_execution_service: Arc::new(Mutex::new(self.query_execution_service)),
Expand All @@ -143,6 +152,7 @@ pub(crate) async fn query(
log,
node_id,
registry_client,
time_source,
validator,
health_status,
signer,
Expand Down Expand Up @@ -188,7 +198,11 @@ pub(crate) async fn query(
// Since spawn blocking requires 'static we can't use any references
let request_c = request.clone();
match tokio::task::spawn_blocking(move || {
validator.validate_request(&request_c, current_time(), &root_of_trust_provider)
validator.validate_request(
&request_c,
time_source.get_relative_time(),
&root_of_trust_provider,
)
})
.await
{
Expand Down
31 changes: 22 additions & 9 deletions rs/http_endpoints/public/src/read_state/canister.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ use http::Request;
use hyper::StatusCode;
use ic_crypto_interfaces_sig_verification::IngressSigVerifier;
use ic_crypto_tree_hash::{sparse_labeled_tree_from_paths, Label, Path, TooLongPathError};
use ic_interfaces::time_source::{SysTimeSource, TimeSource};
use ic_interfaces_registry::RegistryClient;
use ic_interfaces_state_manager::StateReader;
use ic_logger::ReplicaLogger;
Expand All @@ -26,7 +27,6 @@ use ic_types::{
Blob, Certificate, CertificateDelegation, HttpReadStateContent, HttpReadStateResponse,
HttpRequest, HttpRequestEnvelope, MessageId, ReadState, EXPECTED_MESSAGE_ID_LENGTH,
},
time::current_time,
CanisterId, PrincipalId, UserId,
};
use ic_validator::{CanisterIdSet, HttpRequestVerifier};
Expand All @@ -43,6 +43,7 @@ pub struct CanisterReadStateService {
health_status: Arc<AtomicCell<ReplicaHealthStatus>>,
delegation_from_nns: Arc<OnceCell<CertificateDelegation>>,
state_reader: Arc<dyn StateReader<State = ReplicatedState>>,
time_source: Arc<dyn TimeSource>,
validator: Arc<dyn HttpRequestVerifier<ReadState, RegistryRootOfTrustProvider>>,
registry_client: Arc<dyn RegistryClient>,
}
Expand All @@ -53,6 +54,7 @@ pub struct CanisterReadStateServiceBuilder {
malicious_flags: Option<MaliciousFlags>,
delegation_from_nns: Arc<OnceCell<CertificateDelegation>>,
state_reader: Arc<dyn StateReader<State = ReplicatedState>>,
time_source: Option<Arc<dyn TimeSource>>,
ingress_verifier: Arc<dyn IngressSigVerifier + Send + Sync>,
registry_client: Arc<dyn RegistryClient>,
}
Expand All @@ -77,6 +79,7 @@ impl CanisterReadStateServiceBuilder {
malicious_flags: None,
delegation_from_nns,
state_reader,
time_source: None,
ingress_verifier,
registry_client,
}
Expand All @@ -87,6 +90,11 @@ impl CanisterReadStateServiceBuilder {
self
}

pub fn with_time_source(mut self, time_source: Arc<dyn TimeSource>) -> Self {
self.time_source = Some(time_source);
self
}

pub fn with_health_status(
mut self,
health_status: Arc<AtomicCell<ReplicaHealthStatus>>,
Expand All @@ -103,6 +111,7 @@ impl CanisterReadStateServiceBuilder {
.unwrap_or_else(|| Arc::new(AtomicCell::new(ReplicaHealthStatus::Healthy))),
delegation_from_nns: self.delegation_from_nns,
state_reader: self.state_reader,
time_source: self.time_source.unwrap_or(Arc::new(SysTimeSource::new())),
validator: build_validator(self.ingress_verifier, self.malicious_flags),
registry_client: self.registry_client,
};
Expand All @@ -127,6 +136,7 @@ pub(crate) async fn canister_read_state(
health_status,
delegation_from_nns,
state_reader,
time_source,
validator,
registry_client,
}): State<CanisterReadStateService>,
Expand Down Expand Up @@ -165,14 +175,17 @@ pub(crate) async fn canister_read_state(
// Since spawn blocking requires 'static we can't use any references
let request_c = request.clone();
let response = tokio::task::spawn_blocking(move || {
let targets =
match validator.validate_request(&request_c, current_time(), &root_of_trust_provider) {
Ok(targets) => targets,
Err(err) => {
let http_err = validation_error_to_http_error(&request, err, &log);
return (http_err.status, http_err.message).into_response();
}
};
let targets = match validator.validate_request(
&request_c,
time_source.get_relative_time(),
&root_of_trust_provider,
) {
Ok(targets) => targets,
Err(err) => {
let http_err = validation_error_to_http_error(&request, err, &log);
return (http_err.status, http_err.message).into_response();
}
};

let certified_state_reader = match state_reader.get_certified_state_snapshot() {
Some(reader) => reader,
Expand Down
Loading