Merge pull request #802 from TheBlueMatt/2021-01-update-rust-bitcoin

Update rust-bitcoin and add secp256k1 context randomization

Author: TheBlueMatt

.github/workflows/build.yml

@@ -202,7 +202,7 @@ jobs:
           sudo apt-get update
           sudo apt-get -y install build-essential binutils-dev libunwind-dev
       - name: Sanity check fuzz targets on Rust ${{ env.TOOLCHAIN }}
-        run: cd fuzz && cargo test --verbose --color always
+        run: cd fuzz && RUSTFLAGS="--cfg=fuzzing" cargo test --verbose --color always
       - name: Run fuzzers
         run: cd fuzz && ./ci-fuzz.sh
 

background-processor/Cargo.toml

@@ -7,7 +7,7 @@ edition = "2018"
 # See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html
 
 [dependencies]
-bitcoin = "0.24"
+bitcoin = "0.26"
 lightning = { version = "0.0.12", path = "../lightning", features = ["allow_wallclock_use"] }
 lightning-persister = { version = "0.0.1", path = "../lightning-persister" }
 

fuzz/Cargo.toml

@@ -19,11 +19,17 @@ stdin_fuzz = []
 [dependencies]
 afl = { version = "0.4", optional = true }
 lightning = { path = "../lightning", features = ["fuzztarget"] }
-bitcoin = { version = "0.24", features = ["fuzztarget"] }
+bitcoin = { version = "0.26", features = ["fuzztarget", "secp-lowmemory"] }
 hex = "0.3"
 honggfuzz = { version = "0.5", optional = true }
 libfuzzer-sys = { git = "https://github.com/rust-fuzz/libfuzzer-sys.git", optional = true }
 
+[patch.crates-io]
+# Rust-Secp256k1 PR 282. This patch should be dropped once that is merged.
+secp256k1 = { git = 'https://github.com/TheBlueMatt/rust-secp256k1', rev = '32767e0e21e8861701ff7d5957613169d67ff1f8' }
+# bitcoin_hashes PR 111 (without the top commit). This patch should be dropped once that is merged.
+bitcoin_hashes = { git = 'https://github.com/TheBlueMatt/bitcoin_hashes', rev = 'c90d26339a3e34fd2f942aa80298f410cc41b743' }
+
 [build-dependencies]
 cc = "1.0"
 

fuzz/src/full_stack.rs

@@ -13,7 +13,7 @@ rest-client = [ "serde", "serde_json", "chunked_transfer" ]
 rpc-client = [ "serde", "serde_json", "chunked_transfer" ]
 
 [dependencies]
-bitcoin = "0.24"
+bitcoin = "0.26"
 lightning = { version = "0.0.12", path = "../lightning" }
 tokio = { version = "1.0", features = [ "io-util", "net" ], optional = true }
 serde = { version = "1.0", features = ["derive"], optional = true }

lightning-block-sync/Cargo.toml

@@ -15,7 +15,7 @@ crate-type = ["staticlib"
 ,"cdylib"]
 
 [dependencies]
-bitcoin = "0.24"
+bitcoin = "0.26"
 lightning = { version = "0.0.12", path = "../lightning" }
 
 # We eventually want to join the root workspace, but for now, the bindings generation is

lightning-c-bindings/Cargo.toml

