Disable "ahash-compile-time-rng" feature in hashbrown.

[eddyb]

This will ensure that every build uses the same ahash seed, instead of a compile-time randomized one (which is now the default - see rust-lang/hashbrown#124, rust-lang/hashbrown#125, tkaitchuck/aHash#18 and tkaitchuck/aHash#25 for more details/background).

(The other such update PR needed is paritytech/finality-grandpa#90).

[parity-cla-bot]

It looks like @eddyb hasn't signed our Contributor License Agreement, yet.

The purpose of a CLA is to ensure that the guardian of a project's outputs has the necessary ownership or grants of rights over all contributions to allow them to distribute under the chosen licence.
Wikipedia

You can read and sign our full Contributor License Agreement at the following URL: https://cla.parity.io

Once you've signed, please reply to this thread with [clabot:check] to prove it.

Many thanks,

Parity Technologies CLA Bot

[eddyb]

[clabot:check]

[parity-cla-bot]

It looks like @eddyb signed our Contributor License Agreement. 👍

Many thanks,

Parity Technologies CLA Bot

[arkpar]

Does it interfere with no_std build? Randomized seed is extra security against hash collision attacks. And I don't think we require consistent ordering anywhere, as we switched from std to hashbrown not that long ago. I'd keep it as is at least for std

[eddyb]

It depends if you want the build process (and the resulting binaries) to be deterministic (with this change) or not (without) - cc @rphmeier who merged paritytech/finality-grandpa#90.

One thing I'll note is that any published binaries (e.g. WASM blobs on-chain) will have the seed extractable from them anyway, so that should be included in the attack model for hash collisions.

[burdges]

I agree with @eddyb here, if needed then randomness should be obtained at runtime.

At worse, we could handle ephemeral cases inside execute block by constructing the random seed from the parachain block hash or relay chain block hash or VRF output, so that everyone had equal seeds right now, but the seed gets created after the block.

In fact, I'd worry more about alternative builds behaving differently. In particular, it appears MemoryDB::drain returns a hashbrown::HashMap which provides Iterator methods. If this iteration order ever impacts final behavior anywhere then attackers could make nodes behave differently, possibly increasing the attackers' power, either by confusion or maybe by slashing and disabling many nodes.

There are tools like https://docs.rs/ordermap/0.4.2/ordermap/ that iterate over a randomized hashmap predictably or maybe drain could just be replaced by a clear method that returned nothing. Yet, right now we still ban secrets from the execute block function entirely, which makes this change essential for alternative builds, not just reproducible builds.

[rphmeier]

One thing I'll note is that any published binaries (e.g. WASM blobs on-chain) will have the seed extractable from them anyway, so that should be included in the attack model for hash collisions.

This was my reasoning for merging the PR against finality-grandpa. Randomizing the seed only matters if you do so on each run of the program. In the case of on-chain WASM, it's fixed at compile-time. That bears re-evaluation of any of our usages of hash maps in our no-std crates, but that's beyond the scope of the PR.