Check the PK of the source of an error before closing chans from it

When we receive an error message from a peer, it can indicate a
channel which we should close. However, we previously did not
check that the counterparty who sends us such a message is the
counterparty with whom we have the channel, allowing any
connected peer to make us force-close any channel we have as long
as they know the channel id.

This commit simply changes the force-close logic to check that the
sender matches the channel's counterparty node_id, though as noted
in #105, we eventually need to change the indexing anyway to allow
absurdly terrible peers to open channels with us.

Found during review of #777.

diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs
index e353700be16f5bff37627e53b06cd45a8a585114..621013724bc4195cd58b1dac9fb8c6aee915ef0b 100644 (file)
--- a/lightning/src/ln/channelmanager.rs
+++ b/lightning/src/ln/channelmanager.rs
@@ -916,19 +916,22 @@ impl<ChanSigner: ChannelKeys, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref>
                }
        }
 
-       /// Force closes a channel, immediately broadcasting the latest local commitment transaction to
-       /// the chain and rejecting new HTLCs on the given channel. Fails if channel_id is unknown to the manager.
-       pub fn force_close_channel(&self, channel_id: &[u8; 32]) -> Result<(), APIError>{
-               let _consistency_lock = self.total_consistency_lock.read().unwrap();
-
+       fn force_close_channel_with_peer(&self, channel_id: &[u8; 32], peer_node_id: Option<&PublicKey>) -> Result<(), APIError> {
                let mut chan = {
                        let mut channel_state_lock = self.channel_state.lock().unwrap();
                        let channel_state = &mut *channel_state_lock;
-                       if let Some(chan) = channel_state.by_id.remove(channel_id) {
-                               if let Some(short_id) = chan.get_short_channel_id() {
+                       if let hash_map::Entry::Occupied(chan) = channel_state.by_id.entry(channel_id.clone()) {
+                               if let Some(node_id) = peer_node_id {
+                                       if chan.get().get_counterparty_node_id() != *node_id {
+                                               // Error or Ok here doesn't matter - the result is only exposed publicly
+                                               // when peer_node_id is None anyway.
+                                               return Ok(());
+                                       }
+                               }
+                               if let Some(short_id) = chan.get().get_short_channel_id() {
                                        channel_state.short_to_id.remove(&short_id);
                                }
-                               chan
+                               chan.remove_entry().1
                        } else {
                                return Err(APIError::ChannelUnavailable{err: "No such channel".to_owned()});
                        }
@@ -945,6 +948,13 @@ impl<ChanSigner: ChannelKeys, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref>
                Ok(())
        }
 
+       /// Force closes a channel, immediately broadcasting the latest local commitment transaction to
+       /// the chain and rejecting new HTLCs on the given channel. Fails if channel_id is unknown to the manager.
+       pub fn force_close_channel(&self, channel_id: &[u8; 32]) -> Result<(), APIError> {
+               let _consistency_lock = self.total_consistency_lock.read().unwrap();
+               self.force_close_channel_with_peer(channel_id, None)
+       }
+
        /// Force close all channels, immediately broadcasting the latest local commitment transaction
        /// for each to the chain and rejecting new HTLCs on each.
        pub fn force_close_all_channels(&self) {
@@ -3474,12 +3484,12 @@ impl<ChanSigner: ChannelKeys, M: Deref + Sync + Send, T: Deref + Sync + Send, K:
                        for chan in self.list_channels() {
                                if chan.remote_network_id == *counterparty_node_id {
                                        // Untrusted messages from peer, we throw away the error if id points to a non-existent channel
-                                       let _ = self.force_close_channel(&msg.channel_id);
+                                       let _ = self.force_close_channel_with_peer(&chan.channel_id, Some(counterparty_node_id));
                                }
                        }
                } else {
                        // Untrusted messages from peer, we throw away the error if id points to a non-existent channel
-                       let _ = self.force_close_channel(&msg.channel_id);
+                       let _ = self.force_close_channel_with_peer(&msg.channel_id, Some(counterparty_node_id));
                }
        }
 }