Skip to content

Commit d036400

Browse files
TheBlueMattTheBlueMatt
authored
Merge pull request #1840 from valentinewallace/2022-11-htlc-intercept-prefactor
Pre-refactor for HTLC Interception
2 parents 30b9807 + c793549 commit d036400

File tree

3 files changed

+137
-118
lines changed

3 files changed

+137
-118
lines changed

‎lightning/src/ln/channelmanager.rs

+119-102
Original file line numberDiff line numberDiff line change
@@ -113,6 +113,7 @@ pub(super) struct PendingHTLCInfo {
113113
pub(super) incoming_shared_secret: [u8; 32],
114114
payment_hash: PaymentHash,
115115
pub(super) amt_to_forward: u64,
116+
pub(super) amt_incoming: Option<u64>, // Added in 0.0.113
116117
pub(super) outgoing_cltv_value: u32,
117118
}
118119

@@ -129,20 +130,22 @@ pub(super) enum PendingHTLCStatus {
129130
Fail(HTLCFailureMsg),
130131
}
131132

132-
pub(super) enum HTLCForwardInfo {
133-
AddHTLC {
134-
forward_info: PendingHTLCInfo,
133+
pub(super) struct PendingAddHTLCInfo {
134+
pub(super) forward_info: PendingHTLCInfo,
135135

136-
// These fields are produced in `forward_htlcs()` and consumed in
137-
// `process_pending_htlc_forwards()` for constructing the
138-
// `HTLCSource::PreviousHopData` for failed and forwarded
139-
// HTLCs.
140-
//
141-
// Note that this may be an outbound SCID alias for the associated channel.
142-
prev_short_channel_id: u64,
143-
prev_htlc_id: u64,
144-
prev_funding_outpoint: OutPoint,
145-
},
136+
// These fields are produced in `forward_htlcs()` and consumed in
137+
// `process_pending_htlc_forwards()` for constructing the
138+
// `HTLCSource::PreviousHopData` for failed and forwarded
139+
// HTLCs.
140+
//
141+
// Note that this may be an outbound SCID alias for the associated channel.
142+
prev_short_channel_id: u64,
143+
prev_htlc_id: u64,
144+
prev_funding_outpoint: OutPoint,
145+
}
146+
147+
pub(super) enum HTLCForwardInfo {
148+
AddHTLC(PendingAddHTLCInfo),
146149
FailHTLC {
147150
htlc_id: u64,
148151
err_packet: msgs::OnionErrorPacket,
@@ -2194,6 +2197,7 @@ impl<M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelManager<M, T, K, F
21942197
routing,
21952198
payment_hash,
21962199
incoming_shared_secret: shared_secret,
2200+
amt_incoming: Some(amt_msat),
21972201
amt_to_forward: amt_msat,
21982202
outgoing_cltv_value: hop_data.outgoing_cltv_value,
21992203
})
@@ -2290,6 +2294,7 @@ impl<M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelManager<M, T, K, F
22902294
},
22912295
payment_hash: msg.payment_hash.clone(),
22922296
incoming_shared_secret: shared_secret,
2297+
amt_incoming: Some(msg.amount_msat),
22932298
amt_to_forward: next_hop_data.amt_to_forward,
22942299
outgoing_cltv_value: next_hop_data.outgoing_cltv_value,
22952300
})
@@ -2308,7 +2313,7 @@ impl<M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelManager<M, T, K, F
23082313
None => { // unknown_next_peer
23092314
// Note that this is likely a timing oracle for detecting whether an scid is a
23102315
// phantom.
2311-
if fake_scid::is_valid_phantom(&self.fake_scid_rand_bytes, *short_channel_id) {
2316+
if fake_scid::is_valid_phantom(&self.fake_scid_rand_bytes, *short_channel_id, &self.genesis_hash) {
23122317
None
23132318
} else {
23142319
break Some(("Don't have available channel for forwarding as requested.", 0x4000 | 10, None));
@@ -3149,82 +3154,86 @@ impl<M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelManager<M, T, K, F
31493154
() => {
31503155
for forward_info in pending_forwards.drain(..) {
31513156
match forward_info {
3152-
HTLCForwardInfo::AddHTLC { prev_short_channel_id, prev_htlc_id, forward_info: PendingHTLCInfo {
3153-
routing, incoming_shared_secret, payment_hash, amt_to_forward, outgoing_cltv_value },
3154-
prev_funding_outpoint } => {
3155-
macro_rules! failure_handler {
3156-
($msg: expr, $err_code: expr, $err_data: expr, $phantom_ss: expr, $next_hop_unknown: expr) => {
3157-
log_info!(self.logger, "Failed to accept/forward incoming HTLC: {}", $msg);
3158-
3159-
let htlc_source = HTLCSource::PreviousHopData(HTLCPreviousHopData {
3160-
short_channel_id: prev_short_channel_id,
3161-
outpoint: prev_funding_outpoint,
3162-
htlc_id: prev_htlc_id,
3163-
incoming_packet_shared_secret: incoming_shared_secret,
3164-
phantom_shared_secret: $phantom_ss,
3165-
});
3166-
3167-
let reason = if $next_hop_unknown {
3168-
HTLCDestination::UnknownNextHop { requested_forward_scid: short_chan_id }
3169-
} else {
3170-
HTLCDestination::FailedPayment{ payment_hash }
3171-
};
3172-
3173-
failed_forwards.push((htlc_source, payment_hash,
3174-
HTLCFailReason::Reason { failure_code: $err_code, data: $err_data },
3175-
reason
3176-
));
3177-
continue;
3178-
}
3157+
HTLCForwardInfo::AddHTLC(PendingAddHTLCInfo {
3158+
prev_short_channel_id, prev_htlc_id, prev_funding_outpoint,
3159+
forward_info: PendingHTLCInfo {
3160+
routing, incoming_shared_secret, payment_hash, amt_to_forward,
3161+
outgoing_cltv_value, amt_incoming: _
3162+
}
3163+
}) => {
3164+
macro_rules! failure_handler {
3165+
($msg: expr, $err_code: expr, $err_data: expr, $phantom_ss: expr, $next_hop_unknown: expr) => {
3166+
log_info!(self.logger, "Failed to accept/forward incoming HTLC: {}", $msg);
3167+
3168+
let htlc_source = HTLCSource::PreviousHopData(HTLCPreviousHopData {
3169+
short_channel_id: prev_short_channel_id,
3170+
outpoint: prev_funding_outpoint,
3171+
htlc_id: prev_htlc_id,
3172+
incoming_packet_shared_secret: incoming_shared_secret,
3173+
phantom_shared_secret: $phantom_ss,
3174+
});
3175+
3176+
let reason = if $next_hop_unknown {
3177+
HTLCDestination::UnknownNextHop { requested_forward_scid: short_chan_id }
3178+
} else {
3179+
HTLCDestination::FailedPayment{ payment_hash }
3180+
};
3181+
3182+
failed_forwards.push((htlc_source, payment_hash,
3183+
HTLCFailReason::Reason { failure_code: $err_code, data: $err_data },
3184+
reason
3185+
));
3186+
continue;
31793187
}
3180-
macro_rules! fail_forward {
3181-
($msg: expr, $err_code: expr, $err_data: expr, $phantom_ss: expr) => {
3182-
{
3183-
failure_handler!($msg, $err_code, $err_data, $phantom_ss, true);
3184-
}
3188+
}
3189+
macro_rules! fail_forward {
3190+
($msg: expr, $err_code: expr, $err_data: expr, $phantom_ss: expr) => {
3191+
{
3192+
failure_handler!($msg, $err_code, $err_data, $phantom_ss, true);
31853193
}
31863194
}
3187-
macro_rules! failed_payment {
3188-
($msg: expr, $err_code: expr, $err_data: expr, $phantom_ss: expr) => {
3189-
{
3190-
failure_handler!($msg, $err_code, $err_data, $phantom_ss, false);
3191-
}
3195+
}
3196+
macro_rules! failed_payment {
3197+
($msg: expr, $err_code: expr, $err_data: expr, $phantom_ss: expr) => {
3198+
{
3199+
failure_handler!($msg, $err_code, $err_data, $phantom_ss, false);
31923200
}
31933201
}
3194-
if let PendingHTLCRouting::Forward { onion_packet, .. } = routing {
3195-
let phantom_secret_res = self.keys_manager.get_node_secret(Recipient::PhantomNode);
3196-
if phantom_secret_res.is_ok() && fake_scid::is_valid_phantom(&self.fake_scid_rand_bytes, short_chan_id) {
3197-
let phantom_shared_secret = SharedSecret::new(&onion_packet.public_key.unwrap(), &phantom_secret_res.unwrap()).secret_bytes();
3198-
let next_hop = match onion_utils::decode_next_payment_hop(phantom_shared_secret, &onion_packet.hop_data, onion_packet.hmac, payment_hash) {
3199-
Ok(res) => res,
3200-
Err(onion_utils::OnionDecodeErr::Malformed { err_msg, err_code }) => {
3201-
let sha256_of_onion = Sha256::hash(&onion_packet.hop_data).into_inner();
3202-
// In this scenario, the phantom would have sent us an
3203-
// `update_fail_malformed_htlc`, meaning here we encrypt the error as
3204-
// if it came from us (the second-to-last hop) but contains the sha256
3205-
// of the onion.
3206-
failed_payment!(err_msg, err_code, sha256_of_onion.to_vec(), None);
3207-
},
3208-
Err(onion_utils::OnionDecodeErr::Relay { err_msg, err_code }) => {
3209-
failed_payment!(err_msg, err_code, Vec::new(), Some(phantom_shared_secret));
3210-
},
3211-
};
3212-
match next_hop {
3213-
onion_utils::Hop::Receive(hop_data) => {
3214-
match self.construct_recv_pending_htlc_info(hop_data, incoming_shared_secret, payment_hash, amt_to_forward, outgoing_cltv_value, Some(phantom_shared_secret)) {
3215-
Ok(info) => phantom_receives.push((prev_short_channel_id, prev_funding_outpoint, vec![(info, prev_htlc_id)])),
3216-
Err(ReceiveError { err_code, err_data, msg }) => failed_payment!(msg, err_code, err_data, Some(phantom_shared_secret))
3217-
}
3218-
},
3219-
_ => panic!(),
3220-
}
3221-
} else {
3222-
fail_forward!(format!("Unknown short channel id {} for forward HTLC", short_chan_id), 0x4000 | 10, Vec::new(), None);
3202+
}
3203+
if let PendingHTLCRouting::Forward { onion_packet, .. } = routing {
3204+
let phantom_secret_res = self.keys_manager.get_node_secret(Recipient::PhantomNode);
3205+
if phantom_secret_res.is_ok() && fake_scid::is_valid_phantom(&self.fake_scid_rand_bytes, short_chan_id, &self.genesis_hash) {
3206+
let phantom_shared_secret = SharedSecret::new(&onion_packet.public_key.unwrap(), &phantom_secret_res.unwrap()).secret_bytes();
3207+
let next_hop = match onion_utils::decode_next_payment_hop(phantom_shared_secret, &onion_packet.hop_data, onion_packet.hmac, payment_hash) {
3208+
Ok(res) => res,
3209+
Err(onion_utils::OnionDecodeErr::Malformed { err_msg, err_code }) => {
3210+
let sha256_of_onion = Sha256::hash(&onion_packet.hop_data).into_inner();
3211+
// In this scenario, the phantom would have sent us an
3212+
// `update_fail_malformed_htlc`, meaning here we encrypt the error as
3213+
// if it came from us (the second-to-last hop) but contains the sha256
3214+
// of the onion.
3215+
failed_payment!(err_msg, err_code, sha256_of_onion.to_vec(), None);
3216+
},
3217+
Err(onion_utils::OnionDecodeErr::Relay { err_msg, err_code }) => {
3218+
failed_payment!(err_msg, err_code, Vec::new(), Some(phantom_shared_secret));
3219+
},
3220+
};
3221+
match next_hop {
3222+
onion_utils::Hop::Receive(hop_data) => {
3223+
match self.construct_recv_pending_htlc_info(hop_data, incoming_shared_secret, payment_hash, amt_to_forward, outgoing_cltv_value, Some(phantom_shared_secret)) {
3224+
Ok(info) => phantom_receives.push((prev_short_channel_id, prev_funding_outpoint, vec![(info, prev_htlc_id)])),
3225+
Err(ReceiveError { err_code, err_data, msg }) => failed_payment!(msg, err_code, err_data, Some(phantom_shared_secret))
3226+
}
3227+
},
3228+
_ => panic!(),
32233229
}
32243230
} else {
32253231
fail_forward!(format!("Unknown short channel id {} for forward HTLC", short_chan_id), 0x4000 | 10, Vec::new(), None);
32263232
}
3227-
},
3233+
} else {
3234+
fail_forward!(format!("Unknown short channel id {} for forward HTLC", short_chan_id), 0x4000 | 10, Vec::new(), None);
3235+
}
3236+
},
32283237
HTLCForwardInfo::FailHTLC { .. } => {
32293238
// Channel went away before we could fail it. This implies
32303239
// the channel is now on chain and our counterparty is
@@ -3252,11 +3261,13 @@ impl<M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelManager<M, T, K, F
32523261
let mut fail_htlc_msgs = Vec::new();
32533262
for forward_info in pending_forwards.drain(..) {
32543263
match forward_info {
3255-
HTLCForwardInfo::AddHTLC { prev_short_channel_id, prev_htlc_id, forward_info: PendingHTLCInfo {
3256-
routing: PendingHTLCRouting::Forward {
3257-
onion_packet, ..
3258-
}, incoming_shared_secret, payment_hash, amt_to_forward, outgoing_cltv_value },
3259-
prev_funding_outpoint } => {
3264+
HTLCForwardInfo::AddHTLC(PendingAddHTLCInfo {
3265+
prev_short_channel_id, prev_htlc_id, prev_funding_outpoint ,
3266+
forward_info: PendingHTLCInfo {
3267+
incoming_shared_secret, payment_hash, amt_to_forward, outgoing_cltv_value,
3268+
routing: PendingHTLCRouting::Forward { onion_packet, .. }, amt_incoming: _,
3269+
},
3270+
}) => {
32603271
log_trace!(self.logger, "Adding HTLC from short id {} with payment_hash {} to channel with short id {} after delay", prev_short_channel_id, log_bytes!(payment_hash.0), short_chan_id);
32613272
let htlc_source = HTLCSource::PreviousHopData(HTLCPreviousHopData {
32623273
short_channel_id: prev_short_channel_id,
@@ -3377,9 +3388,12 @@ impl<M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelManager<M, T, K, F
33773388
} else {
33783389
for forward_info in pending_forwards.drain(..) {
33793390
match forward_info {
3380-
HTLCForwardInfo::AddHTLC { prev_short_channel_id, prev_htlc_id, forward_info: PendingHTLCInfo {
3381-
routing, incoming_shared_secret, payment_hash, amt_to_forward, .. },
3382-
prev_funding_outpoint } => {
3391+
HTLCForwardInfo::AddHTLC(PendingAddHTLCInfo {
3392+
prev_short_channel_id, prev_htlc_id, prev_funding_outpoint,
3393+
forward_info: PendingHTLCInfo {
3394+
routing, incoming_shared_secret, payment_hash, amt_to_forward, ..
3395+
}
3396+
}) => {
33833397
let (cltv_expiry, onion_payload, payment_data, phantom_shared_secret) = match routing {
33843398
PendingHTLCRouting::Receive { payment_data, incoming_cltv_expiry, phantom_shared_secret } => {
33853399
let _legacy_hop_data = Some(payment_data.clone());
@@ -5089,12 +5103,12 @@ impl<M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelManager<M, T, K, F
50895103
PendingHTLCRouting::ReceiveKeysend { .. } => 0,
50905104
}) {
50915105
hash_map::Entry::Occupied(mut entry) => {
5092-
entry.get_mut().push(HTLCForwardInfo::AddHTLC { prev_short_channel_id, prev_funding_outpoint,
5093-
prev_htlc_id, forward_info });
5106+
entry.get_mut().push(HTLCForwardInfo::AddHTLC(PendingAddHTLCInfo {
5107+
prev_short_channel_id, prev_funding_outpoint, prev_htlc_id, forward_info }));
50945108
},
50955109
hash_map::Entry::Vacant(entry) => {
5096-
entry.insert(vec!(HTLCForwardInfo::AddHTLC { prev_short_channel_id, prev_funding_outpoint,
5097-
prev_htlc_id, forward_info }));
5110+
entry.insert(vec!(HTLCForwardInfo::AddHTLC(PendingAddHTLCInfo {
5111+
prev_short_channel_id, prev_funding_outpoint, prev_htlc_id, forward_info })));
50985112
}
50995113
}
51005114
}
@@ -6459,7 +6473,8 @@ impl_writeable_tlv_based!(PendingHTLCInfo, {
64596473
(2, incoming_shared_secret, required),
64606474
(4, payment_hash, required),
64616475
(6, amt_to_forward, required),
6462-
(8, outgoing_cltv_value, required)
6476+
(8, outgoing_cltv_value, required),
6477+
(9, amt_incoming, option),
64636478
});
64646479

64656480

@@ -6681,18 +6696,20 @@ impl_writeable_tlv_based_enum!(HTLCFailReason,
66816696
},
66826697
;);
66836698

6699+
impl_writeable_tlv_based!(PendingAddHTLCInfo, {
6700+
(0, forward_info, required),
6701+
(2, prev_short_channel_id, required),
6702+
(4, prev_htlc_id, required),
6703+
(6, prev_funding_outpoint, required),
6704+
});
6705+
66846706
impl_writeable_tlv_based_enum!(HTLCForwardInfo,
6685-
(0, AddHTLC) => {
6686-
(0, forward_info, required),
6687-
(2, prev_short_channel_id, required),
6688-
(4, prev_htlc_id, required),
6689-
(6, prev_funding_outpoint, required),
6690-
},
66916707
(1, FailHTLC) => {
66926708
(0, htlc_id, required),
66936709
(2, err_packet, required),
6694-
},
6695-
;);
6710+
};
6711+
(0, AddHTLC)
6712+
);
66966713

66976714
impl_writeable_tlv_based!(PendingInboundPayment, {
66986715
(0, payment_secret, required),

‎lightning/src/ln/onion_route_tests.rs

+9-9
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ use crate::chain::channelmonitor::{ChannelMonitor, CLTV_CLAIM_BUFFER, LATENCY_GR
1515
use crate::chain::keysinterface::{KeysInterface, Recipient};
1616
use crate::ln::{PaymentHash, PaymentSecret};
1717
use crate::ln::channel::EXPIRE_PREV_CONFIG_TICKS;
18-
use crate::ln::channelmanager::{self, ChannelManager, ChannelManagerReadArgs, HTLCForwardInfo, CLTV_FAR_FAR_AWAY, MIN_CLTV_EXPIRY_DELTA, PendingHTLCInfo, PendingHTLCRouting, PaymentId};
18+
use crate::ln::channelmanager::{self, ChannelManager, ChannelManagerReadArgs, HTLCForwardInfo, CLTV_FAR_FAR_AWAY, MIN_CLTV_EXPIRY_DELTA, PendingAddHTLCInfo, PendingHTLCInfo, PendingHTLCRouting, PaymentId};
1919
use crate::ln::onion_utils;
2020
use crate::routing::gossip::{NetworkUpdate, RoutingFees, NodeId};
2121
use crate::routing::router::{get_route, PaymentParameters, Route, RouteHint, RouteHintHop};
@@ -554,7 +554,7 @@ fn test_onion_failure() {
554554
for (_, pending_forwards) in nodes[1].node.forward_htlcs.lock().unwrap().iter_mut() {
555555
for f in pending_forwards.iter_mut() {
556556
match f {
557-
&mut HTLCForwardInfo::AddHTLC { ref mut forward_info, .. } =>
557+
&mut HTLCForwardInfo::AddHTLC(PendingAddHTLCInfo { ref mut forward_info, .. }) =>
558558
forward_info.outgoing_cltv_value += 1,
559559
_ => {},
560560
}
@@ -567,7 +567,7 @@ fn test_onion_failure() {
567567
for (_, pending_forwards) in nodes[1].node.forward_htlcs.lock().unwrap().iter_mut() {
568568
for f in pending_forwards.iter_mut() {
569569
match f {
570-
&mut HTLCForwardInfo::AddHTLC { ref mut forward_info, .. } =>
570+
&mut HTLCForwardInfo::AddHTLC(PendingAddHTLCInfo { ref mut forward_info, .. }) =>
571571
forward_info.amt_to_forward -= 1,
572572
_ => {},
573573
}
@@ -1035,12 +1035,12 @@ fn test_phantom_onion_hmac_failure() {
10351035
let mut forward_htlcs = nodes[1].node.forward_htlcs.lock().unwrap();
10361036
let mut pending_forward = forward_htlcs.get_mut(&phantom_scid).unwrap();
10371037
match pending_forward[0] {
1038-
HTLCForwardInfo::AddHTLC {
1038+
HTLCForwardInfo::AddHTLC(PendingAddHTLCInfo {
10391039
forward_info: PendingHTLCInfo {
10401040
routing: PendingHTLCRouting::Forward { ref mut onion_packet, .. },
10411041
..
10421042
}, ..
1043-
} => {
1043+
}) => {
10441044
onion_packet.hmac[onion_packet.hmac.len() - 1] ^= 1;
10451045
Sha256::hash(&onion_packet.hop_data).into_inner().to_vec()
10461046
},
@@ -1095,12 +1095,12 @@ fn test_phantom_invalid_onion_payload() {
10951095
for (_, pending_forwards) in nodes[1].node.forward_htlcs.lock().unwrap().iter_mut() {
10961096
for f in pending_forwards.iter_mut() {
10971097
match f {
1098-
&mut HTLCForwardInfo::AddHTLC {
1098+
&mut HTLCForwardInfo::AddHTLC(PendingAddHTLCInfo {
10991099
forward_info: PendingHTLCInfo {
11001100
routing: PendingHTLCRouting::Forward { ref mut onion_packet, .. },
11011101
..
11021102
}, ..
1103-
} => {
1103+
}) => {
11041104
// Construct the onion payloads for the entire route and an invalid amount.
11051105
let height = nodes[0].best_block_info().1;
11061106
let session_priv = SecretKey::from_slice(&session_priv).unwrap();
@@ -1166,9 +1166,9 @@ fn test_phantom_final_incorrect_cltv_expiry() {
11661166
for (_, pending_forwards) in nodes[1].node.forward_htlcs.lock().unwrap().iter_mut() {
11671167
for f in pending_forwards.iter_mut() {
11681168
match f {
1169-
&mut HTLCForwardInfo::AddHTLC {
1169+
&mut HTLCForwardInfo::AddHTLC(PendingAddHTLCInfo {
11701170
forward_info: PendingHTLCInfo { ref mut outgoing_cltv_value, .. }, ..
1171-
} => {
1171+
}) => {
11721172
*outgoing_cltv_value += 1;
11731173
},
11741174
_ => panic!("Unexpected forward"),

0 commit comments

Comments
 (0)