Merge pull request #2205 from wpaulino/sign-ecdsa-with-noncedata

Generate local signatures with additional randomness

Author: TheBlueMatt

ci/ci-tests.sh

@@ -35,13 +35,14 @@ for DIR in lightning lightning-invoice lightning-rapid-gossip-sync; do
 	cargo test --verbose --color always --no-default-features --features no-std
 	# check if there is a conflict between no-std and the default std feature
 	cargo test --verbose --color always --features no-std
-	# check that things still pass without grind_signatures
-	# note that outbound_commitment_test only runs in this mode, because of hardcoded signature values
-	cargo test --verbose --color always --no-default-features --features std
 	# check if there is a conflict between no-std and the c_bindings cfg
 	RUSTFLAGS="--cfg=c_bindings" cargo test --verbose --color always --no-default-features --features=no-std
 	popd
 done
+# Note that outbound_commitment_test only runs in this mode because of hardcoded signature values
+pushd lightning
+cargo test --verbose --color always --no-default-features --features=std,_test_vectors
+popd
 # This one only works for lightning-invoice
 pushd lightning-invoice
 # check that compile with no-std and serde works in lightning-invoice

fuzz/src/chanmon_consistency.rs

@@ -240,6 +240,7 @@ impl SignerProvider for KeyProvider {
 			[id, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 9, self.node_secret[31]],
 			channel_value_satoshis,
 			channel_keys_id,
+			channel_keys_id,
 		);
 		let revoked_commitment = self.make_enforcement_state_cell(keys.commitment_seed);
 		EnforcingSigner::new_with_revoked(keys, revoked_commitment, false)
