Skip to content

add block hashes to the randomness used by hashmaps and friends in validation context#9127

Merged
alexggh merged 27 commits intomasterfrom
alexggh/add_randomness
Jul 11, 2025
Merged

add block hashes to the randomness used by hashmaps and friends in validation context#9127
alexggh merged 27 commits intomasterfrom
alexggh/add_randomness

Conversation

@alexggh
Copy link
Copy Markdown
Contributor

@alexggh alexggh commented Jul 8, 2025

#8606 paritytech/trie#221 replaced the usage of BTreeMap with HashMaps in validation context. The keys are already derived with a cryptographic hash function from user data, so users should not be able to manipulate it.

To be on safe side this PR also modifies the TrieCache, TrieRecorder and MemoryDB to use a hasher that on top of the default generated randomness also adds randomness generated from the hash of the relaychain and that of the parachain blocks, which is not something users can control or guess ahead of time.

Signed-off-by: Alexandru Gheorghe <alexandru.gheorghe@parity.io>
@paritytech-review-bot paritytech-review-bot bot requested a review from a team July 8, 2025 07:19
@alexggh alexggh requested review from bkchr and skunert and removed request for a team July 8, 2025 07:19
@paritytech-review-bot paritytech-review-bot bot requested a review from a team July 8, 2025 07:20
Cargo.toml Outdated
gum-proc-macro = { path = "polkadot/node/gum/proc-macro", default-features = false, package = "tracing-gum-proc-macro" }
handlebars = { version = "5.1.0" }
hash-db = { version = "0.16.0", default-features = false }
hash-db = { git = "https://github.com/paritytech/trie.git", branch = "alexggh/allow_usage_of_custom_hasher", default-features = false }
Copy link
Copy Markdown
Contributor Author

@alexggh alexggh Jul 8, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will be removed once: paritytech/trie#224, gets merged and a crate release is done.

@alexggh alexggh changed the title add block hashes to the randomness used by hashmaps and friends add block hashes to the randomness used by hashmaps and friends in validation context Jul 8, 2025
alexggh added 2 commits July 8, 2025 10:26
Signed-off-by: Alexandru Gheorghe <alexandru.gheorghe@parity.io>
Signed-off-by: Alexandru Gheorghe <alexandru.gheorghe@parity.io>
Signed-off-by: Alexandru Gheorghe <alexandru.gheorghe@parity.io>
alexggh added 3 commits July 8, 2025 16:22
Signed-off-by: Alexandru Gheorghe <alexandru.gheorghe@parity.io>
Signed-off-by: Alexandru Gheorghe <alexandru.gheorghe@parity.io>
Signed-off-by: Alexandru Gheorghe <alexandru.gheorghe@parity.io>
@alexggh alexggh requested a review from a team as a code owner July 8, 2025 14:50
alexggh and others added 3 commits July 8, 2025 17:51
Signed-off-by: Alexandru Gheorghe <alexandru.gheorghe@parity.io>
Signed-off-by: Alexandru Gheorghe <alexandru.gheorghe@parity.io>
@alexggh alexggh removed request for a team July 8, 2025 15:01
alexggh and others added 2 commits July 8, 2025 18:10
Signed-off-by: Alexandru Gheorghe <alexandru.gheorghe@parity.io>
@burdges
Copy link
Copy Markdown
Contributor

burdges commented Jul 9, 2025

I first suggested this in #8606 (comment) but I honestly still have no idea how secure this is. Also how badly this breaks depends upon what computation happens, aka this seems not contract safe.

The simple attack is: 1st, do a bunch in a hashmap operations in a block. 2nd, search block hashes until you find one having considerable probing. At some point, I started looking up how the hash map handles landing early in the probe sequence, ala if probes ever trigger reallocation, but never pinned that down enough to figure out the attack difficultly. Anyways if you have a tight hash map and can land early in the probe sequence, then you can produce many probes on each read, which gives an attack.

