Skip to content

Commit 8f4e095

Browse files
committed
Drop rng and SystemTime calls from KeysManager
They were only used for ensuring generated keys were globally unique (ie in case the user opened the same seed at a different time, we need generated keys to be globally unique). Instead, we let the user specify a time in secs/nanos, and provide a precise meaning for the user to understand.
1 parent ef3e9dd commit 8f4e095

File tree

3 files changed

+41
-33
lines changed

3 files changed

+41
-33
lines changed

src/chain/keysinterface.rs

Lines changed: 35 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -9,18 +9,17 @@ use bitcoin::network::constants::Network;
99
use bitcoin::util::bip32::{ExtendedPrivKey, ExtendedPubKey, ChildNumber};
1010

1111
use bitcoin_hashes::{Hash, HashEngine};
12+
use bitcoin_hashes::sha256::HashEngine as Sha256State;
1213
use bitcoin_hashes::sha256::Hash as Sha256;
1314
use bitcoin_hashes::hash160::Hash as Hash160;
1415

1516
use secp256k1::key::{SecretKey, PublicKey};
1617
use secp256k1::Secp256k1;
1718
use secp256k1;
1819

19-
use util::logger::Logger;
20-
use util::rng;
2120
use util::byte_utils;
21+
use util::logger::Logger;
2222

23-
use std::time::{SystemTime, UNIX_EPOCH};
2423
use std::sync::Arc;
2524
use std::sync::atomic::{AtomicUsize, Ordering};
2625

@@ -131,13 +130,31 @@ pub struct KeysManager {
131130
channel_id_master_key: ExtendedPrivKey,
132131
channel_id_child_index: AtomicUsize,
133132

133+
unique_start: Sha256State,
134134
logger: Arc<Logger>,
135135
}
136136

