From c3991602a589b2c9fa76faff49885544cba048b3 Mon Sep 17 00:00:00 2001 From: Antoine Riard Date: Wed, 10 Jul 2019 16:39:10 -0400 Subject: [PATCH] Enforce option_data_loss_protect user-side If we remote peer provide us a revocation secret which doesn't match with next_remote_revocation_number we close the channel If we learn that we are fallen-behind, we send back a CloseDelayBroadcast error, special take care will be take to log error and channel should stale, i.e we expect our honest peer to unilateral close to claim on it our balance Add ChannelError::CloseDelayBroadcast to signal that you need to close the channel but not to broadcast it while however update ChannelMonitor with remote per_commitment_point thanks to our peer being a gentleman --- src/ln/channel.rs | 35 ++++++++++++++++++++++++++++++++--- src/ln/channelmanager.rs | 32 ++++++++++++++++++++++++++++++++ src/ln/channelmonitor.rs | 3 +++ 3 files changed, 67 insertions(+), 3 deletions(-) diff --git a/src/ln/channel.rs b/src/ln/channel.rs index 72b9fab14..5c0ca8fd6 100644 --- a/src/ln/channel.rs +++ b/src/ln/channel.rs @@ -32,7 +32,7 @@ use util::config::{UserConfig,ChannelConfig}; use std; use std::default::Default; -use std::{cmp,mem}; +use std::{cmp,mem,fmt}; use std::sync::{Arc}; #[cfg(test)] @@ -366,10 +366,23 @@ pub const OFFERED_HTLC_SCRIPT_WEIGHT: usize = 133; /// Used to return a simple Error back to ChannelManager. Will get converted to a /// msgs::ErrorAction::SendErrorMessage or msgs::ErrorAction::IgnoreError as appropriate with our /// channel_id in ChannelManager. -#[derive(Debug)] pub(super) enum ChannelError { Ignore(&'static str), Close(&'static str), + CloseDelayBroadcast { + msg: &'static str, + update: Option + }, +} + +impl fmt::Debug for ChannelError { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + match self { + &ChannelError::Ignore(e) => write!(f, "Ignore : {}", e), + &ChannelError::Close(e) => write!(f, "Close : {}", e), + &ChannelError::CloseDelayBroadcast { msg, .. } => write!(f, "CloseDelayBroadcast : {}", msg) + } + } } macro_rules! secp_check { @@ -2500,6 +2513,22 @@ impl Channel { return Err(ChannelError::Close("Peer sent a garbage channel_reestablish")); } + if msg.next_remote_commitment_number > 0 { + match msg.data_loss_protect { + OptionalField::Present(ref data_loss) => { + if chan_utils::build_commitment_secret(self.local_keys.commitment_seed, INITIAL_COMMITMENT_NUMBER - msg.next_remote_commitment_number + 1) != data_loss.your_last_per_commitment_secret { + return Err(ChannelError::Close("Peer sent a garbage channel_reestablish with secret key not matching the commitment height provided")); + } + if msg.next_remote_commitment_number > INITIAL_COMMITMENT_NUMBER - self.cur_local_commitment_transaction_number { + self.channel_monitor.provide_rescue_remote_commitment_tx_info(data_loss.my_current_per_commitment_point); + return Err(ChannelError::CloseDelayBroadcast { msg: "We have fallen behind - we have received proof that if we broadcast remote is going to claim our funds - we can't do any automated broadcasting", update: Some(self.channel_monitor.clone()) + }); + } + }, + OptionalField::Absent => {} + } + } + // Go ahead and unmark PeerDisconnected as various calls we may make check for it (and all // remaining cases either succeed or ErrorMessage-fail). self.channel_state &= !(ChannelState::PeerDisconnected as u32); @@ -2576,7 +2605,7 @@ impl Channel { // now! match self.free_holding_cell_htlcs() { Err(ChannelError::Close(msg)) => return Err(ChannelError::Close(msg)), - Err(ChannelError::Ignore(_)) => panic!("Got non-channel-failing result from free_holding_cell_htlcs"), + Err(ChannelError::Ignore(_)) | Err(ChannelError::CloseDelayBroadcast { .. }) => panic!("Got non-channel-failing result from free_holding_cell_htlcs"), Ok(Some((commitment_update, channel_monitor))) => return Ok((resend_funding_locked, required_revoke, Some(commitment_update), Some(channel_monitor), self.resend_order.clone(), shutdown_msg)), Ok(None) => return Ok((resend_funding_locked, required_revoke, None, None, self.resend_order.clone(), shutdown_msg)), } diff --git a/src/ln/channelmanager.rs b/src/ln/channelmanager.rs index afabcd7cc..2b22cbd89 100644 --- a/src/ln/channelmanager.rs +++ b/src/ln/channelmanager.rs @@ -208,6 +208,15 @@ impl MsgHandleErrInternal { }, }), }, + ChannelError::CloseDelayBroadcast { msg, .. } => HandleError { + err: msg, + action: Some(msgs::ErrorAction::SendErrorMessage { + msg: msgs::ErrorMessage { + channel_id, + data: msg.to_string() + }, + }), + }, }, shutdown_finish: None, } @@ -447,6 +456,7 @@ macro_rules! break_chan_entry { } break Err(MsgHandleErrInternal::from_finish_shutdown(msg, channel_id, chan.force_shutdown(), $self.get_channel_update(&chan).ok())) }, + Err(ChannelError::CloseDelayBroadcast { .. }) => { panic!("Wait is only generated on receipt of channel_reestablish, which is handled by try_chan_entry, we don't bother to support it here"); } } } } @@ -466,6 +476,28 @@ macro_rules! try_chan_entry { } return Err(MsgHandleErrInternal::from_finish_shutdown(msg, channel_id, chan.force_shutdown(), $self.get_channel_update(&chan).ok())) }, + Err(ChannelError::CloseDelayBroadcast { msg, update }) => { + log_error!($self, "Channel {} need to be shutdown but closing transactions not broadcast due to {}", log_bytes!($entry.key()[..]), msg); + let (channel_id, mut chan) = $entry.remove_entry(); + if let Some(short_id) = chan.get_short_channel_id() { + $channel_state.short_to_id.remove(&short_id); + } + if let Some(update) = update { + if let Err(e) = $self.monitor.add_update_monitor(update.get_funding_txo().unwrap(), update) { + match e { + // Upstream channel is dead, but we want at least to fail backward HTLCs to save + // downstream channels. In case of PermanentFailure, we are not going to be able + // to claim back to_remote output on remote commitment transaction. Doesn't + // make a difference here, we are concern about HTLCs circuit, not onchain funds. + ChannelMonitorUpdateErr::PermanentFailure => {}, + ChannelMonitorUpdateErr::TemporaryFailure => {}, + } + } + } + let mut shutdown_res = chan.force_shutdown(); // We drop closing transactions as they are toxic datas and MUST NOT be broadcast + shutdown_res.0.clear(); + return Err(MsgHandleErrInternal::from_finish_shutdown(msg, channel_id, shutdown_res, $self.get_channel_update(&chan).ok())) + } } } } diff --git a/src/ln/channelmonitor.rs b/src/ln/channelmonitor.rs index 2c0a05804..e0ffe8098 100644 --- a/src/ln/channelmonitor.rs +++ b/src/ln/channelmonitor.rs @@ -763,6 +763,9 @@ impl ChannelMonitor { } } + pub(super) fn provide_rescue_remote_commitment_tx_info(&mut self, their_revocation_point: PublicKey) { + } + /// Informs this monitor of the latest local (ie broadcastable) commitment transaction. The /// monitor watches for timeouts and may broadcast it if we approach such a timeout. Thus, it /// is important that any clones of this channel monitor (including remote clones) by kept -- 2.39.5