From: Matt Corallo <git@bluematt.me>
Date: Sat, 16 Jan 2021 02:34:17 +0000 (-0500)
Subject: Check the PK of the source of an error before closing chans from it
X-Git-Url: http://git.bitcoin.ninja/?a=commitdiff_plain;h=5399dfb2d5b480a01076ee1194c64a950d6d6692;p=rust-lightning

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.
---

diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs
index 49c14a043..5ecfc26d5 100644
--- a/lightning/src/ln/channelmanager.rs
+++ b/lightning/src/ln/channelmanager.rs
@@ -916,19 +916,18 @@ 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.
-	pub fn force_close_channel(&self, channel_id: &[u8; 32]) {
-		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>) {
 		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 { return; }
+				}
+				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;
 			}
@@ -943,6 +942,13 @@ 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.
+	pub fn force_close_channel(&self, channel_id: &[u8; 32]) {
+		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) {
@@ -3475,7 +3481,7 @@ impl<ChanSigner: ChannelKeys, M: Deref + Sync + Send, T: Deref + Sync + Send, K:
 				}
 			}
 		} else {
-			self.force_close_channel(&msg.channel_id);
+			self.force_close_channel_with_peer(&msg.channel_id, Some(counterparty_node_id));
 		}
 	}
 }