137137
impl KeysManager {
138138
/// Constructs a KeysManager from a 32-byte seed. If the seed is in some way biased (eg your
139-
/// RNG is busted) this may panic.
140-
pub fn new(seed: &[u8; 32], network: Network, logger: Arc<Logger>) -> KeysManager {
139+
/// RNG is busted) this may panic (but more importantly, you will possibly lose funds).
140+
/// starting_time isn't strictly required to actually be a time, but it must absolutely,
141+
/// without a doubt, be unique to this instance. ie if you start multiple times with the same
142+
/// seed, starting_time must be unique to each run. Thus, the easiest way to achieve this is to
143+
/// simply use the current time (with very high precision).
144+
///
145+
/// The seed MUST be backed up safely prior to use so that the keys can be re-created, however,
146+
/// obviously, starting_time should be unique every time you reload the library - it is only
147+
/// used to generate new ephemeral key data (which will be stored by the individual channel if
148+
/// necessary).
149+
///
150+
/// Note that the seed is required to recover certain on-chain funds independent of
151+
/// ChannelMonitor data, though a current copy of ChannelMonitor data is also required for any
152+
/// channel, and some on-chain during-closing funds.
153+
///
154+
/// Note that until the 0.1 release there is no guarantee of backward compatibility between
155+
/// versions. Once the library is more fully supported, the docs will be updated to include a
156+
/// detailed description of the guarantee.
157+
pub fn new(seed: &[u8; 32], network: Network, logger: Arc<Logger>, starting_time_secs: u64, starting_time_nanos: u32) -> KeysManager {
141158
let secp_ctx = Secp256k1::signing_only();
142159
match ExtendedPrivKey::new_master(network.clone(), seed) {
143160
Ok(master_key) => {
@@ -158,6 +175,12 @@ impl KeysManager {
158175
let channel_master_key = master_key.ckd_priv(&secp_ctx, ChildNumber::from_hardened_idx(3).unwrap()).expect("Your RNG is busted");
159176
let session_master_key = master_key.ckd_priv(&secp_ctx, ChildNumber::from_hardened_idx(4).unwrap()).expect("Your RNG is busted");
160177
let channel_id_master_key = master_key.ckd_priv(&secp_ctx, ChildNumber::from_hardened_idx(5).unwrap()).expect("Your RNG is busted");
178+
179+
let mut unique_start = Sha256::engine();
180+
unique_start.input(&byte_utils::be64_to_array(starting_time_secs));
181+
unique_start.input(&byte_utils::be32_to_array(starting_time_nanos));
182+
unique_start.input(seed);
183+
161184
KeysManager {
162185
secp_ctx,
163186
node_secret,
@@ -170,6 +193,7 @@ impl KeysManager {
170193
channel_id_master_key,
171194
channel_id_child_index: AtomicUsize::new(0),
172195

196+
unique_start,
173197
logger,
174198
}
175199
},
@@ -193,24 +217,15 @@ impl KeysInterface for KeysManager {
193217

194218
fn get_channel_keys(&self, _inbound: bool) -> ChannelKeys {
195219
// We only seriously intend to rely on the channel_master_key for true secure
196-
// entropy, everything else just ensures uniqueness. We generally don't expect
197-
// all clients to have non-broken RNGs here, so we also include the current
198-
// time as a fallback to get uniqueness.
199-
let mut sha = Sha256::engine();
200-
201-
let mut seed = [0u8; 32];
202-
rng::fill_bytes(&mut seed[..]);
203-
sha.input(&seed);
204-
205-
let now = SystemTime::now().duration_since(UNIX_EPOCH).expect("Time went backwards");
206-
sha.input(&byte_utils::be32_to_array(now.subsec_nanos()));
207-
sha.input(&byte_utils::be64_to_array(now.as_secs()));
220+
// entropy, everything else just ensures uniqueness. We rely on the unique_start (ie
221+
// starting_time provided in the constructor) to be unique.
222+
let mut sha = self.unique_start.clone();
208223

209224
let child_ix = self.channel_child_index.fetch_add(1, Ordering::AcqRel);
210225
let child_privkey = self.channel_master_key.ckd_priv(&self.secp_ctx, ChildNumber::from_hardened_idx(child_ix as u32).expect("key space exhausted")).expect("Your RNG is busted");
211226
sha.input(&child_privkey.private_key.key[..]);
212227

213-
seed = Sha256::from_engine(sha).into_inner();
228+
let seed = Sha256::from_engine(sha).into_inner();
214229

215230
let commitment_seed = {
216231
let mut sha = Sha256::engine();
@@ -244,11 +259,7 @@ impl KeysInterface for KeysManager {
244259
}
245260

246261
fn get_session_key(&self) -> SecretKey {
247-
let mut sha = Sha256::engine();
248-
249-
let now = SystemTime::now().duration_since(UNIX_EPOCH).expect("Time went backwards");
250-
sha.input(&byte_utils::be32_to_array(now.subsec_nanos()));
251-
sha.input(&byte_utils::be64_to_array(now.as_secs()));
262+
let mut sha = self.unique_start.clone();
252263

253264
let child_ix = self.session_child_index.fetch_add(1, Ordering::AcqRel);
254265
let child_privkey = self.session_master_key.ckd_priv(&self.secp_ctx, ChildNumber::from_hardened_idx(child_ix as u32).expect("key space exhausted")).expect("Your RNG is busted");
@@ -257,11 +268,7 @@ impl KeysInterface for KeysManager {
257268
}
258269

259270
fn get_channel_id(&self) -> [u8; 32] {
260-
let mut sha = Sha256::engine();
261-
262-
let now = SystemTime::now().duration_since(UNIX_EPOCH).expect("Time went backwards");
263-
sha.input(&byte_utils::be32_to_array(now.subsec_nanos()));
264-
sha.input(&byte_utils::be64_to_array(now.as_secs()));
271+
let mut sha = self.unique_start.clone();
265272

266273
let child_ix = self.channel_id_child_index.fetch_add(1, Ordering::AcqRel);
267274
let child_privkey = self.channel_id_master_key.ckd_priv(&self.secp_ctx, ChildNumber::from_hardened_idx(child_ix as u32).expect("key space exhausted")).expect("Your RNG is busted");

src/ln/functional_tests.rs

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@
55
use chain::transaction::OutPoint;
66
use chain::chaininterface::{ChainListener, ChainWatchInterface};
77
use chain::keysinterface::{KeysInterface, SpendableOutputDescriptor};
8-
use chain::keysinterface;
98
use ln::channel::{COMMITMENT_TX_BASE_WEIGHT, COMMITMENT_TX_WEIGHT_PER_HTLC, BREAKDOWN_TIMEOUT};
109
use ln::channelmanager::{ChannelManager,ChannelManagerReadArgs,HTLCForwardInfo,RAACommitmentOrder, PaymentPreimage, PaymentHash};
1110
use ln::channelmonitor::{ChannelMonitor, CLTV_CLAIM_BUFFER, LATENCY_GRACE_PERIOD_BLOCKS, ManyChannelMonitor, ANTI_REORG_DELAY};
@@ -3225,7 +3224,7 @@ fn test_no_txn_manager_serialize_deserialize() {
32253224

32263225
let mut nodes_0_read = &nodes_0_serialized[..];
32273226
let config = UserConfig::new();
3228-
let keys_manager = Arc::new(keysinterface::KeysManager::new(&nodes[0].node_seed, Network::Testnet, Arc::new(test_utils::TestLogger::new())));
3227+
let keys_manager = Arc::new(test_utils::TestKeysInterface::new(&nodes[0].node_seed, Network::Testnet, Arc::new(test_utils::TestLogger::new())));
32293228
let (_, nodes_0_deserialized) = {
32303229
let mut channel_monitors = HashMap::new();
32313230
channel_monitors.insert(chan_0_monitor.get_funding_txo().unwrap(), &chan_0_monitor);
@@ -3290,7 +3289,7 @@ fn test_simple_manager_serialize_deserialize() {
32903289
assert!(chan_0_monitor_read.is_empty());
32913290

32923291
let mut nodes_0_read = &nodes_0_serialized[..];
3293-
let keys_manager = Arc::new(keysinterface::KeysManager::new(&nodes[0].node_seed, Network::Testnet, Arc::new(test_utils::TestLogger::new())));
3292+
let keys_manager = Arc::new(test_utils::TestKeysInterface::new(&nodes[0].node_seed, Network::Testnet, Arc::new(test_utils::TestLogger::new())));
32943293
let (_, nodes_0_deserialized) = {
32953294
let mut channel_monitors = HashMap::new();
32963295
channel_monitors.insert(chan_0_monitor.get_funding_txo().unwrap(), &chan_0_monitor);
@@ -3354,7 +3353,7 @@ fn test_manager_serialize_deserialize_inconsistent_monitor() {
33543353
}
33553354

33563355
let mut nodes_0_read = &nodes_0_serialized[..];
3357-
let keys_manager = Arc::new(keysinterface::KeysManager::new(&nodes[0].node_seed, Network::Testnet, Arc::new(test_utils::TestLogger::new())));
3356+
let keys_manager = Arc::new(test_utils::TestKeysInterface::new(&nodes[0].node_seed, Network::Testnet, Arc::new(test_utils::TestLogger::new())));
33583357
let (_, nodes_0_deserialized) = <(Sha256dHash, ChannelManager)>::read(&mut nodes_0_read, ChannelManagerReadArgs {
33593358
default_config: UserConfig::new(),
33603359
keys_manager,

src/util/test_utils.rs

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ use bitcoin::network::constants::Network;
1818

1919
use secp256k1::{SecretKey, PublicKey};
2020

21+
use std::time::{SystemTime, UNIX_EPOCH};
2122
use std::sync::{Arc,Mutex};
2223
use std::{mem};
2324

@@ -242,8 +243,9 @@ impl keysinterface::KeysInterface for TestKeysInterface {
242243

243244
impl TestKeysInterface {
244245
pub fn new(seed: &[u8; 32], network: Network, logger: Arc<Logger>) -> Self {
246+
let now = SystemTime::now().duration_since(UNIX_EPOCH).expect("Time went backwards");
245247
Self {
246-
backing: keysinterface::KeysManager::new(seed, network, logger),
248+
backing: keysinterface::KeysManager::new(seed, network, logger, now.as_secs(), now.subsec_nanos()),
247249
override_session_priv: Mutex::new(None),
248250
override_channel_id_priv: Mutex::new(None),
249251
}

0 commit comments

Comments
 (0)