mirrors/rust-lightning
1
0
Fork 0
mirror of https://github.com/lightningdevkit/rust-lightning.git synced 2025-02-24 15:02:20 +01:00
Code Issues Projects Releases Packages Wiki Activity

Stop sending channel_update in onion failures

Browse source 
Per <https://github.com/lightning/bolts/pull/1173>.

Fixes #3277
This commit is contained in:
Leo Nash 2024-09-28 19:41:27 +00:00
parent c7627dfd61
commit 9e30f2560d
4 changed files with 69 additions and 142 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

8
lightning/src/ln/channel.rs
View file

@ -2142,6 +2142,14 @@ impl<SP: Deref> ChannelContext<SP> where SP::Target: SignerProvider {
self.is_usable() && !self.channel_state.is_peer_disconnected()
}
/// Returns false if our last broadcasted channel_update message has the "channel disabled" bit set
pub fn is_enabled(&self) -> bool {
self.is_usable() && match self.channel_update_status {
ChannelUpdateStatus::Enabled | ChannelUpdateStatus::DisabledStaged(_) => true,
ChannelUpdateStatus::Disabled | ChannelUpdateStatus::EnabledStaged(_) => false,
}
}
// Public utilities:
pub fn channel_id(&self) -> ChannelId {

155
lightning/src/ln/channelmanager.rs
View file

@ -62,7 +62,6 @@ use crate::ln::msgs::{ChannelMessageHandler, DecodeError, LightningError};
#[cfg(test)]
use crate::ln::outbound_payment;
use crate::ln::outbound_payment::{OutboundPayments, PendingOutboundPayment, RetryableInvoiceRequest, SendAlongPathArgs, StaleExpiration};
use crate::ln::wire::Encode;
use crate::offers::invoice::{Bolt12Invoice, DEFAULT_RELATIVE_EXPIRY, DerivedSigningPubkey, ExplicitSigningPubkey, InvoiceBuilder, UnsignedBolt12Invoice};
use crate::offers::invoice_error::InvoiceError;
use crate::offers::invoice_request::{DerivedPayerSigningPubkey, InvoiceRequest, InvoiceRequestBuilder};
@ -677,7 +676,7 @@ pub enum FailureCode {
}
impl Into<u16> for FailureCode {
fn into(self) -> u16 {
fn into(self) -> u16 {
match self {
FailureCode::TemporaryNodeFailure => 0x2000 | 2,
FailureCode::RequiredNodeFeatureMissing => 0x4000 | 0x2000 | 3,
@ -3841,18 +3840,18 @@ where
fn can_forward_htlc_to_outgoing_channel(
&self, chan: &mut Channel<SP>, msg: &msgs::UpdateAddHTLC, next_packet: &NextPacketDetails
) -> Result<(), (&'static str, u16, Option<msgs::ChannelUpdate>)> {
) -> Result<(), (&'static str, u16)> {
if !chan.context.should_announce() && !self.default_configuration.accept_forwards_to_priv_channels {
// Note that the behavior here should be identical to the above block - we
// should NOT reveal the existence or non-existence of a private channel if
// we don't allow forwards outbound over them.
return Err(("Refusing to forward to a private channel based on our config.", 0x4000 | 10, None));
return Err(("Refusing to forward to a private channel based on our config.", 0x4000 | 10));
}
if chan.context.get_channel_type().supports_scid_privacy() && next_packet.outgoing_scid != chan.context.outbound_scid_alias() {
// `option_scid_alias` (referred to in LDK as `scid_privacy`) means
// "refuse to forward unless the SCID alias was used", so we pretend
// we don't have the channel here.
return Err(("Refusing to forward over real channel SCID as our counterparty requested.", 0x4000 | 10, None));
return Err(("Refusing to forward over real channel SCID as our counterparty requested.", 0x4000 | 10));
}
// Note that we could technically not return an error yet here and just hope
@ -3860,24 +3859,20 @@ where
// around to doing the actual forward, but better to fail early if we can and
// hopefully an attacker trying to path-trace payments cannot make this occur
// on a small/per-node/per-channel scale.
if !chan.context.is_live() { // channel_disabled
// If the channel_update we're going to return is disabled (i.e. the
// peer has been disabled for some time), return `channel_disabled`,
// otherwise return `temporary_channel_failure`.
let chan_update_opt = self.get_channel_update_for_onion(next_packet.outgoing_scid, chan).ok();
if chan_update_opt.as_ref().map(|u| u.contents.channel_flags & 2 == 2).unwrap_or(false) {
return Err(("Forwarding channel has been disconnected for some time.", 0x1000 | 20, chan_update_opt));
if !chan.context.is_live() {
if !chan.context.is_enabled() {
// channel_disabled
return Err(("Forwarding channel has been disconnected for some time.", 0x1000 | 20));
} else {
return Err(("Forwarding channel is not in a ready state.", 0x1000 | 7, chan_update_opt));
// temporary_channel_failure
return Err(("Forwarding channel is not in a ready state.", 0x1000 | 7));
}
}
if next_packet.outgoing_amt_msat < chan.context.get_counterparty_htlc_minimum_msat() { // amount_below_minimum
let chan_update_opt = self.get_channel_update_for_onion(next_packet.outgoing_scid, chan).ok();
return Err(("HTLC amount was below the htlc_minimum_msat", 0x1000 | 11, chan_update_opt));
return Err(("HTLC amount was below the htlc_minimum_msat", 0x1000 | 11));
}
if let Err((err, code)) = chan.htlc_satisfies_config(msg, next_packet.outgoing_amt_msat, next_packet.outgoing_cltv_value) {
let chan_update_opt = self.get_channel_update_for_onion(next_packet.outgoing_scid, chan).ok();
return Err((err, code, chan_update_opt));
return Err((err, code));
}
Ok(())
@ -3909,7 +3904,7 @@ where
fn can_forward_htlc(
&self, msg: &msgs::UpdateAddHTLC, next_packet_details: &NextPacketDetails
) -> Result<(), (&'static str, u16, Option<msgs::ChannelUpdate>)> {
) -> Result<(), (&'static str, u16)> {
match self.do_funded_channel_callback(next_packet_details.outgoing_scid, |chan: &mut Channel<SP>| {
self.can_forward_htlc_to_outgoing_channel(chan, msg, next_packet_details)
}) {
@ -3922,7 +3917,7 @@ where
fake_scid::is_valid_intercept(&self.fake_scid_rand_bytes, next_packet_details.outgoing_scid, &self.chain_hash)) ||
fake_scid::is_valid_phantom(&self.fake_scid_rand_bytes, next_packet_details.outgoing_scid, &self.chain_hash)
{} else {
return Err(("Don't have available channel for forwarding as requested.", 0x4000 | 10, None));
return Err(("Don't have available channel for forwarding as requested.", 0x4000 | 10));
}
}
}
@ -3931,10 +3926,7 @@ where
if let Err((err_msg, err_code)) = check_incoming_htlc_cltv(
cur_height, next_packet_details.outgoing_cltv_value, msg.cltv_expiry
) {
let chan_update_opt = self.do_funded_channel_callback(next_packet_details.outgoing_scid, |chan: &mut Channel<SP>| {
self.get_channel_update_for_onion(next_packet_details.outgoing_scid, chan).ok()
}).flatten();
return Err((err_msg, err_code, chan_update_opt));
return Err((err_msg, err_code));
}
Ok(())
@ -3942,12 +3934,12 @@ where
fn htlc_failure_from_update_add_err(
&self, msg: &msgs::UpdateAddHTLC, counterparty_node_id: &PublicKey, err_msg: &'static str,
mut err_code: u16, chan_update: Option<msgs::ChannelUpdate>, is_intro_node_blinded_forward: bool,
err_code: u16, is_intro_node_blinded_forward: bool,
shared_secret: &[u8; 32]
) -> HTLCFailureMsg {
let mut res = VecWriter(Vec::with_capacity(chan_update.serialized_length() + 2 + 8 + 2));
if chan_update.is_some() && err_code & 0x1000 == 0x1000 {
let chan_update = chan_update.unwrap();
// at capacity, we write fields `htlc_msat` and `len`
let mut res = VecWriter(Vec::with_capacity(8 + 2));
if err_code & 0x1000 == 0x1000 {
if err_code == 0x1000 | 11 || err_code == 0x1000 | 12 {
msg.amount_msat.write(&mut res).expect("Writes cannot fail");
}
@ -3958,15 +3950,8 @@ where
// TODO: underspecified, follow https://github.com/lightning/bolts/issues/791
0u16.write(&mut res).expect("Writes cannot fail");
}
(chan_update.serialized_length() as u16 + 2).write(&mut res).expect("Writes cannot fail");
msgs::ChannelUpdate::TYPE.write(&mut res).expect("Writes cannot fail");
chan_update.write(&mut res).expect("Writes cannot fail");
} else if err_code & 0x1000 == 0x1000 {
// If we're trying to return an error that requires a `channel_update` but
// we're forwarding to a phantom or intercept "channel" (i.e. cannot
// generate an update), just use the generic "temporary_node_failure"
// instead.
err_code = 0x2000 | 2;
// See https://github.com/lightning/bolts/blob/247e83d/04-onion-routing.md?plain=1#L1414-L1415
(0u16).write(&mut res).expect("Writes cannot fail");
}
log_info!(
@ -4014,9 +3999,9 @@ where
// Perform outbound checks here instead of in [`Self::construct_pending_htlc_info`] because we
// can't hold the outbound peer state lock at the same time as the inbound peer state lock.
self.can_forward_htlc(&msg, &next_packet_details).map_err(|e| {
let (err_msg, err_code, chan_update_opt) = e;
let (err_msg, err_code) = e;
self.htlc_failure_from_update_add_err(
msg, counterparty_node_id, err_msg, err_code, chan_update_opt,
msg, counterparty_node_id, err_msg, err_code,
next_hop.is_intro_node_blinded_forward(), &shared_secret
)
})?;
@ -4125,20 +4110,10 @@ where
Some(id) => id,
};
self.get_channel_update_for_onion(short_channel_id, chan)
}
fn get_channel_update_for_onion(&self, short_channel_id: u64, chan: &Channel<SP>) -> Result<msgs::ChannelUpdate, LightningError> {
let logger = WithChannelContext::from(&self.logger, &chan.context, None);
log_trace!(logger, "Generating channel update for channel {}", chan.context.channel_id());
let were_node_one = self.our_network_pubkey.serialize()[..] < chan.context.get_counterparty_node_id().serialize()[..];
let enabled = chan.context.is_usable() && match chan.channel_update_status() {
ChannelUpdateStatus::Enabled => true,
ChannelUpdateStatus::DisabledStaged(_) => true,
ChannelUpdateStatus::Disabled => false,
ChannelUpdateStatus::EnabledStaged(_) => false,
};
let enabled = chan.context.is_enabled();
let unsigned = msgs::UnsignedChannelUpdate {
chain_hash: self.chain_hash,
@ -5326,16 +5301,9 @@ where
}) {
Some(Ok(_)) => {},
Some(Err((err, code))) => {
let outgoing_chan_update_opt = if let Some(outgoing_scid) = outgoing_scid_opt.as_ref() {
self.do_funded_channel_callback(*outgoing_scid, |chan: &mut Channel<SP>| {
self.get_channel_update_for_onion(*outgoing_scid, chan).ok()
}).flatten()
} else {
None
};
let htlc_fail = self.htlc_failure_from_update_add_err(
&update_add_htlc, &incoming_counterparty_node_id, err, code,
outgoing_chan_update_opt, is_intro_node_blinded_forward, &shared_secret,
is_intro_node_blinded_forward, &shared_secret,
);
let htlc_destination = get_failed_htlc_destination(outgoing_scid_opt, update_add_htlc.payment_hash);
htlc_fails.push((htlc_fail, htlc_destination));
@ -5347,12 +5315,12 @@ where
// Now process the HTLC on the outgoing channel if it's a forward.
if let Some(next_packet_details) = next_packet_details_opt.as_ref() {
if let Err((err, code, chan_update_opt)) = self.can_forward_htlc(
if let Err((err, code)) = self.can_forward_htlc(
&update_add_htlc, next_packet_details
) {
let htlc_fail = self.htlc_failure_from_update_add_err(
&update_add_htlc, &incoming_counterparty_node_id, err, code,
chan_update_opt, is_intro_node_blinded_forward, &shared_secret,
is_intro_node_blinded_forward, &shared_secret,
);
let htlc_destination = get_failed_htlc_destination(outgoing_scid_opt, update_add_htlc.payment_hash);
htlc_fails.push((htlc_fail, htlc_destination));
@ -5633,7 +5601,8 @@ where
}
if let Some(ChannelPhase::Funded(ref mut chan)) = peer_state.channel_by_id.get_mut(&forward_chan_id) {
let (failure_code, data) = self.get_htlc_temp_fail_err_and_data(0x1000|7, short_chan_id, chan);
let failure_code = 0x1000|7;
let data = self.get_htlc_inbound_temp_fail_data(failure_code);
failed_forwards.push((htlc_source, payment_hash,
HTLCFailReason::reason(failure_code, data),
HTLCDestination::NextHopChannel { node_id: Some(chan.context.get_counterparty_node_id()), channel_id: forward_chan_id }
@ -6448,46 +6417,21 @@ where
///
/// This is for failures on the channel on which the HTLC was *received*, not failures
/// forwarding
fn get_htlc_inbound_temp_fail_err_and_data(&self, desired_err_code: u16, chan: &Channel<SP>) -> (u16, Vec<u8>) {
// We can't be sure what SCID was used when relaying inbound towards us, so we have to
// guess somewhat. If its a public channel, we figure best to just use the real SCID (as
// we're not leaking that we have a channel with the counterparty), otherwise we try to use
// an inbound SCID alias before the real SCID.
let scid_pref = if chan.context.should_announce() {
chan.context.get_short_channel_id().or(chan.context.latest_inbound_scid_alias())
} else {
chan.context.latest_inbound_scid_alias().or(chan.context.get_short_channel_id())
};
if let Some(scid) = scid_pref {
self.get_htlc_temp_fail_err_and_data(desired_err_code, scid, chan)
} else {
(0x4000|10, Vec::new())
}
}
/// Gets an HTLC onion failure code and error data for an `UPDATE` error, given the error code
/// that we want to return and a channel.
fn get_htlc_temp_fail_err_and_data(&self, desired_err_code: u16, scid: u64, chan: &Channel<SP>) -> (u16, Vec<u8>) {
debug_assert_eq!(desired_err_code & 0x1000, 0x1000);
if let Ok(upd) = self.get_channel_update_for_onion(scid, chan) {
let mut enc = VecWriter(Vec::with_capacity(upd.serialized_length() + 6));
if desired_err_code == 0x1000 | 20 {
// No flags for `disabled_flags` are currently defined so they're always two zero bytes.
// See https://github.com/lightning/bolts/blob/341ec84/04-onion-routing.md?plain=1#L1008
0u16.write(&mut enc).expect("Writes cannot fail");
}
(upd.serialized_length() as u16 + 2).write(&mut enc).expect("Writes cannot fail");
msgs::ChannelUpdate::TYPE.write(&mut enc).expect("Writes cannot fail");
upd.write(&mut enc).expect("Writes cannot fail");
(desired_err_code, enc.0)
} else {
// If we fail to get a unicast channel_update, it implies we don't yet have an SCID,
// which means we really shouldn't have gotten a payment to be forwarded over this
// channel yet, or if we did it's from a route hint. Either way, returning an error of
// PERM|no_such_channel should be fine.
(0x4000|10, Vec::new())
fn get_htlc_inbound_temp_fail_data(&self, err_code: u16) -> Vec<u8> {
debug_assert_eq!(err_code & 0x1000, 0x1000);
debug_assert_ne!(err_code, 0x1000|11);
debug_assert_ne!(err_code, 0x1000|12);
debug_assert_ne!(err_code, 0x1000|13);
// at capacity, we write fields `disabled_flags` and `len`
let mut enc = VecWriter(Vec::with_capacity(4));
if err_code == 0x1000 | 20 {
// No flags for `disabled_flags` are currently defined so they're always two zero bytes.
// See https://github.com/lightning/bolts/blob/341ec84/04-onion-routing.md?plain=1#L1008
0u16.write(&mut enc).expect("Writes cannot fail");
}
// See https://github.com/lightning/bolts/blob/247e83d/04-onion-routing.md?plain=1#L1414-L1415
(0u16).write(&mut enc).expect("Writes cannot fail");
enc.0
}
// Fail a list of HTLCs that were just freed from the holding cell. The HTLCs need to be
@ -6504,8 +6448,10 @@ where
let peer_state = &mut *peer_state_lock;
match peer_state.channel_by_id.entry(channel_id) {
hash_map::Entry::Occupied(chan_phase_entry) => {
if let ChannelPhase::Funded(chan) = chan_phase_entry.get() {
self.get_htlc_inbound_temp_fail_err_and_data(0x1000|7, &chan)
if let ChannelPhase::Funded(_chan) = chan_phase_entry.get() {
let failure_code = 0x1000|7;
let data = self.get_htlc_inbound_temp_fail_data(failure_code);
(failure_code, data)
} else {
// We shouldn't be trying to fail holding cell HTLCs on an unfunded channel.
debug_assert!(false);
@ -8164,8 +8110,8 @@ where
let reason = if routing.blinded_failure().is_some() {
HTLCFailReason::reason(INVALID_ONION_BLINDING, vec![0; 32])
} else if (error_code & 0x1000) != 0 {
let (real_code, error_data) = self.get_htlc_inbound_temp_fail_err_and_data(error_code, chan);
HTLCFailReason::reason(real_code, error_data)
let error_data = self.get_htlc_inbound_temp_fail_data(error_code);
HTLCFailReason::reason(error_code, error_data)
} else {
HTLCFailReason::from_failure_code(error_code)
}.get_encrypted_failure_packet(incoming_shared_secret, &None);
@ -10190,7 +10136,8 @@ where
let res = f(channel);
if let Ok((channel_ready_opt, mut timed_out_pending_htlcs, announcement_sigs)) = res {
for (source, payment_hash) in timed_out_pending_htlcs.drain(..) {
let (failure_code, data) = self.get_htlc_inbound_temp_fail_err_and_data(0x1000|14 /* expiry_too_soon */, &channel);
let failure_code = 0x1000|14; /* expiry_too_soon */
let data = self.get_htlc_inbound_temp_fail_data(failure_code);
timed_out_htlcs.push((source, payment_hash, HTLCFailReason::reason(failure_code, data),
HTLCDestination::NextHopChannel { node_id: Some(channel.context.get_counterparty_node_id()), channel_id: channel.context.channel_id() }));
}

14
lightning/src/ln/onion_route_tests.rs
View file

@ -1397,9 +1397,12 @@ fn test_phantom_failure_modified_cltv() {
commitment_signed_dance!(nodes[0], nodes[1], update_1.commitment_signed, false);
// Ensure the payment fails with the expected error.
let mut err_data = Vec::new();
err_data.extend_from_slice(&update_add.cltv_expiry.to_be_bytes());
err_data.extend_from_slice(&0u16.to_be_bytes());
let mut fail_conditions = PaymentFailedConditions::new()
.blamed_scid(phantom_scid)
.expected_htlc_error_data(0x2000 | 2, &[]);
.expected_htlc_error_data(0x1000 | 13, &err_data);
expect_payment_failed_conditions(&nodes[0], payment_hash, false, fail_conditions);
}
@ -1437,9 +1440,10 @@ fn test_phantom_failure_expires_too_soon() {
commitment_signed_dance!(nodes[0], nodes[1], update_1.commitment_signed, false);
// Ensure the payment fails with the expected error.
let err_data = 0u16.to_be_bytes();
let mut fail_conditions = PaymentFailedConditions::new()
.blamed_scid(phantom_scid)
.expected_htlc_error_data(0x2000 | 2, &[]);
.expected_htlc_error_data(0x1000 | 14, &err_data);
expect_payment_failed_conditions(&nodes[0], payment_hash, false, fail_conditions);
}
@ -1535,11 +1539,7 @@ fn do_test_phantom_dust_exposure_failure(multiplier_dust_limit: bool) {
commitment_signed_dance!(nodes[0], nodes[1], update_1.commitment_signed, false);
// Ensure the payment fails with the expected error.
let mut err_data = Vec::new();
err_data.extend_from_slice(&(channel.1.serialized_length() as u16 + 2).to_be_bytes());
err_data.extend_from_slice(&ChannelUpdate::TYPE.to_be_bytes());
err_data.extend_from_slice(&channel.1.encode());
let err_data = 0u16.to_be_bytes();
let mut fail_conditions = PaymentFailedConditions::new()
.blamed_scid(route.paths[0].hops.last().as_ref().unwrap().short_channel_id)
.blamed_chan_closed(false)

34
lightning/src/ln/priv_short_conf_tests.rs
View file

@ -12,7 +12,6 @@
//! LSP).
use crate::chain::ChannelMonitorUpdateStatus;
use crate::sign::NodeSigner;
use crate::events::{ClosureReason, Event, HTLCDestination, MessageSendEvent, MessageSendEventsProvider};
use crate::ln::channelmanager::{MIN_CLTV_EXPIRY_DELTA, PaymentId, RecipientOnionFields};
use crate::routing::gossip::RoutingFees;
@ -20,8 +19,7 @@ use crate::routing::router::{PaymentParameters, RouteHint, RouteHintHop};
use crate::ln::features::ChannelTypeFeatures;
use crate::ln::msgs;
use crate::ln::types::ChannelId;
use crate::ln::msgs::{ChannelMessageHandler, RoutingMessageHandler, ChannelUpdate, ErrorAction};
use crate::ln::wire::Encode;
use crate::ln::msgs::{ChannelMessageHandler, RoutingMessageHandler, ErrorAction};
use crate::util::config::{UserConfig, MaxDustHTLCExposure};
use crate::util::ser::Writeable;
@ -29,9 +27,6 @@ use crate::prelude::*;
use crate::ln::functional_test_utils::*;
use bitcoin::constants::ChainHash;
use bitcoin::network::Network;
#[test]
fn test_priv_forwarding_rejection() {
// If we have a private channel with outbound liquidity, and
@ -503,28 +498,7 @@ fn test_scid_alias_returned() {
nodes[0].node.handle_update_fail_htlc(nodes[1].node.get_our_node_id(), &bs_updates.update_fail_htlcs[0]);
commitment_signed_dance!(nodes[0], nodes[1], bs_updates.commitment_signed, false, true);
// Build the expected channel update
let contents = msgs::UnsignedChannelUpdate {
chain_hash: ChainHash::using_genesis_block(Network::Testnet),
short_channel_id: last_hop[0].inbound_scid_alias.unwrap(),
timestamp: 21,
message_flags: 1, // Only must_be_one
channel_flags: 1,
cltv_expiry_delta: accept_forward_cfg.channel_config.cltv_expiry_delta,
htlc_minimum_msat: 1_000,
htlc_maximum_msat: 1_000_000, // Defaults to 10% of the channel value
fee_base_msat: last_hop[0].counterparty.forwarding_info.as_ref().unwrap().fee_base_msat,
fee_proportional_millionths: last_hop[0].counterparty.forwarding_info.as_ref().unwrap().fee_proportional_millionths,
excess_data: Vec::new(),
};
let signature = nodes[1].keys_manager.sign_gossip_message(msgs::UnsignedGossipMessage::ChannelUpdate(&contents)).unwrap();
let msg = msgs::ChannelUpdate { signature, contents };
let mut err_data = Vec::new();
err_data.extend_from_slice(&(msg.serialized_length() as u16 + 2).to_be_bytes());
err_data.extend_from_slice(&ChannelUpdate::TYPE.to_be_bytes());
err_data.extend_from_slice(&msg.encode());
let err_data = 0u16.to_be_bytes();
expect_payment_failed_conditions(&nodes[0], payment_hash, false,
PaymentFailedConditions::new().blamed_scid(last_hop[0].inbound_scid_alias.unwrap())
.blamed_chan_closed(false).expected_htlc_error_data(0x1000|7, &err_data));
@ -546,9 +520,7 @@ fn test_scid_alias_returned() {
let mut err_data = Vec::new();
err_data.extend_from_slice(&10_000u64.to_be_bytes());
err_data.extend_from_slice(&(msg.serialized_length() as u16 + 2).to_be_bytes());
err_data.extend_from_slice(&ChannelUpdate::TYPE.to_be_bytes());
err_data.extend_from_slice(&msg.encode());
err_data.extend_from_slice(&0u16.to_be_bytes());
expect_payment_failed_conditions(&nodes[0], payment_hash, false,
PaymentFailedConditions::new().blamed_scid(last_hop[0].inbound_scid_alias.unwrap())
.blamed_chan_closed(false).expected_htlc_error_data(0x1000|12, &err_data));