fix(upgrader): include module delivery in DR recovery footprint - #647
Open
MRmarioruci wants to merge 2 commits into
Open
fix(upgrader): include module delivery in DR recovery footprint#647MRmarioruci wants to merge 2 commits into
MRmarioruci wants to merge 2 commits into
Conversation
Disaster-recovery committee agreement is matched on a footprint of each InstallCode request. The footprint was (install_mode, wasm_sha256, arg_sha256) and omitted the module delivery, and for a chunked install the wasm_sha256 is the caller-declared hash rather than a recomputed one. Two requests could therefore share a footprint while fetching the module from completely different sources. Because evaluate_requests executes the specific request that crosses quorum, a single committee member could submit a chunked request that copies an honest member's declared hash but points at a store canister of their choosing, match the honest footprint, and have their own request execute. Include wasm_module_extra_chunks (store canister, key, declared hash) in the footprint so requests only count together when they install the same module from the same source. Honest members agreeing on the same chunked delivery still reach quorum; a mismatched delivery no longer collides. The management canister continues to hash-verify the assembled module at install time, so a mismatched delivery fails rather than installing attacker-chosen code. The footprint is a transient in-memory type rebuilt on each evaluation, so this is not a stable-memory change and needs no migration.
|
✅ No security or compliance issues detected. Reviewed everything up to fff59aa. Security Overview
Detected Code Changes
|
Contributor
There was a problem hiding this comment.
Pull request overview
This PR hardens the disaster-recovery committee quorum logic by expanding the InstallCode request “footprint” to include the module delivery details for chunked installs, preventing a committee member from hijacking quorum by submitting a request with the same declared hash/args but a different chunk store source.
Changes:
- Extend
StationRecoveryRequestInstallCodeOperationFootprintto includewasm_module_extra_chunks(delivery identity). - Populate the new footprint field in the
From<&StationRecoveryRequestOperation>mapper. - Add a regression test proving mismatched delivery no longer collides, while matching delivery still reaches quorum.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| core/upgrader/impl/src/model/disaster_recovery.rs | Adds wasm_module_extra_chunks to the in-memory InstallCode footprint so quorum matching includes delivery identity. |
| core/upgrader/impl/src/mappers/disaster_recovery.rs | Ensures the footprint mapping includes the new delivery field when computing request agreement keys. |
| core/upgrader/impl/src/services/disaster_recovery.rs | Adds a unit test covering the quorum-hijack scenario and the expected behavior for matching chunked deliveries. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Change
Disaster-recovery committee agreement matches requests on a footprint of the requested operation. For
InstallCodethat footprint was(install_mode, wasm_sha256, arg_sha256), which omittedwasm_module_extra_chunks. Two requests could therefore match on footprint while specifying different module delivery, and for a chunked install the hash in the footprint is the caller-declared one rather than a recomputed value.The footprint now includes the module delivery, so agreement covers how the module is obtained and not only its declared hash.
Tests
install_code_footprint_includes_module_delivery— requests differing only in module delivery no longer share a footprint.