From d62edd58abfca1929fc3e67eaef0b40ed66fe617 Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Sun, 14 Nov 2021 17:25:39 +0000 Subject: [PATCH] Move node_id signing of ChannelAnnouncement into Signer This removes one more place where we directly access the node_id secret key in `ChannelManager`, slowly marching towards allowing the node_id secret key to be offline in the signer. More importantly, it allows more ChannelAnnouncement logic to move into the `Channel` without having to pass the node secret key around, avoiding the announcement logic being split across two files. --- fuzz/src/chanmon_consistency.rs | 3 +- fuzz/src/full_stack.rs | 6 ++- lightning/src/chain/channelmonitor.rs | 1 + lightning/src/chain/keysinterface.rs | 29 ++++++++----- lightning/src/ln/channel.rs | 45 +++++++++++++-------- lightning/src/ln/channelmanager.rs | 19 ++------- lightning/src/util/enforcing_trait_impls.rs | 3 +- lightning/src/util/test_utils.rs | 5 ++- 8 files changed, 64 insertions(+), 47 deletions(-) diff --git a/fuzz/src/chanmon_consistency.rs b/fuzz/src/chanmon_consistency.rs index ef0ae1da8..8c11ba2c6 100644 --- a/fuzz/src/chanmon_consistency.rs +++ b/fuzz/src/chanmon_consistency.rs @@ -188,6 +188,7 @@ impl KeysInterface for KeyProvider { let id = self.rand_bytes_id.fetch_add(1, atomic::Ordering::Relaxed); let keys = InMemorySigner::new( &secp_ctx, + self.get_node_secret(), SecretKey::from_slice(&[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, 4, self.node_id]).unwrap(), SecretKey::from_slice(&[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, 5, self.node_id]).unwrap(), SecretKey::from_slice(&[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, self.node_id]).unwrap(), @@ -211,7 +212,7 @@ impl KeysInterface for KeyProvider { fn read_chan_signer(&self, buffer: &[u8]) -> Result { let mut reader = std::io::Cursor::new(buffer); - let inner: InMemorySigner = Readable::read(&mut reader)?; + let inner: InMemorySigner = ReadableArgs::read(&mut reader, self.get_node_secret())?; let state = self.make_enforcement_state_cell(inner.commitment_seed); Ok(EnforcingSigner { diff --git a/fuzz/src/full_stack.rs b/fuzz/src/full_stack.rs index c5d091f2d..7db5aadcd 100644 --- a/fuzz/src/full_stack.rs +++ b/fuzz/src/full_stack.rs @@ -45,7 +45,7 @@ use lightning::util::errors::APIError; use lightning::util::events::Event; use lightning::util::enforcing_trait_impls::{EnforcingSigner, EnforcementState}; use lightning::util::logger::Logger; -use lightning::util::ser::Readable; +use lightning::util::ser::ReadableArgs; use utils::test_logger; use utils::test_persister::TestPersister; @@ -293,6 +293,7 @@ impl KeysInterface for KeyProvider { EnforcingSigner::new(if inbound { InMemorySigner::new( &secp_ctx, + self.node_secret.clone(), SecretKey::from_slice(&[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, 1, ctr]).unwrap(), SecretKey::from_slice(&[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, 2, ctr]).unwrap(), SecretKey::from_slice(&[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, 3, ctr]).unwrap(), @@ -305,6 +306,7 @@ impl KeysInterface for KeyProvider { } else { InMemorySigner::new( &secp_ctx, + self.node_secret.clone(), SecretKey::from_slice(&[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, 7, ctr]).unwrap(), SecretKey::from_slice(&[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, 8, ctr]).unwrap(), SecretKey::from_slice(&[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, 9, ctr]).unwrap(), @@ -324,7 +326,7 @@ impl KeysInterface for KeyProvider { } fn read_chan_signer(&self, mut data: &[u8]) -> Result { - let inner: InMemorySigner = Readable::read(&mut data)?; + let inner: InMemorySigner = ReadableArgs::read(&mut data, self.node_secret.clone())?; let state = Arc::new(Mutex::new(EnforcementState::new())); Ok(EnforcingSigner::new_with_revoked( diff --git a/lightning/src/chain/channelmonitor.rs b/lightning/src/chain/channelmonitor.rs index 8786f47db..e6f6ceda2 100644 --- a/lightning/src/chain/channelmonitor.rs +++ b/lightning/src/chain/channelmonitor.rs @@ -3479,6 +3479,7 @@ mod tests { SecretKey::from_slice(&[41; 32]).unwrap(), SecretKey::from_slice(&[41; 32]).unwrap(), SecretKey::from_slice(&[41; 32]).unwrap(), + SecretKey::from_slice(&[41; 32]).unwrap(), [41; 32], 0, [0; 32] diff --git a/lightning/src/chain/keysinterface.rs b/lightning/src/chain/keysinterface.rs index e1cc90674..dffe060d9 100644 --- a/lightning/src/chain/keysinterface.rs +++ b/lightning/src/chain/keysinterface.rs @@ -31,7 +31,7 @@ use bitcoin::secp256k1::recovery::RecoverableSignature; use bitcoin::secp256k1; use util::{byte_utils, transaction_utils}; -use util::ser::{Writeable, Writer, Readable}; +use util::ser::{Writeable, Writer, Readable, ReadableArgs}; use chain::transaction::OutPoint; use ln::{chan_utils, PaymentPreimage}; @@ -346,13 +346,17 @@ pub trait BaseSign { /// chosen to forgo their output as dust. fn sign_closing_transaction(&self, closing_tx: &ClosingTransaction, secp_ctx: &Secp256k1) -> Result; - /// Signs a channel announcement message with our funding key, proving it comes from one - /// of the channel participants. + /// Signs a channel announcement message with our funding key and our node secret key (aka + /// node_id or network_key), proving it comes from one of the channel participants. + /// + /// The first returned signature should be from our node secret key, the second from our + /// funding key. /// /// Note that if this fails or is rejected, the channel will not be publicly announced and /// our counterparty may (though likely will not) close the channel on us for violating the /// protocol. - fn sign_channel_announcement(&self, msg: &UnsignedChannelAnnouncement, secp_ctx: &Secp256k1) -> Result; + fn sign_channel_announcement(&self, msg: &UnsignedChannelAnnouncement, secp_ctx: &Secp256k1) + -> Result<(Signature, Signature), ()>; /// Set the counterparty static channel data, including basepoints, /// counterparty_selected/holder_selected_contest_delay and funding outpoint. @@ -447,6 +451,8 @@ pub struct InMemorySigner { pub commitment_seed: [u8; 32], /// Holder public keys and basepoints pub(crate) holder_channel_pubkeys: ChannelPublicKeys, + /// Private key of our node secret, used for signing channel announcements + node_secret: SecretKey, /// Counterparty public keys and counterparty/holder selected_contest_delay, populated on channel acceptance channel_parameters: Option, /// The total value of this channel @@ -459,6 +465,7 @@ impl InMemorySigner { /// Create a new InMemorySigner pub fn new( secp_ctx: &Secp256k1, + node_secret: SecretKey, funding_key: SecretKey, revocation_base_key: SecretKey, payment_key: SecretKey, @@ -478,6 +485,7 @@ impl InMemorySigner { delayed_payment_base_key, htlc_base_key, commitment_seed, + node_secret, channel_value_satoshis, holder_channel_pubkeys, channel_parameters: None, @@ -720,9 +728,10 @@ impl BaseSign for InMemorySigner { Ok(closing_tx.trust().sign(&self.funding_key, &channel_funding_redeemscript, self.channel_value_satoshis, secp_ctx)) } - fn sign_channel_announcement(&self, msg: &UnsignedChannelAnnouncement, secp_ctx: &Secp256k1) -> Result { + fn sign_channel_announcement(&self, msg: &UnsignedChannelAnnouncement, secp_ctx: &Secp256k1) + -> Result<(Signature, Signature), ()> { let msghash = hash_to_message!(&Sha256dHash::hash(&msg.encode()[..])[..]); - Ok(secp_ctx.sign(&msghash, &self.funding_key)) + Ok((secp_ctx.sign(&msghash, &self.node_secret), secp_ctx.sign(&msghash, &self.funding_key))) } fn ready_channel(&mut self, channel_parameters: &ChannelTransactionParameters) { @@ -757,8 +766,8 @@ impl Writeable for InMemorySigner { } } -impl Readable for InMemorySigner { - fn read(reader: &mut R) -> Result { +impl ReadableArgs for InMemorySigner { + fn read(reader: &mut R, node_secret: SecretKey) -> Result { let _ver = read_ver_prefix!(reader, SERIALIZATION_VERSION); let funding_key = Readable::read(reader)?; @@ -784,6 +793,7 @@ impl Readable for InMemorySigner { payment_key, delayed_payment_base_key, htlc_base_key, + node_secret, commitment_seed, channel_value_satoshis, holder_channel_pubkeys, @@ -937,6 +947,7 @@ impl KeysManager { InMemorySigner::new( &self.secp_ctx, + self.node_secret, funding_key, revocation_base_key, payment_key, @@ -1119,7 +1130,7 @@ impl KeysInterface for KeysManager { } fn read_chan_signer(&self, reader: &[u8]) -> Result { - InMemorySigner::read(&mut io::Cursor::new(reader)) + InMemorySigner::read(&mut io::Cursor::new(reader), self.get_node_secret()) } fn sign_invoice(&self, hrp_bytes: &[u8], invoice_data: &[u5]) -> Result { diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index cc015a681..70342ef18 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -4596,17 +4596,19 @@ impl Channel { }) } - /// Gets an UnsignedChannelAnnouncement, as well as a signature covering it using our - /// bitcoin_key, if available, for this channel. The channel must be publicly announceable and - /// available for use (have exchanged FundingLocked messages in both directions). Should be used - /// for both loose and in response to an AnnouncementSignatures message from the remote peer. + /// Gets an UnsignedChannelAnnouncement for this channel. The channel must be publicly + /// announceable and available for use (have exchanged FundingLocked messages in both + /// directions). Should be used for both loose and in response to an AnnouncementSignatures + /// message from the remote peer. + /// /// Will only fail if we're not in a state where channel_announcement may be sent (including /// closing). + /// /// Note that the "channel must be funded" requirement is stricter than BOLT 7 requires - see /// https://github.com/lightningnetwork/lightning-rfc/issues/468 /// /// This will only return ChannelError::Ignore upon failure. - pub fn get_channel_announcement(&self, node_id: PublicKey, chain_hash: BlockHash) -> Result<(msgs::UnsignedChannelAnnouncement, Signature), ChannelError> { + fn get_channel_announcement(&self, node_id: PublicKey, chain_hash: BlockHash) -> Result { if !self.config.announced_channel { return Err(ChannelError::Ignore("Channel is not available for public announcements".to_owned())); } @@ -4630,19 +4632,30 @@ impl Channel { excess_data: Vec::new(), }; - let sig = self.holder_signer.sign_channel_announcement(&msg, &self.secp_ctx) + Ok(msg) + } + + pub fn get_announcement_sigs(&self, node_pk: PublicKey, genesis_block_hash: BlockHash) -> Result { + let announcement = self.get_channel_announcement(node_pk, genesis_block_hash)?; + let (our_node_sig, our_bitcoin_sig) = self.holder_signer.sign_channel_announcement(&announcement, &self.secp_ctx) .map_err(|_| ChannelError::Ignore("Signer rejected channel_announcement".to_owned()))?; - Ok((msg, sig)) + Ok(msgs::AnnouncementSignatures { + channel_id: self.channel_id(), + short_channel_id: self.get_short_channel_id().unwrap(), + node_signature: our_node_sig, + bitcoin_signature: our_bitcoin_sig, + }) } /// Signs the given channel announcement, returning a ChannelError::Ignore if no keys are /// available. - fn sign_channel_announcement(&self, our_node_secret: &SecretKey, our_node_id: PublicKey, msghash: secp256k1::Message, announcement: msgs::UnsignedChannelAnnouncement, our_bitcoin_sig: Signature) -> Result { + fn sign_channel_announcement(&self, our_node_id: PublicKey, announcement: msgs::UnsignedChannelAnnouncement) -> Result { if let Some((their_node_sig, their_bitcoin_sig)) = self.announcement_sigs { let were_node_one = announcement.node_id_1 == our_node_id; - let our_node_sig = self.secp_ctx.sign(&msghash, our_node_secret); + let (our_node_sig, our_bitcoin_sig) = self.holder_signer.sign_channel_announcement(&announcement, &self.secp_ctx) + .map_err(|_| ChannelError::Ignore("Signer rejected channel_announcement".to_owned()))?; Ok(msgs::ChannelAnnouncement { node_signature_1: if were_node_one { our_node_sig } else { their_node_sig }, node_signature_2: if were_node_one { their_node_sig } else { our_node_sig }, @@ -4658,8 +4671,8 @@ impl Channel { /// Processes an incoming announcement_signatures message, providing a fully-signed /// channel_announcement message which we can broadcast and storing our counterparty's /// signatures for later reconstruction/rebroadcast of the channel_announcement. - pub fn announcement_signatures(&mut self, our_node_secret: &SecretKey, our_node_id: PublicKey, chain_hash: BlockHash, msg: &msgs::AnnouncementSignatures) -> Result { - let (announcement, our_bitcoin_sig) = self.get_channel_announcement(our_node_id.clone(), chain_hash)?; + pub fn announcement_signatures(&mut self, our_node_id: PublicKey, chain_hash: BlockHash, msg: &msgs::AnnouncementSignatures) -> Result { + let announcement = self.get_channel_announcement(our_node_id.clone(), chain_hash)?; let msghash = hash_to_message!(&Sha256d::hash(&announcement.encode()[..])[..]); @@ -4676,18 +4689,17 @@ impl Channel { self.announcement_sigs = Some((msg.node_signature, msg.bitcoin_signature)); - self.sign_channel_announcement(our_node_secret, our_node_id, msghash, announcement, our_bitcoin_sig) + self.sign_channel_announcement(our_node_id, announcement) } /// Gets a signed channel_announcement for this channel, if we previously received an /// announcement_signatures from our counterparty. - pub fn get_signed_channel_announcement(&self, our_node_secret: &SecretKey, our_node_id: PublicKey, chain_hash: BlockHash) -> Option { - let (announcement, our_bitcoin_sig) = match self.get_channel_announcement(our_node_id.clone(), chain_hash) { + pub fn get_signed_channel_announcement(&self, our_node_id: PublicKey, chain_hash: BlockHash) -> Option { + let announcement = match self.get_channel_announcement(our_node_id.clone(), chain_hash) { Ok(res) => res, Err(_) => return None, }; - let msghash = hash_to_message!(&Sha256d::hash(&announcement.encode()[..])[..]); - match self.sign_channel_announcement(our_node_secret, our_node_id, msghash, announcement, our_bitcoin_sig) { + match self.sign_channel_announcement(our_node_id, announcement) { Ok(res) => Some(res), Err(_) => None, } @@ -6270,6 +6282,7 @@ mod tests { let mut signer = InMemorySigner::new( &secp_ctx, + SecretKey::from_slice(&hex::decode("4242424242424242424242424242424242424242424242424242424242424242").unwrap()[..]).unwrap(), SecretKey::from_slice(&hex::decode("30ff4956bbdd3222d44cc5e8a1261dab1e07957bdac5ae88fe3261ef321f3749").unwrap()[..]).unwrap(), SecretKey::from_slice(&hex::decode("0fffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff").unwrap()[..]).unwrap(), SecretKey::from_slice(&hex::decode("1111111111111111111111111111111111111111111111111111111111111111").unwrap()[..]).unwrap(), diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index 0ca9dbd46..27bfc5673 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -2896,20 +2896,7 @@ impl ChannelMana log_trace!(self.logger, "Can't send announcement_signatures for private channel {}", log_bytes!(chan.channel_id())); return None } - - let (announcement, our_bitcoin_sig) = match chan.get_channel_announcement(self.get_our_node_id(), self.genesis_hash.clone()) { - Ok(res) => res, - Err(_) => return None, // Only in case of state precondition violations eg channel is closing - }; - let msghash = hash_to_message!(&Sha256dHash::hash(&announcement.encode()[..])[..]); - let our_node_sig = self.secp_ctx.sign(&msghash, &self.our_network_key); - - Some(msgs::AnnouncementSignatures { - channel_id: chan.channel_id(), - short_channel_id: chan.get_short_channel_id().unwrap(), - node_signature: our_node_sig, - bitcoin_signature: our_bitcoin_sig, - }) + chan.get_announcement_sigs(self.get_our_node_id(), self.genesis_hash.clone()).ok() } #[allow(dead_code)] @@ -2969,7 +2956,7 @@ impl ChannelMana let mut announced_chans = false; for (_, chan) in channel_state.by_id.iter() { - if let Some(msg) = chan.get_signed_channel_announcement(&self.our_network_key, self.get_our_node_id(), self.genesis_hash.clone()) { + if let Some(msg) = chan.get_signed_channel_announcement(self.get_our_node_id(), self.genesis_hash.clone()) { channel_state.pending_msg_events.push(events::MessageSendEvent::BroadcastChannelAnnouncement { msg, update_msg: match self.get_channel_update_for_broadcast(chan) { @@ -4674,7 +4661,7 @@ impl ChannelMana } channel_state.pending_msg_events.push(events::MessageSendEvent::BroadcastChannelAnnouncement { - msg: try_chan_entry!(self, chan.get_mut().announcement_signatures(&self.our_network_key, self.get_our_node_id(), self.genesis_hash.clone(), msg), channel_state, chan), + msg: try_chan_entry!(self, chan.get_mut().announcement_signatures(self.get_our_node_id(), self.genesis_hash.clone(), msg), channel_state, chan), // Note that announcement_signatures fails if the channel cannot be announced, // so get_channel_update_for_broadcast will never fail by the time we get here. update_msg: self.get_channel_update_for_broadcast(chan.get()).unwrap(), diff --git a/lightning/src/util/enforcing_trait_impls.rs b/lightning/src/util/enforcing_trait_impls.rs index 3221d6a80..2e22df7c5 100644 --- a/lightning/src/util/enforcing_trait_impls.rs +++ b/lightning/src/util/enforcing_trait_impls.rs @@ -190,7 +190,8 @@ impl BaseSign for EnforcingSigner { Ok(self.inner.sign_closing_transaction(closing_tx, secp_ctx).unwrap()) } - fn sign_channel_announcement(&self, msg: &msgs::UnsignedChannelAnnouncement, secp_ctx: &Secp256k1) -> Result { + fn sign_channel_announcement(&self, msg: &msgs::UnsignedChannelAnnouncement, secp_ctx: &Secp256k1) + -> Result<(Signature, Signature), ()> { self.inner.sign_channel_announcement(msg, secp_ctx) } diff --git a/lightning/src/util/test_utils.rs b/lightning/src/util/test_utils.rs index 416951441..dee690783 100644 --- a/lightning/src/util/test_utils.rs +++ b/lightning/src/util/test_utils.rs @@ -79,7 +79,8 @@ impl keysinterface::KeysInterface for OnlyReadsKeysInterface { fn get_secure_random_bytes(&self) -> [u8; 32] { [0; 32] } fn read_chan_signer(&self, mut reader: &[u8]) -> Result { - let inner: InMemorySigner = Readable::read(&mut reader)?; + let dummy_sk = SecretKey::from_slice(&[42; 32]).unwrap(); + let inner: InMemorySigner = ReadableArgs::read(&mut reader, dummy_sk)?; let state = Arc::new(Mutex::new(EnforcementState::new())); Ok(EnforcingSigner::new_with_revoked( @@ -519,7 +520,7 @@ impl keysinterface::KeysInterface for TestKeysInterface { fn read_chan_signer(&self, buffer: &[u8]) -> Result { let mut reader = io::Cursor::new(buffer); - let inner: InMemorySigner = Readable::read(&mut reader)?; + let inner: InMemorySigner = ReadableArgs::read(&mut reader, self.get_node_secret())?; let state = self.make_enforcement_state_cell(inner.commitment_seed); Ok(EnforcingSigner::new_with_revoked( -- 2.39.5