diff --git a/CHANGELOG.md b/CHANGELOG.md index 6114b8d306..50966a1ce5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,28 @@ # UNRELEASED +### feat!: asset canister evidence and state hash length-prefix every variable-length field + +The encoding hashed for the evidence of a proposed batch, and for the state hash, now +length-prefixes every variable-length field -- asset keys, content types, content encodings, +header names and values, the declared `sha256`, and asset content -- and hashes the number of +entries in a header map, behind a version-tagged domain separator. Every operation in the +encoding is now self-delimiting, which makes the digest injective over the change a batch +applies, and the encoding is specified under `compute_evidence` in +[docs/design/asset-canister-interface.md](docs/design/asset-canister-interface.md). + +The evidence of a `SetAssetContent` operation now covers `last_chunk` whether or not `chunk_ids` +is empty, matching the content that `commit_batch` stores for the same operation. + +The asset canister now reports `api_version` 3. `dfx deploy --by-proposal` and +`dfx deploy --compute-evidence` report an error against an asset canister that reports a lower +version, instead of computing a value that cannot be compared with it. + +**Upgrade the asset canister and dfx together.** Evidence and state hash values that this release +computes differ from the values earlier releases compute over the same assets. Recompute and +re-verify the evidence of any batch proposed before the upgrade. As before, a proposed batch does +not survive a canister upgrade. + ### fix: `dfx new` projects install again `vite-plugin-environment` 1.1.4 raised its peer dependency to `vite >= 8.0`, which no longer @@ -10,6 +32,13 @@ newly created project. The templates now ask for exactly 1.1.3. ### chore: bump `ic-agent`, `ic-identity-hsm`, `ic-utils` and `ic-transport-types` to 0.47.3 +## Dependencies + +### Frontend canister + +- Module hash: 3fffda14c040852d76ca3c5e6423ae2ece73176254dbb2a1b15e963891e817e1 +- https://github.com/dfinity/sdk/pull/4545 + # 0.32.0 ### feat: Deprecate dfx. All commands will throw off a deprecation warning (this can be disabled with `DFX_WARNING=-deprecation`). diff --git a/docs/design/asset-canister-interface.md b/docs/design/asset-canister-interface.md index ce1bde1c2d..1a376f7a7f 100644 --- a/docs/design/asset-canister-interface.md +++ b/docs/design/asset-canister-interface.md @@ -368,6 +368,58 @@ After the hash computation has completed, the batch will no longer expire. The b Required permission: [Prepare](#permission-prepare) +#### The hashed encoding + +The evidence is `sha256` of the encoding below. Every variable-length field is length-prefixed +and every operation begins with a tag, so each operation is self-delimiting and the encoding is +injective over the change a batch applies: its operations, with the content of each +`SetAssetContent` taken as one byte string. Two batches that apply different changes therefore +never share an encoding. + +It is deliberately *not* injective over the `commit_batch` arguments themselves: two batches that +differ only in how they split the same content across `chunk_ids` and `last_chunk` encode +identically, because the encoding covers the assembled content rather than the chunking. + +Tooling that verifies a proposal recomputes this encoding from source, so it is specified here +rather than left to the implementation. + +The encoding begins with the domain separator `ic-certified-assets v2`, hashed as its 22 bytes +with no length prefix, and is followed by the encoding of each operation, in the order the +operations appear in the arguments. The `v2` in the separator versions this encoding; it is not +the [API version](#api-versions), which is at 3, and the two move independently. + +| Element | Encoded as | +|------------------------|-------------------------------------------------------------------------------| +| `bool` | one byte: `0` for false, `1` for true | +| `opt t` | `2` for none, or `3` followed by the encoding of `t` | +| `nat64` | eight bytes, big endian | +| `blob`, `text` | the length in bytes as a `nat64`, then the bytes | +| a header map | the number of entries as a `nat64`, then each name and value as `text`, sorted by name | +| asset content | the total length in bytes as a `nat64`, then the bytes | + +Each operation is encoded as a one-byte tag followed by its fields, in the order they are listed +in the [operation](#operations) it belongs to -- except `SetAssetContent`, whose `chunk_ids` and +`last_chunk` are not encoded as declared but as the assembled content described below, after the +`sha256` field: + +| Operation | Tag | +|----------------------|-----| +| `CreateAsset` | `4` | +| `SetAssetContent` | `5` | +| `UnsetAssetContent` | `6` | +| `DeleteAsset` | `7` | +| `Clear` | `8` | +| `SetAssetProperties` | `9` | + +`SetAssetContent` encodes the content of the asset after its declared `sha256`, as the total +length of the content followed by the content itself. The content is the chunks named by +`chunk_ids`, in order, followed by `last_chunk` -- exactly what `commit_batch` stores -- so how +the content was divided into chunks does not affect the evidence. + +Asset canisters reporting an [API version](#api-versions) lower than 3 use an earlier encoding +and compute a different value over the same batch. Tooling should compare evidence only against a +canister reporting version 3 or later. + ### Method: `commit_proposed_batch` This method executes the operations previously supplied by [propose_commit_batch()](#method-propose_commit_batch), and deletes the batch. @@ -557,6 +609,12 @@ These are set by the [configure()](#method-configure) method. All limits defaul This version added `SetAssetProperties` to `BatchOperationKind`. +### API Version 3 + +This version hashes the encoding described under +[compute_evidence](#the-hashed-encoding) for the evidence of a proposed batch and for the state +hash. Both values differ from the ones an earlier version computes over the same assets. + ## Permissions ### Permission: `Commit` diff --git a/e2e/tests-dfx/assetscanister.bash b/e2e/tests-dfx/assetscanister.bash index 08dc13b9b1..c2899a779e 100644 --- a/e2e/tests-dfx/assetscanister.bash +++ b/e2e/tests-dfx/assetscanister.bash @@ -37,6 +37,22 @@ delete_batch() { assert_command dfx canister call e2e_project_frontend delete_batch "(record { batch_id=$1; })" } +# Reads the evidence out of the output of `dfx deploy --by-proposal`. +evidence_from_proposal_output() { + echo "$1" | sed -n 's/.*with evidence \([0-9a-f]\{64\}\).*/\1/p' | head -1 +} + +# Renders a hex-encoded evidence value as a candid blob literal. +evidence_blob() { + local hex="$1" + local escaped="" + while [ -n "$hex" ]; do + escaped="$escaped\\${hex:0:2}" + hex="${hex:2}" + done + echo "blob \"$escaped\"" +} + check_permission_failure() { assert_contains "$1" "$output" } @@ -135,14 +151,17 @@ check_permission_failure() { dfx identity get-principal --identity prepare dfx canister call e2e_project_frontend list_permitted '(record { permission = variant { Commit }; })' assert_command dfx deploy e2e_project_frontend --by-proposal --identity prepare - assert_contains "Proposed commit of batch 2 with evidence 164fcc4d933ff9992ab6ab909a4bf350010fa0f4a3e1e247bfc679d3f45254e1. Either commit it by proposal, or delete it." "$output" + assert_match "Proposed commit of batch 2 with evidence [0-9a-f]{64}\. Either commit it by proposal, or delete it\." "$output" + EVIDENCE="$(evidence_from_proposal_output "$output")" assert_command_fail dfx deploy e2e_project_frontend --by-proposal --identity prepare assert_contains "Batch 2 is already proposed. Delete or execute it to propose another." "$output" + # The evidence dfx computes from the project must equal the evidence the asset canister computed + # over the batch it was given. Comparing the two is what makes the proposal reviewable. assert_command dfx deploy e2e_project_frontend --compute-evidence --identity anonymous # shellcheck disable=SC2154 - assert_eq "164fcc4d933ff9992ab6ab909a4bf350010fa0f4a3e1e247bfc679d3f45254e1" + assert_eq "$EVIDENCE" ID=$(dfx canister id e2e_project_frontend) PORT=$(get_webserver_port) @@ -162,9 +181,9 @@ check_permission_failure() { assert_command_fail dfx canister call e2e_project_frontend commit_proposed_batch "$wrong_commit_args" --identity commit assert_match "batch computed evidence .* does not match presented evidence" "$output" - commit_args='(record { batch_id = 2; evidence = blob "\16\4f\cc\4d\93\3f\f9\99\2a\b6\ab\90\9a\4b\f3\50\01\0f\a0\f4\a3\e1\e2\47\bf\c6\79\d3\f4\52\54\e1" } )' + commit_args="(record { batch_id = 2; evidence = $(evidence_blob "$EVIDENCE") } )" assert_command dfx canister call e2e_project_frontend validate_commit_proposed_batch "$commit_args" --identity commit - assert_contains "commit proposed batch 2 with evidence 164f" "$output" + assert_contains "commit proposed batch 2 with evidence $EVIDENCE" "$output" assert_command dfx canister call e2e_project_frontend commit_proposed_batch "$commit_args" --identity commit assert_eq "()" @@ -244,11 +263,12 @@ check_permission_failure() { dfx identity get-principal --identity prepare dfx canister call e2e_project_frontend list_permitted '(record { permission = variant { Commit }; })' assert_command dfx deploy e2e_project_frontend --by-proposal --identity prepare - assert_contains "Proposed commit of batch 2 with evidence 9b72eee7f0d7af2a9b41233c341b1caa0c905ef91405f5f513ffb58f68afee5b. Either commit it by proposal, or delete it." "$output" + assert_match "Proposed commit of batch 2 with evidence [0-9a-f]{64}\. Either commit it by proposal, or delete it\." "$output" + EVIDENCE="$(evidence_from_proposal_output "$output")" assert_command dfx deploy e2e_project_frontend --compute-evidence --identity anonymous # shellcheck disable=SC2154 - assert_eq "9b72eee7f0d7af2a9b41233c341b1caa0c905ef91405f5f513ffb58f68afee5b" + assert_eq "$EVIDENCE" ID=$(dfx canister id e2e_project_frontend) PORT=$(get_webserver_port) @@ -256,9 +276,9 @@ check_permission_failure() { assert_command_fail curl --fail -vv http://localhost:"$PORT"/sample-asset.txt?canisterId="$ID" assert_contains "The requested URL returned error: 404" "$output" - commit_args='(record { batch_id = 2; evidence = blob "\9b\72\ee\e7\f0\d7\af\2a\9b\41\23\3c\34\1b\1c\aa\0c\90\5e\f9\14\05\f5\f5\13\ff\b5\8f\68\af\ee\5b" } )' + commit_args="(record { batch_id = 2; evidence = $(evidence_blob "$EVIDENCE") } )" assert_command dfx canister call e2e_project_frontend validate_commit_proposed_batch "$commit_args" --identity commit - assert_contains "commit proposed batch 2 with evidence 9b72eee7f0d7af2a9b41233c341b1caa0c905ef91405f5f513ffb58f68afee5b" "$output" + assert_contains "commit proposed batch 2 with evidence $EVIDENCE" "$output" assert_command dfx canister call e2e_project_frontend commit_proposed_batch "$commit_args" --identity commit assert_eq "()" @@ -503,7 +523,8 @@ check_permission_failure() { # commit_proposed_batch - EVIDENCE_BLOB="blob \"\e3\b0\c4\42\98\fc\1c\14\9a\fb\f4\c8\99\6f\b9\24\27\ae\41\e4\64\9b\93\4c\a4\95\99\1b\78\52\b8\55\"" + # Evidence of a batch with no operations: `sha256(b"ic-certified-assets v2")`. + EVIDENCE_BLOB="$(evidence_blob 5cf0a08eeb8f1cc3758d410916f6ed888995f1e68e51d696e17bf931d302fd3b)" BATCH_ID="$(create_batch)" args="(record { batch_id=$BATCH_ID; operations=vec{} })" diff --git a/src/canisters/frontend/ic-asset/src/error/compute_evidence.rs b/src/canisters/frontend/ic-asset/src/error/compute_evidence.rs index d6137119f7..c90e4c1792 100644 --- a/src/canisters/frontend/ic-asset/src/error/compute_evidence.rs +++ b/src/canisters/frontend/ic-asset/src/error/compute_evidence.rs @@ -10,6 +10,21 @@ use super::AssembleCommitBatchArgumentError; /// Errors related to computing evidence for a proposed update. #[derive(Error, Debug)] pub enum ComputeEvidenceError { + /// Failed when querying the asset canister for its API version. + #[error("Failed to query asset canister API version")] + ApiVersionQueryFailed(#[source] AgentError), + + /// The asset canister computes evidence with an older encoding than this tool does. + #[error( + "The asset canister reports API version {canister_api_version}, but computing evidence to compare with it requires API version {required_api_version} or later. Upgrade the asset canister first." + )] + EvidenceApiVersionTooLow { + /// The API version the asset canister reports. + canister_api_version: u16, + /// The API version required to compute comparable evidence. + required_api_version: u16, + }, + /// Failed when assembling commit_batch argument. #[error(transparent)] AssembleCommitBatchArgumentFailed(#[from] AssembleCommitBatchArgumentError), diff --git a/src/canisters/frontend/ic-asset/src/error/prepare_sync_for_proposal.rs b/src/canisters/frontend/ic-asset/src/error/prepare_sync_for_proposal.rs index 8886cc4488..d6163b7f75 100644 --- a/src/canisters/frontend/ic-asset/src/error/prepare_sync_for_proposal.rs +++ b/src/canisters/frontend/ic-asset/src/error/prepare_sync_for_proposal.rs @@ -9,6 +9,17 @@ pub enum PrepareSyncForProposalError { #[error("Failed to query asset canister API version")] ApiVersionQueryFailed(#[source] AgentError), + /// The asset canister computes evidence with an older encoding than this tool does. + #[error( + "The asset canister reports API version {canister_api_version}, but proposing a batch requires API version {required_api_version} or later. Upgrade the asset canister first." + )] + EvidenceApiVersionTooLow { + /// The API version the asset canister reports. + canister_api_version: u16, + /// The API version required to compute comparable evidence. + required_api_version: u16, + }, + /// Failed while requesting that the asset canister compute evidence. #[error("Failed to compute evidence")] ComputeEvidence(#[source] AgentError), diff --git a/src/canisters/frontend/ic-asset/src/evidence/mod.rs b/src/canisters/frontend/ic-asset/src/evidence/mod.rs index 4ee815c964..2a38c6d5da 100644 --- a/src/canisters/frontend/ic-asset/src/evidence/mod.rs +++ b/src/canisters/frontend/ic-asset/src/evidence/mod.rs @@ -3,7 +3,8 @@ use crate::asset::content::Content; use crate::asset::content_encoder::ContentEncoder::{self, Brotli, Gzip}; use crate::batch_upload::operations::AssetDeletionReason::Obsolete; use crate::batch_upload::operations::assemble_batch_operations; -use crate::batch_upload::plumbing::{MAX_CHUNK_SIZE, ProjectAsset, make_project_assets}; +use crate::batch_upload::plumbing::{ProjectAsset, make_project_assets}; +use crate::canister_api::methods::api_version::api_version; use crate::canister_api::methods::asset_properties::get_assets_properties; use crate::canister_api::methods::list::list_assets; use crate::canister_api::types::asset::SetAssetPropertiesArguments; @@ -37,6 +38,28 @@ const TAG_DELETE_ASSET: [u8; 1] = [7]; const TAG_CLEAR: [u8; 1] = [8]; const TAG_SET_ASSET_PROPERTIES: [u8; 1] = [9]; +/// Domain separator and version of the encoding hashed for the evidence of a proposed batch and +/// for the state hash. Every variable-length field is length-prefixed and every operation +/// carries a tag, so each operation is self-delimiting and the encoding is injective over the +/// change a batch applies: its operations, with the content of each `SetAssetContent` taken as +/// one byte string. It is deliberately *not* injective over the batch arguments themselves -- +/// two batches that differ only in how they split that content across chunks encode identically. +/// +/// The suffix is `v2` because this is the second encoding of this data; the first carried no +/// separator and no version, so no digest anywhere has a `v1` separator. It is not the asset +/// canister API version, which is at 3 and moves independently. +/// +/// Must match `ENCODING_DOMAIN` in `ic-certified-assets`, as must the rest of the encoding: the +/// point of computing these hashes here is to compare them with the values the asset canister +/// computes. +const ENCODING_DOMAIN: &[u8] = b"ic-certified-assets v2"; + +/// The lowest asset canister API version that computes evidence with the encoding above. An +/// asset canister reporting less than this computes a different value over the same batch, so +/// there is nothing to compare against and we say so instead of returning a digest that will not +/// match. +pub(crate) const EVIDENCE_API_VERSION: u16 = 3; + /// Compute the hash ("evidence") over the batch operations required to update the assets pub async fn compute_evidence( canister: &Canister<'_>, @@ -44,6 +67,16 @@ pub async fn compute_evidence( logger: &Logger, progress: Option<&dyn AssetSyncProgressRenderer>, ) -> Result { + let canister_api_version = api_version(canister) + .await + .map_err(ComputeEvidenceError::ApiVersionQueryFailed)?; + if canister_api_version < EVIDENCE_API_VERSION { + return Err(ComputeEvidenceError::EvidenceApiVersionTooLow { + canister_api_version, + required_api_version: EVIDENCE_API_VERSION, + }); + } + let asset_descriptors = gather_asset_descriptors(dirs, logger)?; let canister_assets = list_assets(canister) @@ -84,6 +117,7 @@ pub async fn compute_evidence( trace!(logger, "{:#?}", operations); let mut sha = Sha256::new(); + sha.update(ENCODING_DOMAIN); for op in operations { hash_operation(&mut sha, &op, &project_assets)?; } @@ -102,6 +136,7 @@ pub fn compute_state_hash(dirs: &[&Path], logger: &Logger) -> Result MAX_CHUNK_SIZE { - for chunk in content_data.chunks(MAX_CHUNK_SIZE) { - hasher.update(chunk); - } - } else { - hasher.update(content_data); - } + // Length-prefixes the content, which makes the operation self-delimiting even though the + // asset canister hashes the content bytes chunk by chunk. + hash_len(hasher, content_data.len()); + + // The content is hashed as one byte string. The asset canister hashes it chunk by chunk, + // which comes to the same thing: sha256 is streaming, so how the bytes are divided up on the + // way in does not affect the digest. + hasher.update(content_data); } fn hash_unset_asset_content(hasher: &mut Sha256, args: &UnsetAssetContentArguments) { hasher.update(TAG_UNSET_ASSET_CONTENT); - hasher.update(&args.key); - hasher.update(&args.content_encoding); + hash_str(hasher, &args.key); + hash_str(hasher, &args.content_encoding); } fn hash_delete_asset(hasher: &mut Sha256, args: &DeleteAssetArguments) { hasher.update(TAG_DELETE_ASSET); - hasher.update(&args.key); + hash_str(hasher, &args.key); } fn hash_clear(hasher: &mut Sha256, _args: &ClearArguments) { hasher.update(TAG_CLEAR); } +/// Hashes the length of a repeated or variable-length field, so that the encoding of the field +/// cannot be confused with the encoding of a shorter or longer one. +fn hash_len(hasher: &mut Sha256, len: usize) { + hasher.update((len as u64).to_be_bytes()); +} + +/// Hashes a variable-length byte string, prefixed with its length, so that the boundaries of the +/// string are part of the encoding. +fn hash_bytes(hasher: &mut Sha256, bytes: &[u8]) { + hash_len(hasher, bytes.len()); + hasher.update(bytes); +} + +/// Hashes a variable-length text field, prefixed with its length. +fn hash_str(hasher: &mut Sha256, s: &str) { + hash_bytes(hasher, s.as_bytes()); +} + fn hash_opt_bool(hasher: &mut Sha256, b: Option) { if let Some(b) = b { hasher.update(TAG_SOME); @@ -275,7 +326,7 @@ fn hash_opt_bool(hasher: &mut Sha256, b: Option) { fn hash_opt_vec_u8(hasher: &mut Sha256, buf: Option<&Vec>) { if let Some(buf) = buf { hasher.update(TAG_SOME); - hasher.update(buf); + hash_bytes(hasher, buf); } else { hasher.update(TAG_NONE); } @@ -284,10 +335,11 @@ fn hash_opt_vec_u8(hasher: &mut Sha256, buf: Option<&Vec>) { fn hash_headers(hasher: &mut Sha256, headers: Option<&BTreeMap>) { if let Some(headers) = headers { hasher.update(TAG_SOME); + hash_len(hasher, headers.len()); for k in headers.keys() { let v = headers.get(k).unwrap(); - hasher.update(k); - hasher.update(v); + hash_str(hasher, k); + hash_str(hasher, v); } } else { hasher.update(TAG_NONE); @@ -296,7 +348,7 @@ fn hash_headers(hasher: &mut Sha256, headers: Option<&BTreeMap>) fn hash_set_asset_properties(hasher: &mut Sha256, args: &SetAssetPropertiesArguments) { hasher.update(TAG_SET_ASSET_PROPERTIES); - hasher.update(&args.key); + hash_str(hasher, &args.key); if let Some(max_age) = args.max_age { hasher.update(TAG_SOME); if let Some(max_age) = max_age { @@ -332,3 +384,95 @@ fn hash_set_asset_properties(hasher: &mut Sha256, args: &SetAssetPropertiesArgum hasher.update(TAG_NONE); } } + +#[cfg(test)] +mod tests { + use super::*; + + /// A fixed batch and the evidence the encoding above produces for it. + /// + /// `ic-certified-assets` has the same vector in + /// `tests::evidence_computation::evidence_of_known_batch`. The asset canister and this crate + /// have to hash a batch to the same value -- comparing the two is the whole point of computing + /// evidence here -- but the two implementations are separate, so each one pins the vector and + /// a change to either encoding that is not made to the other shows up as a failure here. + /// + /// The batch covers every operation. `SetAssetProperties` is the one that most needs it: it + /// is the only operation whose arguments differ in type between the two crates, since headers + /// arrive here as a `Vec<(String, String)>` and in the canister as a `BTreeMap`, and the + /// conversion between them is written by hand. The vector below lists those headers out of + /// order on purpose, so that the conversion has to sort them the way the canister does. + const KNOWN_BATCH_EVIDENCE: &str = + "5e8a8c1ccf35e60bfcc332d76c798d806c9a0d76ed3ac59e7c1ecb00c8b28089"; + + #[test] + fn evidence_of_known_batch() { + const CONTENT: &[u8] = b""; + + let mut hasher = Sha256::new(); + hasher.update(ENCODING_DOMAIN); + + hash_create_asset( + &mut hasher, + &CreateAssetArguments { + key: "/index.html".to_string(), + content_type: "text/html".to_string(), + max_age: Some(600), + headers: Some(BTreeMap::from([ + ("X-Frame-Options".to_string(), "DENY".to_string()), + ("X-XSS-Protection".to_string(), "1; mode=block".to_string()), + ])), + enable_aliasing: Some(true), + allow_raw_access: Some(false), + }, + ); + + let content_sha256: [u8; 32] = Sha256::digest(CONTENT).into(); + hash_set_asset_content_raw( + &mut hasher, + &SetAssetContentArguments { + key: "/index.html".to_string(), + content_encoding: "identity".to_string(), + chunk_ids: vec![], + last_chunk: None, + sha256: Some(content_sha256.to_vec()), + }, + CONTENT, + ); + + hash_unset_asset_content( + &mut hasher, + &UnsetAssetContentArguments { + key: "/index.html".to_string(), + content_encoding: "gzip".to_string(), + }, + ); + + // Exercises all three shapes of an `opt opt` field: set, explicitly cleared, and absent. + hash_set_asset_properties( + &mut hasher, + &SetAssetPropertiesArguments { + key: "/index.html".to_string(), + max_age: Some(Some(300)), + headers: Some(Some(vec![ + ("X-Frame-Options".to_string(), "DENY".to_string()), + ("Referrer-Policy".to_string(), "same-origin".to_string()), + ])), + allow_raw_access: Some(None), + is_aliased: None, + }, + ); + + hash_delete_asset( + &mut hasher, + &DeleteAssetArguments { + key: "/obsolete.txt".to_string(), + }, + ); + + hash_clear(&mut hasher, &ClearArguments {}); + + let evidence: [u8; 32] = hasher.finalize().into(); + assert_eq!(hex::encode(evidence), KNOWN_BATCH_EVIDENCE); + } +} diff --git a/src/canisters/frontend/ic-asset/src/sync.rs b/src/canisters/frontend/ic-asset/src/sync.rs index 76e5b0b95d..9c86914d0b 100644 --- a/src/canisters/frontend/ic-asset/src/sync.rs +++ b/src/canisters/frontend/ic-asset/src/sync.rs @@ -31,6 +31,7 @@ use crate::error::SyncError; use crate::error::SyncError::{ApiVersionQueryFailed, CommitBatchFailed}; use crate::error::UploadContentError; use crate::error::UploadContentError::{CreateBatchFailed, ListAssetsFailed}; +use crate::evidence::EVIDENCE_API_VERSION; use crate::progress::{AssetSyncProgressRenderer, AssetSyncState}; use candid::Nat; use ic_agent::AgentError; @@ -299,6 +300,12 @@ pub async fn prepare_sync_for_proposal( let canister_api_version = api_version(canister) .await .map_err(PrepareSyncForProposalError::ApiVersionQueryFailed)?; + if canister_api_version < EVIDENCE_API_VERSION { + return Err(PrepareSyncForProposalError::EvidenceApiVersionTooLow { + canister_api_version, + required_api_version: EVIDENCE_API_VERSION, + }); + } let arg = upload_content_and_assemble_sync_operations( canister, canister_api_version, diff --git a/src/canisters/frontend/ic-certified-assets/src/evidence.rs b/src/canisters/frontend/ic-certified-assets/src/evidence.rs index e6abf2278f..66c7c8d8d9 100644 --- a/src/canisters/frontend/ic-certified-assets/src/evidence.rs +++ b/src/canisters/frontend/ic-certified-assets/src/evidence.rs @@ -25,6 +25,27 @@ const TAG_DELETE_ASSET: [u8; 1] = [7]; const TAG_CLEAR: [u8; 1] = [8]; const TAG_SET_ASSET_PROPERTIES: [u8; 1] = [9]; +/// Domain separator and version of the encoding hashed by [`EvidenceComputation`], both for the +/// evidence of a proposed batch and for the state hash. +/// +/// Every variable-length field of the encoding is length-prefixed and every operation carries a +/// tag, so each operation is self-delimiting and the encoding is injective over the change a +/// batch applies: its operations, with the content of each `SetAssetContent` taken as one byte +/// string. It is deliberately *not* injective over `CommitBatchArguments` itself -- two batches +/// that differ only in how they split that content across chunks encode identically. +/// +/// The evidence and the state hash share this prefix on purpose: the state hash of an asset +/// canister equals the evidence of the batch that would build it from empty, which is what makes +/// the two comparable. +/// +/// The suffix is `v2` because this is the second encoding of this data: the first, which every +/// asset canister installed before API version 3 still computes, carried no separator and no +/// version at all. So there is no digest anywhere with a `v1` separator, and the number counts +/// encodings rather than separators. It is not the canister's [`crate::api_version`], which is +/// at 3 and moves independently. Bump this suffix whenever the encoding changes, so that a hash +/// computed under one version can never equal a hash computed under another. +const ENCODING_DOMAIN: &[u8] = b"ic-certified-assets v2"; + pub enum EvidenceComputation { NextOperation { operation_index: usize, @@ -80,7 +101,7 @@ impl EvidenceComputation { NextOperation { operation_index, hasher, - } => next_operation(args, operation_index, hasher), + } => next_operation(args, operation_index, hasher, chunks), NextChunkIndex { operation_index, chunk_index, @@ -114,7 +135,11 @@ fn next_operation( args: &CommitBatchArguments, operation_index: usize, mut hasher: Sha256, + chunks: &HashMap, ) -> EvidenceComputation { + if operation_index == 0 { + hasher.update(ENCODING_DOMAIN); + } match args.operations.get(operation_index) { None => { let sha256: [u8; 32] = hasher.finalize().into(); @@ -128,7 +153,7 @@ fn next_operation( } } Some(SetAssetContent(args)) => { - hash_set_asset_content(&mut hasher, args); + hash_set_asset_content(&mut hasher, args, set_asset_content_len(args, chunks)); NextChunkIndex { operation_index, chunk_index: 0, @@ -176,14 +201,15 @@ fn next_chunk_index( if let Some(SetAssetContent(sac)) = args.operations.get(operation_index) { if let Some(chunk_id) = sac.chunk_ids.get(chunk_index) { hash_chunk_by_id(&mut hasher, chunk_id, chunks); - if chunk_index + 1 < sac.chunk_ids.len() { - return NextChunkIndex { - operation_index, - chunk_index: chunk_index + 1, - hasher, - }; - } - } else if let Some(chunk_content) = sac.last_chunk.as_ref() { + return NextChunkIndex { + operation_index, + chunk_index: chunk_index + 1, + hasher, + }; + } + // `set_asset_content` appends `last_chunk` to the chunks named by `chunk_ids`, so the + // evidence covers it in the same position and regardless of how many chunk ids precede it. + if let Some(chunk_content) = sac.last_chunk.as_ref() { hash_chunk_by_content(&mut hasher, chunk_content); } } @@ -193,6 +219,22 @@ fn next_chunk_index( } } +/// The number of content bytes that the evidence covers for a `SetAssetContent` operation, which +/// is exactly the content `set_asset_content` assembles from the same arguments. Chunk ids that +/// are not present are skipped here and by [`hash_chunk_by_id`] alike. +fn set_asset_content_len( + args: &SetAssetContentArguments, + chunks: &HashMap, +) -> usize { + let from_chunk_ids: usize = args + .chunk_ids + .iter() + .filter_map(|chunk_id| chunks.get(chunk_id)) + .map(|chunk| chunk.content.len()) + .sum(); + from_chunk_ids + args.last_chunk.as_ref().map_or(0, |chunk| chunk.len()) +} + fn hash_chunk_by_id(hasher: &mut Sha256, chunk_id: &ChunkId, chunks: &HashMap) { if let Some(chunk) = chunks.get(chunk_id) { hasher.update(&chunk.content); @@ -205,8 +247,8 @@ fn hash_chunk_by_content(hasher: &mut Sha256, chunk_content: &[u8]) { fn hash_create_asset(hasher: &mut Sha256, args: &CreateAssetArguments) { hasher.update(TAG_CREATE_ASSET); - hasher.update(&args.key); - hasher.update(&args.content_type); + hash_str(hasher, &args.key); + hash_str(hasher, &args.content_type); if let Some(max_age) = args.max_age { hasher.update(TAG_SOME); hasher.update(max_age.to_be_bytes()); @@ -214,26 +256,35 @@ fn hash_create_asset(hasher: &mut Sha256, args: &CreateAssetArguments) { hasher.update(TAG_NONE); } hash_headers(hasher, args.headers.as_ref()); - hash_opt_bool(hasher, args.allow_raw_access); hash_opt_bool(hasher, args.enable_aliasing); + hash_opt_bool(hasher, args.allow_raw_access); } -fn hash_set_asset_content(hasher: &mut Sha256, args: &SetAssetContentArguments) { +/// `content_len` is the total number of content bytes hashed after this call, by +/// [`hash_chunk_by_id`] and [`hash_chunk_by_content`]. Hashing it here length-prefixes the +/// content, which makes the operation self-delimiting even though the content bytes themselves +/// are hashed in chunks. +fn hash_set_asset_content( + hasher: &mut Sha256, + args: &SetAssetContentArguments, + content_len: usize, +) { hasher.update(TAG_SET_ASSET_CONTENT); - hasher.update(&args.key); - hasher.update(&args.content_encoding); + hash_str(hasher, &args.key); + hash_str(hasher, &args.content_encoding); hash_opt_bytebuf(hasher, args.sha256.as_ref()); + hash_len(hasher, content_len); } fn hash_unset_asset_content(hasher: &mut Sha256, args: &UnsetAssetContentArguments) { hasher.update(TAG_UNSET_ASSET_CONTENT); - hasher.update(&args.key); - hasher.update(&args.content_encoding); + hash_str(hasher, &args.key); + hash_str(hasher, &args.content_encoding); } fn hash_delete_asset(hasher: &mut Sha256, args: &DeleteAssetArguments) { hasher.update(TAG_DELETE_ASSET); - hasher.update(&args.key); + hash_str(hasher, &args.key); } fn hash_clear(hasher: &mut Sha256, _args: &ClearArguments) { @@ -242,7 +293,7 @@ fn hash_clear(hasher: &mut Sha256, _args: &ClearArguments) { fn hash_set_asset_properties(hasher: &mut Sha256, args: &SetAssetPropertiesArguments) { hasher.update(TAG_SET_ASSET_PROPERTIES); - hasher.update(&args.key); + hash_str(hasher, &args.key); if let Some(max_age) = args.max_age { hasher.update(TAG_SOME); if let Some(max_age) = max_age { @@ -274,6 +325,24 @@ fn hash_set_asset_properties(hasher: &mut Sha256, args: &SetAssetPropertiesArgum } } +/// Hashes the length of a repeated or variable-length field, so that the encoding of the field +/// cannot be confused with the encoding of a shorter or longer one. +fn hash_len(hasher: &mut Sha256, len: usize) { + hasher.update((len as u64).to_be_bytes()); +} + +/// Hashes a variable-length byte string, prefixed with its length, so that the boundaries of the +/// string are part of the encoding. +fn hash_bytes(hasher: &mut Sha256, bytes: &[u8]) { + hash_len(hasher, bytes.len()); + hasher.update(bytes); +} + +/// Hashes a variable-length text field, prefixed with its length. +fn hash_str(hasher: &mut Sha256, s: &str) { + hash_bytes(hasher, s.as_bytes()); +} + fn hash_opt_bool(hasher: &mut Sha256, b: Option) { if let Some(b) = b { hasher.update(TAG_SOME); @@ -286,7 +355,7 @@ fn hash_opt_bool(hasher: &mut Sha256, b: Option) { fn hash_opt_bytebuf(hasher: &mut Sha256, buf: Option<&ByteBuf>) { if let Some(buf) = buf { hasher.update(TAG_SOME); - hasher.update(buf); + hash_bytes(hasher, buf); } else { hasher.update(TAG_NONE); } @@ -295,10 +364,11 @@ fn hash_opt_bytebuf(hasher: &mut Sha256, buf: Option<&ByteBuf>) { fn hash_headers(hasher: &mut Sha256, headers: Option<&BTreeMap>) { if let Some(headers) = headers { hasher.update(TAG_SOME); + hash_len(hasher, headers.len()); for k in headers.keys().sorted() { let v = headers.get(k).unwrap(); - hasher.update(k); - hasher.update(v); + hash_str(hasher, k); + hash_str(hasher, v); } } else { hasher.update(TAG_NONE); @@ -332,6 +402,10 @@ fn next_virtual_step( virtual_state: VirtualState, mut hasher: Sha256, ) -> EvidenceComputation { + if current_key_index == 0 && matches!(virtual_state, VirtualState::CreateAsset) { + hasher.update(ENCODING_DOMAIN); + } + if current_key_index >= sorted_keys.len() { let sha256: [u8; 32] = hasher.finalize().into(); return EvidenceComputation::Computed(ByteBuf::from(sha256)); @@ -388,7 +462,12 @@ fn next_virtual_step( last_chunk: None, sha256: Some(ByteBuf::from(enc.sha256)), }; - hash_set_asset_content(&mut hasher, &args); + let content_len = enc + .content_chunks + .iter() + .map(|chunk| chunk.len()) + .sum::(); + hash_set_asset_content(&mut hasher, &args, content_len); EvidenceComputation::Virtual { sorted_keys, diff --git a/src/canisters/frontend/ic-certified-assets/src/lib.rs b/src/canisters/frontend/ic-certified-assets/src/lib.rs index 57d073265b..5e8d5d125e 100644 --- a/src/canisters/frontend/ic-certified-assets/src/lib.rs +++ b/src/canisters/frontend/ic-certified-assets/src/lib.rs @@ -42,7 +42,7 @@ thread_local! { } pub fn api_version() -> u16 { - 2 + 3 } pub fn authorize(other: Principal) { diff --git a/src/canisters/frontend/ic-certified-assets/src/tests.rs b/src/canisters/frontend/ic-certified-assets/src/tests.rs index d439e785d0..5b899266c1 100644 --- a/src/canisters/frontend/ic-certified-assets/src/tests.rs +++ b/src/canisters/frontend/ic-certified-assets/src/tests.rs @@ -2546,6 +2546,532 @@ mod evidence_computation { use crate::types::BatchOperation::SetAssetContent; use crate::types::{ClearArguments, ComputeEvidenceArguments, UnsetAssetContentArguments}; + /// The seven headers `dfx` applies to an asset under its standard security policy, which is + /// the header map a reviewer of a frontend proposal is most likely to be checking. + fn standard_security_headers() -> BTreeMap { + BTreeMap::from([ + ( + "Content-Security-Policy".to_string(), + "default-src 'self';script-src 'self'".to_string(), + ), + ( + "Permissions-Policy".to_string(), + "geolocation=()".to_string(), + ), + ("Referrer-Policy".to_string(), "same-origin".to_string()), + ( + "Strict-Transport-Security".to_string(), + "max-age=31536000; includeSubDomains".to_string(), + ), + ("X-Content-Type-Options".to_string(), "nosniff".to_string()), + ("X-Frame-Options".to_string(), "DENY".to_string()), + ("X-XSS-Protection".to_string(), "1; mode=block".to_string()), + ]) + } + + /// The same header bytes as [`standard_security_headers`], concatenated in the same order but + /// divided into name and value at one different position, leaving a single header that carries + /// all of them and none of the seven names. + fn repartitioned_security_headers() -> BTreeMap { + let headers = standard_security_headers(); + let mut concatenated = String::new(); + for (name, value) in headers.iter() { + concatenated.push_str(name); + concatenated.push_str(value); + } + let split_at = "Content-Security-Policydefault-src".len(); + let (name, value) = concatenated.split_at(split_at); + assert!(!headers.contains_key(name)); + BTreeMap::from([(name.to_string(), value.to_string())]) + } + + /// Proposes `operations` as a batch of its own and returns the evidence computed over it. + fn evidence_of( + state: &mut State, + system_context: &SystemContext, + operations: Vec, + ) -> ByteBuf { + let batch_id = state.create_batch(system_context).unwrap(); + state + .propose_commit_batch(CommitBatchArguments { + batch_id: batch_id.clone(), + operations, + }) + .unwrap(); + let evidence = run_computation_until_completion(|_progress| { + state.compute_evidence(&ComputeEvidenceArguments { + batch_id: batch_id.clone(), + max_iterations: None, + }) + }) + .unwrap(); + delete_batch(state, batch_id); + evidence + } + + /// Proposes a single `SetAssetContent` operation carrying `content` as one chunk and returns + /// the evidence computed over it. + fn evidence_of_content( + state: &mut State, + system_context: &SystemContext, + key: &str, + content_encoding: &str, + content: &[u8], + ) -> ByteBuf { + let batch_id = state.create_batch(system_context).unwrap(); + let chunk_id = state + .create_chunk( + CreateChunkArg { + batch_id: batch_id.clone(), + content: ByteBuf::from(content.to_vec()), + }, + system_context, + ) + .unwrap(); + state + .propose_commit_batch(CommitBatchArguments { + batch_id: batch_id.clone(), + operations: vec![SetAssetContent(SetAssetContentArguments { + key: key.to_string(), + content_encoding: content_encoding.to_string(), + chunk_ids: vec![chunk_id], + last_chunk: None, + sha256: None, + })], + }) + .unwrap(); + let evidence = run_computation_until_completion(|_progress| { + state.compute_evidence(&ComputeEvidenceArguments { + batch_id: batch_id.clone(), + max_iterations: None, + }) + }) + .unwrap(); + delete_batch(state, batch_id); + evidence + } + + #[test] + fn header_map_partition_affects_evidence() { + let mut state = State::default(); + let system_context = mock_system_context(); + + let create_asset = |headers: BTreeMap| { + vec![BatchOperation::CreateAsset(CreateAssetArguments { + key: "/index.html".to_string(), + content_type: "text/html".to_string(), + max_age: None, + headers: Some(headers), + enable_aliasing: None, + allow_raw_access: None, + })] + }; + assert_ne!( + evidence_of( + &mut state, + &system_context, + create_asset(standard_security_headers()) + ), + evidence_of( + &mut state, + &system_context, + create_asset(repartitioned_security_headers()) + ), + ); + + let set_asset_properties = |headers: BTreeMap| { + vec![BatchOperation::SetAssetProperties( + SetAssetPropertiesArguments { + key: "/index.html".to_string(), + max_age: None, + headers: Some(Some(headers)), + allow_raw_access: None, + is_aliased: None, + }, + )] + }; + assert_ne!( + evidence_of( + &mut state, + &system_context, + set_asset_properties(standard_security_headers()) + ), + evidence_of( + &mut state, + &system_context, + set_asset_properties(repartitioned_security_headers()) + ), + ); + } + + #[test] + fn header_count_affects_evidence() { + let mut state = State::default(); + let system_context = mock_system_context(); + + let create_asset = |headers: BTreeMap| { + vec![BatchOperation::CreateAsset(CreateAssetArguments { + key: "/index.html".to_string(), + content_type: "text/html".to_string(), + max_age: None, + headers: Some(headers), + enable_aliasing: None, + allow_raw_access: None, + })] + }; + // Both maps concatenate to the same bytes, in the same order, so only the number of + // entries and where each one ends tells them apart. + assert_ne!( + evidence_of( + &mut state, + &system_context, + create_asset(BTreeMap::from([("a".to_string(), "bc".to_string())])) + ), + evidence_of( + &mut state, + &system_context, + create_asset(BTreeMap::from([ + ("a".to_string(), "b".to_string()), + ("c".to_string(), "".to_string()), + ])) + ), + ); + } + + #[test] + fn key_content_type_boundary_affects_evidence() { + let mut state = State::default(); + let system_context = mock_system_context(); + + let create_asset = |key: &str, content_type: &str| { + vec![BatchOperation::CreateAsset(CreateAssetArguments { + key: key.to_string(), + content_type: content_type.to_string(), + max_age: None, + headers: None, + enable_aliasing: None, + allow_raw_access: None, + })] + }; + assert_ne!( + evidence_of( + &mut state, + &system_context, + create_asset("/index.html", "text/html") + ), + evidence_of( + &mut state, + &system_context, + create_asset("/index.htmltext/", "html") + ), + ); + } + + #[test] + fn key_content_encoding_boundary_affects_evidence() { + let mut state = State::default(); + let system_context = mock_system_context(); + + let unset_asset_content = |key: &str, content_encoding: &str| { + vec![BatchOperation::UnsetAssetContent( + UnsetAssetContentArguments { + key: key.to_string(), + content_encoding: content_encoding.to_string(), + }, + )] + }; + assert_ne!( + evidence_of( + &mut state, + &system_context, + unset_asset_content("/index.html", "identity") + ), + evidence_of( + &mut state, + &system_context, + unset_asset_content("/index.htmliden", "tity") + ), + ); + } + + /// A key ends its operation, so the evidence has to distinguish a batch from one whose first + /// key runs on into a rendering of the operations that follow it. + #[test] + fn key_cannot_absorb_following_operations() { + let mut state = State::default(); + let system_context = mock_system_context(); + + let honest = vec![ + BatchOperation::DeleteAsset(DeleteAssetArguments { + key: "/leaked.txt".to_string(), + }), + BatchOperation::SetAssetProperties(SetAssetPropertiesArguments { + key: "/index.html".to_string(), + max_age: None, + headers: None, + allow_raw_access: None, + is_aliased: None, + }), + ]; + // `TAG_SET_ASSET_PROPERTIES`, the key of the second operation, and the four `TAG_NONE` + // bytes of its remaining fields, all of which are valid UTF-8 and so can appear in a key. + let collapsed_key = format!( + "/leaked.txt{}{}{}", + "\u{9}", "/index.html", "\u{2}\u{2}\u{2}\u{2}" + ); + let collapsed = vec![BatchOperation::DeleteAsset(DeleteAssetArguments { + key: collapsed_key, + })]; + + assert_ne!( + evidence_of(&mut state, &system_context, honest), + evidence_of(&mut state, &system_context, collapsed), + ); + } + + /// Content ends its operation too, with the same requirement as a trailing key. + #[test] + fn content_cannot_absorb_following_operations() { + let mut state = State::default(); + let system_context = mock_system_context(); + + let batch_id = state.create_batch(&system_context).unwrap(); + let chunk_id = state + .create_chunk( + CreateChunkArg { + batch_id: batch_id.clone(), + content: ByteBuf::from(b"asset content".to_vec()), + }, + &system_context, + ) + .unwrap(); + state + .propose_commit_batch(CommitBatchArguments { + batch_id: batch_id.clone(), + operations: vec![ + SetAssetContent(SetAssetContentArguments { + key: "/main.js".to_string(), + content_encoding: "identity".to_string(), + chunk_ids: vec![chunk_id], + last_chunk: None, + sha256: None, + }), + BatchOperation::DeleteAsset(DeleteAssetArguments { + key: "/leaked.txt".to_string(), + }), + ], + }) + .unwrap(); + let honest = run_computation_until_completion(|_progress| { + state.compute_evidence(&ComputeEvidenceArguments { + batch_id: batch_id.clone(), + max_iterations: None, + }) + }) + .unwrap(); + delete_batch(&mut state, batch_id); + + // The same content, run on into `TAG_DELETE_ASSET` and the key of the operation that + // followed it, with that operation dropped. + let collapsed = evidence_of_content( + &mut state, + &system_context, + "/main.js", + "identity", + b"asset content\x07/leaked.txt", + ); + + assert_ne!(honest, collapsed); + } + + /// Content split differently over the same chunks is the same content, so it has to produce + /// the same evidence -- the length prefix covers the whole content, not each chunk. + #[test] + fn chunk_boundaries_do_not_affect_evidence() { + let mut state = State::default(); + let system_context = mock_system_context(); + + let one_chunk = evidence_of_content( + &mut state, + &system_context, + "/main.js", + "identity", + b"asset content", + ); + + let batch_id = state.create_batch(&system_context).unwrap(); + let mut chunk_ids = vec![]; + for content in [&b"asset "[..], &b"content"[..]] { + chunk_ids.push( + state + .create_chunk( + CreateChunkArg { + batch_id: batch_id.clone(), + content: ByteBuf::from(content.to_vec()), + }, + &system_context, + ) + .unwrap(), + ); + } + state + .propose_commit_batch(CommitBatchArguments { + batch_id: batch_id.clone(), + operations: vec![SetAssetContent(SetAssetContentArguments { + key: "/main.js".to_string(), + content_encoding: "identity".to_string(), + chunk_ids, + last_chunk: None, + sha256: None, + })], + }) + .unwrap(); + let two_chunks = run_computation_until_completion(|_progress| { + state.compute_evidence(&ComputeEvidenceArguments { + batch_id: batch_id.clone(), + max_iterations: None, + }) + }) + .unwrap(); + delete_batch(&mut state, batch_id); + + assert_eq!(one_chunk, two_chunks); + } + + /// A fixed batch and the evidence the encoding produces for it. + /// + /// `ic-asset` has the same vector in `evidence::tests::evidence_of_known_batch`. This crate + /// and `ic-asset` have to hash a batch to the same value -- comparing the two is the whole + /// point of computing evidence -- but the two implementations are separate, so each one pins + /// the vector and a change to either encoding that is not made to the other shows up as a + /// failure here. The batch covers every operation. + #[test] + fn evidence_of_known_batch() { + const CONTENT: &[u8] = b""; + const KNOWN_BATCH_EVIDENCE: &str = + "5e8a8c1ccf35e60bfcc332d76c798d806c9a0d76ed3ac59e7c1ecb00c8b28089"; + + let mut state = State::default(); + let system_context = mock_system_context(); + + let batch_id = state.create_batch(&system_context).unwrap(); + let chunk_id = state + .create_chunk( + CreateChunkArg { + batch_id: batch_id.clone(), + content: ByteBuf::from(CONTENT.to_vec()), + }, + &system_context, + ) + .unwrap(); + + let content_sha256: [u8; 32] = sha2::Sha256::digest(CONTENT).into(); + state + .propose_commit_batch(CommitBatchArguments { + batch_id: batch_id.clone(), + operations: vec![ + BatchOperation::CreateAsset(CreateAssetArguments { + key: "/index.html".to_string(), + content_type: "text/html".to_string(), + max_age: Some(600), + headers: Some(BTreeMap::from([ + ("X-Frame-Options".to_string(), "DENY".to_string()), + ("X-XSS-Protection".to_string(), "1; mode=block".to_string()), + ])), + enable_aliasing: Some(true), + allow_raw_access: Some(false), + }), + SetAssetContent(SetAssetContentArguments { + key: "/index.html".to_string(), + content_encoding: "identity".to_string(), + chunk_ids: vec![chunk_id], + last_chunk: None, + sha256: Some(ByteBuf::from(content_sha256)), + }), + BatchOperation::UnsetAssetContent(UnsetAssetContentArguments { + key: "/index.html".to_string(), + content_encoding: "gzip".to_string(), + }), + // Exercises all three shapes of an `opt opt` field: set, explicitly cleared, + // and absent. `ic-asset` reaches this operation through a different argument + // type and a hand-written conversion, so it is the one most worth pinning. + BatchOperation::SetAssetProperties(SetAssetPropertiesArguments { + key: "/index.html".to_string(), + max_age: Some(Some(300)), + headers: Some(Some(BTreeMap::from([ + ("X-Frame-Options".to_string(), "DENY".to_string()), + ("Referrer-Policy".to_string(), "same-origin".to_string()), + ]))), + allow_raw_access: Some(None), + is_aliased: None, + }), + BatchOperation::DeleteAsset(DeleteAssetArguments { + key: "/obsolete.txt".to_string(), + }), + BatchOperation::Clear(ClearArguments {}), + ], + }) + .unwrap(); + + let evidence = run_computation_until_completion(|_progress| { + state.compute_evidence(&ComputeEvidenceArguments { + batch_id: batch_id.clone(), + max_iterations: None, + }) + }) + .unwrap(); + + assert_eq!(hex::encode(evidence.as_slice()), KNOWN_BATCH_EVIDENCE); + } + + /// `set_asset_content` stores the chunks named by `chunk_ids` followed by `last_chunk`, so the + /// evidence has to cover `last_chunk` even when `chunk_ids` is not empty. + #[test] + fn last_chunk_affects_evidence_alongside_chunk_ids() { + let mut state = State::default(); + let system_context = mock_system_context(); + + let evidence_with_last_chunk = |state: &mut State, last_chunk: &[u8]| { + let batch_id = state.create_batch(&system_context).unwrap(); + let chunk_id = state + .create_chunk( + CreateChunkArg { + batch_id: batch_id.clone(), + content: ByteBuf::from(b"asset content".to_vec()), + }, + &system_context, + ) + .unwrap(); + state + .propose_commit_batch(CommitBatchArguments { + batch_id: batch_id.clone(), + operations: vec![SetAssetContent(SetAssetContentArguments { + key: "/main.js".to_string(), + content_encoding: "identity".to_string(), + chunk_ids: vec![chunk_id], + last_chunk: Some(ByteBuf::from(last_chunk.to_vec())), + sha256: None, + })], + }) + .unwrap(); + let evidence = run_computation_until_completion(|_progress| { + state.compute_evidence(&ComputeEvidenceArguments { + batch_id: batch_id.clone(), + max_iterations: None, + }) + }) + .unwrap(); + delete_batch(state, batch_id); + evidence + }; + + assert_eq!(b"const x = 1; // aaa".len(), b"const x = 2; // bbb".len()); + assert_ne!( + evidence_with_last_chunk(&mut state, b"const x = 1; // aaa"), + evidence_with_last_chunk(&mut state, b"const x = 2; // bbb"), + ); + } + #[test] fn evidence_with_set_single_chunk_asset_content() { let mut state = State::default(); @@ -4235,6 +4761,8 @@ mod validate_commit_proposed_batch { unreachable!() }; + // The evidence of a batch with no operations is the hash of the encoding's domain + // separator alone: `sha256(b"ic-certified-assets v2")`. assert_eq!( state .validate_commit_proposed_batch(CommitProposedBatchArguments { @@ -4242,7 +4770,7 @@ mod validate_commit_proposed_batch { evidence: evidence.clone(), },) .unwrap(), - "commit proposed batch 0 with evidence e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855" + "commit proposed batch 0 with evidence 5cf0a08eeb8f1cc3758d410916f6ed888995f1e68e51d696e17bf931d302fd3b" ); run_computation_until_completion(|progress| { diff --git a/src/distributed/assetstorage.wasm.gz b/src/distributed/assetstorage.wasm.gz index 9a5396f125..7426af496b 100755 Binary files a/src/distributed/assetstorage.wasm.gz and b/src/distributed/assetstorage.wasm.gz differ