From 9f7de472fb5e5371f8d697941b6ab443fadb220f Mon Sep 17 00:00:00 2001 From: Wilmer Paulino Date: Wed, 11 Oct 2023 09:42:05 -0700 Subject: [PATCH] Send bogus ChannelReestablish for unknown channels Unfortunately, lnd doesn't force close on errors (https://github.com/lightningnetwork/lnd/blob/abb1e3463f3a83bbb843d5c399869dbe930ad94f/htlcswitch/link.go#L2119). One of the few ways to get an lnd counterparty to force close is by replicating what they do when restoring static channel backups (SCBs). They send an invalid `ChannelReestablish` with `0` commitment numbers and an invalid `your_last_per_commitment_secret`. Since we received a `ChannelReestablish` for a channel that doesn't exist, we can assume it's likely the channel closed from our point of view, but it remains open on the counterparty's side. By sending this bogus `ChannelReestablish` message now as a response to theirs, we trigger them to force close broadcasting their latest state. If the closing transaction from our point of view remains unconfirmed, it'll enter a race with the counterparty's to-be-broadcast latest commitment transaction. --- lightning/src/ln/channel.rs | 2 +- lightning/src/ln/channelmanager.rs | 100 ++++++++++++++++++++++++++++- lightning/src/ln/payment_tests.rs | 8 +-- lightning/src/ln/reload_tests.rs | 20 +++--- lightning/src/ln/shutdown_tests.rs | 4 +- 5 files changed, 114 insertions(+), 20 deletions(-) diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index a61a8de8..2359d1ef 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -3992,7 +3992,7 @@ impl Channel where if msg.next_local_commitment_number >= INITIAL_COMMITMENT_NUMBER || msg.next_remote_commitment_number >= INITIAL_COMMITMENT_NUMBER || msg.next_local_commitment_number == 0 { - return Err(ChannelError::Close("Peer sent a garbage channel_reestablish (usually an lnd node with lost state asking us to force-close for them)".to_owned())); + return Err(ChannelError::Close("Peer sent an invalid channel_reestablish to force close in a non-standard way".to_owned())); } if msg.next_remote_commitment_number > 0 { diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index 24ca65af..35107f10 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -6785,7 +6785,10 @@ where let peer_state_mutex = per_peer_state.get(counterparty_node_id) .ok_or_else(|| { debug_assert!(false); - MsgHandleErrInternal::send_err_msg_no_close(format!("Can't find a peer matching the passed counterparty node_id {}", counterparty_node_id), msg.channel_id) + MsgHandleErrInternal::send_err_msg_no_close( + format!("Can't find a peer matching the passed counterparty node_id {}", counterparty_node_id), + msg.channel_id + ) })?; let mut peer_state_lock = peer_state_mutex.lock().unwrap(); let peer_state = &mut *peer_state_lock; @@ -6829,7 +6832,39 @@ where "Got a channel_reestablish message for an unfunded channel!".into())), chan_phase_entry); } }, - hash_map::Entry::Vacant(_) => return Err(MsgHandleErrInternal::send_err_msg_no_close(format!("Got a message for a channel from the wrong node! No such channel for the passed counterparty_node_id {}", counterparty_node_id), msg.channel_id)) + hash_map::Entry::Vacant(_) => { + log_debug!(self.logger, "Sending bogus ChannelReestablish for unknown channel {} to force channel closure", + log_bytes!(msg.channel_id.0)); + // Unfortunately, lnd doesn't force close on errors + // (https://github.com/lightningnetwork/lnd/blob/abb1e3463f3a83bbb843d5c399869dbe930ad94f/htlcswitch/link.go#L2119). + // One of the few ways to get an lnd counterparty to force close is by + // replicating what they do when restoring static channel backups (SCBs). They + // send an invalid `ChannelReestablish` with `0` commitment numbers and an + // invalid `your_last_per_commitment_secret`. + // + // Since we received a `ChannelReestablish` for a channel that doesn't exist, we + // can assume it's likely the channel closed from our point of view, but it + // remains open on the counterparty's side. By sending this bogus + // `ChannelReestablish` message now as a response to theirs, we trigger them to + // force close broadcasting their latest state. If the closing transaction from + // our point of view remains unconfirmed, it'll enter a race with the + // counterparty's to-be-broadcast latest commitment transaction. + peer_state.pending_msg_events.push(MessageSendEvent::SendChannelReestablish { + node_id: *counterparty_node_id, + msg: msgs::ChannelReestablish { + channel_id: msg.channel_id, + next_local_commitment_number: 0, + next_remote_commitment_number: 0, + your_last_per_commitment_secret: [1u8; 32], + my_current_per_commitment_point: PublicKey::from_slice(&[2u8; 33]).unwrap(), + next_funding_txid: None, + }, + }); + return Err(MsgHandleErrInternal::send_err_msg_no_close( + format!("Got a message for a channel from the wrong node! No such channel for the passed counterparty_node_id {}", + counterparty_node_id), msg.channel_id) + ) + } } }; @@ -11219,6 +11254,67 @@ mod tests { let payment_preimage = PaymentPreimage([42; 32]); assert_eq!(format!("{}", &payment_preimage), "2a2a2a2a2a2a2a2a2a2a2a2a2a2a2a2a2a2a2a2a2a2a2a2a2a2a2a2a2a2a2a2a"); } + + #[test] + fn test_trigger_lnd_force_close() { + let chanmon_cfg = create_chanmon_cfgs(2); + let node_cfg = create_node_cfgs(2, &chanmon_cfg); + let user_config = test_default_channel_config(); + let node_chanmgr = create_node_chanmgrs(2, &node_cfg, &[Some(user_config), Some(user_config)]); + let nodes = create_network(2, &node_cfg, &node_chanmgr); + + // Open a channel, immediately disconnect each other, and broadcast Alice's latest state. + let (_, _, chan_id, funding_tx) = create_announced_chan_between_nodes(&nodes, 0, 1); + nodes[0].node.peer_disconnected(&nodes[1].node.get_our_node_id()); + nodes[1].node.peer_disconnected(&nodes[0].node.get_our_node_id()); + nodes[0].node.force_close_broadcasting_latest_txn(&chan_id, &nodes[1].node.get_our_node_id()).unwrap(); + check_closed_broadcast(&nodes[0], 1, true); + check_added_monitors(&nodes[0], 1); + check_closed_event!(nodes[0], 1, ClosureReason::HolderForceClosed, [nodes[1].node.get_our_node_id()], 100000); + { + let txn = nodes[0].tx_broadcaster.txn_broadcast(); + assert_eq!(txn.len(), 1); + check_spends!(txn[0], funding_tx); + } + + // Since they're disconnected, Bob won't receive Alice's `Error` message. Reconnect them + // such that Bob sends a `ChannelReestablish` to Alice since the channel is still open from + // their side. + nodes[0].node.peer_connected(&nodes[1].node.get_our_node_id(), &msgs::Init { + features: nodes[1].node.init_features(), networks: None, remote_network_address: None + }, true).unwrap(); + nodes[1].node.peer_connected(&nodes[0].node.get_our_node_id(), &msgs::Init { + features: nodes[0].node.init_features(), networks: None, remote_network_address: None + }, false).unwrap(); + assert!(nodes[0].node.get_and_clear_pending_msg_events().is_empty()); + let channel_reestablish = get_event_msg!( + nodes[1], MessageSendEvent::SendChannelReestablish, nodes[0].node.get_our_node_id() + ); + nodes[0].node.handle_channel_reestablish(&nodes[1].node.get_our_node_id(), &channel_reestablish); + + // Alice should respond with an error since the channel isn't known, but a bogus + // `ChannelReestablish` should be sent first, such that we actually trigger Bob to force + // close even if it was an lnd node. + let msg_events = nodes[0].node.get_and_clear_pending_msg_events(); + assert_eq!(msg_events.len(), 2); + if let MessageSendEvent::SendChannelReestablish { node_id, msg } = &msg_events[0] { + assert_eq!(*node_id, nodes[1].node.get_our_node_id()); + assert_eq!(msg.next_local_commitment_number, 0); + assert_eq!(msg.next_remote_commitment_number, 0); + nodes[1].node.handle_channel_reestablish(&nodes[0].node.get_our_node_id(), &msg); + } else { panic!() }; + check_closed_broadcast(&nodes[1], 1, true); + check_added_monitors(&nodes[1], 1); + let expected_close_reason = ClosureReason::ProcessingError { + err: "Peer sent an invalid channel_reestablish to force close in a non-standard way".to_string() + }; + check_closed_event!(nodes[1], 1, expected_close_reason, [nodes[0].node.get_our_node_id()], 100000); + { + let txn = nodes[1].tx_broadcaster.txn_broadcast(); + assert_eq!(txn.len(), 1); + check_spends!(txn[0], funding_tx); + } + } } #[cfg(ldk_bench)] diff --git a/lightning/src/ln/payment_tests.rs b/lightning/src/ln/payment_tests.rs index 26ecbb0b..f35b67e9 100644 --- a/lightning/src/ln/payment_tests.rs +++ b/lightning/src/ln/payment_tests.rs @@ -706,8 +706,8 @@ fn do_retry_with_no_persist(confirm_before_reload: bool) { let bs_reestablish = get_chan_reestablish_msgs!(nodes[1], nodes[0]).pop().unwrap(); nodes[0].node.handle_channel_reestablish(&nodes[1].node.get_our_node_id(), &bs_reestablish); let as_err = nodes[0].node.get_and_clear_pending_msg_events(); - assert_eq!(as_err.len(), 1); - match as_err[0] { + assert_eq!(as_err.len(), 2); + match as_err[1] { MessageSendEvent::HandleError { node_id, action: msgs::ErrorAction::SendErrorMessage { ref msg } } => { assert_eq!(node_id, nodes[1].node.get_our_node_id()); nodes[1].node.handle_error(&nodes[0].node.get_our_node_id(), msg); @@ -881,9 +881,9 @@ fn do_test_completed_payment_not_retryable_on_reload(use_dust: bool) { let bs_reestablish = get_chan_reestablish_msgs!(nodes[1], nodes[0]).pop().unwrap(); nodes[0].node.handle_channel_reestablish(&nodes[1].node.get_our_node_id(), &bs_reestablish); let as_err = nodes[0].node.get_and_clear_pending_msg_events(); - assert_eq!(as_err.len(), 1); + assert_eq!(as_err.len(), 2); let bs_commitment_tx; - match as_err[0] { + match as_err[1] { MessageSendEvent::HandleError { node_id, action: msgs::ErrorAction::SendErrorMessage { ref msg } } => { assert_eq!(node_id, nodes[1].node.get_our_node_id()); nodes[1].node.handle_error(&nodes[0].node.get_our_node_id(), msg); diff --git a/lightning/src/ln/reload_tests.rs b/lightning/src/ln/reload_tests.rs index 5f4009d6..9ea13f13 100644 --- a/lightning/src/ln/reload_tests.rs +++ b/lightning/src/ln/reload_tests.rs @@ -589,18 +589,16 @@ fn do_test_data_loss_protect(reconnect_panicing: bool) { nodes[0].node.handle_channel_reestablish(&nodes[1].node.get_our_node_id(), &retry_reestablish[0]); let mut err_msgs_0 = Vec::with_capacity(1); - for msg in nodes[0].node.get_and_clear_pending_msg_events() { - if let MessageSendEvent::HandleError { ref action, .. } = msg { - match action { - &ErrorAction::SendErrorMessage { ref msg } => { - assert_eq!(msg.data, format!("Got a message for a channel from the wrong node! No such channel for the passed counterparty_node_id {}", &nodes[1].node.get_our_node_id())); - err_msgs_0.push(msg.clone()); - }, - _ => panic!("Unexpected event!"), - } - } else { - panic!("Unexpected event!"); + if let MessageSendEvent::HandleError { ref action, .. } = nodes[0].node.get_and_clear_pending_msg_events()[1] { + match action { + &ErrorAction::SendErrorMessage { ref msg } => { + assert_eq!(msg.data, format!("Got a message for a channel from the wrong node! No such channel for the passed counterparty_node_id {}", &nodes[1].node.get_our_node_id())); + err_msgs_0.push(msg.clone()); + }, + _ => panic!("Unexpected event!"), } + } else { + panic!("Unexpected event!"); } assert_eq!(err_msgs_0.len(), 1); nodes[1].node.handle_error(&nodes[0].node.get_our_node_id(), &err_msgs_0[0]); diff --git a/lightning/src/ln/shutdown_tests.rs b/lightning/src/ln/shutdown_tests.rs index 47361693..9bc133f4 100644 --- a/lightning/src/ln/shutdown_tests.rs +++ b/lightning/src/ln/shutdown_tests.rs @@ -623,8 +623,8 @@ fn do_test_shutdown_rebroadcast(recv_count: u8) { nodes[0].node.handle_channel_reestablish(&nodes[1].node.get_our_node_id(), &node_1_2nd_reestablish); let msg_events = nodes[0].node.get_and_clear_pending_msg_events(); - assert_eq!(msg_events.len(), 1); - if let MessageSendEvent::HandleError { ref action, .. } = msg_events[0] { + assert_eq!(msg_events.len(), 2); + if let MessageSendEvent::HandleError { ref action, .. } = msg_events[1] { match action { &ErrorAction::SendErrorMessage { ref msg } => { nodes[1].node.handle_error(&nodes[0].node.get_our_node_id(), &msg); -- 2.30.2