From 4d33af867baf2b6439f2541a83ac9205143aeff3 Mon Sep 17 00:00:00 2001 From: Linwei Shang Date: Mon, 17 Aug 2026 16:23:51 -0400 Subject: [PATCH 1/3] feat: sign a canister call for submission from another machine MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `icp canister call --sign-only ` composes and signs a call and writes it out instead of submitting it, so a machine that holds the key can prepare a call with no network at all and a machine with network can submit it without holding the key. `-` writes to stdout. Nothing in the signing path reaches the network: the Candid interface comes from `--candid` or the canister's local build artifact rather than from the canister itself, and a `fetch` root key is refused up front with a pointer at `--root-key mainnet` rather than left to time out. `--proxy` is rejected — the envelope would target the proxy and return a `ProxyResult` the submitting side would have to unwrap. `--valid-from ` places the submission window, as a duration from now or an RFC 3339 timestamp. The window is always five minutes wide, because the IC rejects an ingress message whose expiry is further ahead than that, so the flag places the window rather than sizing it. This is deliberately not dfx's `--expire-after`, which names the window's *end* and leaves the user to subtract the five minutes. The file format lives in the `icp` crate as `signed_message`, since it is library surface rather than CLI plumbing, and `--sign-only` on other call-making commands can reuse it. Its `Destination` is tagged canister-or-subnet rather than a bare principal: the two route to different endpoints and the discriminant cannot be re-derived, and `ic-agent`'s `From for EffectiveId` silently picks the canister endpoint. `validate()` re-derives everything from the envelope and refuses a file whose metadata disagrees. `Agent` creation grows an ingress-expiry override, used only for signing: `sign_request_status` derives its expiry from the agent and truncates the seconds, so without pinning it the pre-signed status check would land in a different five-minute window than the call it waits on. The chosen expiry is minute-aligned to make that truncation a no-op, and signing verifies the two agree before writing anything. Co-Authored-By: Claude Opus 5 (1M context) --- CHANGELOG.md | 9 +- Cargo.lock | 3 + Cargo.toml | 3 +- crates/icp-cli/Cargo.toml | 1 + crates/icp-cli/src/commands/canister/call.rs | 379 ++++++- .../icp-cli/tests/canister_call_sign_tests.rs | 404 ++++++++ crates/icp/Cargo.toml | 2 + crates/icp/src/agent.rs | 59 +- crates/icp/src/context/mod.rs | 45 +- crates/icp/src/lib.rs | 1 + crates/icp/src/network/access.rs | 2 +- crates/icp/src/signed_message.rs | 974 ++++++++++++++++++ docs/reference/cli.md | 6 + 13 files changed, 1835 insertions(+), 53 deletions(-) create mode 100644 crates/icp-cli/tests/canister_call_sign_tests.rs create mode 100644 crates/icp/src/signed_message.rs diff --git a/CHANGELOG.md b/CHANGELOG.md index 55879de88..0eb1ed2c8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,7 +2,8 @@ Convention: changes to experimental features live in a dedicated `## Experimental` subsection under each version. Experimental features may receive breaking changes between releases without a major version -bump. Currently experimental: project bundling, project dependencies +bump. Currently experimental: project bundling, project dependencies, +air-gapped signing --> # Unreleased @@ -11,6 +12,12 @@ bump. Currently experimental: project bundling, project dependencies * feat: `icp completions ` prints a shell completion script for `bash`, `zsh`, `fish`, `powershell`, or `elvish` to stdout. See the [installation guide](docs/guides/installation.md#shell-completions) for where to put it. * fix: `icp canister logs` output formats are corrected. `--json` now emits machine-readable JSON and the default emits the human-readable lines (the two were swapped), and `--follow --json` emits newline-delimited JSON, one record per line, streamed as each record arrives. This is breaking for scripts: parsing the default output as JSON now requires `--json`, and consumers of `--follow --json` must read one JSON object per line. +## Experimental + +* feat(signing): `icp canister call --sign-only ` composes and signs a call and writes it to a JSON file instead of submitting it, so a machine that holds the key can prepare a call with no network at all and a machine with network can submit it without holding the key. `-` writes to stdout. Nothing is fetched while signing: the Candid interface comes from `--candid` or from the canister's local build artifact rather than from the canister itself, and `--root-key` must name a key (`mainnet` or a hex-encoded key) rather than `fetch`. `--proxy` is not supported. The command that submits the file, `icp message send`, follows separately. + * `--valid-from ` places the message's submission window, as a duration from now (`55m`, `2h`) or an RFC 3339 timestamp; it defaults to now. The window is always five minutes wide, because the IC rejects an ingress message whose expiry is further ahead than that — so this places the window rather than sizing it. Note that this is not dfx's `--expire-after`, which names the window's *end* and leaves you to subtract the five minutes yourself. + * The file records the signed envelope, where to submit it, a tagged canister-or-subnet destination, the Candid interface, and a human-readable summary of what was signed. An update also carries a pre-signed `request_status` read, so the submitting machine can await the outcome with no key of its own; it shares the call's expiry, so both live in the same window. + # v1.3.0 * feat: a canister environment variable's value can now be read from a file, by writing `var: { path: }` in place of `var: value`. The path resolves against the canister's directory — including in an environment override, matching `init_args` — and surrounding whitespace is trimmed off the file's contents. The file is read when the project is loaded, so a missing file fails before anything is deployed. `icp project bundle` writes the value into the bundled manifest inline, rejecting a file outside the project as it does for other manifest file references. diff --git a/Cargo.lock b/Cargo.lock index 1d9ca596b..39dec0859 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3608,6 +3608,7 @@ version = "1.3.0" dependencies = [ "async-dropper", "async-trait", + "base64", "bigdecimal", "bip32", "bollard", @@ -3659,6 +3660,7 @@ dependencies = [ "sec1", "semver", "serde", + "serde_cbor", "serde_json", "serde_yaml", "sha2 0.11.0", @@ -3749,6 +3751,7 @@ dependencies = [ "semver", "send_ctrlc", "serde", + "serde_cbor", "serde_json", "serde_yaml", "serial_test", diff --git a/Cargo.toml b/Cargo.toml index ced350649..30710a835 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -94,6 +94,7 @@ send_ctrlc = "0.6" semver = "1" serial_test = { version = "3.2.0", features = ["file_locks"] } serde = { version = "1.0", features = ["derive"] } +serde_cbor = "0.11.2" serde_json = "1.0" serde_yaml = "0.9.34" sha2 = { version = "0.11.0", features = ["zeroize"] } @@ -104,7 +105,7 @@ sysinfo = "0.38.4" tar = "0.4.46" tempfile = "3" test-tag = "0.1" -time = { version = "0.3.47", features = ["formatting", "macros"] } +time = { version = "0.3.47", features = ["formatting", "macros", "parsing"] } tiny-bip39 = "2.0.0" tokio = { version = "1.45.0", features = ["macros", "rt-multi-thread"] } tracing = "0.1.41" diff --git a/crates/icp-cli/Cargo.toml b/crates/icp-cli/Cargo.toml index 5ad3e0f0b..3b85f287b 100644 --- a/crates/icp-cli/Cargo.toml +++ b/crates/icp-cli/Cargo.toml @@ -90,6 +90,7 @@ cryptoki.workspace = true predicates.workspace = true rand.workspace = true send_ctrlc.workspace = true +serde_cbor.workspace = true serde_yaml.workspace = true serial_test.workspace = true test-tag.workspace = true diff --git a/crates/icp-cli/src/commands/canister/call.rs b/crates/icp-cli/src/commands/canister/call.rs index 17df2d102..df6c1b830 100644 --- a/crates/icp-cli/src/commands/canister/call.rs +++ b/crates/icp-cli/src/commands/canister/call.rs @@ -7,18 +7,25 @@ use candid_parser::utils::CandidSource; use clap::{Args, ValueEnum, ValueHint}; use dialoguer::console::Term; use ic_agent::Agent; -use icp::context::Context; +use ic_agent::agent::EffectiveId; +use icp::context::{Context, EnvironmentSelection, NetworkSelection}; use icp::manifest::ArgsFormat; -use icp::parsers::CyclesAmount; +use icp::network::{Configuration as NetworkConfiguration, RootKeySpec}; +use icp::parsers::{CyclesAmount, DurationAmount}; use icp::prelude::*; +use icp::signed_message::{self, Destination, Request, RequestType, SignedMessage, Summary}; use serde::Serialize; use std::io::{self, Write}; +use std::str::FromStr; +use time::{OffsetDateTime, format_description::well_known::Rfc3339}; use tracing::{error, warn}; +use url::Url; use crate::{ commands::args::{self, load_args}, operations::misc::fetch_canister_metadata, operations::proxy::update_or_proxy_raw, + operations::wasm::extract_candid_service, }; /// How to interpret and display the call response blob. @@ -94,18 +101,76 @@ pub(crate) struct CallArgs { /// Output command results as JSON #[arg(long)] pub(crate) json: bool, + + /// Sign the call and write it to FILE instead of submitting it, so it can be + /// submitted later from a machine that has network access but not your key. + /// `-` writes to stdout. + /// + /// Nothing is sent, and nothing is fetched: the interface comes from + /// `--candid` or the local build artifact rather than from the canister, so + /// this works with no network at all. `--root-key` must name a key rather + /// than `fetch`, and `--proxy` is not supported. + #[arg(long, value_name = "FILE", conflicts_with = "proxy", value_hint = ValueHint::FilePath)] + pub(crate) sign_only: Option, + + /// When the signed message's five-minute submission window opens: a duration + /// from now (`55m`, `2h`) or an RFC 3339 timestamp + /// (`2026-08-17T10:07:00Z`). Defaults to now. + /// + /// The window is always five minutes wide — the IC will not accept an + /// ingress message expiring further ahead than that — so this places it + /// rather than sizing it. + #[arg(long, value_name = "WHEN", requires = "sign_only")] + pub(crate) valid_from: Option, +} + +/// When a signed message's five-minute submission window opens. +#[derive(Clone, Debug)] +pub(crate) enum ValidFrom { + /// A duration from now. + In(time::Duration), + /// An absolute instant. + At(OffsetDateTime), +} + +impl FromStr for ValidFrom { + type Err = String; + + fn from_str(s: &str) -> Result { + if let Ok(at) = OffsetDateTime::parse(s, &Rfc3339) { + return Ok(ValidFrom::At(at)); + } + let seconds = DurationAmount::from_str(s) + .ok() + .map(|d| d.get()) + .and_then(|secs| i64::try_from(secs).ok()); + match seconds { + Some(seconds) => Ok(ValidFrom::In(time::Duration::seconds(seconds))), + None => Err(format!( + "'{s}' is neither a duration ('55m', '2h') nor an RFC 3339 timestamp \ + ('2026-08-17T10:07:00Z')" + )), + } + } } pub(crate) async fn exec(ctx: &Context, args: &CallArgs) -> Result<(), anyhow::Error> { let selections = args.cmd_args.selections(); - let agent = ctx - .get_agent( - &selections.identity, - &selections.network, - &selections.environment, - ) - .await?; + // Signing is meant to work on a machine with no network, so no agent is + // built up front: the one used to sign is created last, once the submission + // window it has to expire on is known. + let agent = match args.sign_only { + Some(_) => None, + None => Some( + ctx.get_agent( + &selections.identity, + &selections.network, + &selections.environment, + ) + .await?, + ), + }; let cid = ctx .get_canister_id( &selections.canister, @@ -114,9 +179,12 @@ pub(crate) async fn exec(ctx: &Context, args: &CallArgs) -> Result<(), anyhow::E ) .await?; - let candid_types = match &args.candid { - Some(path) => Some(load_candid_from_file(path)?), - None => get_candid_type(&agent, cid).await, + let candid_types = match (&args.candid, &agent) { + (Some(path), _) => Some(load_candid_from_file(path)?), + (None, Some(agent)) => get_candid_type(agent, cid).await, + // Fetching `candid:service` is a network round trip, so signing falls + // back to the interface of whatever this project last built. + (None, None) => local_candid_type(ctx, &selections.canister).await, }; let method = if let Some(method) = &args.method { @@ -135,11 +203,12 @@ pub(crate) async fn exec(ctx: &Context, args: &CallArgs) -> Result<(), anyhow::E methods[selection].to_string() } else { bail!( - "method name was not provided and could not fetch candid type to assist method selection" + "method name was not provided and no Candid interface is available to assist method selection" ); }; - let declared_method = - candid_types.and_then(|i| Some((i.env.clone(), i.get_method(&method)?.clone()))); + let declared_method = candid_types + .as_ref() + .and_then(|i| Some((i.env.clone(), i.get_method(&method)?.clone()))); enum ResolvedArgs { Candid(IDLArgs), Bytes(Vec), @@ -178,11 +247,11 @@ pub(crate) async fn exec(ctx: &Context, args: &CallArgs) -> Result<(), anyhow::E bail!("arguments must be provided when --args-format is not candid"); } (None, None) => bail!( - "arguments were not provided and could not fetch candid type to assist building arguments" + "arguments were not provided and no Candid interface is available to assist building arguments" ), (None, Some(ResolvedArgs::Bytes(bytes))) => bytes, (None, Some(ResolvedArgs::Candid(arguments))) => { - warn!("could not fetch candid type, serializing arguments with inferred types."); + warn!("no Candid interface is available, serializing arguments with inferred types."); arguments .to_bytes() .context("failed to serialize candid arguments")? @@ -210,16 +279,32 @@ pub(crate) async fn exec(ctx: &Context, args: &CallArgs) -> Result<(), anyhow::E .context("failed to serialize candid arguments with specific types")?, }; + // Preemptive check: error if Candid shows this is an update method + if args.query + && let Some((_, func)) = &declared_method + && !func.is_query() + { + bail!( + "`{method}` is an update method, not a query method. \ + Run the command without `--query`.", + ); + } + + if let Some(out) = &args.sign_only { + return sign_only( + ctx, + args, + out, + cid, + &method, + arg_bytes, + candid_types.as_ref().map(|i| i.source.as_str()), + ) + .await; + } + + let agent = agent.expect("an agent is built whenever the call is submitted"); let res = if args.query { - // Preemptive check: error if Candid shows this is an update method - if let Some((_, func)) = &declared_method - && !func.is_query() - { - bail!( - "`{method}` is an update method, not a query method. \ - Run the command without `--query`.", - ); - } agent .query(&cid, &method) .with_arg(arg_bytes) @@ -272,6 +357,207 @@ pub(crate) async fn exec(ctx: &Context, args: &CallArgs) -> Result<(), anyhow::E Ok(()) } +/// Signs the call and writes it out for another machine to submit, instead of +/// submitting it here. +/// +/// Nothing in this path reaches the network. The interface has already been +/// resolved without one, the root key is recorded rather than fetched, and the +/// agent exists only to hold the key and the expiry. +async fn sign_only( + ctx: &Context, + args: &CallArgs, + out: &Path, + cid: Principal, + method: &str, + arg_bytes: Vec, + interface: Option<&str>, +) -> Result<(), anyhow::Error> { + let selections = args.cmd_args.selections(); + let (url, root_key) = + resolve_network_offline(ctx, &selections.network, &selections.environment).await?; + + let now = OffsetDateTime::now_utc(); + let opens_at = match &args.valid_from { + Some(ValidFrom::At(at)) => *at, + Some(ValidFrom::In(duration)) => now + *duration, + None => now, + }; + let valid_until = floor_to_minute(opens_at + signed_message::SUBMISSION_WINDOW); + let valid_from = valid_until - signed_message::SUBMISSION_WINDOW; + if valid_until <= now { + bail!( + "`--valid-from` puts the submission window at {} to {}, which has already closed", + signed_message::format_timestamp(valid_from), + signed_message::format_timestamp(valid_until), + ); + } + + let agent = ctx + .get_agent_for_signing(&selections.identity, &url, valid_until) + .await?; + let sender = agent + .get_principal() + .map_err(|e| anyhow!("failed to determine the signing identity's principal: {e}"))?; + + let request = if args.query { + let signed = agent + .query(&cid, method) + .with_arg(arg_bytes.clone()) + .expire_at(valid_until) + .sign() + .context("failed to sign the query")?; + Request { + request_type: RequestType::Query, + envelope: signed.signed_query, + // A query answers immediately, so there is nothing to poll for. + request_id: None, + status_check: None, + } + } else { + let signed = agent + .update(&cid, method) + .with_arg(arg_bytes.clone()) + .expire_at(valid_until) + .sign() + .context("failed to sign the call")?; + let status_check = agent + .sign_request_status(EffectiveId::Canister(cid), signed.request_id) + .context("failed to sign the request-status check")?; + + // Both envelopes have to expire at the same instant: a window is + // `[expiry - 5min, expiry]`, so a status check with an expiry of its own + // would be waiting on the call from a different window. It gets its + // expiry from the agent, which is why the agent's was pinned above. + anyhow::ensure!( + status_check.ingress_expiry == signed.ingress_expiry, + "the call and its status check landed in different submission windows \ + ({} vs {}); this is a bug", + signed.ingress_expiry, + status_check.ingress_expiry, + ); + + Request { + request_type: RequestType::Update, + envelope: signed.signed_update, + request_id: Some(signed.request_id.to_string()), + status_check: Some(status_check.signed_request_status), + } + }; + let request_type = request.request_type; + + let message = SignedMessage { + format: signed_message::FORMAT.to_string(), + version: signed_message::VERSION, + request, + network: signed_message::Network { url, root_key }, + // Every call this command can compose is routed by canister id. A + // subnet-scoped destination is legal in the format, but nothing produces + // one yet. + destination: Destination::Canister(cid), + candid: interface.map(str::to_owned), + summary: Summary { + sender, + canister_id: cid, + method: method.to_owned(), + arg: arg_bytes, + signed_at: signed_message::format_timestamp(now), + valid_from: signed_message::format_timestamp(valid_from), + valid_until: signed_message::format_timestamp(valid_until), + }, + }; + + // Refuse to hand over a file we would not accept back. Signing is the last + // thing the air-gapped machine does, so a mistake found on the other side is + // found too late. + message + .validate(now) + .context("the signed message failed its own validation")?; + + if out == "-" { + let mut stdout = io::stdout(); + writeln!(stdout, "{}", message.to_json()?)?; + stdout.flush()?; + } else { + message.save(out)?; + } + + eprintln!( + "Signed a {} call to '{method}' on {cid}, as {sender}.", + request_type.as_str(), + ); + eprintln!( + "It can be submitted between {} and {} — a five-minute window.", + signed_message::format_timestamp(valid_from), + signed_message::format_timestamp(valid_until), + ); + if out != "-" { + eprintln!("Written to {out}."); + } + if interface.is_none() { + warn!( + "no Candid interface was available, so the message carries none; \ + whoever submits it will see the argument and reply undecoded unless they supply one" + ); + } + + Ok(()) +} + +/// Resolves where a signed message says to submit itself, without touching the +/// network. +/// +/// The signing machine may be air-gapped, and the only thing `call` needs a root +/// key for is verifying a reply it will never see — so the key is recorded for +/// the submitting machine rather than resolved here, and a network configured to +/// fetch one is rejected outright instead of hanging until it times out. +async fn resolve_network_offline( + ctx: &Context, + network: &NetworkSelection, + environment: &EnvironmentSelection, +) -> Result<(Url, RootKeySpec), anyhow::Error> { + let net = match (environment, network) { + (EnvironmentSelection::Named(_), NetworkSelection::Named(_)) + | (EnvironmentSelection::Named(_), NetworkSelection::Url(_, _)) => { + bail!("You can't specify both an environment and a network") + } + (_, NetworkSelection::Default) => ctx.get_environment(environment).await?.network, + (EnvironmentSelection::Default, _) => ctx.get_network(network).await?, + }; + + match net.configuration.clone() { + NetworkConfiguration::Connected { connected } => { + if connected.root_key == RootKeySpec::Fetch { + bail!( + "network '{}' fetches its root key from {}, which `--sign-only` cannot do — \ + signing must work with no network. Name the key instead: `--root-key mainnet`, \ + or a hex-encoded root key.", + net.name, + connected.api_url, + ); + } + Ok((connected.api_url, connected.root_key)) + } + // A managed network's root key comes out of the descriptor this machine + // wrote when it started the network: a local file, not a request. + NetworkConfiguration::Managed { .. } => { + let access = ctx.network.access(&net).await?; + Ok((access.api_url, RootKeySpec::Explicit(access.root_key))) + } + } +} + +/// Rounds an ingress expiry down to a whole minute. +/// +/// `ic-agent` truncates the seconds off any ingress expiry it derives itself, +/// which is how the pre-signed status check gets one. Choosing a minute-aligned +/// expiry for the call makes that truncation a no-op, so the two envelopes name +/// the same instant and share one window. +fn floor_to_minute(t: OffsetDateTime) -> OffsetDateTime { + t.replace_nanosecond(0) + .and_then(|t| t.replace_second(0)) + .expect("0 is a valid second and nanosecond") +} + /// A response decoded according to the requested `CallOutputMode`. enum Decoded { Candid(IDLArgs), @@ -369,13 +655,24 @@ pub(crate) fn print_candid_for_term(term: &mut Term, args: &IDLArgs) -> io::Resu /// - has an actor in the IDL file. If anything fails, it returns None. async fn get_candid_type(agent: &Agent, canister_id: Principal) -> Option { let candid_interface = fetch_canister_metadata(agent, canister_id, "candid:service").await?; - let candid_source = CandidSource::Text(&candid_interface); - let (type_env, ty) = candid_source.load().ok()?; - let actor = ty?; - Some(CanisterInterface { - env: type_env, - ty: actor, - }) + CanisterInterface::from_text(candid_interface).ok() +} + +/// Gets the Candid interface a project canister was last built with, from the +/// `candid:service` metadata of its build artifact. +/// +/// Best effort, and offline: it stands in for [`get_candid_type`] when the +/// canister cannot be reached, so a canister that was never built, or was named +/// by principal rather than by name, simply yields nothing. +async fn local_candid_type( + ctx: &Context, + canister: &icp::context::CanisterSelection, +) -> Option { + let icp::context::CanisterSelection::Named(name) = canister else { + return None; + }; + let wasm = ctx.artifacts.lookup(name).await.ok()?; + CanisterInterface::from_text(extract_candid_service(&wasm)?).ok() } /// Loads a Candid interface from a local `.did` file. @@ -383,6 +680,8 @@ async fn get_candid_type(agent: &Agent, canister_id: Principal) -> Option Result { + // Parsed from the path rather than from the text below, so that a `.did` + // file importing another one still resolves. let candid_source = CandidSource::File(path.as_std_path()); let (type_env, ty) = candid_source .load() @@ -392,15 +691,29 @@ fn load_candid_from_file(path: &Path) -> Result Result { + let (env, ty) = CandidSource::Text(&source) + .load() + .context("failed to parse Candid interface")?; + let ty = ty.context("Candid interface does not declare a service")?; + Ok(CanisterInterface { env, ty, source }) + } + fn methods(&self) -> impl Iterator { let ty = if let TypeInner::Class(_, t) = &*self.ty.0 { t diff --git a/crates/icp-cli/tests/canister_call_sign_tests.rs b/crates/icp-cli/tests/canister_call_sign_tests.rs new file mode 100644 index 000000000..a42b0236d --- /dev/null +++ b/crates/icp-cli/tests/canister_call_sign_tests.rs @@ -0,0 +1,404 @@ +//! `icp canister call --sign-only`: composing and signing a call on a machine +//! that holds the key and has no network. +//! +//! None of these tests start a network, and the URLs they name are deliberately +//! unreachable — that is the point of the feature, and a test that quietly +//! depended on a reachable network would stop testing it. + +use indoc::formatdoc; +use predicates::prelude::PredicateBooleanExt; +use predicates::str::contains; +use serde_json::Value; + +use crate::common::TestContext; +use icp::fs::write_string; +use icp::prelude::*; + +mod common; + +/// A canister id to sign calls to. Nothing is ever sent to it. +const TARGET: &str = "ryjl3-tyaaa-aaaaa-aaaba-cai"; + +/// A network that cannot be reached, with a root key that needs no round trip +/// to resolve. +const UNREACHABLE: &str = "http://127.0.0.1:1"; + +const GREET_DID: &str = r#"service : { "greet" : (text) -> (text) query }"#; + +/// The `ingress_expiry` of a base64-encoded, CBOR-encoded authentication envelope. +fn envelope_expiry(encoded: &str) -> u64 { + use base64::{Engine as _, engine::general_purpose::STANDARD}; + let cbor = STANDARD.decode(encoded).expect("envelope must be base64"); + let envelope: ic_agent::agent::Envelope = + serde_cbor::from_slice(&cbor).expect("envelope must be CBOR"); + envelope.content.ingress_expiry() +} + +fn read_message(path: &Path) -> Value { + let text = icp::fs::read_to_string(path).expect("the message file must exist"); + serde_json::from_str(&text).expect("a signed message must be JSON a human can read") +} + +/// Signing needs no project, no network, and no canister-name resolution: +/// a principal, an interface, and a key are enough. +#[test] +fn signs_an_update_with_no_network_and_no_project() { + let ctx = TestContext::new(); + let did = ctx.home_path().join("service.did"); + write_string(&did, GREET_DID).expect("failed to write candid file"); + let out = ctx.home_path().join("message.json"); + + ctx.icp() + .args([ + "canister", + "call", + "--network", + UNREACHABLE, + "--root-key", + "mainnet", + "--candid", + did.as_str(), + "--sign-only", + out.as_str(), + TARGET, + "greet", + "(\"world\")", + ]) + .assert() + .success() + .stderr(contains("five-minute window")); + + let message = read_message(&out); + assert_eq!(message["format"], "icp-signed-message"); + assert_eq!(message["version"], 1); + assert_eq!(message["request"]["type"], "update"); + assert_eq!(message["network"]["url"], format!("{UNREACHABLE}/")); + assert_eq!(message["network"]["root_key"], "mainnet"); + // Tagged, so the submitting machine knows which endpoint shape routes it. + assert_eq!(message["destination"]["canister"], TARGET); + assert_eq!(message["summary"]["canister_id"], TARGET); + assert_eq!(message["summary"]["method"], "greet"); + // The interface travels with the message: the submitting machine has no + // project to resolve one from. + assert_eq!(message["candid"], GREET_DID); + + // An update carries what it takes to await the outcome with no key. + let request_id = message["request"]["request_id"] + .as_str() + .expect("an update records its request id"); + assert_eq!( + request_id.len(), + 64, + "request id is hex-encoded: {request_id}" + ); + + // Both envelopes must expire at the same instant. The window is + // `[expiry - 5min, expiry]`, so a status check with an expiry of its own + // would be waiting on the call from a different window. + let call_expiry = envelope_expiry( + message["request"]["envelope"] + .as_str() + .expect("the envelope is base64 text"), + ); + let status_expiry = envelope_expiry( + message["request"]["status_check"] + .as_str() + .expect("an update records a pre-signed status check"), + ); + assert_eq!( + call_expiry, status_expiry, + "the call and its status check must share one submission window" + ); +} + +/// The window is five minutes wide, always; `--valid-from` only places it. +#[test] +fn valid_from_places_a_five_minute_window() { + let ctx = TestContext::new(); + let did = ctx.home_path().join("service.did"); + write_string(&did, GREET_DID).expect("failed to write candid file"); + + let sign = |out: &Path, valid_from: Option<&str>| { + let mut cmd = ctx.icp(); + cmd.args([ + "canister", + "call", + "--network", + UNREACHABLE, + "--root-key", + "mainnet", + "--candid", + did.as_str(), + "--sign-only", + out.as_str(), + TARGET, + "greet", + "(\"world\")", + ]); + if let Some(valid_from) = valid_from { + cmd.args(["--valid-from", valid_from]); + } + cmd.assert().success(); + read_message(out) + }; + + let window = |message: &Value| { + let parse = |field: &str| { + time::OffsetDateTime::parse( + message["summary"][field].as_str().expect("a timestamp"), + &time::format_description::well_known::Rfc3339, + ) + .expect("timestamps are RFC 3339") + }; + (parse("valid_from"), parse("valid_until")) + }; + + // Default: the window is open now. + let now_message = sign(&ctx.home_path().join("now.json"), None); + let (from, until) = window(&now_message); + assert_eq!(until - from, time::Duration::minutes(5)); + assert!( + from <= time::OffsetDateTime::now_utc(), + "a message signed for now must be submittable immediately, but opens at {from}" + ); + + // Deferred: the same five minutes, an hour out. + let later_message = sign(&ctx.home_path().join("later.json"), Some("1h")); + let (later_from, later_until) = window(&later_message); + assert_eq!(later_until - later_from, time::Duration::minutes(5)); + let deferred = later_from - from; + assert!( + (deferred - time::Duration::hours(1)).abs() < time::Duration::minutes(2), + "`--valid-from 1h` should open the window about an hour later, not {deferred}" + ); + + // An RFC 3339 timestamp names the opening directly. + let at_message = sign( + &ctx.home_path().join("at.json"), + Some("2126-08-17T10:07:00Z"), + ); + assert_eq!(at_message["summary"]["valid_from"], "2126-08-17T10:07:00Z"); + assert_eq!(at_message["summary"]["valid_until"], "2126-08-17T10:12:00Z"); +} + +/// A query answers immediately, so there is nothing to poll for and no +/// status check to pre-sign. +#[test] +fn signs_a_query_without_a_status_check() { + let ctx = TestContext::new(); + let did = ctx.home_path().join("service.did"); + write_string(&did, GREET_DID).expect("failed to write candid file"); + let out = ctx.home_path().join("query.json"); + + ctx.icp() + .args([ + "canister", + "call", + "--network", + UNREACHABLE, + "--root-key", + "mainnet", + "--candid", + did.as_str(), + "--query", + "--sign-only", + out.as_str(), + TARGET, + "greet", + "(\"world\")", + ]) + .assert() + .success(); + + let message = read_message(&out); + assert_eq!(message["request"]["type"], "query"); + assert!( + message["request"].get("request_id").is_none(), + "a query identifies nothing to poll: {message}" + ); + assert!( + message["request"].get("status_check").is_none(), + "a query has no status to check: {message}" + ); +} + +/// `-` writes the message to stdout, so it can be piped rather than filed. +#[test] +fn writes_to_stdout() { + let ctx = TestContext::new(); + let did = ctx.home_path().join("service.did"); + write_string(&did, GREET_DID).expect("failed to write candid file"); + + let output = ctx + .icp() + .args([ + "canister", + "call", + "--network", + UNREACHABLE, + "--root-key", + "mainnet", + "--candid", + did.as_str(), + "--sign-only", + "-", + TARGET, + "greet", + "(\"world\")", + ]) + .output() + .expect("failed to run the signing command"); + assert!( + output.status.success(), + "stderr: {}", + String::from_utf8_lossy(&output.stderr) + ); + + let message: Value = serde_json::from_slice(&output.stdout) + .expect("stdout must be the message and nothing else"); + assert_eq!(message["format"], "icp-signed-message"); +} + +/// The interface a signed message carries comes from the project's own build +/// when `--candid` is not given — the canister itself cannot be asked. +#[test] +fn embeds_the_interface_from_the_local_build() { + let ctx = TestContext::new(); + let project_dir = ctx.create_project_dir("icp"); + let wasm = ctx.make_asset("example_icp_mo.wasm"); + + let pm = formatdoc! {r#" + canisters: + - name: my-canister + build: + steps: + - type: script + command: cp '{wasm}' "$ICP_WASM_OUTPUT_PATH" + + networks: + - name: offline-network + mode: connected + url: {UNREACHABLE} + root-key: mainnet + + environments: + - name: offline + network: offline-network + "#}; + write_string(&project_dir.join("icp.yaml"), &pm).expect("failed to write project manifest"); + + // Build and link, both of which are local. Between them the project knows + // the canister's id and its interface without ever reaching the network. + ctx.icp() + .current_dir(&project_dir) + .args(["build", "my-canister", "--environment", "offline"]) + .assert() + .success(); + ctx.icp() + .current_dir(&project_dir) + .args([ + "canister", + "link", + "my-canister", + TARGET, + "--environment", + "offline", + ]) + .assert() + .success(); + + let out = project_dir.join("message.json"); + ctx.icp() + .current_dir(&project_dir) + .args([ + "canister", + "call", + "--environment", + "offline", + "--sign-only", + out.as_str(), + "my-canister", + "greet", + "(\"world\")", + ]) + .assert() + .success(); + + let message = read_message(&out); + assert_eq!(message["destination"]["canister"], TARGET); + let candid = message["candid"] + .as_str() + .expect("the build artifact's interface should have been embedded"); + assert!( + candid.contains("greet"), + "embedded interface should describe the called method: {candid}" + ); +} + +/// A root key that has to be fetched cannot be resolved without a network, so +/// it is refused up front rather than left to time out. +#[test] +fn refuses_to_fetch_a_root_key() { + let ctx = TestContext::new(); + let out = ctx.home_path().join("message.json"); + + ctx.icp() + .args([ + "canister", + "call", + "--network", + UNREACHABLE, + "--root-key", + "fetch", + "--sign-only", + out.as_str(), + TARGET, + "greet", + "(\"world\")", + ]) + .assert() + .failure() + .stderr(contains("--root-key mainnet")); +} + +/// Signing a proxied call would target the proxy and return a `ProxyResult` the +/// submitting side would have to unwrap; out of scope for v1. +#[test] +fn rejects_proxy() { + let ctx = TestContext::new(); + + ctx.icp() + .args([ + "canister", + "call", + "--sign-only", + "message.json", + "--proxy", + "aaaaa-aa", + TARGET, + "greet", + ]) + .assert() + .failure() + .stderr(contains("--proxy").and(contains("--sign-only"))); +} + +/// `--valid-from` only means something for a message that is being signed. +#[test] +fn valid_from_requires_sign_only() { + let ctx = TestContext::new(); + + ctx.icp() + .args([ + "canister", + "call", + "--valid-from", + "1h", + TARGET, + "greet", + "(\"world\")", + ]) + .assert() + .failure() + .stderr(contains("--sign-only")); +} diff --git a/crates/icp/Cargo.toml b/crates/icp/Cargo.toml index 40690d523..40f310b15 100644 --- a/crates/icp/Cargo.toml +++ b/crates/icp/Cargo.toml @@ -8,6 +8,7 @@ publish.workspace = true [dependencies] async-dropper = { workspace = true } async-trait = { workspace = true } +base64 = { workspace = true } bigdecimal = { workspace = true } bip32 = { workspace = true } bollard = { workspace = true } @@ -57,6 +58,7 @@ scrypt = { workspace = true } semver = { workspace = true } sec1 = { workspace = true } serde = { workspace = true } +serde_cbor = { workspace = true } serde_json = { workspace = true } serde_yaml = { workspace = true } sha2 = { workspace = true } diff --git a/crates/icp/src/agent.rs b/crates/icp/src/agent.rs index d662aacd8..b0df5c6de 100644 --- a/crates/icp/src/agent.rs +++ b/crates/icp/src/agent.rs @@ -12,30 +12,59 @@ pub enum CreateAgentError { Agent { source: AgentError }, } +/// How far ahead of now an agent dates the messages it expires, unless the +/// caller pins something else. +const DEFAULT_INGRESS_EXPIRY: Duration = Duration::from_secs(4 * MINUTE); + #[async_trait] pub trait Create: Sync + Send { - async fn create(&self, id: Arc, url: &str) -> Result; + /// Builds an agent talking to `url` as `id`. + /// + /// `ingress_expiry` pins how far ahead of now the agent dates the messages it + /// derives an expiry for. Pass `None` for the default. Pass `Some` only when + /// the expiry is itself part of the output — signing a message here for + /// another machine to submit, where the call envelope and the pre-signed + /// `request_status` that accompanies it have to land in the same submission + /// window. A pinned expiry is used verbatim, so the + /// `ICP_CLI_TEST_ADVANCE_TIME_MS` clock offset applies to the default only. + async fn create( + &self, + id: Arc, + url: &str, + ingress_expiry: Option, + ) -> Result; } pub struct Creator; #[async_trait] impl Create for Creator { - async fn create(&self, id: Arc, url: &str) -> Result { - let mut b = Agent::builder().with_url(url).with_arc_identity(id); - let default_ingress_expiry = Duration::from_secs(4 * MINUTE); - if let Ok(ms) = std::env::var("ICP_CLI_TEST_ADVANCE_TIME_MS") { - b = b.with_ingress_expiry( - default_ingress_expiry - + Duration::from_millis( - ms.parse::() - .expect("ICP_CLI_TEST_ADVANCE_TIME_MS must be set to an int"), - ), - ); - } else { - b = b.with_ingress_expiry(default_ingress_expiry); - } + async fn create( + &self, + id: Arc, + url: &str, + ingress_expiry: Option, + ) -> Result { + let ingress_expiry = + ingress_expiry.unwrap_or_else(|| DEFAULT_INGRESS_EXPIRY + test_time_advance()); + + let b = Agent::builder() + .with_url(url) + .with_arc_identity(id) + .with_ingress_expiry(ingress_expiry); Ok(b.build().context(AgentSnafu)?) } } + +/// How far a test has advanced the replica's clock past this machine's, so the +/// default ingress expiry stays ahead of replica time. +fn test_time_advance() -> Duration { + match std::env::var("ICP_CLI_TEST_ADVANCE_TIME_MS") { + Ok(ms) => Duration::from_millis( + ms.parse::() + .expect("ICP_CLI_TEST_ADVANCE_TIME_MS must be set to an int"), + ), + Err(_) => Duration::ZERO, + } +} diff --git a/crates/icp/src/context/mod.rs b/crates/icp/src/context/mod.rs index e05e7fb41..3ae5ea72a 100644 --- a/crates/icp/src/context/mod.rs +++ b/crates/icp/src/context/mod.rs @@ -16,6 +16,7 @@ use crate::{ use candid::Principal; use ic_agent::{Agent, Identity}; use snafu::{OptionExt, ResultExt, Snafu}; +use time::OffsetDateTime; mod init; @@ -413,7 +414,7 @@ impl Context { ) -> Result { let agent = self .agent - .create(id, network_access.api_url.as_str()) + .create(id, network_access.api_url.as_str(), None) .await?; agent.set_root_key(network_access.root_key); Ok(agent) @@ -426,10 +427,34 @@ impl Context { url: &Url, ) -> Result { let id = self.get_identity(identity, None).await?; - let agent = self.agent.create(id, url.as_str()).await?; + let agent = self.agent.create(id, url.as_str(), None).await?; Ok(agent) } + /// Creates an agent for signing a message that a different machine will submit. + /// + /// Unlike the other constructors this resolves no root key: nothing is + /// verified on this side, and resolving one can mean a network round trip the + /// signing machine is unable to make. The agent's ingress expiry is pinned so + /// that the expiry it derives on its own — for the pre-signed + /// `request_status` that accompanies an update — lands exactly on + /// `expire_at`, the instant the call envelope itself expires. + pub async fn get_agent_for_signing( + &self, + identity: &IdentitySelection, + url: &Url, + expire_at: OffsetDateTime, + ) -> Result { + // Loading the identity can block on a password prompt or a hardware + // token, so measure what is left of the window only once it is in hand. + let id = self.get_identity(identity, None).await?; + let remaining = (expire_at - OffsetDateTime::now_utc()) + .try_into() + .ok() + .context(SigningWindowClosedSnafu { expire_at })?; + Ok(self.agent.create(id, url.as_str(), Some(remaining)).await?) + } + pub async fn get_agent( &self, identity: &IdentitySelection, @@ -784,6 +809,22 @@ pub enum GetAgentForUrlError { }, } +#[derive(Debug, Snafu)] +pub enum GetAgentForSigningError { + #[snafu(transparent)] + GetIdentity { source: GetIdentityError }, + + #[snafu(display( + "the submission window closed at {expire_at} before the message could be signed" + ))] + SigningWindowClosed { expire_at: OffsetDateTime }, + + #[snafu(transparent)] + AgentCreate { + source: crate::agent::CreateAgentError, + }, +} + #[derive(Debug, Snafu)] pub enum GetAgentError { #[snafu(transparent)] diff --git a/crates/icp/src/lib.rs b/crates/icp/src/lib.rs index 357228627..c5433ce82 100644 --- a/crates/icp/src/lib.rs +++ b/crates/icp/src/lib.rs @@ -38,6 +38,7 @@ pub mod prelude; pub mod project; pub mod settings; pub mod signal; +pub mod signed_message; pub mod store_artifact; pub mod store_id; pub mod telemetry_data; diff --git a/crates/icp/src/network/access.rs b/crates/icp/src/network/access.rs index 05dbe3b4c..89fc10bed 100644 --- a/crates/icp/src/network/access.rs +++ b/crates/icp/src/network/access.rs @@ -160,7 +160,7 @@ async fn fetch_root_key( "fetching the root key from {api_url}; its provenance is not verified (trust-on-first-use)" ); let bootstrap = agent - .create(Arc::new(AnonymousIdentity), api_url.as_str()) + .create(Arc::new(AnonymousIdentity), api_url.as_str(), None) .await .context(CreateBootstrapAgentSnafu { url: api_url.clone(), diff --git a/crates/icp/src/signed_message.rs b/crates/icp/src/signed_message.rs new file mode 100644 index 000000000..1f93d9b29 --- /dev/null +++ b/crates/icp/src/signed_message.rs @@ -0,0 +1,974 @@ +//! The `icp-signed-message` file format, version 1. +//! +//! One canister call, composed and signed on a machine that holds the key and +//! has no network, carried to a machine that has network and no key, and +//! submitted there. `icp canister call --sign-only` writes these files and `icp +//! message send` reads them. +//! +//! The file is JSON so a human courier can look at what they are carrying, and +//! its four objects mirror the trust tiers: +//! +//! 1. **Authenticated** — everything inside `request.envelope`. The courier +//! cannot alter any of it without invalidating the signature. +//! 2. **Acted upon, not authenticated** — `network` and `destination`. The +//! sending machine has to trust these to know where to send; they cannot +//! change *what* executes. +//! 3. **Display only** — `candid` and `summary`. Never used for a decision; +//! everything shown to the operator is re-derived from the envelope. + +use crate::network::RootKeySpec; +use crate::prelude::*; +use base64::engine::general_purpose::STANDARD as BASE64; +use candid::Principal; +use ic_agent::agent::{ + EffectiveId, EnvelopeContent, signed_query_inspect, signed_request_status_inspect, + signed_update_inspect, +}; +use ic_agent::{AgentError, RequestId}; +use serde::{Deserialize, Serialize}; +use snafu::prelude::*; +use time::{Duration, OffsetDateTime, format_description::well_known::Rfc3339}; +use url::Url; + +/// The `format` field every version 1 file carries. +pub const FORMAT: &str = "icp-signed-message"; + +/// The only format version this build reads or writes. An unknown version is +/// refused outright rather than parsed optimistically; fields a reader may +/// safely ignore are added without bumping it. +pub const VERSION: u32 = 1; + +/// How wide the submission window always is. +/// +/// The IC accepts an ingress message only while its `ingress_expiry` is in the +/// future *and* no more than `MAX_INGRESS_TTL` ahead of replica time, so the +/// submittable window is always `[ingress_expiry - 5min, ingress_expiry]`. The +/// width is not a parameter; only its placement is, which is what `--valid-from` +/// sets. +pub const SUBMISSION_WINDOW: Duration = Duration::minutes(5); + +/// The management canister methods the IC routes by effective *subnet* id rather +/// than by canister id, and so the only ones a `subnet` destination may name. +const SUBNET_SCOPED_UPDATE_METHODS: [&str; 2] = + ["create_canister", "provisional_create_canister_with_cycles"]; +const SUBNET_SCOPED_QUERY_METHODS: [&str; 1] = ["list_canisters"]; + +/// A canister call signed for submission from another machine. +#[derive(Debug, Clone, Serialize, Deserialize)] +pub struct SignedMessage { + /// Identifies the file. Always [`FORMAT`]. + pub format: String, + + /// A hard parse gate; see [`VERSION`]. + pub version: u32, + + pub request: Request, + pub network: Network, + pub destination: Destination, + + /// The canister's interface, as `.did` source text — not a path, which would + /// point at a file the submitting machine does not have. Display only: it + /// renders the argument in the confirmation summary and decodes the reply. + /// Optional, because the submitting machine is online and can fall back to + /// fetching `candid:service` itself. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub candid: Option, + + pub summary: Summary, +} + +/// The signed request, and — for an update — what is needed to await its result +/// without a key. +#[derive(Debug, Clone, Serialize, Deserialize)] +pub struct Request { + #[serde(rename = "type")] + pub request_type: RequestType, + + /// The CBOR authentication envelope. The only authenticated content here. + #[serde(with = "base64_bytes")] + pub envelope: Vec, + + /// Hex-encoded request id, recomputed from the envelope on the way in. + /// Update only — a query answers immediately, so it identifies nothing. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub request_id: Option, + + /// A pre-signed `read_state` for `request_status/`, so the + /// submitting machine can poll for the outcome with no key of its own. + /// Update only. + #[serde( + default, + skip_serializing_if = "Option::is_none", + with = "base64_bytes::option" + )] + pub status_check: Option>, +} + +#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)] +#[serde(rename_all = "lowercase")] +pub enum RequestType { + Update, + Query, +} + +impl RequestType { + pub fn as_str(self) -> &'static str { + match self { + RequestType::Update => "update", + RequestType::Query => "query", + } + } +} + +/// Where to submit. The envelope carries no URL, and the reply's certificate has +/// to be verified against a root key the submitting machine may not have +/// configured. +#[derive(Debug, Clone, Serialize, Deserialize)] +pub struct Network { + pub url: Url, + pub root_key: RootKeySpec, +} + +/// Which endpoint shape routes the request, mirroring +/// [`ic_agent::agent::EffectiveId`]. +/// +/// Deliberately tagged rather than a bare principal: canister-scoped and +/// subnet-scoped requests go to different endpoints, and the discriminant cannot +/// be re-derived — it usually equals the canister id, but for management-canister +/// calls it comes from the argument, and for a few it is a subnet id. +/// +/// There is deliberately no `From`: `ic-agent` has one, and it yields +/// `EffectiveId::Canister`, so a bare principal handed to `update_signed` routes +/// to the canister endpoint without complaining. Construct the variant. +#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)] +#[serde(rename_all = "lowercase")] +pub enum Destination { + Canister(Principal), + Subnet(Principal), +} + +impl Destination { + pub fn to_effective_id(self) -> EffectiveId { + match self { + Destination::Canister(p) => EffectiveId::Canister(p), + Destination::Subnet(p) => EffectiveId::Subnet(p), + } + } +} + +/// A human-readable echo of the envelope, for eyeballing the raw file. Never +/// acted upon — the submitting machine re-derives all of it and refuses the file +/// if the two disagree. +#[derive(Debug, Clone, Serialize, Deserialize)] +pub struct Summary { + pub sender: Principal, + pub canister_id: Principal, + pub method: String, + + #[serde(with = "base64_bytes")] + pub arg: Vec, + + pub signed_at: String, + + /// Both ends of the window are recorded so the file answers "when can I send + /// this?" without arithmetic. + pub valid_from: String, + pub valid_until: String, +} + +/// Where now sits relative to the message's five-minute submission window. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum WindowState { + /// The window has not opened yet — a state the signer asked for, not a puzzle. + NotYetValid, + Valid, + Expired, +} + +/// Everything a submitter needs, all of it re-derived from the signed envelope +/// rather than read out of the file's metadata. +#[derive(Debug, Clone)] +#[allow( + dead_code, + reason = "read by the submitting side, which lands separately" +)] +pub struct Validated { + pub request_type: RequestType, + pub sender: Principal, + pub canister_id: Principal, + pub method: String, + pub arg: Vec, + + /// Update only. + pub request_id: Option, + + pub valid_from: OffsetDateTime, + pub valid_until: OffsetDateTime, + pub window: WindowState, +} + +impl SignedMessage { + /// Writes the message to `path`. + pub fn save(&self, path: &Path) -> Result<(), Error> { + crate::fs::json::save(path, self).context(SaveSnafu { path }) + } + + /// Renders the message exactly as [`SignedMessage::save`] would write it, for + /// callers writing somewhere other than a file. + pub fn to_json(&self) -> Result { + serde_json::to_string_pretty(self).context(SerializeSnafu) + } + + /// Reads a message from `path`. The result is unvalidated — call + /// [`SignedMessage::validate`] before acting on any of it. + pub fn load(path: &Path) -> Result { + crate::fs::json::load(path).context(LoadSnafu { path }) + } + + /// Checks the file against its envelope and reports where `now` falls in the + /// submission window. + /// + /// Everything the caller goes on to display or act upon comes back in + /// [`Validated`], decoded from the envelope; a metadata field that disagrees + /// with the envelope means a malformed or tampered file and is an error, not + /// a silent preference. The window state is *reported*, not enforced, so a + /// caller can still show an expired message. + pub fn validate(&self, now: OffsetDateTime) -> Result { + ensure!( + self.format == FORMAT, + UnknownFormatSnafu { + format: self.format.clone() + } + ); + ensure!( + self.version == VERSION, + UnsupportedVersionSnafu { + version: self.version + } + ); + + let content: EnvelopeContent = decode_envelope(&self.request.envelope)?; + + let (sender, canister_id, method, arg, ingress_expiry) = match &content { + EnvelopeContent::Call { + sender, + canister_id, + method_name, + arg, + ingress_expiry, + .. + } => { + ensure!( + self.request.request_type == RequestType::Update, + RequestTypeMismatchSnafu { + declared: self.request.request_type, + envelope: "update", + } + ); + ( + *sender, + *canister_id, + method_name.clone(), + arg.clone(), + *ingress_expiry, + ) + } + EnvelopeContent::Query { + sender, + canister_id, + method_name, + arg, + ingress_expiry, + .. + } => { + ensure!( + self.request.request_type == RequestType::Query, + RequestTypeMismatchSnafu { + declared: self.request.request_type, + envelope: "query", + } + ); + ( + *sender, + *canister_id, + method_name.clone(), + arg.clone(), + *ingress_expiry, + ) + } + EnvelopeContent::ReadState { .. } => { + return RequestTypeMismatchSnafu { + declared: self.request.request_type, + envelope: "read_state", + } + .fail(); + } + }; + + // The summary is what a human reads out of the file, so it has to be the + // envelope's own story. `ic-agent`'s inspectors compare exactly the + // fields the envelope carries. + let inspect = match self.request.request_type { + RequestType::Update => signed_update_inspect( + self.summary.sender, + self.summary.canister_id, + &self.summary.method, + &self.summary.arg, + ingress_expiry, + self.request.envelope.clone(), + ), + RequestType::Query => signed_query_inspect( + self.summary.sender, + self.summary.canister_id, + &self.summary.method, + &self.summary.arg, + ingress_expiry, + self.request.envelope.clone(), + ), + }; + inspect.context(SummaryMismatchSnafu)?; + + let valid_until = timestamp_from_nanos(ingress_expiry)?; + let valid_from = valid_until - SUBMISSION_WINDOW; + ensure!( + self.summary.valid_from == format_timestamp(valid_from) + && self.summary.valid_until == format_timestamp(valid_until), + WindowMismatchSnafu { + recorded_from: self.summary.valid_from.clone(), + recorded_until: self.summary.valid_until.clone(), + envelope_from: format_timestamp(valid_from), + envelope_until: format_timestamp(valid_until), + } + ); + + let request_id = match self.request.request_type { + RequestType::Update => { + let computed = content.to_request_id(); + let recorded = self + .request + .request_id + .as_deref() + .context(MissingRequestIdSnafu)?; + ensure!( + recorded == computed.to_string(), + RequestIdMismatchSnafu { + recorded: recorded.to_owned(), + computed: computed.to_string(), + } + ); + + let status_check = self + .request + .status_check + .as_ref() + .context(MissingStatusCheckSnafu)?; + // Sharing the call's expiry is the point: a status check with a + // later expiry would itself be premature, and one with an earlier + // expiry would die before the call it is waiting on. + signed_request_status_inspect( + sender, + &computed, + ingress_expiry, + status_check.clone(), + ) + .context(StatusCheckMismatchSnafu)?; + + Some(computed) + } + RequestType::Query => { + ensure!( + self.request.request_id.is_none() && self.request.status_check.is_none(), + QueryCarriesUpdateFieldsSnafu + ); + None + } + }; + + self.check_destination(&method)?; + + let window = if now < valid_from { + WindowState::NotYetValid + } else if now > valid_until { + WindowState::Expired + } else { + WindowState::Valid + }; + + Ok(Validated { + request_type: self.request.request_type, + sender, + canister_id, + method, + arg, + request_id, + valid_from, + valid_until, + window, + }) + } + + /// A `subnet` destination is legal only for the management-canister methods + /// the interface spec routes that way; anything else is malformed. + fn check_destination(&self, method: &str) -> Result<(), Error> { + let Destination::Subnet(subnet) = self.destination else { + return Ok(()); + }; + let permitted = match self.request.request_type { + RequestType::Update => SUBNET_SCOPED_UPDATE_METHODS.contains(&method), + RequestType::Query => SUBNET_SCOPED_QUERY_METHODS.contains(&method), + }; + ensure!( + permitted && self.summary.canister_id == Principal::management_canister(), + IllegalSubnetDestinationSnafu { + subnet, + method: method.to_owned(), + } + ); + Ok(()) + } +} + +/// CBOR-decodes an authentication envelope down to the content that was signed. +fn decode_envelope(envelope: &[u8]) -> Result { + let envelope: ic_agent::agent::Envelope = + serde_cbor::from_slice(envelope).context(MalformedEnvelopeSnafu)?; + Ok(envelope.content.into_owned()) +} + +fn timestamp_from_nanos(nanos: u64) -> Result { + OffsetDateTime::from_unix_timestamp_nanos(i128::from(nanos)) + .ok() + .context(UnrepresentableExpirySnafu { nanos }) +} + +/// Formats an instant the way the file records it: RFC 3339 in UTC. +/// +/// Every timestamp written into a file goes through here, which is what lets +/// [`SignedMessage::validate`] check the recorded window against the envelope by +/// comparing rendered strings — no reparsing, and no precision lost either way. +pub fn format_timestamp(t: OffsetDateTime) -> String { + t.format(&Rfc3339) + .expect("an OffsetDateTime is always representable as RFC 3339") +} + +#[derive(Debug, Snafu)] +pub enum Error { + #[snafu(display("failed to write the signed message to {path}"))] + Save { + source: crate::fs::json::Error, + path: PathBuf, + }, + + #[snafu(display("failed to serialize the signed message"))] + Serialize { source: serde_json::Error }, + + #[snafu(display("failed to read the signed message at {path}"))] + Load { + source: crate::fs::json::Error, + path: PathBuf, + }, + + #[snafu(display("not an {FORMAT} file: its format is '{format}'"))] + UnknownFormat { format: String }, + + #[snafu(display( + "signed message format version {version} is not supported; this build reads version {VERSION}" + ))] + UnsupportedVersion { version: u32 }, + + #[snafu(display("the signed request could not be decoded"))] + MalformedEnvelope { source: serde_cbor::Error }, + + #[snafu(display( + "the file declares a {} request but its envelope is a {envelope} request", + declared.as_str() + ))] + RequestTypeMismatch { + declared: RequestType, + envelope: &'static str, + }, + + #[snafu(display("the summary does not match the signed request"))] + SummaryMismatch { + #[snafu(source(from(AgentError, Box::new)))] + source: Box, + }, + + #[snafu(display( + "the summary records a submission window of {recorded_from} to {recorded_until}, \ + but the signed request expires over {envelope_from} to {envelope_until}" + ))] + WindowMismatch { + recorded_from: String, + recorded_until: String, + envelope_from: String, + envelope_until: String, + }, + + #[snafu(display("an update message must carry a request_id"))] + MissingRequestId, + + #[snafu(display( + "the recorded request_id {recorded} is not the one the signed request hashes to ({computed})" + ))] + RequestIdMismatch { recorded: String, computed: String }, + + #[snafu(display("an update message must carry a status_check"))] + MissingStatusCheck, + + #[snafu(display("the status_check does not read the status of this request"))] + StatusCheckMismatch { + #[snafu(source(from(AgentError, Box::new)))] + source: Box, + }, + + #[snafu(display( + "a query message answers immediately and must carry neither request_id nor status_check" + ))] + QueryCarriesUpdateFields, + + #[snafu(display( + "'{method}' cannot be routed to subnet {subnet}: only management canister calls the \ + interface spec scopes to a subnet may name a subnet destination" + ))] + IllegalSubnetDestination { subnet: Principal, method: String }, + + #[snafu(display("the signed request expires at {nanos}ns, which is not a representable time"))] + UnrepresentableExpiry { nanos: u64 }, +} + +/// Byte fields are base64 rather than hex: an argument can be large — an +/// air-gapped `install_code` carries a whole Wasm module — where hex doubles the +/// file and base64 adds a third. +mod base64_bytes { + use super::BASE64; + use base64::Engine as _; + use serde::{Deserialize as _, Deserializer, Serializer, de::Error as _}; + + pub(super) fn serialize(bytes: &[u8], s: S) -> Result { + s.serialize_str(&BASE64.encode(bytes)) + } + + pub(super) fn deserialize<'de, D: Deserializer<'de>>(d: D) -> Result, D::Error> { + let encoded = String::deserialize(d)?; + BASE64.decode(&encoded).map_err(D::Error::custom) + } + + pub(super) mod option { + use super::*; + + pub(in super::super) fn serialize( + bytes: &Option>, + s: S, + ) -> Result { + match bytes { + Some(bytes) => super::serialize(bytes, s), + None => s.serialize_none(), + } + } + + pub(in super::super) fn deserialize<'de, D: Deserializer<'de>>( + d: D, + ) -> Result>, D::Error> { + let encoded = Option::::deserialize(d)?; + encoded + .map(|encoded| BASE64.decode(&encoded).map_err(D::Error::custom)) + .transpose() + } + } +} + +#[cfg(test)] +mod tests { + use super::*; + use base64::Engine as _; + use camino_tempfile::tempdir; + use ic_agent::{Agent, identity::AnonymousIdentity}; + + const CANISTER: &str = "ryjl3-tyaaa-aaaaa-aaaba-cai"; + const SUBNET: &str = "fuqsr-in2lc-zbcjj-ydmcw-pzq7h-4xm2z-pto4i-dcyee-5z4rz-x63ji-nae"; + + fn canister() -> Principal { + Principal::from_text(CANISTER).expect("a valid principal") + } + + /// An agent built the way `--sign-only` builds one: no root key, and an + /// ingress expiry pinned so the status check it derives lands on + /// `valid_until`. + fn signing_agent(valid_until: OffsetDateTime) -> Agent { + let remaining = (valid_until - OffsetDateTime::now_utc()) + .try_into() + .expect("the test window must be in the future"); + Agent::builder() + .with_url("http://localhost:1") + .with_identity(AnonymousIdentity) + .with_ingress_expiry(remaining) + .build() + .expect("building an agent makes no request") + } + + /// A minute-aligned window opening `offset` from now, as the signing path + /// produces it. + fn window(offset: Duration) -> (OffsetDateTime, OffsetDateTime) { + let until = (OffsetDateTime::now_utc() + offset + SUBMISSION_WINDOW) + .replace_nanosecond(0) + .and_then(|t| t.replace_second(0)) + .expect("0 is a valid second and nanosecond"); + (until - SUBMISSION_WINDOW, until) + } + + fn update_message_for( + canister_id: Principal, + method: &str, + offset: Duration, + ) -> (SignedMessage, OffsetDateTime, OffsetDateTime) { + let (valid_from, valid_until) = window(offset); + let agent = signing_agent(valid_until); + let signed = agent + .update(&canister_id, method) + .with_arg(b"arg".to_vec()) + .expire_at(valid_until) + .sign() + .expect("signing makes no request"); + let status_check = agent + .sign_request_status(EffectiveId::Canister(canister_id), signed.request_id) + .expect("signing makes no request"); + assert_eq!( + status_check.ingress_expiry, signed.ingress_expiry, + "a minute-aligned expiry must survive ic-agent's own truncation" + ); + + let message = SignedMessage { + format: FORMAT.to_string(), + version: VERSION, + request: Request { + request_type: RequestType::Update, + envelope: signed.signed_update, + request_id: Some(signed.request_id.to_string()), + status_check: Some(status_check.signed_request_status), + }, + network: Network { + url: "https://icp-api.io".parse().expect("a valid url"), + root_key: RootKeySpec::Mainnet, + }, + destination: Destination::Canister(canister_id), + candid: Some(r#"service : { "greet" : (text) -> (text) }"#.to_string()), + summary: Summary { + sender: signed.sender, + canister_id, + method: method.to_string(), + arg: b"arg".to_vec(), + signed_at: format_timestamp(OffsetDateTime::now_utc()), + valid_from: format_timestamp(valid_from), + valid_until: format_timestamp(valid_until), + }, + }; + (message, valid_from, valid_until) + } + + fn update_message() -> (SignedMessage, OffsetDateTime, OffsetDateTime) { + update_message_for(canister(), "greet", Duration::ZERO) + } + + fn query_message() -> SignedMessage { + let (valid_from, valid_until) = window(Duration::ZERO); + let agent = signing_agent(valid_until); + let signed = agent + .query(&canister(), "greet") + .with_arg(b"arg".to_vec()) + .expire_at(valid_until) + .sign() + .expect("signing makes no request"); + + SignedMessage { + format: FORMAT.to_string(), + version: VERSION, + request: Request { + request_type: RequestType::Query, + envelope: signed.signed_query, + request_id: None, + status_check: None, + }, + network: Network { + url: "https://icp-api.io".parse().expect("a valid url"), + root_key: RootKeySpec::Mainnet, + }, + destination: Destination::Canister(canister()), + candid: None, + summary: Summary { + sender: signed.sender, + canister_id: canister(), + method: "greet".to_string(), + arg: b"arg".to_vec(), + signed_at: format_timestamp(OffsetDateTime::now_utc()), + valid_from: format_timestamp(valid_from), + valid_until: format_timestamp(valid_until), + }, + } + } + + #[test] + fn round_trips_through_a_file() { + let (message, _, _) = update_message(); + let dir = tempdir().expect("temp dir"); + let path = dir.path().join("message.json"); + message.save(&path).expect("save"); + + let loaded = SignedMessage::load(&path).expect("load"); + let validated = loaded + .validate(OffsetDateTime::now_utc()) + .expect("a message we just signed must validate"); + + assert_eq!(validated.canister_id, canister()); + assert_eq!(validated.method, "greet"); + assert_eq!(validated.arg, b"arg"); + assert_eq!(validated.request_type, RequestType::Update); + assert_eq!(validated.window, WindowState::Valid); + assert_eq!( + validated + .request_id + .expect("an update carries one") + .to_string(), + message.request.request_id.expect("an update carries one"), + ); + } + + #[test] + fn writes_the_documented_json_shape() { + let (message, _, valid_until) = update_message(); + let json: serde_json::Value = + serde_json::from_str(&message.to_json().expect("serialize")).expect("valid json"); + + assert_eq!(json["format"], FORMAT); + assert_eq!(json["version"], VERSION); + assert_eq!(json["request"]["type"], "update"); + // Tagged, so a submitter can tell a canister route from a subnet one. + assert_eq!(json["destination"]["canister"], CANISTER); + assert_eq!(json["network"]["root_key"], "mainnet"); + assert_eq!( + json["summary"]["valid_until"], + format_timestamp(valid_until) + ); + // Base64, not hex: an argument can be a whole Wasm module. + assert_eq!( + BASE64 + .decode(json["summary"]["arg"].as_str().expect("a string")) + .expect("base64"), + b"arg", + ); + } + + #[test] + fn query_message_omits_the_update_only_fields() { + let message = query_message(); + let json: serde_json::Value = + serde_json::from_str(&message.to_json().expect("serialize")).expect("valid json"); + assert_eq!(json["request"]["type"], "query"); + assert!(json["request"].get("request_id").is_none()); + assert!(json["request"].get("status_check").is_none()); + + let validated = message + .validate(OffsetDateTime::now_utc()) + .expect("validate"); + assert_eq!(validated.request_type, RequestType::Query); + assert!(validated.request_id.is_none()); + } + + #[test] + fn query_carrying_update_fields_is_rejected() { + let (update, _, _) = update_message(); + let mut message = query_message(); + message.request.status_check = update.request.status_check; + + assert!(matches!( + message.validate(OffsetDateTime::now_utc()), + Err(Error::QueryCarriesUpdateFields), + )); + } + + #[test] + fn declared_type_must_match_the_envelope() { + let mut message = query_message(); + message.request.request_type = RequestType::Update; + + assert!(matches!( + message.validate(OffsetDateTime::now_utc()), + Err(Error::RequestTypeMismatch { .. }), + )); + } + + #[test] + fn tampered_summary_is_rejected() { + for tamper in [ + (|m: &mut SignedMessage| m.summary.method = "transfer".to_string()) as fn(&mut _), + |m: &mut SignedMessage| m.summary.arg = b"other".to_vec(), + |m: &mut SignedMessage| m.summary.canister_id = Principal::management_canister(), + |m: &mut SignedMessage| m.summary.sender = Principal::management_canister(), + ] { + let (mut message, _, _) = update_message(); + tamper(&mut message); + assert!( + matches!( + message.validate(OffsetDateTime::now_utc()), + Err(Error::SummaryMismatch { .. }), + ), + "a summary that disagrees with the envelope is an error, not a preference", + ); + } + } + + #[test] + fn tampered_window_is_rejected() { + let (mut message, _, valid_until) = update_message(); + message.summary.valid_until = format_timestamp(valid_until + Duration::hours(1)); + + assert!(matches!( + message.validate(OffsetDateTime::now_utc()), + Err(Error::WindowMismatch { .. }), + )); + } + + #[test] + fn request_id_must_be_the_envelope_hash() { + let (mut message, _, _) = update_message(); + message.request.request_id = Some("00".repeat(32)); + + assert!(matches!( + message.validate(OffsetDateTime::now_utc()), + Err(Error::RequestIdMismatch { .. }), + )); + } + + #[test] + fn status_check_must_read_this_requests_status() { + let (mut message, _, valid_until) = update_message(); + // A status check signed for some *other* call: correctly formed, same + // sender, same window — but it would poll the wrong request. + let agent = signing_agent(valid_until); + let other = agent + .update(&canister(), "greet") + .with_arg(b"different".to_vec()) + .expire_at(valid_until) + .sign() + .expect("signing makes no request"); + let elsewhere = agent + .sign_request_status(EffectiveId::Canister(canister()), other.request_id) + .expect("signing makes no request"); + message.request.status_check = Some(elsewhere.signed_request_status); + + assert!(matches!( + message.validate(OffsetDateTime::now_utc()), + Err(Error::StatusCheckMismatch { .. }), + )); + } + + #[test] + fn missing_update_fields_are_rejected() { + let (mut message, _, _) = update_message(); + let status_check = message.request.status_check.take(); + assert!(matches!( + message.validate(OffsetDateTime::now_utc()), + Err(Error::MissingStatusCheck), + )); + + message.request.status_check = status_check; + message.request.request_id = None; + assert!(matches!( + message.validate(OffsetDateTime::now_utc()), + Err(Error::MissingRequestId), + )); + } + + #[test] + fn reports_each_window_state() { + let (message, valid_from, valid_until) = update_message_for( + canister(), + "greet", + // Placed in the future so every state can be asked about. + Duration::hours(1), + ); + + let state = |now| message.validate(now).expect("validate").window; + assert_eq!( + state(valid_from - Duration::seconds(1)), + WindowState::NotYetValid + ); + assert_eq!(state(valid_from), WindowState::Valid); + assert_eq!(state(valid_until), WindowState::Valid); + assert_eq!( + state(valid_until + Duration::seconds(1)), + WindowState::Expired + ); + } + + #[test] + fn the_window_is_always_five_minutes_wide() { + let (message, valid_from, valid_until) = update_message(); + assert_eq!(valid_until - valid_from, SUBMISSION_WINDOW); + + let validated = message + .validate(OffsetDateTime::now_utc()) + .expect("validate"); + assert_eq!(validated.valid_from, valid_from); + assert_eq!(validated.valid_until, valid_until); + } + + #[test] + fn subnet_destination_is_legal_only_for_subnet_scoped_methods() { + let subnet = Principal::from_text(SUBNET).expect("a valid principal"); + + let (mut creating, _, _) = update_message_for( + Principal::management_canister(), + "create_canister", + Duration::ZERO, + ); + creating.destination = Destination::Subnet(subnet); + creating + .validate(OffsetDateTime::now_utc()) + .expect("canister creation is routed by subnet"); + + let (mut greeting, _, _) = update_message(); + greeting.destination = Destination::Subnet(subnet); + assert!(matches!( + greeting.validate(OffsetDateTime::now_utc()), + Err(Error::IllegalSubnetDestination { .. }), + )); + + // Right method, but on an ordinary canister rather than the management one. + let (mut impostor, _, _) = + update_message_for(canister(), "create_canister", Duration::ZERO); + impostor.destination = Destination::Subnet(subnet); + assert!(matches!( + impostor.validate(OffsetDateTime::now_utc()), + Err(Error::IllegalSubnetDestination { .. }), + )); + } + + #[test] + fn unknown_format_and_version_are_refused_outright() { + let (mut message, _, _) = update_message(); + message.version = VERSION + 1; + assert!(matches!( + message.validate(OffsetDateTime::now_utc()), + Err(Error::UnsupportedVersion { .. }), + )); + + message.version = VERSION; + message.format = "quill".to_string(); + assert!(matches!( + message.validate(OffsetDateTime::now_utc()), + Err(Error::UnknownFormat { .. }), + )); + } + + #[test] + fn malformed_envelope_is_refused() { + let (mut message, _, _) = update_message(); + message.request.envelope = b"not cbor".to_vec(); + + assert!(matches!( + message.validate(OffsetDateTime::now_utc()), + Err(Error::MalformedEnvelope { .. }), + )); + } +} diff --git a/docs/reference/cli.md b/docs/reference/cli.md index ac32476a9..4ca081d5b 100644 --- a/docs/reference/cli.md +++ b/docs/reference/cli.md @@ -214,6 +214,12 @@ Make a canister call Print raw response as hex * `--json` — Output command results as JSON +* `--sign-only ` — Sign the call and write it to FILE instead of submitting it, so it can be submitted later from a machine that has network access but not your key. `-` writes to stdout. + + Nothing is sent, and nothing is fetched: the interface comes from `--candid` or the local build artifact rather than from the canister, so this works with no network at all. `--root-key` must name a key rather than `fetch`, and `--proxy` is not supported. +* `--valid-from ` — When the signed message's five-minute submission window opens: a duration from now (`55m`, `2h`) or an RFC 3339 timestamp (`2026-08-17T10:07:00Z`). Defaults to now. + + The window is always five minutes wide — the IC will not accept an ingress message expiring further ahead than that — so this places it rather than sizing it. From 3d51bf844f3d0307115774815c14b83c94815986 Mon Sep 17 00:00:00 2001 From: Linwei Shang Date: Tue, 18 Aug 2026 10:56:47 -0400 Subject: [PATCH 2/3] refactor: name the signed message's request type CallType MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `RequestType` did not say whether it meant the called method's kind or the API used to invoke it — a real ambiguity, since the two do not correspond: an update method can only go through `/call`, a composite query only through `/query`, and a query method through either. It is the API, and the codebase already had a name for exactly that distinction: `sync-plugin.wit`'s `enum call-type { update, query }`, which dispatches `agent.update()` against `agent.query()` for the same reason. Adopt it, and document what the type selects and why it cannot be re-derived from the interface. The wire format is unchanged: the field keeps `#[serde(rename = "type")]` and the values stay `update` / `query`, as the design specifies and as both the plugin ABI and quill spell them. Co-Authored-By: Claude Opus 5 (1M context) --- crates/icp-cli/src/commands/canister/call.rs | 10 +-- crates/icp/src/signed_message.rs | 79 ++++++++++++-------- 2 files changed, 52 insertions(+), 37 deletions(-) diff --git a/crates/icp-cli/src/commands/canister/call.rs b/crates/icp-cli/src/commands/canister/call.rs index df6c1b830..7f8039e25 100644 --- a/crates/icp-cli/src/commands/canister/call.rs +++ b/crates/icp-cli/src/commands/canister/call.rs @@ -13,7 +13,7 @@ use icp::manifest::ArgsFormat; use icp::network::{Configuration as NetworkConfiguration, RootKeySpec}; use icp::parsers::{CyclesAmount, DurationAmount}; use icp::prelude::*; -use icp::signed_message::{self, Destination, Request, RequestType, SignedMessage, Summary}; +use icp::signed_message::{self, CallType, Destination, Request, SignedMessage, Summary}; use serde::Serialize; use std::io::{self, Write}; use std::str::FromStr; @@ -407,7 +407,7 @@ async fn sign_only( .sign() .context("failed to sign the query")?; Request { - request_type: RequestType::Query, + call_type: CallType::Query, envelope: signed.signed_query, // A query answers immediately, so there is nothing to poll for. request_id: None, @@ -437,13 +437,13 @@ async fn sign_only( ); Request { - request_type: RequestType::Update, + call_type: CallType::Update, envelope: signed.signed_update, request_id: Some(signed.request_id.to_string()), status_check: Some(status_check.signed_request_status), } }; - let request_type = request.request_type; + let call_type = request.call_type; let message = SignedMessage { format: signed_message::FORMAT.to_string(), @@ -483,7 +483,7 @@ async fn sign_only( eprintln!( "Signed a {} call to '{method}' on {cid}, as {sender}.", - request_type.as_str(), + call_type.as_str(), ); eprintln!( "It can be submitted between {} and {} — a five-minute window.", diff --git a/crates/icp/src/signed_message.rs b/crates/icp/src/signed_message.rs index 1f93d9b29..49fc5905f 100644 --- a/crates/icp/src/signed_message.rs +++ b/crates/icp/src/signed_message.rs @@ -82,7 +82,7 @@ pub struct SignedMessage { #[derive(Debug, Clone, Serialize, Deserialize)] pub struct Request { #[serde(rename = "type")] - pub request_type: RequestType, + pub call_type: CallType, /// The CBOR authentication envelope. The only authenticated content here. #[serde(with = "base64_bytes")] @@ -104,18 +104,33 @@ pub struct Request { pub status_check: Option>, } +/// Which submission API the signed envelope targets — *not* what kind of method +/// is being called. +/// +/// A method is an update method, a query method, or a composite query, but there +/// are only two ways to invoke one: `/call` for replicated execution and +/// `/query` for non-replicated. The mapping is not one-to-one — an update method +/// can only go through `/call`, a composite query only through `/query`, and a +/// query method through either — so which one was signed for has to be recorded +/// here rather than re-derived from the interface. +/// +/// [`CallType::Update`] is the envelope's `call` request type, spelled "update" +/// to match `sync-plugin.wit`'s `enum call-type { update, query }`, which draws +/// the same distinction for the same reason. Checked against the envelope's own +/// discriminant on the way in; selects `update_signed` over `query_signed` on +/// the way out. #[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)] #[serde(rename_all = "lowercase")] -pub enum RequestType { +pub enum CallType { Update, Query, } -impl RequestType { +impl CallType { pub fn as_str(self) -> &'static str { match self { - RequestType::Update => "update", - RequestType::Query => "query", + CallType::Update => "update", + CallType::Query => "query", } } } @@ -193,7 +208,7 @@ pub enum WindowState { reason = "read by the submitting side, which lands separately" )] pub struct Validated { - pub request_type: RequestType, + pub call_type: CallType, pub sender: Principal, pub canister_id: Principal, pub method: String, @@ -259,9 +274,9 @@ impl SignedMessage { .. } => { ensure!( - self.request.request_type == RequestType::Update, - RequestTypeMismatchSnafu { - declared: self.request.request_type, + self.request.call_type == CallType::Update, + CallTypeMismatchSnafu { + declared: self.request.call_type, envelope: "update", } ); @@ -282,9 +297,9 @@ impl SignedMessage { .. } => { ensure!( - self.request.request_type == RequestType::Query, - RequestTypeMismatchSnafu { - declared: self.request.request_type, + self.request.call_type == CallType::Query, + CallTypeMismatchSnafu { + declared: self.request.call_type, envelope: "query", } ); @@ -297,8 +312,8 @@ impl SignedMessage { ) } EnvelopeContent::ReadState { .. } => { - return RequestTypeMismatchSnafu { - declared: self.request.request_type, + return CallTypeMismatchSnafu { + declared: self.request.call_type, envelope: "read_state", } .fail(); @@ -308,8 +323,8 @@ impl SignedMessage { // The summary is what a human reads out of the file, so it has to be the // envelope's own story. `ic-agent`'s inspectors compare exactly the // fields the envelope carries. - let inspect = match self.request.request_type { - RequestType::Update => signed_update_inspect( + let inspect = match self.request.call_type { + CallType::Update => signed_update_inspect( self.summary.sender, self.summary.canister_id, &self.summary.method, @@ -317,7 +332,7 @@ impl SignedMessage { ingress_expiry, self.request.envelope.clone(), ), - RequestType::Query => signed_query_inspect( + CallType::Query => signed_query_inspect( self.summary.sender, self.summary.canister_id, &self.summary.method, @@ -341,8 +356,8 @@ impl SignedMessage { } ); - let request_id = match self.request.request_type { - RequestType::Update => { + let request_id = match self.request.call_type { + CallType::Update => { let computed = content.to_request_id(); let recorded = self .request @@ -375,7 +390,7 @@ impl SignedMessage { Some(computed) } - RequestType::Query => { + CallType::Query => { ensure!( self.request.request_id.is_none() && self.request.status_check.is_none(), QueryCarriesUpdateFieldsSnafu @@ -395,7 +410,7 @@ impl SignedMessage { }; Ok(Validated { - request_type: self.request.request_type, + call_type: self.request.call_type, sender, canister_id, method, @@ -413,9 +428,9 @@ impl SignedMessage { let Destination::Subnet(subnet) = self.destination else { return Ok(()); }; - let permitted = match self.request.request_type { - RequestType::Update => SUBNET_SCOPED_UPDATE_METHODS.contains(&method), - RequestType::Query => SUBNET_SCOPED_QUERY_METHODS.contains(&method), + let permitted = match self.request.call_type { + CallType::Update => SUBNET_SCOPED_UPDATE_METHODS.contains(&method), + CallType::Query => SUBNET_SCOPED_QUERY_METHODS.contains(&method), }; ensure!( permitted && self.summary.canister_id == Principal::management_canister(), @@ -483,8 +498,8 @@ pub enum Error { "the file declares a {} request but its envelope is a {envelope} request", declared.as_str() ))] - RequestTypeMismatch { - declared: RequestType, + CallTypeMismatch { + declared: CallType, envelope: &'static str, }, @@ -642,7 +657,7 @@ mod tests { format: FORMAT.to_string(), version: VERSION, request: Request { - request_type: RequestType::Update, + call_type: CallType::Update, envelope: signed.signed_update, request_id: Some(signed.request_id.to_string()), status_check: Some(status_check.signed_request_status), @@ -684,7 +699,7 @@ mod tests { format: FORMAT.to_string(), version: VERSION, request: Request { - request_type: RequestType::Query, + call_type: CallType::Query, envelope: signed.signed_query, request_id: None, status_check: None, @@ -722,7 +737,7 @@ mod tests { assert_eq!(validated.canister_id, canister()); assert_eq!(validated.method, "greet"); assert_eq!(validated.arg, b"arg"); - assert_eq!(validated.request_type, RequestType::Update); + assert_eq!(validated.call_type, CallType::Update); assert_eq!(validated.window, WindowState::Valid); assert_eq!( validated @@ -770,7 +785,7 @@ mod tests { let validated = message .validate(OffsetDateTime::now_utc()) .expect("validate"); - assert_eq!(validated.request_type, RequestType::Query); + assert_eq!(validated.call_type, CallType::Query); assert!(validated.request_id.is_none()); } @@ -789,11 +804,11 @@ mod tests { #[test] fn declared_type_must_match_the_envelope() { let mut message = query_message(); - message.request.request_type = RequestType::Update; + message.request.call_type = CallType::Update; assert!(matches!( message.validate(OffsetDateTime::now_utc()), - Err(Error::RequestTypeMismatch { .. }), + Err(Error::CallTypeMismatch { .. }), )); } From c90812ff4279249e2ea7acd2b7d61543225a739e Mon Sep 17 00:00:00 2001 From: Linwei Shang Date: Tue, 18 Aug 2026 11:45:33 -0400 Subject: [PATCH 3/3] fix: reject an unrepresentable --valid-from instead of panicking MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `--valid-from` accepts any duration that fits a `u64` of seconds, which reaches far past the last representable instant, and `OffsetDateTime`'s `+` panics rather than saturating — so `--valid-from 999999999999d` aborted the process. Place the window with checked arithmetic and report the input as out of range. Alongside, three things the same review surfaced: - Re-read the clock before the message validates itself, and refuse to write one whose window has already closed. Unlocking the identity and signing on a hardware token both take time, and a file that can no longer be submitted is worse than an error. - `--valid-from` promises the instant the window opens, but the expiry is floored to the minute to survive `ic-agent`'s own truncation, so the window can open up to 59 seconds early. Say so in the help text. - `summary.signed_at` is the one summary field the envelope cannot corroborate, since an ingress envelope carries no signing time. Document it as a note from the signer rather than leaving `Summary` claiming that all of it is re-derived and checked. Co-Authored-By: Claude Opus 5 (1M context) --- crates/icp-cli/src/commands/canister/call.rs | 48 ++++++++++++++----- .../icp-cli/tests/canister_call_sign_tests.rs | 29 +++++++++++ crates/icp/src/signed_message.rs | 9 +++- docs/reference/cli.md | 2 +- 4 files changed, 74 insertions(+), 14 deletions(-) diff --git a/crates/icp-cli/src/commands/canister/call.rs b/crates/icp-cli/src/commands/canister/call.rs index 7f8039e25..2790cc001 100644 --- a/crates/icp-cli/src/commands/canister/call.rs +++ b/crates/icp-cli/src/commands/canister/call.rs @@ -13,7 +13,9 @@ use icp::manifest::ArgsFormat; use icp::network::{Configuration as NetworkConfiguration, RootKeySpec}; use icp::parsers::{CyclesAmount, DurationAmount}; use icp::prelude::*; -use icp::signed_message::{self, CallType, Destination, Request, SignedMessage, Summary}; +use icp::signed_message::{ + self, CallType, Destination, Request, SignedMessage, Summary, WindowState, +}; use serde::Serialize; use std::io::{self, Write}; use std::str::FromStr; @@ -119,7 +121,9 @@ pub(crate) struct CallArgs { /// /// The window is always five minutes wide — the IC will not accept an /// ingress message expiring further ahead than that — so this places it - /// rather than sizing it. + /// rather than sizing it. It is rounded down to the whole minute, and so may + /// open up to 59 seconds earlier than asked; the file records the window it + /// actually got. #[arg(long, value_name = "WHEN", requires = "sign_only")] pub(crate) valid_from: Option, } @@ -377,13 +381,26 @@ async fn sign_only( resolve_network_offline(ctx, &selections.network, &selections.environment).await?; let now = OffsetDateTime::now_utc(); + // Checked throughout: `--valid-from` accepts any duration that fits a `u64` + // of seconds, which reaches far past the last representable instant, and + // `OffsetDateTime`'s `+` panics rather than saturating. + let out_of_range = || { + anyhow!( + "`--valid-from` is too far from now to place a submission window in representable time" + ) + }; let opens_at = match &args.valid_from { Some(ValidFrom::At(at)) => *at, - Some(ValidFrom::In(duration)) => now + *duration, + Some(ValidFrom::In(duration)) => now.checked_add(*duration).ok_or_else(out_of_range)?, None => now, }; - let valid_until = floor_to_minute(opens_at + signed_message::SUBMISSION_WINDOW); - let valid_from = valid_until - signed_message::SUBMISSION_WINDOW; + let valid_until = opens_at + .checked_add(signed_message::SUBMISSION_WINDOW) + .map(floor_to_minute) + .ok_or_else(out_of_range)?; + let valid_from = valid_until + .checked_sub(signed_message::SUBMISSION_WINDOW) + .ok_or_else(out_of_range)?; if valid_until <= now { bail!( "`--valid-from` puts the submission window at {} to {}, which has already closed", @@ -450,9 +467,11 @@ async fn sign_only( version: signed_message::VERSION, request, network: signed_message::Network { url, root_key }, - // Every call this command can compose is routed by canister id. A - // subnet-scoped destination is legal in the format, but nothing produces - // one yet. + // Routed by the target's own canister id, which is what `call` does + // online too — it passes no effective canister id to + // `update_or_proxy_raw`. So a management-canister call records + // `aaaaa-aa` and is misrouted here exactly as it already is online. A + // subnet destination is legal in the format, but nothing produces one yet. destination: Destination::Canister(cid), candid: interface.map(str::to_owned), summary: Summary { @@ -468,10 +487,17 @@ async fn sign_only( // Refuse to hand over a file we would not accept back. Signing is the last // thing the air-gapped machine does, so a mistake found on the other side is - // found too late. - message - .validate(now) + // found too late. The clock is re-read rather than reused from above: + // unlocking the identity and signing on a hardware token both take time. + let validated = message + .validate(OffsetDateTime::now_utc()) .context("the signed message failed its own validation")?; + anyhow::ensure!( + validated.window != WindowState::Expired, + "the submission window closed at {} while the message was being signed, \ + so it could no longer be submitted; nothing was written", + signed_message::format_timestamp(valid_until), + ); if out == "-" { let mut stdout = io::stdout(); diff --git a/crates/icp-cli/tests/canister_call_sign_tests.rs b/crates/icp-cli/tests/canister_call_sign_tests.rs index a42b0236d..f5ca54978 100644 --- a/crates/icp-cli/tests/canister_call_sign_tests.rs +++ b/crates/icp-cli/tests/canister_call_sign_tests.rs @@ -383,6 +383,35 @@ fn rejects_proxy() { .stderr(contains("--proxy").and(contains("--sign-only"))); } +/// A duration the parser accepts can still reach past the last representable +/// instant. That has to be a CLI error, not a panic. +#[test] +fn rejects_an_unrepresentable_valid_from() { + let ctx = TestContext::new(); + let out = ctx.home_path().join("message.json"); + + ctx.icp() + .args([ + "canister", + "call", + "--network", + UNREACHABLE, + "--root-key", + "mainnet", + "--sign-only", + out.as_str(), + "--valid-from", + "999999999999d", + TARGET, + "greet", + "(\"world\")", + ]) + .assert() + .failure() + .stderr(contains("too far from now").and(contains("--valid-from"))) + .stderr(contains("panicked").not()); +} + /// `--valid-from` only means something for a message that is being signed. #[test] fn valid_from_requires_sign_only() { diff --git a/crates/icp/src/signed_message.rs b/crates/icp/src/signed_message.rs index 49fc5905f..0d0bff541 100644 --- a/crates/icp/src/signed_message.rs +++ b/crates/icp/src/signed_message.rs @@ -172,8 +172,9 @@ impl Destination { } /// A human-readable echo of the envelope, for eyeballing the raw file. Never -/// acted upon — the submitting machine re-derives all of it and refuses the file -/// if the two disagree. +/// acted upon: [`SignedMessage::validate`] re-derives each field from the +/// envelope and refuses a file whose summary disagrees — with the one exception +/// of [`Summary::signed_at`], which the envelope does not carry. #[derive(Debug, Clone, Serialize, Deserialize)] pub struct Summary { pub sender: Principal, @@ -183,6 +184,10 @@ pub struct Summary { #[serde(with = "base64_bytes")] pub arg: Vec, + /// When the signer says it signed this. An ingress envelope records no + /// signing time, so this is the one field here that nothing can corroborate: + /// a courier can change it and the file still validates. Show it as a note + /// from the signer, never as a verified time. pub signed_at: String, /// Both ends of the window are recorded so the file answers "when can I send diff --git a/docs/reference/cli.md b/docs/reference/cli.md index 4ca081d5b..2b9e49ea0 100644 --- a/docs/reference/cli.md +++ b/docs/reference/cli.md @@ -219,7 +219,7 @@ Make a canister call Nothing is sent, and nothing is fetched: the interface comes from `--candid` or the local build artifact rather than from the canister, so this works with no network at all. `--root-key` must name a key rather than `fetch`, and `--proxy` is not supported. * `--valid-from ` — When the signed message's five-minute submission window opens: a duration from now (`55m`, `2h`) or an RFC 3339 timestamp (`2026-08-17T10:07:00Z`). Defaults to now. - The window is always five minutes wide — the IC will not accept an ingress message expiring further ahead than that — so this places it rather than sizing it. + The window is always five minutes wide — the IC will not accept an ingress message expiring further ahead than that — so this places it rather than sizing it. It is rounded down to the whole minute, and so may open up to 59 seconds earlier than asked; the file records the window it actually got.