I've no idea if there are worse attacks that exploit the weakness of hash brown vs say sip hasher, but that where not beiong. contract safe really emerges. Afaik nobody has ever proven really nice properties of sip hasher beyond hiding the key, so not sure what'd be better.

As for optimizing this, you could do hash(blockhash * hash-to-curve(blockhash)) for this randomness, so that every attack attempt required 25 us. It'd do nothing against parallellism though, so not worth the effort without the ECC host calls.

Anyways..

If we must BTreeMap then we could first take this appraoch because its easy, and not that bad, but also make an RFP for my other PerfectMap proposal, and hope that happens before anyone exploits this for griefing us.

PerfectMap amounts to the collator "proving" that it picked a "nearly perfect" seed. It just hard caps the probing like a cuckoo table does, but panic instead of rebuilding. Avoiding rebuilding should improve write performance. It's the hash map analog of the collator doing an O(n log n) sort on then verifying the sort in O(n) in the block.

…tion.rs

Co-authored-by: Bastian Köcher <git@kchr.de>
@alexggh
Copy link
Copy Markdown
Contributor Author

alexggh commented Jul 9, 2025

The simple attack is: 1st, do a bunch in a hashmap operations in a block. 2nd, search block hashes until you find one having considerable probing. At some point, I started looking up how the hash map handles landing early in the probe sequence, ala if probes ever trigger reallocation, but never pinned that down enough to figure out the attack difficultly. Anyways if you have a tight hash map and can land early in the probe sequence, then you can produce many probes on each read, which gives an attack.

Not sure, I understand this, the extra seed is derived from the relay chain parent blockhash and the hash of the current parachain block(which is also impacted by the transactions in it), so this is not something the user can control apriori or manipulate and it changes every block.

