Move node_id signing of ChannelAnnouncement into Signer
authorMatt Corallo <git@bluematt.me>
Sun, 14 Nov 2021 17:25:39 +0000 (17:25 +0000)
committerMatt Corallo <git@bluematt.me>
Tue, 21 Dec 2021 00:16:55 +0000 (00:16 +0000)
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
fuzz/src/full_stack.rs
lightning/src/chain/channelmonitor.rs
lightning/src/chain/keysinterface.rs
lightning/src/ln/channel.rs
lightning/src/ln/channelmanager.rs
lightning/src/util/enforcing_trait_impls.rs
lightning/src/util/test_utils.rs

index f41b17a1430da1bd439a816802c23432538bd6a7..4d84859ff101c0657523cc4e80d77f58c6ac8f28 100644 (file)
@@ -187,6 +187,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(),
@@ -210,7 +211,7 @@ impl KeysInterface 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.get_node_secret())?;
                let state = self.make_enforcement_state_cell(inner.commitment_seed);
 
                Ok(EnforcingSigner {
index 34c6f554c18e3a550759415f649e872c002383b3..6a55eb0c05a69fb313cdde90d56858d14d71a3c3 100644 (file)
@@ -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;
@@ -292,6 +292,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(),
@@ -304,6 +305,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(),
@@ -323,7 +325,7 @@ impl KeysInterface for KeyProvider {
        }
 
        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.node_secret.clone())?;
                let state = Arc::new(Mutex::new(EnforcementState::new()));
 
                Ok(EnforcingSigner::new_with_revoked(
index 5f456c7bfe86ba91ca3a73a19a93e491126adde7..8a41c4d10405ded1eb1a78ad6fe76abdc69f8d81 100644 (file)
@@ -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]
index 9e0cfb06e619c1219ada5c2e5621de1a317e432f..730de35d327dbbadb3d3aa07628abe6601f0920e 100644 (file)
@@ -30,7 +30,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;
@@ -330,13 +330,17 @@ pub trait BaseSign {
        /// chosen to forgo their output as dust.
        fn sign_closing_transaction(&self, closing_tx: &ClosingTransaction, secp_ctx: &Secp256k1<secp256k1::All>) -> Result<Signature, ()>;
 
-       /// 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<secp256k1::All>) -> Result<Signature, ()>;
+       fn sign_channel_announcement(&self, msg: &UnsignedChannelAnnouncement, secp_ctx: &Secp256k1<secp256k1::All>)
+               -> Result<(Signature, Signature), ()>;
 
        /// Set the counterparty static channel data, including basepoints,
        /// counterparty_selected/holder_selected_contest_delay and funding outpoint.
@@ -430,6 +434,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<ChannelTransactionParameters>,
        /// The total value of this channel
@@ -442,6 +448,7 @@ impl InMemorySigner {
        /// Create a new InMemorySigner
        pub fn new<C: Signing>(
                secp_ctx: &Secp256k1<C>,
+               node_secret: SecretKey,
                funding_key: SecretKey,
                revocation_base_key: SecretKey,
                payment_key: SecretKey,
@@ -461,6 +468,7 @@ impl InMemorySigner {
                        delayed_payment_base_key,
                        htlc_base_key,
                        commitment_seed,
+                       node_secret,
                        channel_value_satoshis,
                        holder_channel_pubkeys,
                        channel_parameters: None,
@@ -694,9 +702,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<secp256k1::All>) -> Result<Signature, ()> {
+       fn sign_channel_announcement(&self, msg: &UnsignedChannelAnnouncement, secp_ctx: &Secp256k1<secp256k1::All>)
+       -> 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) {
@@ -731,8 +740,8 @@ impl Writeable for InMemorySigner {
        }
 }
 
-impl Readable for InMemorySigner {
-       fn read<R: io::Read>(reader: &mut R) -> Result<Self, DecodeError> {
+impl ReadableArgs<SecretKey> for InMemorySigner {
+       fn read<R: io::Read>(reader: &mut R, node_secret: SecretKey) -> Result<Self, DecodeError> {
                let _ver = read_ver_prefix!(reader, SERIALIZATION_VERSION);
 
                let funding_key = Readable::read(reader)?;
@@ -758,6 +767,7 @@ impl Readable for InMemorySigner {
                        payment_key,
                        delayed_payment_base_key,
                        htlc_base_key,
+                       node_secret,
                        commitment_seed,
                        channel_value_satoshis,
                        holder_channel_pubkeys,
@@ -911,6 +921,7 @@ impl KeysManager {
 
                InMemorySigner::new(
                        &self.secp_ctx,
+                       self.node_secret,
                        funding_key,
                        revocation_base_key,
                        payment_key,
@@ -1088,7 +1099,7 @@ impl KeysInterface 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.get_node_secret())
        }
 
        fn sign_invoice(&self, invoice_preimage: Vec<u8>) -> Result<RecoverableSignature, ()> {
index ca6c6781fde467ea5d0fd9b712843d4b0c9ca624..abfd768a113bc7c2043fa4ed0da22a647e9dbce2 100644 (file)
@@ -4544,17 +4544,19 @@ impl<Signer: Sign> Channel<Signer> {
                })
        }
 
-       /// 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<msgs::UnsignedChannelAnnouncement, ChannelError> {
                if !self.config.announced_channel {
                        return Err(ChannelError::Ignore("Channel is not available for public announcements".to_owned()));
                }
@@ -4578,19 +4580,30 @@ impl<Signer: Sign> Channel<Signer> {
                        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<msgs::AnnouncementSignatures, ChannelError> {
+               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<msgs::ChannelAnnouncement, ChannelError> {
+       fn sign_channel_announcement(&self, our_node_id: PublicKey, announcement: msgs::UnsignedChannelAnnouncement) -> Result<msgs::ChannelAnnouncement, ChannelError> {
                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 },
@@ -4606,8 +4619,8 @@ impl<Signer: Sign> Channel<Signer> {
        /// 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<msgs::ChannelAnnouncement, ChannelError> {
-               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<msgs::ChannelAnnouncement, ChannelError> {
+               let announcement = self.get_channel_announcement(our_node_id.clone(), chain_hash)?;
 
                let msghash = hash_to_message!(&Sha256d::hash(&announcement.encode()[..])[..]);
 
@@ -4624,18 +4637,17 @@ impl<Signer: Sign> Channel<Signer> {
 
                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<msgs::ChannelAnnouncement> {
-               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<msgs::ChannelAnnouncement> {
+               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,
                }
@@ -6174,6 +6186,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(),
index c51d17f67b9df16701321b687e23d533e191cd68..ce6289200e9bcda1d559c701973f2ba5ec439aed 100644 (file)
@@ -2892,20 +2892,7 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> 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)]
@@ -2965,7 +2952,7 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> 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) {
@@ -4670,7 +4657,7 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> 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(),
index 58137e584d478a09d492005affd240ca93b963cf..7a5ab0827fc4a82052144e512830a241997aef1b 100644 (file)
@@ -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<secp256k1::All>) -> Result<Signature, ()> {
+       fn sign_channel_announcement(&self, msg: &msgs::UnsignedChannelAnnouncement, secp_ctx: &Secp256k1<secp256k1::All>)
+       -> Result<(Signature, Signature), ()> {
                self.inner.sign_channel_announcement(msg, secp_ctx)
        }
 
index 01c637b41ee6c0351f615c1b9bebc10be0ac11cb..66c5f2c9b03518a74d2d513a7033ed6a5a531dcb 100644 (file)
@@ -78,7 +78,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<Self::Signer, msgs::DecodeError> {
-               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(
@@ -517,7 +518,7 @@ impl keysinterface::KeysInterface for TestKeysInterface {
        fn read_chan_signer(&self, buffer: &[u8]) -> Result<Self::Signer, msgs::DecodeError> {
                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(