From: Matt Corallo Date: Wed, 28 Dec 2022 17:44:33 +0000 (+0000) Subject: Ensure `derive_channel_keys` doesn't panic if per-run seed is high X-Git-Url: http://git.bitcoin.ninja/index.cgi?a=commitdiff_plain;h=b60253c61c4cc537b46f6393440a93279bd46f56;p=rust-lightning Ensure `derive_channel_keys` doesn't panic if per-run seed is high b04d1b868fe28bea2e4c711e6e6d2470d2b98d77 changed the way we calculate the `channel_keys_id` to include the 128-bit `user_channel_id` as well, shifting the counter up four bytes and the `starting_time_nanos` field up into the second four bytes. In `derive_channel_keys` we hash the full `channel_keys_id` with an HD-derived key from our master seed. Previously, that key was derived with an index of the per-restart counter, re-calculated by pulling the second four bytes out of the `user_channel_id`. Because the `channel_keys_id` fields were shifted up four bytes, that is now a reference to the `starting_time_nanos` value. This should be fine, the derivation doesn't really add any value here, its all being hashed anyway, except that derivation IDs must be below 2^31. This implies that we panic if the user passes a `starting_time_nanos` which has the high bit set. For those using the nanosecond part of the current time this isn't an issue - the value cannot exceed 1_000_000, which does not have the high bit set, however, some users may use some other per-run seed. Thus, here we simply drop the high bit from the seed, ensuring we don't panic. Note that this is backwards compatible as it only changes the key derivation in cases where we previously panicked. Ideally we'd drop the derivation entirely, but that would break backwards compatibility of key derivation. --- diff --git a/lightning/src/chain/keysinterface.rs b/lightning/src/chain/keysinterface.rs index 1426ff5ce..0a464193d 100644 --- a/lightning/src/chain/keysinterface.rs +++ b/lightning/src/chain/keysinterface.rs @@ -1051,7 +1051,9 @@ impl KeysManager { // We only seriously intend to rely on the channel_master_key for true secure // entropy, everything else just ensures uniqueness. We rely on the unique_start (ie // starting_time provided in the constructor) to be unique. - let child_privkey = self.channel_master_key.ckd_priv(&self.secp_ctx, ChildNumber::from_hardened_idx(chan_id as u32).expect("key space exhausted")).expect("Your RNG is busted"); + let child_privkey = self.channel_master_key.ckd_priv(&self.secp_ctx, + ChildNumber::from_hardened_idx((chan_id as u32) % (1 << 31)).expect("key space exhausted") + ).expect("Your RNG is busted"); unique_start.input(&child_privkey.private_key[..]); let seed = Sha256::from_engine(unique_start).into_inner();