Signed-off-by: Alexandru Gheorghe <alexandru.gheorghe@parity.io>
@alexggh alexggh added the A4-backport-unstable2507 Pull request must be backported to the unstable2507 release branch label Jul 11, 2025
@alexggh alexggh added this pull request to the merge queue Jul 11, 2025
Merged via the queue into master with commit 7058819 Jul 11, 2025
343 of 354 checks passed
@alexggh alexggh deleted the alexggh/add_randomness branch July 11, 2025 16:20
paritytech-release-backport-bot bot pushed a commit that referenced this pull request Jul 11, 2025
…lidation context (#9127)

#8606
paritytech/trie#221 replaced the usage of
BTreeMap with HashMaps in validation context. The keys are already
derived with a cryptographic hash function from user data, so users
should not be able to manipulate it.

To be on safe side this PR also modifies the TrieCache, TrieRecorder and
MemoryDB to use a hasher that on top of the default generated randomness
also adds randomness generated from the hash of the relaychain and that
of the parachain blocks, which is not something users can control or
guess ahead of time.

---------

Signed-off-by: Alexandru Gheorghe <alexandru.gheorghe@parity.io>
Co-authored-by: Bastian Köcher <git@kchr.de>
Co-authored-by: cmd[bot] <41898282+github-actions[bot]@users.noreply.github.com>
(cherry picked from commit 7058819)
@paritytech-release-backport-bot
Copy link
Copy Markdown

Successfully created backport PR for stable2506:

EgorPopelyaev pushed a commit that referenced this pull request Jul 14, 2025
Backport #9127 into `stable2506` from alexggh.

See the
[documentation](https://github.com/paritytech/polkadot-sdk/blob/master/docs/BACKPORT.md)
on how to use this bot.

<!--
  # To be used by other automation, do not modify:
  original-pr-number: #${pull_number}
-->

Signed-off-by: Alexandru Gheorghe <alexandru.gheorghe@parity.io>
Co-authored-by: Alexandru Gheorghe <49718502+alexggh@users.noreply.github.com>
Co-authored-by: Bastian Köcher <git@kchr.de>
Co-authored-by: cmd[bot] <41898282+github-actions[bot]@users.noreply.github.com>
@alexggh alexggh added A4-backport-unstable2507 Pull request must be backported to the unstable2507 release branch and removed A4-backport-unstable2507 Pull request must be backported to the unstable2507 release branch labels Jul 17, 2025
@paritytech-release-backport-bot
Copy link
Copy Markdown

Created backport PR for stable:

Please cherry-pick the changes locally and resolve any conflicts.

git fetch origin backport-9127-to-stable
git worktree add --checkout .worktree/backport-9127-to-stable backport-9127-to-stable
cd .worktree/backport-9127-to-stable
git reset --hard HEAD^
git cherry-pick -x 7058819a45ed5b74cedd6d21698f1c2ac2445d6b
git push --force-with-lease

@paritytech-release-backport-bot
Copy link
Copy Markdown

Backport failed for unstable: couldn't find remote ref unstable.
Please ensure that this Github repo has a branch named unstable.

@paritytech-release-backport-bot
Copy link
Copy Markdown

Backport failed for stable2506, because it was unable to cherry-pick the commit(s).

Please cherry-pick the changes locally and resolve any conflicts.

git fetch origin stable2506
git worktree add -d .worktree/backport-9127-to-stable2506 origin/stable2506
cd .worktree/backport-9127-to-stable2506
git switch --create backport-9127-to-stable2506
git cherry-pick -x 7058819a45ed5b74cedd6d21698f1c2ac2445d6b

paritytech-release-backport-bot bot pushed a commit that referenced this pull request Jul 17, 2025
…lidation context (#9127)

#8606
paritytech/trie#221 replaced the usage of
BTreeMap with HashMaps in validation context. The keys are already
derived with a cryptographic hash function from user data, so users
should not be able to manipulate it.

To be on safe side this PR also modifies the TrieCache, TrieRecorder and
MemoryDB to use a hasher that on top of the default generated randomness
also adds randomness generated from the hash of the relaychain and that
of the parachain blocks, which is not something users can control or
guess ahead of time.

---------

Signed-off-by: Alexandru Gheorghe <alexandru.gheorghe@parity.io>
Co-authored-by: Bastian Köcher <git@kchr.de>
Co-authored-by: cmd[bot] <41898282+github-actions[bot]@users.noreply.github.com>
(cherry picked from commit 7058819)
@paritytech-release-backport-bot
Copy link
Copy Markdown

Successfully created backport PR for unstable2507:

acatangiu added a commit that referenced this pull request Jul 17, 2025
Backport #9127 into `unstable2507` from alexggh.

See the
[documentation](https://github.com/paritytech/polkadot-sdk/blob/master/docs/BACKPORT.md)
on how to use this bot.

<!--
  # To be used by other automation, do not modify:
  original-pr-number: #${pull_number}
-->

Signed-off-by: Alexandru Gheorghe <alexandru.gheorghe@parity.io>
Co-authored-by: Alexandru Gheorghe <49718502+alexggh@users.noreply.github.com>
Co-authored-by: Bastian Köcher <git@kchr.de>
Co-authored-by: cmd[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: Adrian Catangiu <adrian@parity.io>
alvicsam pushed a commit that referenced this pull request Oct 17, 2025
…lidation context (#9127)

#8606
paritytech/trie#221 replaced the usage of
BTreeMap with HashMaps in validation context. The keys are already
derived with a cryptographic hash function from user data, so users
should not be able to manipulate it.

To be on safe side this PR also modifies the TrieCache, TrieRecorder and
MemoryDB to use a hasher that on top of the default generated randomness
also adds randomness generated from the hash of the relaychain and that
of the parachain blocks, which is not something users can control or
guess ahead of time.

---------

Signed-off-by: Alexandru Gheorghe <alexandru.gheorghe@parity.io>
Co-authored-by: Bastian Köcher <git@kchr.de>
Co-authored-by: cmd[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A4-backport-stable2506 Pull request must be backported to the stable2506 release branch A4-backport-unstable2507 Pull request must be backported to the unstable2507 release branch T9-cumulus This PR/Issue is related to cumulus.

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

5 participants