From 4ebe64fafd2ca0e17aab31fb026ea3910bcaecde Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Fri, 13 Dec 2019 01:57:45 -0500 Subject: [PATCH] Use external key signer to generate closing transaction signatures --- lightning/src/chain/keysinterface.rs | 22 ++++++-- lightning/src/ln/channel.rs | 60 +++++++++++++++------ lightning/src/util/enforcing_trait_impls.rs | 4 ++ 3 files changed, 68 insertions(+), 18 deletions(-) diff --git a/lightning/src/chain/keysinterface.rs b/lightning/src/chain/keysinterface.rs index 2a68299c..2a8910b0 100644 --- a/lightning/src/chain/keysinterface.rs +++ b/lightning/src/chain/keysinterface.rs @@ -142,7 +142,13 @@ pub trait ChannelKeys : Send { /// TODO: Document the things someone using this interface should enforce before signing. /// TODO: Add more input vars to enable better checking (preferably removing commitment_tx and /// making the callee generate it via some util function we expose)! - fn sign_remote_commitment(&self, channel_value_satoshis: u64, channel_funding_script: &Script, feerate_per_kw: u64, commitment_tx: &Transaction, keys: &TxCreationKeys, htlcs: &[&HTLCOutputInCommitment], to_self_delay: u16, secp_ctx: &Secp256k1) -> Result<(Signature, Vec), ()>; + fn sign_remote_commitment(&self, channel_value_satoshis: u64, channel_funding_redeemscript: &Script, absolute_fee: u64, commitment_tx: &Transaction, keys: &TxCreationKeys, htlcs: &[&HTLCOutputInCommitment], to_self_delay: u16, secp_ctx: &Secp256k1) -> Result<(Signature, Vec), ()>; + + /// Create a signature for a (proposed) closing transaction. + /// + /// Note that, due to rounding, there may be one "missing" satoshi, and either party may have + /// chosen to forgo their output as dust. + fn sign_closing_transaction(&self, channel_value_satoshis: u64, channel_funding_redeemscript: &Script, closing_tx: &Transaction, secp_ctx: &Secp256k1) -> Result; /// Signs a channel announcement message with our funding key, proving it comes from one /// of the channel participants. @@ -178,9 +184,9 @@ impl ChannelKeys for InMemoryChannelKeys { fn htlc_base_key(&self) -> &SecretKey { &self.htlc_base_key } fn commitment_seed(&self) -> &[u8; 32] { &self.commitment_seed } - fn sign_remote_commitment(&self, channel_value_satoshis: u64, channel_funding_script: &Script, feerate_per_kw: u64, commitment_tx: &Transaction, keys: &TxCreationKeys, htlcs: &[&HTLCOutputInCommitment], to_self_delay: u16, secp_ctx: &Secp256k1) -> Result<(Signature, Vec), ()> { + fn sign_remote_commitment(&self, channel_value_satoshis: u64, channel_funding_redeemscript: &Script, feerate_per_kw: u64, commitment_tx: &Transaction, keys: &TxCreationKeys, htlcs: &[&HTLCOutputInCommitment], to_self_delay: u16, secp_ctx: &Secp256k1) -> Result<(Signature, Vec), ()> { if commitment_tx.input.len() != 1 { return Err(()); } - let commitment_sighash = hash_to_message!(&bip143::SighashComponents::new(&commitment_tx).sighash_all(&commitment_tx.input[0], &channel_funding_script, channel_value_satoshis)[..]); + let commitment_sighash = hash_to_message!(&bip143::SighashComponents::new(&commitment_tx).sighash_all(&commitment_tx.input[0], &channel_funding_redeemscript, channel_value_satoshis)[..]); let commitment_sig = secp_ctx.sign(&commitment_sighash, &self.funding_key); let commitment_txid = commitment_tx.txid(); @@ -202,6 +208,16 @@ impl ChannelKeys for InMemoryChannelKeys { Ok((commitment_sig, htlc_sigs)) } + fn sign_closing_transaction(&self, channel_value_satoshis: u64, channel_funding_redeemscript: &Script, closing_tx: &Transaction, secp_ctx: &Secp256k1) -> Result { + if closing_tx.input.len() != 1 { return Err(()); } + if closing_tx.input[0].witness.len() != 0 { return Err(()); } + if closing_tx.output.len() > 2 { return Err(()); } + + let sighash = hash_to_message!(&bip143::SighashComponents::new(closing_tx) + .sighash_all(&closing_tx.input[0], &channel_funding_redeemscript, channel_value_satoshis)[..]); + Ok(secp_ctx.sign(&sighash, &self.funding_key)) + } + fn sign_channel_announcement(&self, msg: &msgs::UnsignedChannelAnnouncement, secp_ctx: &Secp256k1) -> Result { let msghash = hash_to_message!(&Sha256dHash::hash(&msg.encode()[..])[..]); Ok(secp_ctx.sign(&msghash, &self.funding_key)) diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index c7f074ff..f1330fb9 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -303,7 +303,7 @@ pub(super) struct Channel { #[cfg(not(test))] last_local_commitment_txn: Vec, - last_sent_closing_fee: Option<(u64, u64)>, // (feerate, fee) + last_sent_closing_fee: Option<(u64, u64, Signature)>, // (feerate, fee, our_sig) /// The hash of the block in which the funding transaction reached our CONF_TARGET. We use this /// to detect unconfirmation after a serialize-unserialize roundtrip where we may not see a full @@ -2665,14 +2665,16 @@ impl Channel { let proposed_total_fee_satoshis = proposed_feerate * tx_weight / 1000; let (closing_tx, total_fee_satoshis) = self.build_closing_transaction(proposed_total_fee_satoshis, false); - let funding_redeemscript = self.get_funding_redeemscript(); - let 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.local_keys + .sign_closing_transaction(self.channel_value_satoshis, &self.get_funding_redeemscript(), &closing_tx, &self.secp_ctx) + .ok(); + if our_sig.is_none() { return None; } - self.last_sent_closing_fee = Some((proposed_feerate, total_fee_satoshis)); + self.last_sent_closing_fee = Some((proposed_feerate, total_fee_satoshis, our_sig.clone().unwrap())); Some(msgs::ClosingSigned { channel_id: self.channel_id, fee_satoshis: total_fee_satoshis, - signature: self.secp_ctx.sign(&sighash, &self.local_keys.funding_key()), + signature: our_sig.unwrap(), }) } @@ -2749,6 +2751,28 @@ impl Channel { Ok((our_shutdown, self.maybe_propose_first_closing_signed(fee_estimator), dropped_outbound_htlcs)) } + fn build_signed_closing_transaction(&self, tx: &mut Transaction, their_sig: &Signature, our_sig: &Signature) { + if tx.input.len() != 1 { panic!("Tried to sign closing transaction that had input count != 1!"); } + if tx.input[0].witness.len() != 0 { panic!("Tried to re-sign closing transaction"); } + if tx.output.len() > 2 { panic!("Tried to sign bogus closing transaction"); } + + tx.input[0].witness.push(Vec::new()); // First is the multisig dummy + + let our_funding_key = PublicKey::from_secret_key(&self.secp_ctx, self.local_keys.funding_key()).serialize(); + let their_funding_key = self.their_funding_pubkey.unwrap().serialize(); + if our_funding_key[..] < their_funding_key[..] { + tx.input[0].witness.push(our_sig.serialize_der().to_vec()); + tx.input[0].witness.push(their_sig.serialize_der().to_vec()); + } else { + tx.input[0].witness.push(their_sig.serialize_der().to_vec()); + tx.input[0].witness.push(our_sig.serialize_der().to_vec()); + } + tx.input[0].witness[1].push(SigHashType::All as u8); + tx.input[0].witness[2].push(SigHashType::All as u8); + + tx.input[0].witness.push(self.get_funding_redeemscript().into_bytes()); + } + pub fn closing_signed(&mut self, fee_estimator: &FeeEstimator, msg: &msgs::ClosingSigned) -> Result<(Option, Option), ChannelError> { if self.channel_state & BOTH_SIDES_SHUTDOWN_MASK != BOTH_SIDES_SHUTDOWN_MASK { return Err(ChannelError::Close("Remote end sent us a closing_signed before both sides provided a shutdown")); @@ -2781,9 +2805,9 @@ impl Channel { }, }; - if let Some((_, last_fee)) = self.last_sent_closing_fee { + if let Some((_, last_fee, our_sig)) = self.last_sent_closing_fee { if last_fee == msg.fee_satoshis { - self.sign_commitment_transaction(&mut closing_tx, &msg.signature); + self.build_signed_closing_transaction(&mut closing_tx, &msg.signature, &our_sig); self.channel_state = ChannelState::ShutdownComplete as u32; self.channel_update_count += 1; return Ok((None, Some(closing_tx))); @@ -2794,9 +2818,10 @@ 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 = 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)); + let our_sig = self.local_keys + .sign_closing_transaction(self.channel_value_satoshis, &funding_redeemscript, &closing_tx, &self.secp_ctx) + .map_err(|_| ChannelError::Close("External signer refused to sign closing transaction"))?; + self.last_sent_closing_fee = Some(($new_feerate, used_total_fee, our_sig.clone())); return Ok((Some(msgs::ClosingSigned { channel_id: self.channel_id, fee_satoshis: used_total_fee, @@ -2809,7 +2834,7 @@ impl Channel { if self.channel_outbound { let our_max_feerate = fee_estimator.get_est_sat_per_1000_weight(ConfirmationTarget::Normal); if proposed_sat_per_kw > our_max_feerate { - if let Some((last_feerate, _)) = self.last_sent_closing_fee { + if let Some((last_feerate, _, _)) = self.last_sent_closing_fee { if our_max_feerate <= last_feerate { return Err(ChannelError::Close("Unable to come to consensus about closing feerate, remote wanted something higher than our Normal feerate")); } @@ -2819,7 +2844,7 @@ impl Channel { } else { let our_min_feerate = fee_estimator.get_est_sat_per_1000_weight(ConfirmationTarget::Background); if proposed_sat_per_kw < our_min_feerate { - if let Some((last_feerate, _)) = self.last_sent_closing_fee { + if let Some((last_feerate, _, _)) = self.last_sent_closing_fee { if our_min_feerate >= last_feerate { return Err(ChannelError::Close("Unable to come to consensus about closing feerate, remote wanted something lower than our Background feerate")); } @@ -2828,7 +2853,11 @@ impl Channel { } } - let our_sig = self.sign_commitment_transaction(&mut closing_tx, &msg.signature); + let our_sig = self.local_keys + .sign_closing_transaction(self.channel_value_satoshis, &funding_redeemscript, &closing_tx, &self.secp_ctx) + .map_err(|_| ChannelError::Close("External signer refused to sign closing transaction"))?; + self.build_signed_closing_transaction(&mut closing_tx, &msg.signature, &our_sig); + self.channel_state = ChannelState::ShutdownComplete as u32; self.channel_update_count += 1; @@ -3855,10 +3884,11 @@ impl Writeable for Channel { } match self.last_sent_closing_fee { - Some((feerate, fee)) => { + Some((feerate, fee, sig)) => { 1u8.write(writer)?; feerate.write(writer)?; fee.write(writer)?; + sig.write(writer)?; }, None => 0u8.write(writer)?, } @@ -4022,7 +4052,7 @@ impl> ReadableArgs>::read(reader)? { 0 => None, - 1 => Some((Readable::read(reader)?, Readable::read(reader)?)), + 1 => Some((Readable::read(reader)?, Readable::read(reader)?, Readable::read(reader)?)), _ => return Err(DecodeError::InvalidValue), }; diff --git a/lightning/src/util/enforcing_trait_impls.rs b/lightning/src/util/enforcing_trait_impls.rs index 0415351c..e9c82c1b 100644 --- a/lightning/src/util/enforcing_trait_impls.rs +++ b/lightning/src/util/enforcing_trait_impls.rs @@ -52,6 +52,10 @@ impl ChannelKeys for EnforcingChannelKeys { Ok(self.inner.sign_remote_commitment(channel_value_satoshis, channel_funding_script, feerate_per_kw, commitment_tx, keys, htlcs, to_self_delay, secp_ctx).unwrap()) } + fn sign_closing_transaction(&self, channel_value_satoshis: u64, channel_funding_redeemscript: &Script, closing_tx: &Transaction, secp_ctx: &Secp256k1) -> Result { + Ok(self.inner.sign_closing_transaction(channel_value_satoshis, channel_funding_redeemscript, closing_tx, secp_ctx).unwrap()) + } + fn sign_channel_announcement(&self, msg: &msgs::UnsignedChannelAnnouncement, secp_ctx: &Secp256k1) -> Result { self.inner.sign_channel_announcement(msg, secp_ctx) } -- 2.30.2