diff --git a/lightning/src/chain/keysinterface.rs b/lightning/src/chain/keysinterface.rs index 26d1f07c7c4..63c87ad21f4 100644 --- a/lightning/src/chain/keysinterface.rs +++ b/lightning/src/chain/keysinterface.rs @@ -23,7 +23,7 @@ use util::byte_utils; use util::ser::{Writeable, Writer, Readable}; use ln::chan_utils; -use ln::chan_utils::{TxCreationKeys, HTLCOutputInCommitment, make_funding_redeemscript, ChannelPublicKeys, LocalCommitmentTransaction}; +use ln::chan_utils::{HTLCOutputInCommitment, make_funding_redeemscript, ChannelPublicKeys, LocalCommitmentTransaction, PreCalculatedTxCreationKeys}; use ln::msgs; use std::sync::atomic::{AtomicUsize, Ordering}; @@ -223,7 +223,7 @@ pub trait ChannelKeys : Send+Clone { // 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, feerate_per_kw: u32, commitment_tx: &Transaction, keys: &TxCreationKeys, htlcs: &[&HTLCOutputInCommitment], secp_ctx: &Secp256k1) -> Result<(Signature, Vec), ()>; + fn sign_remote_commitment(&self, feerate_per_kw: u32, commitment_tx: &Transaction, keys: &PreCalculatedTxCreationKeys, htlcs: &[&HTLCOutputInCommitment], secp_ctx: &Secp256k1) -> Result<(Signature, Vec), ()>; /// Create a signature for a local commitment transaction. This will only ever be called with /// the same local_commitment_tx (or a copy thereof), though there are currently no guarantees @@ -461,8 +461,9 @@ impl ChannelKeys for InMemoryChannelKeys { fn pubkeys(&self) -> &ChannelPublicKeys { &self.local_channel_pubkeys } fn key_derivation_params(&self) -> (u64, u64) { self.key_derivation_params } - fn sign_remote_commitment(&self, feerate_per_kw: u32, commitment_tx: &Transaction, keys: &TxCreationKeys, htlcs: &[&HTLCOutputInCommitment], secp_ctx: &Secp256k1) -> Result<(Signature, Vec), ()> { + fn sign_remote_commitment(&self, feerate_per_kw: u32, commitment_tx: &Transaction, pre_keys: &PreCalculatedTxCreationKeys, htlcs: &[&HTLCOutputInCommitment], secp_ctx: &Secp256k1) -> Result<(Signature, Vec), ()> { if commitment_tx.input.len() != 1 { return Err(()); } + let keys = pre_keys.trust_key_derivation(); let funding_pubkey = PublicKey::from_secret_key(secp_ctx, &self.funding_key); let accepted_data = self.accepted_channel_data.as_ref().expect("must accept before signing"); diff --git a/lightning/src/ln/chan_utils.rs b/lightning/src/ln/chan_utils.rs index ba175733abe..b5d4d490774 100644 --- a/lightning/src/ln/chan_utils.rs +++ b/lightning/src/ln/chan_utils.rs @@ -267,23 +267,52 @@ pub fn derive_public_revocation_key(secp_ctx: &Secp2 /// The set of public keys which are used in the creation of one commitment transaction. /// These are derived from the channel base keys and per-commitment data. +/// +/// These keys are assumed to be good, either because the code derived them from +/// channel basepoints via the new function, or they were obtained via +/// PreCalculatedTxCreationKeys.trust_key_derivation because we trusted the source of the +/// pre-calculated keys. #[derive(PartialEq, Clone)] pub struct TxCreationKeys { /// The per-commitment public key which was used to derive the other keys. pub per_commitment_point: PublicKey, /// The revocation key which is used to allow the owner of the commitment transaction to /// provide their counterparty the ability to punish them if they broadcast an old state. - pub(crate) revocation_key: PublicKey, + pub revocation_key: PublicKey, /// A's HTLC Key - pub(crate) a_htlc_key: PublicKey, + pub a_htlc_key: PublicKey, /// B's HTLC Key - pub(crate) b_htlc_key: PublicKey, + pub b_htlc_key: PublicKey, /// A's Payment Key (which isn't allowed to be spent from for some delay) - pub(crate) a_delayed_payment_key: PublicKey, + pub a_delayed_payment_key: PublicKey, } impl_writeable!(TxCreationKeys, 33*6, { per_commitment_point, revocation_key, a_htlc_key, b_htlc_key, a_delayed_payment_key }); +/// The per-commitment point and a set of pre-calculated public keys used for transaction creation +/// in the signer. +/// The pre-calculated keys are an optimization, because ChannelKeys has enough +/// information to re-derive them. +pub struct PreCalculatedTxCreationKeys(TxCreationKeys); + +impl PreCalculatedTxCreationKeys { + /// Create a new PreCalculatedTxCreationKeys from TxCreationKeys + pub fn new(keys: TxCreationKeys) -> Self { + PreCalculatedTxCreationKeys(keys) + } + + /// The pre-calculated transaction creation public keys. + /// An external validating signer should not trust these keys. + pub fn trust_key_derivation(&self) -> &TxCreationKeys { + &self.0 + } + + /// The transaction per-commitment point + pub fn per_comitment_point(&self) -> &PublicKey { + &self.0.per_commitment_point + } +} + /// One counterparty's public keys which do not change over the life of a channel. #[derive(Clone, PartialEq)] pub struct ChannelPublicKeys { @@ -318,7 +347,8 @@ impl_writeable!(ChannelPublicKeys, 33*5, { impl TxCreationKeys { - pub(crate) fn new(secp_ctx: &Secp256k1, per_commitment_point: &PublicKey, a_delayed_payment_base: &PublicKey, a_htlc_base: &PublicKey, b_revocation_base: &PublicKey, b_htlc_base: &PublicKey) -> Result { + /// Create a new TxCreationKeys from channel base points and the per-commitment point + pub fn new(secp_ctx: &Secp256k1, per_commitment_point: &PublicKey, a_delayed_payment_base: &PublicKey, a_htlc_base: &PublicKey, b_revocation_base: &PublicKey, b_htlc_base: &PublicKey) -> Result { Ok(TxCreationKeys { per_commitment_point: per_commitment_point.clone(), revocation_key: derive_public_revocation_key(&secp_ctx, &per_commitment_point, &b_revocation_base)?, @@ -510,8 +540,7 @@ pub struct LocalCommitmentTransaction { // Which order the signatures should go in when constructing the final commitment tx witness. // The user should be able to reconstruc this themselves, so we don't bother to expose it. our_sig_first: bool, - /// The key derivation parameters for this commitment transaction - pub local_keys: TxCreationKeys, + pub(crate) local_keys: TxCreationKeys, /// The feerate paid per 1000-weight-unit in this commitment transaction. This value is /// controlled by the channel initiator. pub feerate_per_kw: u32, @@ -576,6 +605,12 @@ impl LocalCommitmentTransaction { } } + /// The pre-calculated transaction creation public keys. + /// An external validating signer should not trust these keys. + pub fn trust_key_derivation(&self) -> &TxCreationKeys { + &self.local_keys + } + /// Get the txid of the local commitment transaction contained in this /// LocalCommitmentTransaction pub fn txid(&self) -> Txid { diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index 719d03c6e37..3d3fecbdfca 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -19,7 +19,7 @@ use ln::msgs; use ln::msgs::{DecodeError, OptionalField, DataLossProtect}; use ln::channelmonitor::{ChannelMonitor, ChannelMonitorUpdate, ChannelMonitorUpdateStep, HTLC_FAIL_BACK_BUFFER}; use ln::channelmanager::{PendingHTLCStatus, HTLCSource, HTLCFailReason, HTLCFailureMsg, PendingHTLCInfo, RAACommitmentOrder, PaymentPreimage, PaymentHash, BREAKDOWN_TIMEOUT, MAX_LOCAL_BREAKDOWN_TIMEOUT}; -use ln::chan_utils::{CounterpartyCommitmentSecrets, LocalCommitmentTransaction, TxCreationKeys, HTLCOutputInCommitment, HTLC_SUCCESS_TX_WEIGHT, HTLC_TIMEOUT_TX_WEIGHT, make_funding_redeemscript, ChannelPublicKeys}; +use ln::chan_utils::{CounterpartyCommitmentSecrets, LocalCommitmentTransaction, TxCreationKeys, HTLCOutputInCommitment, HTLC_SUCCESS_TX_WEIGHT, HTLC_TIMEOUT_TX_WEIGHT, make_funding_redeemscript, ChannelPublicKeys, PreCalculatedTxCreationKeys}; use ln::chan_utils; use chain::chaininterface::{FeeEstimator,ConfirmationTarget}; use chain::transaction::OutPoint; @@ -1484,7 +1484,8 @@ 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, logger).0; - let remote_signature = self.local_keys.sign_remote_commitment(self.feerate_per_kw, &remote_initial_commitment_tx, &remote_keys, &Vec::new(), &self.secp_ctx) + let pre_remote_keys = PreCalculatedTxCreationKeys::new(remote_keys); + let remote_signature = self.local_keys.sign_remote_commitment(self.feerate_per_kw, &remote_initial_commitment_tx, &pre_remote_keys, &Vec::new(), &self.secp_ctx) .map_err(|_| ChannelError::Close("Failed to get signatures for new commitment_signed".to_owned()))?.0; // We sign the "remote" commitment transaction, allowing them to broadcast the tx if they wish. @@ -3525,7 +3526,8 @@ impl Channel { fn get_outbound_funding_created_signature(&mut self, logger: &L) -> Result where L::Target: Logger { 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, logger).0; - Ok(self.local_keys.sign_remote_commitment(self.feerate_per_kw, &remote_initial_commitment_tx, &remote_keys, &Vec::new(), &self.secp_ctx) + let pre_remote_keys = PreCalculatedTxCreationKeys::new(remote_keys); + Ok(self.local_keys.sign_remote_commitment(self.feerate_per_kw, &remote_initial_commitment_tx, &pre_remote_keys, &Vec::new(), &self.secp_ctx) .map_err(|_| ChannelError::Close("Failed to get signatures for new commitment_signed".to_owned()))?.0) } @@ -3878,10 +3880,12 @@ impl Channel { htlcs.push(htlc); } - let res = self.local_keys.sign_remote_commitment(feerate_per_kw, &remote_commitment_tx.0, &remote_keys, &htlcs, &self.secp_ctx) + let pre_remote_keys = PreCalculatedTxCreationKeys::new(remote_keys); + let res = self.local_keys.sign_remote_commitment(feerate_per_kw, &remote_commitment_tx.0, &pre_remote_keys, &htlcs, &self.secp_ctx) .map_err(|_| ChannelError::Close("Failed to get signatures for new commitment_signed".to_owned()))?; signature = res.0; htlc_signatures = res.1; + let remote_keys = pre_remote_keys.trust_key_derivation(); log_trace!(logger, "Signed remote commitment tx {} with redeemscript {} -> {}", encode::serialize_hex(&remote_commitment_tx.0), @@ -3891,7 +3895,7 @@ impl Channel { for (ref htlc_sig, ref htlc) in htlc_signatures.iter().zip(htlcs) { log_trace!(logger, "Signed remote HTLC tx {} with redeemscript {} with pubkey {} -> {}", encode::serialize_hex(&chan_utils::build_htlc_transaction(&remote_commitment_tx.0.txid(), feerate_per_kw, self.our_to_self_delay, htlc, &remote_keys.a_delayed_payment_key, &remote_keys.revocation_key)), - encode::serialize_hex(&chan_utils::get_htlc_redeemscript(&htlc, &remote_keys)), + encode::serialize_hex(&chan_utils::get_htlc_redeemscript(&htlc, remote_keys)), log_bytes!(remote_keys.a_htlc_key.serialize()), log_bytes!(htlc_sig.serialize_compact()[..])); } diff --git a/lightning/src/ln/functional_tests.rs b/lightning/src/ln/functional_tests.rs index 240a5741b91..e1254ee771e 100644 --- a/lightning/src/ln/functional_tests.rs +++ b/lightning/src/ln/functional_tests.rs @@ -52,6 +52,7 @@ use std::sync::atomic::Ordering; use std::{mem, io}; use ln::functional_test_utils::*; +use ln::chan_utils::PreCalculatedTxCreationKeys; #[test] fn test_insane_channel_opens() { @@ -1694,7 +1695,8 @@ fn test_fee_spike_violation_fails_htlc() { let local_chan_lock = nodes[0].node.channel_state.lock().unwrap(); let local_chan = local_chan_lock.by_id.get(&chan.2).unwrap(); let local_chan_keys = local_chan.get_local_keys(); - local_chan_keys.sign_remote_commitment(feerate_per_kw, &commit_tx, &commit_tx_keys, &[&accepted_htlc_info], &secp_ctx).unwrap() + let pre_commit_tx_keys = PreCalculatedTxCreationKeys::new(commit_tx_keys); + local_chan_keys.sign_remote_commitment(feerate_per_kw, &commit_tx, &pre_commit_tx_keys, &[&accepted_htlc_info], &secp_ctx).unwrap() }; let commit_signed_msg = msgs::CommitmentSigned { diff --git a/lightning/src/util/enforcing_trait_impls.rs b/lightning/src/util/enforcing_trait_impls.rs index 1864fffbbc2..9f297b48103 100644 --- a/lightning/src/util/enforcing_trait_impls.rs +++ b/lightning/src/util/enforcing_trait_impls.rs @@ -1,4 +1,4 @@ -use ln::chan_utils::{HTLCOutputInCommitment, TxCreationKeys, ChannelPublicKeys, LocalCommitmentTransaction}; +use ln::chan_utils::{HTLCOutputInCommitment, TxCreationKeys, ChannelPublicKeys, LocalCommitmentTransaction, PreCalculatedTxCreationKeys}; use ln::{chan_utils, msgs}; use chain::keysinterface::{ChannelKeys, InMemoryChannelKeys}; @@ -60,9 +60,9 @@ impl ChannelKeys for EnforcingChannelKeys { fn pubkeys(&self) -> &ChannelPublicKeys { self.inner.pubkeys() } fn key_derivation_params(&self) -> (u64, u64) { self.inner.key_derivation_params() } - fn sign_remote_commitment(&self, feerate_per_kw: u32, commitment_tx: &Transaction, keys: &TxCreationKeys, htlcs: &[&HTLCOutputInCommitment], secp_ctx: &Secp256k1) -> Result<(Signature, Vec), ()> { + fn sign_remote_commitment(&self, feerate_per_kw: u32, commitment_tx: &Transaction, pre_keys: &PreCalculatedTxCreationKeys, htlcs: &[&HTLCOutputInCommitment], secp_ctx: &Secp256k1) -> Result<(Signature, Vec), ()> { if commitment_tx.input.len() != 1 { panic!("lightning commitment transactions have a single input"); } - self.check_keys(secp_ctx, keys); + self.check_keys(secp_ctx, pre_keys.trust_key_derivation()); let obscured_commitment_transaction_number = (commitment_tx.lock_time & 0xffffff) as u64 | ((commitment_tx.input[0].sequence as u64 & 0xffffff) << 3*8); { @@ -75,7 +75,7 @@ impl ChannelKeys for EnforcingChannelKeys { commitment_data.1 = cmp::max(commitment_number, commitment_data.1) } - Ok(self.inner.sign_remote_commitment(feerate_per_kw, commitment_tx, keys, htlcs, secp_ctx).unwrap()) + Ok(self.inner.sign_remote_commitment(feerate_per_kw, commitment_tx, pre_keys, htlcs, secp_ctx).unwrap()) } fn sign_local_commitment(&self, local_commitment_tx: &LocalCommitmentTransaction, secp_ctx: &Secp256k1) -> Result {