Ensure Message always unwraps in fuzztarget

Hashes cant be all-0s, so we can normally unwrap, but fuzztarget
can generate all-0 hashes, so we have to handle it and swap for
something else.
This commit is contained in:
Matt Corallo 2019-01-17 17:36:49 -05:00
parent f109d13e22
commit 8678bda576
6 changed files with 48 additions and 28 deletions

View file

@ -11,7 +11,7 @@ use bitcoin_hashes::sha256::Hash as Sha256;
use bitcoin_hashes::hash160::Hash as Hash160;
use secp256k1::key::{PublicKey,SecretKey};
use secp256k1::{Secp256k1,Message,Signature};
use secp256k1::{Secp256k1,Signature};
use secp256k1;
use ln::msgs;
@ -1067,7 +1067,7 @@ impl Channel {
let funding_redeemscript = self.get_funding_redeemscript();
let sighash = Message::from_slice(&bip143::SighashComponents::new(&tx).sighash_all(&tx.input[0], &funding_redeemscript, self.channel_value_satoshis)[..]).unwrap();
let sighash = hash_to_message!(&bip143::SighashComponents::new(&tx).sighash_all(&tx.input[0], &funding_redeemscript, self.channel_value_satoshis)[..]);
let our_sig = self.secp_ctx.sign(&sighash, &self.local_keys.funding_key);
tx.input[0].witness.push(Vec::new()); // First is the multisig dummy
@ -1104,7 +1104,7 @@ impl Channel {
let htlc_redeemscript = chan_utils::get_htlc_redeemscript(&htlc, &keys);
let our_htlc_key = secp_check!(chan_utils::derive_private_key(&self.secp_ctx, &keys.per_commitment_point, &self.local_keys.htlc_base_key), "Derived invalid key, peer is maliciously selecting parameters");
let sighash = Message::from_slice(&bip143::SighashComponents::new(&tx).sighash_all(&tx.input[0], &htlc_redeemscript, htlc.amount_msat / 1000)[..]).unwrap();
let sighash = hash_to_message!(&bip143::SighashComponents::new(&tx).sighash_all(&tx.input[0], &htlc_redeemscript, htlc.amount_msat / 1000)[..]);
let is_local_tx = PublicKey::from_secret_key(&self.secp_ctx, &our_htlc_key) == keys.a_htlc_key;
Ok((htlc_redeemscript, self.secp_ctx.sign(&sighash, &our_htlc_key), is_local_tx))
}
@ -1408,7 +1408,7 @@ impl Channel {
let local_keys = self.build_local_transaction_keys(self.cur_local_commitment_transaction_number)?;
let mut local_initial_commitment_tx = self.build_commitment_transaction(self.cur_local_commitment_transaction_number, &local_keys, true, false, self.feerate_per_kw).0;
let local_sighash = Message::from_slice(&bip143::SighashComponents::new(&local_initial_commitment_tx).sighash_all(&local_initial_commitment_tx.input[0], &funding_script, self.channel_value_satoshis)[..]).unwrap();
let local_sighash = hash_to_message!(&bip143::SighashComponents::new(&local_initial_commitment_tx).sighash_all(&local_initial_commitment_tx.input[0], &funding_script, self.channel_value_satoshis)[..]);
// They sign the "local" commitment transaction...
secp_check!(self.secp_ctx.verify(&local_sighash, &sig, &self.their_funding_pubkey.unwrap()), "Invalid funding_created signature from peer");
@ -1418,7 +1418,7 @@ impl Channel {
let remote_keys = self.build_remote_transaction_keys()?;
let remote_initial_commitment_tx = self.build_commitment_transaction(self.cur_remote_commitment_transaction_number, &remote_keys, false, false, self.feerate_per_kw).0;
let remote_sighash = Message::from_slice(&bip143::SighashComponents::new(&remote_initial_commitment_tx).sighash_all(&remote_initial_commitment_tx.input[0], &funding_script, self.channel_value_satoshis)[..]).unwrap();
let remote_sighash = hash_to_message!(&bip143::SighashComponents::new(&remote_initial_commitment_tx).sighash_all(&remote_initial_commitment_tx.input[0], &funding_script, self.channel_value_satoshis)[..]);
// We sign the "remote" commitment transaction, allowing them to broadcast the tx if they wish.
Ok((remote_initial_commitment_tx, local_initial_commitment_tx, self.secp_ctx.sign(&remote_sighash, &self.local_keys.funding_key), local_keys))
@ -1487,7 +1487,7 @@ impl Channel {
let local_keys = self.build_local_transaction_keys(self.cur_local_commitment_transaction_number)?;
let mut local_initial_commitment_tx = self.build_commitment_transaction(self.cur_local_commitment_transaction_number, &local_keys, true, false, self.feerate_per_kw).0;
let local_sighash = Message::from_slice(&bip143::SighashComponents::new(&local_initial_commitment_tx).sighash_all(&local_initial_commitment_tx.input[0], &funding_script, self.channel_value_satoshis)[..]).unwrap();
let local_sighash = hash_to_message!(&bip143::SighashComponents::new(&local_initial_commitment_tx).sighash_all(&local_initial_commitment_tx.input[0], &funding_script, self.channel_value_satoshis)[..]);
// They sign the "local" commitment transaction, allowing us to broadcast the tx if we wish.
secp_check!(self.secp_ctx.verify(&local_sighash, &msg.signature, &self.their_funding_pubkey.unwrap()), "Invalid funding_signed signature from peer");
@ -1699,7 +1699,7 @@ impl Channel {
(commitment_tx.0, commitment_tx.1, htlcs_cloned)
};
let local_commitment_txid = local_commitment_tx.0.txid();
let local_sighash = Message::from_slice(&bip143::SighashComponents::new(&local_commitment_tx.0).sighash_all(&local_commitment_tx.0.input[0], &funding_script, self.channel_value_satoshis)[..]).unwrap();
let local_sighash = hash_to_message!(&bip143::SighashComponents::new(&local_commitment_tx.0).sighash_all(&local_commitment_tx.0.input[0], &funding_script, self.channel_value_satoshis)[..]);
secp_check!(self.secp_ctx.verify(&local_sighash, &msg.signature, &self.their_funding_pubkey.unwrap()), "Invalid commitment tx signature from peer");
//If channel fee was updated by funder confirm funder can afford the new fee rate when applied to the current local commitment transaction
@ -1725,7 +1725,7 @@ impl Channel {
if let Some(_) = htlc.transaction_output_index {
let mut htlc_tx = self.build_htlc_transaction(&local_commitment_txid, &htlc, true, &local_keys, feerate_per_kw);
let htlc_redeemscript = chan_utils::get_htlc_redeemscript(&htlc, &local_keys);
let htlc_sighash = Message::from_slice(&bip143::SighashComponents::new(&htlc_tx).sighash_all(&htlc_tx.input[0], &htlc_redeemscript, htlc.amount_msat / 1000)[..]).unwrap();
let htlc_sighash = hash_to_message!(&bip143::SighashComponents::new(&htlc_tx).sighash_all(&htlc_tx.input[0], &htlc_redeemscript, htlc.amount_msat / 1000)[..]);
secp_check!(self.secp_ctx.verify(&htlc_sighash, &msg.htlc_signatures[idx], &local_keys.b_htlc_key), "Invalid HTLC tx signature from peer");
let htlc_sig = if htlc.offered {
let htlc_sig = self.sign_htlc_transaction(&mut htlc_tx, &msg.htlc_signatures[idx], &None, &htlc, &local_keys)?;
@ -2456,7 +2456,7 @@ impl Channel {
let (closing_tx, total_fee_satoshis) = self.build_closing_transaction(proposed_total_fee_satoshis, false);
let funding_redeemscript = self.get_funding_redeemscript();
let sighash = Message::from_slice(&bip143::SighashComponents::new(&closing_tx).sighash_all(&closing_tx.input[0], &funding_redeemscript, self.channel_value_satoshis)[..]).unwrap();
let sighash = hash_to_message!(&bip143::SighashComponents::new(&closing_tx).sighash_all(&closing_tx.input[0], &funding_redeemscript, self.channel_value_satoshis)[..]);
self.last_sent_closing_fee = Some((proposed_feerate, total_fee_satoshis));
Some(msgs::ClosingSigned {
@ -2558,7 +2558,7 @@ impl Channel {
if used_total_fee != msg.fee_satoshis {
return Err(ChannelError::Close("Remote sent us a closing_signed with a fee greater than the value they can claim"));
}
let mut sighash = Message::from_slice(&bip143::SighashComponents::new(&closing_tx).sighash_all(&closing_tx.input[0], &funding_redeemscript, self.channel_value_satoshis)[..]).unwrap();
let mut sighash = hash_to_message!(&bip143::SighashComponents::new(&closing_tx).sighash_all(&closing_tx.input[0], &funding_redeemscript, self.channel_value_satoshis)[..]);
match self.secp_ctx.verify(&sighash, &msg.signature, &self.their_funding_pubkey.unwrap()) {
Ok(_) => {},
@ -2566,7 +2566,7 @@ impl Channel {
// The remote end may have decided to revoke their output due to inconsistent dust
// limits, so check for that case by re-checking the signature here.
closing_tx = self.build_closing_transaction(msg.fee_satoshis, true).0;
sighash = Message::from_slice(&bip143::SighashComponents::new(&closing_tx).sighash_all(&closing_tx.input[0], &funding_redeemscript, self.channel_value_satoshis)[..]).unwrap();
sighash = hash_to_message!(&bip143::SighashComponents::new(&closing_tx).sighash_all(&closing_tx.input[0], &funding_redeemscript, self.channel_value_satoshis)[..]);
secp_check!(self.secp_ctx.verify(&sighash, &msg.signature, &self.their_funding_pubkey.unwrap()), "Invalid closing tx signature from peer");
},
};
@ -2584,7 +2584,7 @@ impl Channel {
($new_feerate: expr) => {
let closing_tx_max_weight = Self::get_closing_transaction_weight(&self.get_closing_scriptpubkey(), self.their_shutdown_scriptpubkey.as_ref().unwrap());
let (closing_tx, used_total_fee) = self.build_closing_transaction($new_feerate * closing_tx_max_weight / 1000, false);
sighash = Message::from_slice(&bip143::SighashComponents::new(&closing_tx).sighash_all(&closing_tx.input[0], &funding_redeemscript, self.channel_value_satoshis)[..]).unwrap();
sighash = hash_to_message!(&bip143::SighashComponents::new(&closing_tx).sighash_all(&closing_tx.input[0], &funding_redeemscript, self.channel_value_satoshis)[..]);
let our_sig = self.secp_ctx.sign(&sighash, &self.local_keys.funding_key);
self.last_sent_closing_fee = Some(($new_feerate, used_total_fee));
return Ok((Some(msgs::ClosingSigned {
@ -2993,7 +2993,7 @@ impl Channel {
let remote_keys = self.build_remote_transaction_keys()?;
let remote_initial_commitment_tx = self.build_commitment_transaction(self.cur_remote_commitment_transaction_number, &remote_keys, false, false, self.feerate_per_kw).0;
let remote_sighash = Message::from_slice(&bip143::SighashComponents::new(&remote_initial_commitment_tx).sighash_all(&remote_initial_commitment_tx.input[0], &funding_script, self.channel_value_satoshis)[..]).unwrap();
let remote_sighash = hash_to_message!(&bip143::SighashComponents::new(&remote_initial_commitment_tx).sighash_all(&remote_initial_commitment_tx.input[0], &funding_script, self.channel_value_satoshis)[..]);
// We sign the "remote" commitment transaction, allowing them to broadcast the tx if they wish.
Ok((self.secp_ctx.sign(&remote_sighash, &self.local_keys.funding_key), remote_initial_commitment_tx))
@ -3080,7 +3080,7 @@ impl Channel {
excess_data: Vec::new(),
};
let msghash = Message::from_slice(&Sha256dHash::from_data(&msg.encode()[..])[..]).unwrap();
let msghash = hash_to_message!(&Sha256dHash::from_data(&msg.encode()[..])[..]);
let sig = self.secp_ctx.sign(&msghash, &self.local_keys.funding_key);
Ok((msg, sig))
@ -3295,7 +3295,7 @@ impl Channel {
let remote_keys = self.build_remote_transaction_keys()?;
let remote_commitment_tx = self.build_commitment_transaction(self.cur_remote_commitment_transaction_number, &remote_keys, false, true, feerate_per_kw);
let remote_commitment_txid = remote_commitment_tx.0.txid();
let remote_sighash = Message::from_slice(&bip143::SighashComponents::new(&remote_commitment_tx.0).sighash_all(&remote_commitment_tx.0.input[0], &funding_script, self.channel_value_satoshis)[..]).unwrap();
let remote_sighash = hash_to_message!(&bip143::SighashComponents::new(&remote_commitment_tx.0).sighash_all(&remote_commitment_tx.0.input[0], &funding_script, self.channel_value_satoshis)[..]);
let our_sig = self.secp_ctx.sign(&remote_sighash, &self.local_keys.funding_key);
let mut htlc_sigs = Vec::with_capacity(remote_commitment_tx.1);
@ -3303,7 +3303,7 @@ impl Channel {
if let Some(_) = htlc.transaction_output_index {
let htlc_tx = self.build_htlc_transaction(&remote_commitment_txid, htlc, false, &remote_keys, feerate_per_kw);
let htlc_redeemscript = chan_utils::get_htlc_redeemscript(&htlc, &remote_keys);
let htlc_sighash = Message::from_slice(&bip143::SighashComponents::new(&htlc_tx).sighash_all(&htlc_tx.input[0], &htlc_redeemscript, htlc.amount_msat / 1000)[..]).unwrap();
let htlc_sighash = hash_to_message!(&bip143::SighashComponents::new(&htlc_tx).sighash_all(&htlc_tx.input[0], &htlc_redeemscript, htlc.amount_msat / 1000)[..]);
let our_htlc_key = secp_check!(chan_utils::derive_private_key(&self.secp_ctx, &remote_keys.per_commitment_point, &self.local_keys.htlc_base_key), "Derived invalid key, peer is maliciously selecting parameters");
htlc_sigs.push(self.secp_ctx.sign(&htlc_sighash, &our_htlc_key));
}

View file

@ -20,7 +20,7 @@ use bitcoin_hashes::sha256::Hash as Sha256;
use bitcoin_hashes::cmp::fixed_time_eq;
use secp256k1::key::{SecretKey,PublicKey};
use secp256k1::{Secp256k1,Message};
use secp256k1::Secp256k1;
use secp256k1::ecdh::SharedSecret;
use secp256k1;
@ -937,7 +937,7 @@ impl ChannelManager {
};
let msg_hash = Sha256dHash::from_data(&unsigned.encode()[..]);
let sig = self.secp_ctx.sign(&Message::from_slice(&msg_hash[..]).unwrap(), &self.our_network_key);
let sig = self.secp_ctx.sign(&hash_to_message!(&msg_hash[..]), &self.our_network_key);
Ok(msgs::ChannelUpdate {
signature: sig,
@ -1130,7 +1130,7 @@ impl ChannelManager {
Ok(res) => res,
Err(_) => return None, // Only in case of state precondition violations eg channel is closing
};
let msghash = Message::from_slice(&Sha256dHash::from_data(&announcement.encode()[..])[..]).unwrap();
let msghash = hash_to_message!(&Sha256dHash::from_data(&announcement.encode()[..])[..]);
let our_node_sig = self.secp_ctx.sign(&msghash, &self.our_network_key);
Some(msgs::AnnouncementSignatures {
@ -2101,7 +2101,7 @@ impl ChannelManager {
try_chan_entry!(self, chan.get_mut().get_channel_announcement(our_node_id.clone(), self.genesis_hash.clone()), channel_state, chan);
let were_node_one = announcement.node_id_1 == our_node_id;
let msghash = Message::from_slice(&Sha256dHash::from_data(&announcement.encode()[..])[..]).unwrap();
let msghash = hash_to_message!(&Sha256dHash::from_data(&announcement.encode()[..])[..]);
if self.secp_ctx.verify(&msghash, &msg.node_signature, if were_node_one { &announcement.node_id_2 } else { &announcement.node_id_1 }).is_err() ||
self.secp_ctx.verify(&msghash, &msg.bitcoin_signature, if were_node_one { &announcement.bitcoin_key_2 } else { &announcement.bitcoin_key_1 }).is_err() {
try_chan_entry!(self, Err(ChannelError::Close("Bad announcement_signatures node_signature")), channel_state, chan);

View file

@ -24,7 +24,7 @@ use bitcoin_hashes::Hash;
use bitcoin_hashes::sha256::Hash as Sha256;
use bitcoin_hashes::hash160::Hash as Hash160;
use secp256k1::{Secp256k1,Message,Signature};
use secp256k1::{Secp256k1,Signature};
use secp256k1::key::{SecretKey,PublicKey};
use secp256k1;
@ -1105,7 +1105,7 @@ impl ChannelMonitor {
let htlc = &per_commitment_option.unwrap()[$htlc_idx.unwrap()].0;
chan_utils::get_htlc_redeemscript_with_explicit_keys(htlc, &a_htlc_key, &b_htlc_key, &revocation_pubkey)
};
let sighash = ignore_error!(Message::from_slice(&$sighash_parts.sighash_all(&$input, &redeemscript, $amount)[..]));
let sighash = hash_to_message!(&$sighash_parts.sighash_all(&$input, &redeemscript, $amount)[..]);
let revocation_key = ignore_error!(chan_utils::derive_private_revocation_key(&self.secp_ctx, &per_commitment_key, &revocation_base_key));
(self.secp_ctx.sign(&sighash, &revocation_key), redeemscript)
},
@ -1327,7 +1327,7 @@ impl ChannelMonitor {
Storage::Local { ref htlc_base_key, .. } => {
let htlc = &per_commitment_option.unwrap()[$input.sequence as usize].0;
let redeemscript = chan_utils::get_htlc_redeemscript_with_explicit_keys(htlc, &a_htlc_key, &b_htlc_key, &revocation_pubkey);
let sighash = ignore_error!(Message::from_slice(&$sighash_parts.sighash_all(&$input, &redeemscript, $amount)[..]));
let sighash = hash_to_message!(&$sighash_parts.sighash_all(&$input, &redeemscript, $amount)[..]);
let htlc_key = ignore_error!(chan_utils::derive_private_key(&self.secp_ctx, revocation_point, &htlc_base_key));
(self.secp_ctx.sign(&sighash, &htlc_key), redeemscript)
},
@ -1512,7 +1512,7 @@ impl ChannelMonitor {
let sig = match self.key_storage {
Storage::Local { ref revocation_base_key, .. } => {
let sighash = ignore_error!(Message::from_slice(&sighash_parts.sighash_all(&spend_tx.input[0], &redeemscript, amount)[..]));
let sighash = hash_to_message!(&sighash_parts.sighash_all(&spend_tx.input[0], &redeemscript, amount)[..]);
let revocation_key = ignore_error!(chan_utils::derive_private_revocation_key(&self.secp_ctx, &per_commitment_key, &revocation_base_key));
self.secp_ctx.sign(&sighash, &revocation_key)
}

View file

@ -4,7 +4,7 @@
//! interrogate it to get routes for your own payments.
use secp256k1::key::PublicKey;
use secp256k1::{Secp256k1,Message};
use secp256k1::Secp256k1;
use secp256k1;
use bitcoin::util::hash::Sha256dHash;
@ -239,7 +239,7 @@ macro_rules! secp_verify_sig {
impl RoutingMessageHandler for Router {
fn handle_node_announcement(&self, msg: &msgs::NodeAnnouncement) -> Result<bool, HandleError> {
let msg_hash = Message::from_slice(&Sha256dHash::from_data(&msg.contents.encode()[..])[..]).unwrap();
let msg_hash = hash_to_message!(&Sha256dHash::from_data(&msg.contents.encode()[..])[..]);
secp_verify_sig!(self.secp_ctx, &msg_hash, &msg.signature, &msg.contents.node_id);
if msg.contents.features.requires_unknown_bits() {
@ -272,7 +272,7 @@ impl RoutingMessageHandler for Router {
return Err(HandleError{err: "Channel announcement node had a channel with itself", action: Some(ErrorAction::IgnoreError)});
}
let msg_hash = Message::from_slice(&Sha256dHash::from_data(&msg.contents.encode()[..])[..]).unwrap();
let msg_hash = hash_to_message!(&Sha256dHash::from_data(&msg.contents.encode()[..])[..]);
secp_verify_sig!(self.secp_ctx, &msg_hash, &msg.node_signature_1, &msg.contents.node_id_1);
secp_verify_sig!(self.secp_ctx, &msg_hash, &msg.node_signature_2, &msg.contents.node_id_2);
secp_verify_sig!(self.secp_ctx, &msg_hash, &msg.bitcoin_signature_1, &msg.contents.bitcoin_key_1);
@ -448,7 +448,7 @@ impl RoutingMessageHandler for Router {
};
}
}
let msg_hash = Message::from_slice(&Sha256dHash::from_data(&msg.contents.encode()[..])[..]).unwrap();
let msg_hash = hash_to_message!(&Sha256dHash::from_data(&msg.contents.encode()[..])[..]);
if msg.contents.flags & 1 == 1 {
dest_node_id = channel.one_to_two.src_node_id.clone();
secp_verify_sig!(self.secp_ctx, &msg_hash, &msg.signature, &channel.two_to_one.src_node_id);

17
src/util/fuzz_wrappers.rs Normal file
View file

@ -0,0 +1,17 @@
macro_rules! hash_to_message {
($slice: expr) => {
{
#[cfg(not(feature = "fuzztarget"))]
{
::secp256k1::Message::from_slice($slice).unwrap()
}
#[cfg(feature = "fuzztarget")]
{
match ::secp256k1::Message::from_slice($slice) {
Ok(msg) => msg,
Err(_) => ::secp256k1::Message::from_slice(&[1; 32]).unwrap()
}
}
}
}
}

View file

@ -27,3 +27,6 @@ pub use self::rng::{reset_rng_state, fill_bytes};
#[cfg(test)]
pub(crate) mod test_utils;
#[macro_use]
pub(crate) mod fuzz_wrappers;