@@ -248,7 +249,7 @@ impl SignerProvider for KeyProvider {
 	fn read_chan_signer(&self, buffer: &[u8]) -> Result<Self::Signer, DecodeError> {
 		let mut reader = std::io::Cursor::new(buffer);
 
-		let inner: InMemorySigner = Readable::read(&mut reader)?;
+		let inner: InMemorySigner = ReadableArgs::read(&mut reader, self)?;
 		let state = self.make_enforcement_state_cell(inner.commitment_seed);
 
 		Ok(EnforcingSigner {

fuzz/src/full_stack.rs

@@ -48,7 +48,7 @@ use lightning::util::config::UserConfig;
 use lightning::util::errors::APIError;
 use lightning::util::enforcing_trait_impls::{EnforcingSigner, EnforcementState};
 use lightning::util::logger::Logger;
-use lightning::util::ser::{Readable, Writeable};
+use lightning::util::ser::{Readable, ReadableArgs, Writeable};
 
 use crate::utils::test_logger;
 use crate::utils::test_persister::TestPersister;
@@ -347,6 +347,7 @@ impl SignerProvider for KeyProvider {
 				[0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 6, ctr],
 				channel_value_satoshis,
 				channel_keys_id,
+				channel_keys_id,
 			)
 		} else {
 			InMemorySigner::new(
@@ -359,12 +360,13 @@ impl SignerProvider for KeyProvider {
 				[0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 12, ctr],
 				channel_value_satoshis,
 				channel_keys_id,
+				channel_keys_id,
 			)
 		}, state, false)
 	}
 
 	fn read_chan_signer(&self, mut data: &[u8]) -> Result<EnforcingSigner, DecodeError> {
-		let inner: InMemorySigner = Readable::read(&mut data)?;
+		let inner: InMemorySigner = ReadableArgs::read(&mut data, self)?;
 		let state = Arc::new(Mutex::new(EnforcementState::new()));
 
 		Ok(EnforcingSigner::new_with_revoked(

lightning/Cargo.toml

@@ -29,6 +29,8 @@ max_level_trace = []
 # This is unsafe to use in production because it may result in the counterparty publishing taking our funds.
 unsafe_revoked_tx_signing = []
 _bench_unstable = []
+# Override signing to not include randomness when generating signatures for test vectors.
+_test_vectors = []
 
 no-std = ["hashbrown", "bitcoin/no-std", "core2/alloc"]
 std = ["bitcoin/std"]

lightning/src/chain/channelmonitor.rs

@@ -4235,6 +4235,7 @@ mod tests {
 			[41; 32],
 			0,
 			[0; 32],
+			[0; 32],
 		);
 
 		let counterparty_pubkeys = ChannelPublicKeys {

lightning/src/chain/keysinterface.rs

@@ -32,8 +32,8 @@ use bitcoin::secp256k1::ecdsa::RecoverableSignature;
 use bitcoin::{PackedLockTime, secp256k1, Sequence, Witness};
 
 use crate::util::transaction_utils;
-use crate::util::crypto::{hkdf_extract_expand_twice, sign};
-use crate::util::ser::{Writeable, Writer, Readable};
+use crate::util::crypto::{hkdf_extract_expand_twice, sign, sign_with_aux_rand};
+use crate::util::ser::{Writeable, Writer, Readable, ReadableArgs};
 use crate::chain::transaction::OutPoint;
 #[cfg(anchors)]
 use crate::events::bump_transaction::HTLCDescriptor;
@@ -45,6 +45,7 @@ use crate::ln::script::ShutdownScript;
 
 use crate::prelude::*;
 use core::convert::TryInto;
+use core::ops::Deref;
 use core::sync::atomic::{AtomicUsize, Ordering};
 use crate::io::{self, Error};
 use crate::ln::msgs::{DecodeError, MAX_VALUE_MSAT};
@@ -553,7 +554,6 @@ pub trait SignerProvider {
 	fn get_shutdown_scriptpubkey(&self) -> ShutdownScript;
 }
 
-#[derive(Clone)]
 /// A simple implementation of [`WriteableEcdsaChannelSigner`] that just keeps the private keys in memory.
 ///
 /// This implementation performs no policy checks and is insufficient by itself as
@@ -580,6 +580,30 @@ pub struct InMemorySigner {
 	channel_value_satoshis: u64,
 	/// Key derivation parameters.
 	channel_keys_id: [u8; 32],
+	/// Seed from which all randomness produced is derived from.
+	rand_bytes_unique_start: [u8; 32],
+	/// Tracks the number of times we've produced randomness to ensure we don't return the same
+	/// bytes twice.
+	rand_bytes_index: AtomicCounter,
+}
+
+impl Clone for InMemorySigner {
+	fn clone(&self) -> Self {
+		Self {
+			funding_key: self.funding_key.clone(),
+			revocation_base_key: self.revocation_base_key.clone(),
+			payment_key: self.payment_key.clone(),
+			delayed_payment_base_key: self.delayed_payment_base_key.clone(),
+			htlc_base_key: self.htlc_base_key.clone(),
+			commitment_seed: self.commitment_seed.clone(),
+			holder_channel_pubkeys: self.holder_channel_pubkeys.clone(),
+			channel_parameters: self.channel_parameters.clone(),
+			channel_value_satoshis: self.channel_value_satoshis,
+			channel_keys_id: self.channel_keys_id,
+			rand_bytes_unique_start: self.get_secure_random_bytes(),
+			rand_bytes_index: AtomicCounter::new(),
+		}
+	}
 }
 
 impl InMemorySigner {
@@ -594,6 +618,7 @@ impl InMemorySigner {
 		commitment_seed: [u8; 32],
 		channel_value_satoshis: u64,
 		channel_keys_id: [u8; 32],
+		rand_bytes_unique_start: [u8; 32],
 	) -> InMemorySigner {
 		let holder_channel_pubkeys =
 			InMemorySigner::make_holder_keys(secp_ctx, &funding_key, &revocation_base_key,
@@ -610,6 +635,8 @@ impl InMemorySigner {
 			holder_channel_pubkeys,
 			channel_parameters: None,
 			channel_keys_id,
+			rand_bytes_unique_start,
+			rand_bytes_index: AtomicCounter::new(),
 		}
 	}
 
@@ -686,7 +713,7 @@ impl InMemorySigner {
 		let remotepubkey = self.pubkeys().payment_point;
 		let witness_script = bitcoin::Address::p2pkh(&::bitcoin::PublicKey{compressed: true, inner: remotepubkey}, Network::Testnet).script_pubkey();
 		let sighash = hash_to_message!(&sighash::SighashCache::new(spend_tx).segwit_signature_hash(input_idx, &witness_script, descriptor.output.value, EcdsaSighashType::All).unwrap()[..]);
-		let remotesig = sign(secp_ctx, &sighash, &self.payment_key);
+		let remotesig = sign_with_aux_rand(secp_ctx, &sighash, &self.payment_key, &self);
 		let payment_script = bitcoin::Address::p2wpkh(&::bitcoin::PublicKey{compressed: true, inner: remotepubkey}, Network::Bitcoin).unwrap().script_pubkey();
 
 		if payment_script != descriptor.output.script_pubkey { return Err(()); }
@@ -722,7 +749,7 @@ impl InMemorySigner {
 		let delayed_payment_pubkey = PublicKey::from_secret_key(&secp_ctx, &delayed_payment_key);
 		let witness_script = chan_utils::get_revokeable_redeemscript(&descriptor.revocation_pubkey, descriptor.to_self_delay, &delayed_payment_pubkey);
 		let sighash = hash_to_message!(&sighash::SighashCache::new(spend_tx).segwit_signature_hash(input_idx, &witness_script, descriptor.output.value, EcdsaSighashType::All).unwrap()[..]);
-		let local_delayedsig = sign(secp_ctx, &sighash, &delayed_payment_key);
+		let local_delayedsig = sign_with_aux_rand(secp_ctx, &sighash, &delayed_payment_key, &self);
 		let payment_script = bitcoin::Address::p2wsh(&witness_script, Network::Bitcoin).script_pubkey();
 
 		if descriptor.output.script_pubkey != payment_script { return Err(()); }
@@ -736,6 +763,15 @@ impl InMemorySigner {
 	}
 }
 
+impl EntropySource for InMemorySigner {
+	fn get_secure_random_bytes(&self) -> [u8; 32] {
+		let index = self.rand_bytes_index.get_increment();
+		let mut nonce = [0u8; 16];
+		nonce[..8].copy_from_slice(&index.to_be_bytes());
+		ChaCha20::get_single_block(&self.rand_bytes_unique_start, &nonce)
+	}
+}
+
 impl ChannelSigner for InMemorySigner {
 	fn get_per_commitment_point(&self, idx: u64, secp_ctx: &Secp256k1<secp256k1::All>) -> PublicKey {
 		let commitment_secret = SecretKey::from_slice(&chan_utils::build_commitment_secret(&self.commitment_seed, idx)).unwrap();
@@ -774,7 +810,7 @@ impl EcdsaChannelSigner for InMemorySigner {
 		let channel_funding_redeemscript = make_funding_redeemscript(&funding_pubkey, &self.counterparty_pubkeys().funding_pubkey);
 
 		let built_tx = trusted_tx.built_transaction();
-		let commitment_sig = built_tx.sign(&self.funding_key, &channel_funding_redeemscript, self.channel_value_satoshis, secp_ctx);
+		let commitment_sig = built_tx.sign_counterparty_commitment(&self.funding_key, &channel_funding_redeemscript, self.channel_value_satoshis, secp_ctx);
 		let commitment_txid = built_tx.txid;
 
 		let mut htlc_sigs = Vec::with_capacity(commitment_tx.htlcs().len());
@@ -799,9 +835,9 @@ impl EcdsaChannelSigner for InMemorySigner {
 		let funding_pubkey = PublicKey::from_secret_key(secp_ctx, &self.funding_key);
 		let funding_redeemscript = make_funding_redeemscript(&funding_pubkey, &self.counterparty_pubkeys().funding_pubkey);
 		let trusted_tx = commitment_tx.trust();
-		let sig = trusted_tx.built_transaction().sign(&self.funding_key, &funding_redeemscript, self.channel_value_satoshis, secp_ctx);
+		let sig = trusted_tx.built_transaction().sign_holder_commitment(&self.funding_key, &funding_redeemscript, self.channel_value_satoshis, &self, secp_ctx);
 		let channel_parameters = self.get_channel_parameters();
-		let htlc_sigs = trusted_tx.get_htlc_sigs(&self.htlc_base_key, &channel_parameters.as_holder_broadcastable(), secp_ctx)?;
+		let htlc_sigs = trusted_tx.get_htlc_sigs(&self.htlc_base_key, &channel_parameters.as_holder_broadcastable(), &self, secp_ctx)?;
 		Ok((sig, htlc_sigs))
 	}
 
@@ -810,9 +846,9 @@ impl EcdsaChannelSigner for InMemorySigner {
 		let funding_pubkey = PublicKey::from_secret_key(secp_ctx, &self.funding_key);
 		let funding_redeemscript = make_funding_redeemscript(&funding_pubkey, &self.counterparty_pubkeys().funding_pubkey);
 		let trusted_tx = commitment_tx.trust();
-		let sig = trusted_tx.built_transaction().sign(&self.funding_key, &funding_redeemscript, self.channel_value_satoshis, secp_ctx);
+		let sig = trusted_tx.built_transaction().sign_holder_commitment(&self.funding_key, &funding_redeemscript, self.channel_value_satoshis, &self, secp_ctx);
 		let channel_parameters = self.get_channel_parameters();
-		let htlc_sigs = trusted_tx.get_htlc_sigs(&self.htlc_base_key, &channel_parameters.as_holder_broadcastable(), secp_ctx)?;
+		let htlc_sigs = trusted_tx.get_htlc_sigs(&self.htlc_base_key, &channel_parameters.as_holder_broadcastable(), &self, secp_ctx)?;
 		Ok((sig, htlc_sigs))
 	}
 
@@ -826,7 +862,7 @@ impl EcdsaChannelSigner for InMemorySigner {
 		};
 		let mut sighash_parts = sighash::SighashCache::new(justice_tx);
 		let sighash = hash_to_message!(&sighash_parts.segwit_signature_hash(input, &witness_script, amount, EcdsaSighashType::All).unwrap()[..]);
-		return Ok(sign(secp_ctx, &sighash, &revocation_key))
+		return Ok(sign_with_aux_rand(secp_ctx, &sighash, &revocation_key, &self))
 	}
 
 	fn sign_justice_revoked_htlc(&self, justice_tx: &Transaction, input: usize, amount: u64, per_commitment_key: &SecretKey, htlc: &HTLCOutputInCommitment, secp_ctx: &Secp256k1<secp256k1::All>) -> Result<Signature, ()> {
@@ -840,7 +876,7 @@ impl EcdsaChannelSigner for InMemorySigner {
 		};
 		let mut sighash_parts = sighash::SighashCache::new(justice_tx);
 		let sighash = hash_to_message!(&sighash_parts.segwit_signature_hash(input, &witness_script, amount, EcdsaSighashType::All).unwrap()[..]);
-		return Ok(sign(secp_ctx, &sighash, &revocation_key))
+		return Ok(sign_with_aux_rand(secp_ctx, &sighash, &revocation_key, &self))
 	}
 
 	#[cfg(anchors)]
@@ -858,7 +894,7 @@ impl EcdsaChannelSigner for InMemorySigner {
 		let our_htlc_private_key = chan_utils::derive_private_key(
 			&secp_ctx, &per_commitment_point, &self.htlc_base_key
 		);
-		Ok(sign(&secp_ctx, &hash_to_message!(sighash), &our_htlc_private_key))
+		Ok(sign_with_aux_rand(&secp_ctx, &hash_to_message!(sighash), &our_htlc_private_key, &self))
 	}
 
 	fn sign_counterparty_htlc_transaction(&self, htlc_tx: &Transaction, input: usize, amount: u64, per_commitment_point: &PublicKey, htlc: &HTLCOutputInCommitment, secp_ctx: &Secp256k1<secp256k1::All>) -> Result<Signature, ()> {
@@ -869,7 +905,7 @@ impl EcdsaChannelSigner for InMemorySigner {
 		let witness_script = chan_utils::get_htlc_redeemscript_with_explicit_keys(&htlc, self.opt_anchors(), &counterparty_htlcpubkey, &htlcpubkey, &revocation_pubkey);
 		let mut sighash_parts = sighash::SighashCache::new(htlc_tx);
 		let sighash = hash_to_message!(&sighash_parts.segwit_signature_hash(input, &witness_script, amount, EcdsaSighashType::All).unwrap()[..]);
-		Ok(sign(secp_ctx, &sighash, &htlc_key))
+		Ok(sign_with_aux_rand(secp_ctx, &sighash, &htlc_key, &self))
 	}
 
 	fn sign_closing_transaction(&self, closing_tx: &ClosingTransaction, secp_ctx: &Secp256k1<secp256k1::All>) -> Result<Signature, ()> {
@@ -885,14 +921,14 @@ impl EcdsaChannelSigner for InMemorySigner {
 		let sighash = sighash::SighashCache::new(&*anchor_tx).segwit_signature_hash(
 			input, &witness_script, ANCHOR_OUTPUT_VALUE_SATOSHI, EcdsaSighashType::All,
 		).unwrap();
-		Ok(sign(secp_ctx, &hash_to_message!(&sighash[..]), &self.funding_key))
+		Ok(sign_with_aux_rand(secp_ctx, &hash_to_message!(&sighash[..]), &self.funding_key, &self))
 	}
 
 	fn sign_channel_announcement_with_funding_key(
 		&self, msg: &UnsignedChannelAnnouncement, secp_ctx: &Secp256k1<secp256k1::All>
 	) -> Result<Signature, ()> {
 		let msghash = hash_to_message!(&Sha256dHash::hash(&msg.encode()[..])[..]);
-		Ok(sign(secp_ctx, &msghash, &self.funding_key))
+		Ok(secp_ctx.sign_ecdsa(&msghash, &self.funding_key))
 	}
 }
 
@@ -922,8 +958,8 @@ impl Writeable for InMemorySigner {
 	}
 }
 
-impl Readable for InMemorySigner {
-	fn read<R: io::Read>(reader: &mut R) -> Result<Self, DecodeError> {
+impl<ES: Deref> ReadableArgs<ES> for InMemorySigner where ES::Target: EntropySource {
+	fn read<R: io::Read>(reader: &mut R, entropy_source: ES) -> Result<Self, DecodeError> {
 		let _ver = read_ver_prefix!(reader, SERIALIZATION_VERSION);
 
 		let funding_key = Readable::read(reader)?;
@@ -953,6 +989,8 @@ impl Readable for InMemorySigner {
 			holder_channel_pubkeys,
 			channel_parameters: counterparty_channel_data,
 			channel_keys_id: keys_id,
+			rand_bytes_unique_start: entropy_source.get_secure_random_bytes(),
+			rand_bytes_index: AtomicCounter::new(),
 		})
 	}
 }
@@ -1107,6 +1145,7 @@ impl KeysManager {
 		let payment_key = key_step!(b"payment key", revocation_base_key);
 		let delayed_payment_base_key = key_step!(b"delayed payment base key", payment_key);
 		let htlc_base_key = key_step!(b"HTLC base key", delayed_payment_base_key);
+		let prng_seed = self.get_secure_random_bytes();
 
 		InMemorySigner::new(
 			&self.secp_ctx,
@@ -1118,6 +1157,7 @@ impl KeysManager {
 			commitment_seed,
 			channel_value_satoshis,
 			params.clone(),
+			prng_seed,
 		)
 	}
 
@@ -1233,7 +1273,7 @@ impl KeysManager {
 					if payment_script != output.script_pubkey { return Err(()); };
 
 					let sighash = hash_to_message!(&sighash::SighashCache::new(&spend_tx).segwit_signature_hash(input_idx, &witness_script, output.value, EcdsaSighashType::All).unwrap()[..]);
-					let sig = sign(secp_ctx, &sighash, &secret.private_key);
+					let sig = sign_with_aux_rand(secp_ctx, &sighash, &secret.private_key, &self);
 					let mut sig_ser = sig.serialize_der().to_vec();
 					sig_ser.push(EcdsaSighashType::All as u8);
 					spend_tx.input[input_idx].witness.push(sig_ser);
@@ -1295,7 +1335,7 @@ impl NodeSigner for KeysManager {
 
 	fn sign_gossip_message(&self, msg: UnsignedGossipMessage) -> Result<Signature, ()> {
 		let msg_hash = hash_to_message!(&Sha256dHash::hash(&msg.encode()[..])[..]);
-		Ok(sign(&self.secp_ctx, &msg_hash, &self.node_secret))
+		Ok(self.secp_ctx.sign_ecdsa(&msg_hash, &self.node_secret))
 	}
 }
 
@@ -1323,7 +1363,7 @@ impl SignerProvider for KeysManager {
 	}
 
 	fn read_chan_signer(&self, reader: &[u8]) -> Result<Self::Signer, DecodeError> {
-		InMemorySigner::read(&mut io::Cursor::new(reader))
+		InMemorySigner::read(&mut io::Cursor::new(reader), self)
 	}
 
 	fn get_destination_script(&self) -> Script {