@@ -72,8 +72,8 @@ pub enum Secp256k1Error {
 	InvalidSecretKey,
 	InvalidRecoveryId,
 	InvalidTweak,
+	TweakCheckFailed,
 	NotEnoughMemory,
-	CallbackPanicked,
 }
 impl Secp256k1Error {
 	pub(crate) fn from_rust(err: SecpError) -> Self {
@@ -85,6 +85,7 @@ impl Secp256k1Error {
 			SecpError::InvalidSecretKey => Secp256k1Error::InvalidSecretKey,
 			SecpError::InvalidRecoveryId => Secp256k1Error::InvalidRecoveryId,
 			SecpError::InvalidTweak => Secp256k1Error::InvalidTweak,
+			SecpError::TweakCheckFailed => Secp256k1Error::TweakCheckFailed,
 			SecpError::NotEnoughMemory => Secp256k1Error::NotEnoughMemory,
 		}
 	}

lightning-c-bindings/src/c_types/mod.rs

@@ -10,7 +10,7 @@ For Rust-Lightning clients which wish to make direct connections to Lightning P2
 """
 
 [dependencies]
-bitcoin = "0.24"
+bitcoin = "0.26"
 lightning = { version = "0.0.12", path = "../lightning" }
 tokio = { version = "1.0", features = [ "io-util", "macros", "rt", "sync", "net", "time" ] }
 

lightning-net-tokio/Cargo.toml

@@ -8,15 +8,15 @@ Utilities to manage channel data persistence and retrieval.
 """
 
 [dependencies]
-bitcoin = "0.24"
+bitcoin = "0.26"
 lightning = { version = "0.0.12", path = "../lightning" }
 libc = "0.2"
 
 [target.'cfg(windows)'.dependencies]
 winapi = { version = "0.3", features = ["winbase"] }
 
 [dev-dependencies.bitcoin]
-version = "0.24"
+version = "0.26"
 features = ["bitcoinconsensus"]
 
 [dev-dependencies]

lightning-persister/Cargo.toml

@@ -27,13 +27,13 @@ unsafe_revoked_tx_signing = []
 unstable = []
 
 [dependencies]
-bitcoin = "0.24"
+bitcoin = "0.26"
 
 hex = { version = "0.3", optional = true }
 regex = { version = "0.1.80", optional = true }
 
 [dev-dependencies.bitcoin]
-version = "0.24"
+version = "0.26"
 features = ["bitcoinconsensus"]
 
 [dev-dependencies]

lightning/Cargo.toml

@@ -951,7 +951,7 @@ impl<Signer: Sign> Writeable for ChannelMonitor<Signer> {
 }
 
 impl<Signer: Sign> ChannelMonitor<Signer> {
-	pub(crate) fn new(keys: Signer, shutdown_pubkey: &PublicKey,
+	pub(crate) fn new(secp_ctx: Secp256k1<secp256k1::All>, keys: Signer, shutdown_pubkey: &PublicKey,
 	                  on_counterparty_tx_csv: u16, destination_script: &Script, funding_info: (OutPoint, Script),
 	                  channel_parameters: &ChannelTransactionParameters,
 	                  funding_redeemscript: Script, channel_value_satoshis: u64,
@@ -972,8 +972,6 @@ impl<Signer: Sign> ChannelMonitor<Signer> {
 		let channel_keys_id = keys.channel_keys_id();
 		let holder_revocation_basepoint = keys.pubkeys().revocation_basepoint;
 
-		let secp_ctx = Secp256k1::new();
-
 		// block for Rust 1.34 compat
 		let (holder_commitment_tx, current_holder_commitment_number) = {
 			let trusted_tx = initial_holder_commitment_tx.trust();
@@ -994,7 +992,8 @@ impl<Signer: Sign> ChannelMonitor<Signer> {
 		};
 
 		let onchain_tx_handler =
-			OnchainTxHandler::new(destination_script.clone(), keys, channel_parameters.clone(), initial_holder_commitment_tx);
+			OnchainTxHandler::new(destination_script.clone(), keys,
+			channel_parameters.clone(), initial_holder_commitment_tx, secp_ctx.clone());
 
 		let mut outputs_to_watch = HashMap::new();
 		outputs_to_watch.insert(funding_info.0.txid, vec![(funding_info.0.index as u32, funding_info.1.clone())]);
@@ -2558,6 +2557,9 @@ impl<'a, Signer: Sign, K: KeysInterface<Signer = Signer>> ReadableArgs<&'a K>
 		let lockdown_from_offchain = Readable::read(reader)?;
 		let holder_tx_signed = Readable::read(reader)?;
 
+		let mut secp_ctx = Secp256k1::new();
+		secp_ctx.seeded_randomize(&keys_manager.get_secure_random_bytes());
+
 		Ok((last_block_hash.clone(), ChannelMonitor {
 			latest_update_id,
 			commitment_transaction_number_obscure_factor,
@@ -2603,7 +2605,7 @@ impl<'a, Signer: Sign, K: KeysInterface<Signer = Signer>> ReadableArgs<&'a K>
 			holder_tx_signed,
 
 			last_block_hash,
-			secp_ctx: Secp256k1::new(),
+			secp_ctx,
 		}))
 	}
 }
@@ -2718,7 +2720,7 @@ mod tests {
 		};
 		// Prune with one old state and a holder commitment tx holding a few overlaps with the
 		// old state.
-		let mut monitor = ChannelMonitor::new(keys,
+		let mut monitor = ChannelMonitor::new(Secp256k1::new(), keys,
 		                                      &PublicKey::from_secret_key(&secp_ctx, &SecretKey::from_slice(&[42; 32]).unwrap()), 0, &Script::new(),
 		                                      (OutPoint { txid: Txid::from_slice(&[43; 32]).unwrap(), index: 0 }, Script::new()),
 		                                      &channel_parameters,

lightning/src/chain/channelmonitor.rs

@@ -744,8 +744,10 @@ pub struct KeysManager {
 	shutdown_pubkey: PublicKey,
 	channel_master_key: ExtendedPrivKey,
 	channel_child_index: AtomicUsize,
+
 	rand_bytes_master_key: ExtendedPrivKey,
 	rand_bytes_child_index: AtomicUsize,
+	rand_bytes_unique_start: Sha256State,
 
 	seed: [u8; 32],
 	starting_time_secs: u64,
@@ -794,31 +796,36 @@ impl KeysManager {
 				let channel_master_key = master_key.ckd_priv(&secp_ctx, ChildNumber::from_hardened_idx(3).unwrap()).expect("Your RNG is busted");
 				let rand_bytes_master_key = master_key.ckd_priv(&secp_ctx, ChildNumber::from_hardened_idx(4).unwrap()).expect("Your RNG is busted");
 
-				KeysManager {
+				let mut rand_bytes_unique_start = Sha256::engine();
+				rand_bytes_unique_start.input(&byte_utils::be64_to_array(starting_time_secs));
+				rand_bytes_unique_start.input(&byte_utils::be32_to_array(starting_time_nanos));
+				rand_bytes_unique_start.input(seed);
+
+				let mut res = KeysManager {
 					secp_ctx,
 					node_secret,
+
 					destination_script,
 					shutdown_pubkey,
+
 					channel_master_key,
 					channel_child_index: AtomicUsize::new(0),
+
 					rand_bytes_master_key,
 					rand_bytes_child_index: AtomicUsize::new(0),
+					rand_bytes_unique_start,
 
 					seed: *seed,
 					starting_time_secs,
 					starting_time_nanos,
-				}
+				};
+				let secp_seed = res.get_secure_random_bytes();
+				res.secp_ctx.seeded_randomize(&secp_seed);
+				res
 			},
 			Err(_) => panic!("Your rng is busted"),
 		}
 	}
-	fn derive_unique_start(&self) -> Sha256State {
-		let mut unique_start = Sha256::engine();
-		unique_start.input(&byte_utils::be64_to_array(self.starting_time_secs));
-		unique_start.input(&byte_utils::be32_to_array(self.starting_time_nanos));
-		unique_start.input(&self.seed);
-		unique_start
-	}
 	/// Derive an old set of Sign for per-channel secrets based on a key derivation
 	/// parameters.
 	/// Key derivation parameters are accessible through a per-channel secrets
@@ -1017,7 +1024,7 @@ impl KeysInterface for KeysManager {
 	}
 
 	fn get_secure_random_bytes(&self) -> [u8; 32] {
-		let mut sha = self.derive_unique_start();
+		let mut sha = self.rand_bytes_unique_start.clone();
 
 		let child_ix = self.rand_bytes_child_index.fetch_add(1, Ordering::AcqRel);
 		let child_privkey = self.rand_bytes_master_key.ckd_priv(&self.secp_ctx, ChildNumber::from_hardened_idx(child_ix as u32).expect("key space exhausted")).expect("Your RNG is busted");

lightning/src/chain/keysinterface.rs

@@ -19,7 +19,7 @@
 //! instead of having a rather-separate lightning appendage to a wallet.
 
 #![cfg_attr(not(any(feature = "fuzztarget", feature = "_test_utils")), deny(missing_docs))]
-#![forbid(unsafe_code)]
+#![cfg_attr(not(any(test, feature = "fuzztarget", feature = "_test_utils")), forbid(unsafe_code))]
 
 // In general, rust is absolutely horrid at supporting users doing things like,
 // for example, compiling Rust code for real environments. Disable useless lints

lightning/src/lib.rs

@@ -521,13 +521,16 @@ impl<Signer: Sign> Channel<Signer> {
 
 		let feerate = fee_estimator.get_est_sat_per_1000_weight(ConfirmationTarget::Normal);
 
+		let mut secp_ctx = Secp256k1::new();
+		secp_ctx.seeded_randomize(&keys_provider.get_secure_random_bytes());
+
 		Ok(Channel {
 			user_id,
 			config: config.channel_options.clone(),
 
 			channel_id: keys_provider.get_secure_random_bytes(),
 			channel_state: ChannelState::OurInitSent as u32,
-			secp_ctx: Secp256k1::new(),
+			secp_ctx,
 			channel_value_satoshis,
 
 			latest_monitor_update_id: 0,
@@ -755,13 +758,16 @@ impl<Signer: Sign> Channel<Signer> {
 			}
 		} else { None };
 
+		let mut secp_ctx = Secp256k1::new();
+		secp_ctx.seeded_randomize(&keys_provider.get_secure_random_bytes());
+
 		let chan = Channel {
 			user_id,
 			config: local_config,
 
 			channel_id: msg.temporary_channel_id,
 			channel_state: (ChannelState::OurInitSent as u32) | (ChannelState::TheirInitSent as u32),
-			secp_ctx: Secp256k1::new(),
+			secp_ctx,
 
 			latest_monitor_update_id: 0,
 
@@ -1564,7 +1570,7 @@ impl<Signer: Sign> Channel<Signer> {
 		let funding_redeemscript = self.get_funding_redeemscript();
 		let funding_txo_script = funding_redeemscript.to_v0_p2wsh();
 		let obscure_factor = get_commitment_transaction_number_obscure_factor(&self.get_holder_pubkeys().payment_point, &self.get_counterparty_pubkeys().payment_point, self.is_outbound());
-		let mut channel_monitor = ChannelMonitor::new(self.holder_signer.clone(),
+		let mut channel_monitor = ChannelMonitor::new(self.secp_ctx.clone(), self.holder_signer.clone(),
 		                                              &self.shutdown_pubkey, self.get_holder_selected_contest_delay(),
 		                                              &self.destination_script, (funding_txo, funding_txo_script.clone()),
 		                                              &self.channel_transaction_parameters,
@@ -1634,7 +1640,7 @@ impl<Signer: Sign> Channel<Signer> {
 		let funding_txo = self.get_funding_txo().unwrap();
 		let funding_txo_script = funding_redeemscript.to_v0_p2wsh();
 		let obscure_factor = get_commitment_transaction_number_obscure_factor(&self.get_holder_pubkeys().payment_point, &self.get_counterparty_pubkeys().payment_point, self.is_outbound());
-		let mut channel_monitor = ChannelMonitor::new(self.holder_signer.clone(),
+		let mut channel_monitor = ChannelMonitor::new(self.secp_ctx.clone(), self.holder_signer.clone(),
 		                                              &self.shutdown_pubkey, self.get_holder_selected_contest_delay(),
 		                                              &self.destination_script, (funding_txo, funding_txo_script),
 		                                              &self.channel_transaction_parameters,
@@ -4081,7 +4087,8 @@ impl<Signer: Sign> Channel<Signer> {
 			signature = res.0;
 			htlc_signatures = res.1;
 
-			log_trace!(logger, "Signed remote commitment tx {} with redeemscript {} -> {}",
+			log_trace!(logger, "Signed remote commitment tx {} (txid {}) with redeemscript {} -> {}",
+				encode::serialize_hex(&counterparty_commitment_tx.0.trust().built_transaction().transaction),
 				&counterparty_commitment_txid,
 				encode::serialize_hex(&self.get_funding_redeemscript()),
 				log_bytes!(signature.serialize_compact()[..]));
@@ -4608,13 +4615,16 @@ impl<'a, Signer: Sign, K: Deref> ReadableArgs<&'a K> for Channel<Signer>
 		let counterparty_shutdown_scriptpubkey = Readable::read(reader)?;
 		let commitment_secrets = Readable::read(reader)?;
 
+		let mut secp_ctx = Secp256k1::new();
+		secp_ctx.seeded_randomize(&keys_source.get_secure_random_bytes());
+
 		Ok(Channel {
 			user_id,
 
 			config,
 			channel_id,
 			channel_state,
-			secp_ctx: Secp256k1::new(),
+			secp_ctx,
 			channel_value_satoshis,
 
 			latest_monitor_update_id,

lightning/src/ln/channel.rs

@@ -766,7 +766,8 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
 	/// Users need to notify the new ChannelManager when a new block is connected or
 	/// disconnected using its `block_connected` and `block_disconnected` methods.
 	pub fn new(network: Network, fee_est: F, chain_monitor: M, tx_broadcaster: T, logger: L, keys_manager: K, config: UserConfig, current_blockchain_height: usize) -> Self {
-		let secp_ctx = Secp256k1::new();
+		let mut secp_ctx = Secp256k1::new();
+		secp_ctx.seeded_randomize(&keys_manager.get_secure_random_bytes());
 
 		ChannelManager {
 			default_configuration: config.clone(),
@@ -4129,6 +4130,9 @@ impl<'a, Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref>
 
 		let last_node_announcement_serial: u32 = Readable::read(reader)?;
 
+		let mut secp_ctx = Secp256k1::new();
+		secp_ctx.seeded_randomize(&args.keys_manager.get_secure_random_bytes());
+
 		let channel_manager = ChannelManager {
 			genesis_hash,
 			fee_estimator: args.fee_estimator,
@@ -4137,7 +4141,7 @@ impl<'a, Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref>
 
 			latest_block_height: AtomicUsize::new(latest_block_height as usize),
 			last_block_hash: Mutex::new(last_block_hash),
-			secp_ctx: Secp256k1::new(),
+			secp_ctx,
 
 			channel_state: Mutex::new(ChannelHolder {
 				by_id,

lightning/src/ln/channelmanager.rs

@@ -406,6 +406,9 @@ impl<'a, K: KeysInterface> ReadableArgs<&'a K> for OnchainTxHandler<K::Signer> {
 		}
 		let latest_height = Readable::read(reader)?;
 
+		let mut secp_ctx = Secp256k1::new();
+		secp_ctx.seeded_randomize(&keys_manager.get_secure_random_bytes());
+
 		Ok(OnchainTxHandler {
 			destination_script,
 			holder_commitment,
@@ -418,13 +421,13 @@ impl<'a, K: KeysInterface> ReadableArgs<&'a K> for OnchainTxHandler<K::Signer> {
 			pending_claim_requests,
 			onchain_events_waiting_threshold_conf,
 			latest_height,
-			secp_ctx: Secp256k1::new(),
+			secp_ctx,
 		})
 	}
 }
 
 impl<ChannelSigner: Sign> OnchainTxHandler<ChannelSigner> {
-	pub(crate) fn new(destination_script: Script, signer: ChannelSigner, channel_parameters: ChannelTransactionParameters, holder_commitment: HolderCommitmentTransaction) -> Self {
+	pub(crate) fn new(destination_script: Script, signer: ChannelSigner, channel_parameters: ChannelTransactionParameters, holder_commitment: HolderCommitmentTransaction, secp_ctx: Secp256k1<secp256k1::All>) -> Self {
 		OnchainTxHandler {
 			destination_script,
 			holder_commitment,
@@ -438,7 +441,7 @@ impl<ChannelSigner: Sign> OnchainTxHandler<ChannelSigner> {
 			onchain_events_waiting_threshold_conf: HashMap::new(),
 			latest_height: 0,
 
-			secp_ctx: Secp256k1::new(),
+			secp_ctx,
 		}
 	}