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
This commit is contained in:
Antoine Riard 2019-07-10 16:39:10 -04:00
parent 2869e50d67
commit c3991602a5
3 changed files with 67 additions and 3 deletions

View file

@ -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<ChannelMonitor>
},
}
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)),
}

View file

@ -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()))
}
}
}
}

View file

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