From 0052b2c5c3017aec1a80b6476fad441b63a67a8c Mon Sep 17 00:00:00 2001 From: Antoine Riard Date: Tue, 9 Jul 2019 14:38:30 -0400 Subject: [PATCH] Provide peer local_features to handle_open_channel/accept_channel Peer may send us a shutdown_scriptpubkey in open_channel or accept_channel messages. Before to enforce this policy on channel closing, we want to be sure that our peer has opt-in to it. Extend LocalFeatures new method visibilty from crate to public for fuzz tests --- fuzz/fuzz_targets/chanmon_fail_consistency.rs | 6 +++--- src/ln/channel.rs | 6 +++--- src/ln/channelmanager.rs | 17 +++++++++-------- src/ln/functional_test_utils.rs | 6 +++--- src/ln/functional_tests.rs | 4 ++-- src/ln/msgs.rs | 7 ++++--- src/ln/peer_handler.rs | 4 ++-- src/util/test_utils.rs | 5 +++-- 8 files changed, 29 insertions(+), 26 deletions(-) diff --git a/fuzz/fuzz_targets/chanmon_fail_consistency.rs b/fuzz/fuzz_targets/chanmon_fail_consistency.rs index 5a8790903..c26ff88a7 100644 --- a/fuzz/fuzz_targets/chanmon_fail_consistency.rs +++ b/fuzz/fuzz_targets/chanmon_fail_consistency.rs @@ -36,7 +36,7 @@ use lightning::ln::channelmonitor; use lightning::ln::channelmonitor::{ChannelMonitorUpdateErr, HTLCUpdate}; use lightning::ln::channelmanager::{ChannelManager, PaymentHash, PaymentPreimage}; use lightning::ln::router::{Route, RouteHop}; -use lightning::ln::msgs::{CommitmentUpdate, ChannelMessageHandler, ErrorAction, HandleError, UpdateAddHTLC}; +use lightning::ln::msgs::{CommitmentUpdate, ChannelMessageHandler, ErrorAction, HandleError, UpdateAddHTLC, LocalFeatures}; use lightning::util::{reset_rng_state, fill_bytes, events}; use lightning::util::logger::Logger; use lightning::util::config::UserConfig; @@ -168,7 +168,7 @@ pub fn do_test(data: &[u8]) { } else { panic!("Wrong event type"); } }; - $dest.handle_open_channel(&$source.get_our_node_id(), &open_channel).unwrap(); + $dest.handle_open_channel(&$source.get_our_node_id(), LocalFeatures::new(), &open_channel).unwrap(); let accept_channel = { let events = $dest.get_and_clear_pending_msg_events(); assert_eq!(events.len(), 1); @@ -177,7 +177,7 @@ pub fn do_test(data: &[u8]) { } else { panic!("Wrong event type"); } }; - $source.handle_accept_channel(&$dest.get_our_node_id(), &accept_channel).unwrap(); + $source.handle_accept_channel(&$dest.get_our_node_id(), LocalFeatures::new(), &accept_channel).unwrap(); { let events = $source.get_and_clear_pending_events(); assert_eq!(events.len(), 1); diff --git a/src/ln/channel.rs b/src/ln/channel.rs index 63f0f41ae..cee70e5c3 100644 --- a/src/ln/channel.rs +++ b/src/ln/channel.rs @@ -16,7 +16,7 @@ use secp256k1::{Secp256k1,Signature}; use secp256k1; use ln::msgs; -use ln::msgs::{DecodeError, OptionalField}; +use ln::msgs::{DecodeError, OptionalField, LocalFeatures}; use ln::channelmonitor::ChannelMonitor; use ln::channelmanager::{PendingHTLCStatus, HTLCSource, HTLCFailReason, HTLCFailureMsg, PendingForwardHTLCInfo, RAACommitmentOrder, PaymentPreimage, PaymentHash}; use ln::chan_utils::{TxCreationKeys,HTLCOutputInCommitment,HTLC_SUCCESS_TX_WEIGHT,HTLC_TIMEOUT_TX_WEIGHT}; @@ -522,7 +522,7 @@ impl Channel { /// Creates a new channel from a remote sides' request for one. /// Assumes chain_hash has already been checked and corresponds with what we expect! - pub fn new_from_req(fee_estimator: &FeeEstimator, keys_provider: &Arc, their_node_id: PublicKey, msg: &msgs::OpenChannel, user_id: u64, logger: Arc, config: &UserConfig) -> Result { + pub fn new_from_req(fee_estimator: &FeeEstimator, keys_provider: &Arc, their_node_id: PublicKey, _their_local_features: LocalFeatures, msg: &msgs::OpenChannel, user_id: u64, logger: Arc, config: &UserConfig) -> Result { let chan_keys = keys_provider.get_channel_keys(true); let mut local_config = (*config).channel_options.clone(); @@ -1341,7 +1341,7 @@ impl Channel { // Message handlers: - pub fn accept_channel(&mut self, msg: &msgs::AcceptChannel, config: &UserConfig) -> Result<(), ChannelError> { + pub fn accept_channel(&mut self, msg: &msgs::AcceptChannel, config: &UserConfig, _their_local_features: LocalFeatures) -> Result<(), ChannelError> { // Check sanity of message fields: if !self.channel_outbound { return Err(ChannelError::Close("Got an accept_channel message from an inbound peer")); diff --git a/src/ln/channelmanager.rs b/src/ln/channelmanager.rs index 1bf3f0fec..14491f1aa 100644 --- a/src/ln/channelmanager.rs +++ b/src/ln/channelmanager.rs @@ -31,6 +31,7 @@ use ln::channel::{Channel, ChannelError}; use ln::channelmonitor::{ChannelMonitor, ChannelMonitorUpdateErr, ManyChannelMonitor, CLTV_CLAIM_BUFFER, LATENCY_GRACE_PERIOD_BLOCKS, ANTI_REORG_DELAY}; use ln::router::Route; use ln::msgs; +use ln::msgs::LocalFeatures; use ln::onion_utils; use ln::msgs::{ChannelMessageHandler, DecodeError, HandleError}; use chain::keysinterface::KeysInterface; @@ -1702,12 +1703,12 @@ impl ChannelManager { } } - fn internal_open_channel(&self, their_node_id: &PublicKey, msg: &msgs::OpenChannel) -> Result<(), MsgHandleErrInternal> { + fn internal_open_channel(&self, their_node_id: &PublicKey, their_local_features: LocalFeatures, msg: &msgs::OpenChannel) -> Result<(), MsgHandleErrInternal> { if msg.chain_hash != self.genesis_hash { return Err(MsgHandleErrInternal::send_err_msg_no_close("Unknown genesis block hash", msg.temporary_channel_id.clone())); } - let channel = Channel::new_from_req(&*self.fee_estimator, &self.keys_manager, their_node_id.clone(), msg, 0, Arc::clone(&self.logger), &self.default_configuration) + let channel = Channel::new_from_req(&*self.fee_estimator, &self.keys_manager, their_node_id.clone(), their_local_features, msg, 0, Arc::clone(&self.logger), &self.default_configuration) .map_err(|e| MsgHandleErrInternal::from_chan_no_close(e, msg.temporary_channel_id))?; let mut channel_state_lock = self.channel_state.lock().unwrap(); let channel_state = channel_state_lock.borrow_parts(); @@ -1724,7 +1725,7 @@ impl ChannelManager { Ok(()) } - fn internal_accept_channel(&self, their_node_id: &PublicKey, msg: &msgs::AcceptChannel) -> Result<(), MsgHandleErrInternal> { + fn internal_accept_channel(&self, their_node_id: &PublicKey, their_local_features: LocalFeatures, msg: &msgs::AcceptChannel) -> Result<(), MsgHandleErrInternal> { let (value, output_script, user_id) = { let mut channel_lock = self.channel_state.lock().unwrap(); let channel_state = channel_lock.borrow_parts(); @@ -1734,7 +1735,7 @@ impl ChannelManager { //TODO: see issue #153, need a consistent behavior on obnoxious behavior from random node return Err(MsgHandleErrInternal::send_err_msg_no_close("Got a message for a channel from the wrong node!", msg.temporary_channel_id)); } - try_chan_entry!(self, chan.get_mut().accept_channel(&msg, &self.default_configuration), channel_state, chan); + try_chan_entry!(self, chan.get_mut().accept_channel(&msg, &self.default_configuration, their_local_features), channel_state, chan); (chan.get().get_value_satoshis(), chan.get().get_funding_redeemscript().to_v0_p2wsh(), chan.get().get_user_id()) }, //TODO: same as above @@ -2525,14 +2526,14 @@ impl ChainListener for ChannelManager { impl ChannelMessageHandler for ChannelManager { //TODO: Handle errors and close channel (or so) - fn handle_open_channel(&self, their_node_id: &PublicKey, msg: &msgs::OpenChannel) -> Result<(), HandleError> { + fn handle_open_channel(&self, their_node_id: &PublicKey, their_local_features: LocalFeatures, msg: &msgs::OpenChannel) -> Result<(), HandleError> { let _ = self.total_consistency_lock.read().unwrap(); - handle_error!(self, self.internal_open_channel(their_node_id, msg)) + handle_error!(self, self.internal_open_channel(their_node_id, their_local_features, msg)) } - fn handle_accept_channel(&self, their_node_id: &PublicKey, msg: &msgs::AcceptChannel) -> Result<(), HandleError> { + fn handle_accept_channel(&self, their_node_id: &PublicKey, their_local_features: LocalFeatures, msg: &msgs::AcceptChannel) -> Result<(), HandleError> { let _ = self.total_consistency_lock.read().unwrap(); - handle_error!(self, self.internal_accept_channel(their_node_id, msg)) + handle_error!(self, self.internal_accept_channel(their_node_id, their_local_features, msg)) } fn handle_funding_created(&self, their_node_id: &PublicKey, msg: &msgs::FundingCreated) -> Result<(), HandleError> { diff --git a/src/ln/functional_test_utils.rs b/src/ln/functional_test_utils.rs index 14ba7e5ff..dedc2d500 100644 --- a/src/ln/functional_test_utils.rs +++ b/src/ln/functional_test_utils.rs @@ -7,7 +7,7 @@ use chain::keysinterface::KeysInterface; use ln::channelmanager::{ChannelManager,RAACommitmentOrder, PaymentPreimage, PaymentHash}; use ln::router::{Route, Router}; use ln::msgs; -use ln::msgs::{ChannelMessageHandler,RoutingMessageHandler}; +use ln::msgs::{ChannelMessageHandler,RoutingMessageHandler, LocalFeatures}; use util::test_utils; use util::events::{Event, EventsProvider, MessageSendEvent, MessageSendEventsProvider}; use util::errors::APIError; @@ -174,8 +174,8 @@ macro_rules! get_feerate { pub fn create_chan_between_nodes_with_value_init(node_a: &Node, node_b: &Node, channel_value: u64, push_msat: u64) -> Transaction { node_a.node.create_channel(node_b.node.get_our_node_id(), channel_value, push_msat, 42).unwrap(); - node_b.node.handle_open_channel(&node_a.node.get_our_node_id(), &get_event_msg!(node_a, MessageSendEvent::SendOpenChannel, node_b.node.get_our_node_id())).unwrap(); - node_a.node.handle_accept_channel(&node_b.node.get_our_node_id(), &get_event_msg!(node_b, MessageSendEvent::SendAcceptChannel, node_a.node.get_our_node_id())).unwrap(); + node_b.node.handle_open_channel(&node_a.node.get_our_node_id(), LocalFeatures::new(), &get_event_msg!(node_a, MessageSendEvent::SendOpenChannel, node_b.node.get_our_node_id())).unwrap(); + node_a.node.handle_accept_channel(&node_b.node.get_our_node_id(), LocalFeatures::new(), &get_event_msg!(node_b, MessageSendEvent::SendAcceptChannel, node_a.node.get_our_node_id())).unwrap(); let chan_id = *node_a.network_chan_count.borrow(); let tx; diff --git a/src/ln/functional_tests.rs b/src/ln/functional_tests.rs index 210a4e91f..1397be15e 100644 --- a/src/ln/functional_tests.rs +++ b/src/ln/functional_tests.rs @@ -13,7 +13,7 @@ use ln::channel::{ACCEPTED_HTLC_SCRIPT_WEIGHT, OFFERED_HTLC_SCRIPT_WEIGHT}; use ln::onion_utils; use ln::router::{Route, RouteHop}; use ln::msgs; -use ln::msgs::{ChannelMessageHandler,RoutingMessageHandler,HTLCFailChannelUpdate}; +use ln::msgs::{ChannelMessageHandler,RoutingMessageHandler,HTLCFailChannelUpdate, LocalFeatures}; use util::test_utils; use util::events::{Event, EventsProvider, MessageSendEvent, MessageSendEventsProvider}; use util::errors::APIError; @@ -4861,7 +4861,7 @@ fn bolt2_open_channel_sending_node_checks_part1() { //This test needs to be on i let push_msat=10001; nodes[0].node.create_channel(nodes[1].node.get_our_node_id(), channel_value_satoshis, push_msat, 42).unwrap(); let node0_to_1_send_open_channel = get_event_msg!(nodes[0], MessageSendEvent::SendOpenChannel, nodes[1].node.get_our_node_id()); - nodes[1].node.handle_open_channel(&nodes[0].node.get_our_node_id(), &node0_to_1_send_open_channel).unwrap(); + nodes[1].node.handle_open_channel(&nodes[0].node.get_our_node_id(), LocalFeatures::new(), &node0_to_1_send_open_channel).unwrap(); //Create a second channel with a channel_id collision assert!(nodes[0].node.create_channel(nodes[0].node.get_our_node_id(), channel_value_satoshis, push_msat, 42).is_err()); diff --git a/src/ln/msgs.rs b/src/ln/msgs.rs index 6aa9e05b2..10e83e08d 100644 --- a/src/ln/msgs.rs +++ b/src/ln/msgs.rs @@ -59,7 +59,8 @@ pub struct LocalFeatures { } impl LocalFeatures { - pub(crate) fn new() -> LocalFeatures { + /// Create a blank LocalFeatures flags (visibility extended for fuzz tests) + pub fn new() -> LocalFeatures { LocalFeatures { flags: Vec::new(), } @@ -611,9 +612,9 @@ pub enum OptionalField { pub trait ChannelMessageHandler : events::MessageSendEventsProvider + Send + Sync { //Channel init: /// Handle an incoming open_channel message from the given peer. - fn handle_open_channel(&self, their_node_id: &PublicKey, msg: &OpenChannel) -> Result<(), HandleError>; + fn handle_open_channel(&self, their_node_id: &PublicKey, their_local_features: LocalFeatures, msg: &OpenChannel) -> Result<(), HandleError>; /// Handle an incoming accept_channel message from the given peer. - fn handle_accept_channel(&self, their_node_id: &PublicKey, msg: &AcceptChannel) -> Result<(), HandleError>; + fn handle_accept_channel(&self, their_node_id: &PublicKey, their_local_features: LocalFeatures, msg: &AcceptChannel) -> Result<(), HandleError>; /// Handle an incoming funding_created message from the given peer. fn handle_funding_created(&self, their_node_id: &PublicKey, msg: &FundingCreated) -> Result<(), HandleError>; /// Handle an incoming funding_signed message from the given peer. diff --git a/src/ln/peer_handler.rs b/src/ln/peer_handler.rs index cfdc6f1cb..a9784327c 100644 --- a/src/ln/peer_handler.rs +++ b/src/ln/peer_handler.rs @@ -659,11 +659,11 @@ impl PeerManager { // Channel control: 32 => { let msg = try_potential_decodeerror!(msgs::OpenChannel::read(&mut reader)); - try_potential_handleerror!(self.message_handler.chan_handler.handle_open_channel(&peer.their_node_id.unwrap(), &msg)); + try_potential_handleerror!(self.message_handler.chan_handler.handle_open_channel(&peer.their_node_id.unwrap(), peer.their_local_features.clone().unwrap(), &msg)); }, 33 => { let msg = try_potential_decodeerror!(msgs::AcceptChannel::read(&mut reader)); - try_potential_handleerror!(self.message_handler.chan_handler.handle_accept_channel(&peer.their_node_id.unwrap(), &msg)); + try_potential_handleerror!(self.message_handler.chan_handler.handle_accept_channel(&peer.their_node_id.unwrap(), peer.their_local_features.clone().unwrap(), &msg)); }, 34 => { diff --git a/src/util/test_utils.rs b/src/util/test_utils.rs index 5d9b00688..f88ded97e 100644 --- a/src/util/test_utils.rs +++ b/src/util/test_utils.rs @@ -4,6 +4,7 @@ use chain::transaction::OutPoint; use chain::keysinterface; use ln::channelmonitor; use ln::msgs; +use ln::msgs::LocalFeatures; use ln::msgs::{HandleError}; use ln::channelmonitor::HTLCUpdate; use util::events; @@ -96,10 +97,10 @@ impl TestChannelMessageHandler { } impl msgs::ChannelMessageHandler for TestChannelMessageHandler { - fn handle_open_channel(&self, _their_node_id: &PublicKey, _msg: &msgs::OpenChannel) -> Result<(), HandleError> { + fn handle_open_channel(&self, _their_node_id: &PublicKey, _their_local_features: LocalFeatures, _msg: &msgs::OpenChannel) -> Result<(), HandleError> { Err(HandleError { err: "", action: None }) } - fn handle_accept_channel(&self, _their_node_id: &PublicKey, _msg: &msgs::AcceptChannel) -> Result<(), HandleError> { + fn handle_accept_channel(&self, _their_node_id: &PublicKey, _their_local_features: LocalFeatures, _msg: &msgs::AcceptChannel) -> Result<(), HandleError> { Err(HandleError { err: "", action: None }) } fn handle_funding_created(&self, _their_node_id: &PublicKey, _msg: &msgs::FundingCreated) -> Result<(), HandleError> { -- 2.39.5