Skip to content

#1199 Followup #1324

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 6 commits into from
Feb 28, 2022
Merged

#1199 Followup #1324

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
1f6700c
Follow-up nits from #1199 (phantom node support)
valentinewallace Feb 19, 2022
bafd141
Add phantom shared secret to PendingHTLCRouting::Receive
valentinewallace Feb 25, 2022
f1aba79
Add phantom shared secret to HTLCPreviousHopData
valentinewallace Feb 25, 2022
3faea33
Fix phantom malformed onion error packet
valentinewallace Feb 25, 2022
26fe879
Correctly wrap phantom onion errors
valentinewallace Feb 25, 2022
694ef1e
Fix bug where we encode flags field into all updates on htlc fail
valentinewallace Feb 24, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions fuzz/src/full_stack.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -390,6 +390,9 @@ pub fn do_test(data: &[u8], logger: &Arc<dyn Logger>) {
best_block: BestBlock::from_genesis(network),
};
let channelmanager = Arc::new(ChannelManager::new(fee_est.clone(), monitor.clone(), broadcast.clone(), Arc::clone(&logger), keys_manager.clone(), config, params));
// Adding new calls to `KeysInterface::get_secure_random_bytes` during startup can change all the
// keys subsequently generated in this test. Rather than regenerating all the messages manually,
// it's easier to just increment the counter here so the keys don't change.
keys_manager.counter.fetch_sub(1, Ordering::AcqRel);
let our_id = PublicKey::from_secret_key(&Secp256k1::signing_only(), &keys_manager.get_node_secret(Recipient::Node).unwrap());
let network_graph = Arc::new(NetworkGraph::new(genesis_block(network).block_hash()));
Expand Down
74 changes: 47 additions & 27 deletions lightning/src/ln/channelmanager.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -358,14 +358,15 @@ mod inbound_payment {
// our payment, which we can use to decode errors or inform the user that the payment was sent.

#[derive(Clone)] // See Channel::revoke_and_ack for why, tl;dr: Rust bug
enum PendingHTLCRouting {
pub(super) enum PendingHTLCRouting {
Forward {
onion_packet: msgs::OnionPacket,
short_channel_id: u64, // This should be NonZero<u64> eventually when we bump MSRV
},
Receive {
payment_data: msgs::FinalOnionHopData,
incoming_cltv_expiry: u32, // Used to track when we should expire pending HTLCs that go unclaimed
phantom_shared_secret: Option<[u8; 32]>,
},
ReceiveKeysend {
payment_preimage: PaymentPreimage,
Expand All @@ -375,8 +376,8 @@ enum PendingHTLCRouting {

#[derive(Clone)] // See Channel::revoke_and_ack for why, tl;dr: Rust bug
pub(super) struct PendingHTLCInfo {
routing: PendingHTLCRouting,
incoming_shared_secret: [u8; 32],
pub(super) routing: PendingHTLCRouting,
pub(super) incoming_shared_secret: [u8; 32],
payment_hash: PaymentHash,
pub(super) amt_to_forward: u64,
pub(super) outgoing_cltv_value: u32,
Expand Down Expand Up @@ -419,6 +420,7 @@ pub(crate) struct HTLCPreviousHopData {
short_channel_id: u64,
htlc_id: u64,
incoming_packet_shared_secret: [u8; 32],
phantom_shared_secret: Option<[u8; 32]>,

// This field is consumed by `claim_funds_from_hop()` when updating a force-closed backwards
// channel with a preimage provided by the forward channel.
Expand Down Expand Up @@ -2072,7 +2074,7 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
}

fn construct_recv_pending_htlc_info(&self, hop_data: msgs::OnionHopData, shared_secret: [u8; 32],
payment_hash: PaymentHash, amt_msat: u64, cltv_expiry: u32) -> Result<PendingHTLCInfo, ReceiveError>
payment_hash: PaymentHash, amt_msat: u64, cltv_expiry: u32, phantom_shared_secret: Option<[u8; 32]>) -> Result<PendingHTLCInfo, ReceiveError>
{
// final_incorrect_cltv_expiry
if hop_data.outgoing_cltv_value != cltv_expiry {
Expand Down Expand Up @@ -2129,6 +2131,7 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
PendingHTLCRouting::Receive {
payment_data: data,
incoming_cltv_expiry: hop_data.outgoing_cltv_value,
phantom_shared_secret,
}
} else if let Some(payment_preimage) = keysend_preimage {
// We need to check that the sender knows the keysend preimage before processing this
Expand Down Expand Up @@ -2232,7 +2235,7 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
let pending_forward_info = match next_hop {
onion_utils::Hop::Receive(next_hop_data) => {
// OUR PAYMENT!
match self.construct_recv_pending_htlc_info(next_hop_data, shared_secret, msg.payment_hash, msg.amount_msat, msg.cltv_expiry) {
match self.construct_recv_pending_htlc_info(next_hop_data, shared_secret, msg.payment_hash, msg.amount_msat, msg.cltv_expiry, None) {
Ok(info) => {
// Note that we could obviously respond immediately with an update_fulfill_htlc
// message, however that would leak that we are the recipient of this payment, so
Expand Down Expand Up @@ -3012,17 +3015,18 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
routing, incoming_shared_secret, payment_hash, amt_to_forward, outgoing_cltv_value },
prev_funding_outpoint } => {
macro_rules! fail_forward {
($msg: expr, $err_code: expr, $err_data: expr) => {
($msg: expr, $err_code: expr, $err_data: expr, $phantom_ss: expr) => {
{
log_info!(self.logger, "Failed to accept/forward incoming HTLC: {}", $msg);
let htlc_source = HTLCSource::PreviousHopData(HTLCPreviousHopData {
short_channel_id: short_chan_id,
short_channel_id: prev_short_channel_id,
outpoint: prev_funding_outpoint,
htlc_id: prev_htlc_id,
incoming_packet_shared_secret: incoming_shared_secret,
phantom_shared_secret: $phantom_ss,
});
failed_forwards.push((htlc_source, payment_hash,
HTLCFailReason::Reason { failure_code: $err_code, data: $err_data }
HTLCFailReason::Reason { failure_code: $err_code, data: $err_data }
));
continue;
}
Expand All @@ -3031,44 +3035,46 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
if let PendingHTLCRouting::Forward { onion_packet, .. } = routing {
let phantom_secret_res = self.keys_manager.get_node_secret(Recipient::PhantomNode);
if phantom_secret_res.is_ok() && fake_scid::is_valid_phantom(&self.fake_scid_rand_bytes, short_chan_id) {
let shared_secret = {
let phantom_shared_secret = {
let mut arr = [0; 32];
arr.copy_from_slice(&SharedSecret::new(&onion_packet.public_key.unwrap(), &phantom_secret_res.unwrap())[..]);
arr
};
let next_hop = match onion_utils::decode_next_hop(shared_secret, &onion_packet.hop_data, onion_packet.hmac, payment_hash) {
let next_hop = match onion_utils::decode_next_hop(phantom_shared_secret, &onion_packet.hop_data, onion_packet.hmac, payment_hash) {
Ok(res) => res,
Err(onion_utils::OnionDecodeErr::Malformed { err_msg, err_code }) => {
fail_forward!(err_msg, err_code, Vec::new());
let sha256_of_onion = Sha256::hash(&onion_packet.hop_data).into_inner();
// In this scenario, the phantom would have sent us an
// `update_fail_malformed_htlc`, meaning here we encrypt the error as
// if it came from us (the second-to-last hop) but contains the sha256
// of the onion.
fail_forward!(err_msg, err_code, sha256_of_onion.to_vec(), None);
},
Err(onion_utils::OnionDecodeErr::Relay { err_msg, err_code }) => {
fail_forward!(err_msg, err_code, Vec::new());
fail_forward!(err_msg, err_code, Vec::new(), Some(phantom_shared_secret));
},
};
match next_hop {
onion_utils::Hop::Receive(hop_data) => {
match self.construct_recv_pending_htlc_info(hop_data, shared_secret, payment_hash, amt_to_forward, outgoing_cltv_value) {
match self.construct_recv_pending_htlc_info(hop_data, incoming_shared_secret, payment_hash, amt_to_forward, outgoing_cltv_value, Some(phantom_shared_secret)) {
Ok(info) => phantom_receives.push((prev_short_channel_id, prev_funding_outpoint, vec![(info, prev_htlc_id)])),
Err(ReceiveError { err_code, err_data, msg }) => fail_forward!(msg, err_code, err_data)
Err(ReceiveError { err_code, err_data, msg }) => fail_forward!(msg, err_code, err_data, Some(phantom_shared_secret))
}
},
_ => panic!(),
}
} else {
fail_forward!(format!("Unknown short channel id {} for forward HTLC", short_chan_id), 0x4000 | 10, Vec::new());
fail_forward!(format!("Unknown short channel id {} for forward HTLC", short_chan_id), 0x4000 | 10, Vec::new(), None);
}
} else {
fail_forward!(format!("Unknown short channel id {} for forward HTLC", short_chan_id), 0x4000 | 10, Vec::new());
fail_forward!(format!("Unknown short channel id {} for forward HTLC", short_chan_id), 0x4000 | 10, Vec::new(), None);
}
},
HTLCForwardInfo::FailHTLC { .. } => {
// Channel went away before we could fail it. This implies
// the channel is now on chain and our counterparty is
// trying to broadcast the HTLC-Timeout, but that's their
// problem, not ours.
//
// `fail_htlc_backwards_internal` is never called for
// phantom payments, so this is unreachable for them.
}
}
}
Expand All @@ -3091,6 +3097,8 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
outpoint: prev_funding_outpoint,
htlc_id: prev_htlc_id,
incoming_packet_shared_secret: incoming_shared_secret,
// Phantom payments are only PendingHTLCRouting::Receive.
phantom_shared_secret: None,
});
match chan.get_mut().send_htlc(amt_to_forward, payment_hash, outgoing_cltv_value, htlc_source.clone(), onion_packet, &self.logger) {
Err(e) => {
Expand Down Expand Up @@ -3207,11 +3215,11 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
HTLCForwardInfo::AddHTLC { prev_short_channel_id, prev_htlc_id, forward_info: PendingHTLCInfo {
routing, incoming_shared_secret, payment_hash, amt_to_forward, .. },
prev_funding_outpoint } => {
let (cltv_expiry, onion_payload) = match routing {
PendingHTLCRouting::Receive { payment_data, incoming_cltv_expiry } =>
(incoming_cltv_expiry, OnionPayload::Invoice(payment_data)),
let (cltv_expiry, onion_payload, phantom_shared_secret) = match routing {
PendingHTLCRouting::Receive { payment_data, incoming_cltv_expiry, phantom_shared_secret } =>
(incoming_cltv_expiry, OnionPayload::Invoice(payment_data), phantom_shared_secret),
PendingHTLCRouting::ReceiveKeysend { payment_preimage, incoming_cltv_expiry } =>
(incoming_cltv_expiry, OnionPayload::Spontaneous(payment_preimage)),
(incoming_cltv_expiry, OnionPayload::Spontaneous(payment_preimage), None),
_ => {
panic!("short_channel_id == 0 should imply any pending_forward entries are of type Receive");
}
Expand All @@ -3222,6 +3230,7 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
outpoint: prev_funding_outpoint,
htlc_id: prev_htlc_id,
incoming_packet_shared_secret: incoming_shared_secret,
phantom_shared_secret,
},
value: amt_to_forward,
cltv_expiry,
Expand All @@ -3239,6 +3248,7 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
outpoint: prev_funding_outpoint,
htlc_id: $htlc.prev_hop.htlc_id,
incoming_packet_shared_secret: $htlc.prev_hop.incoming_packet_shared_secret,
phantom_shared_secret,
}), payment_hash,
HTLCFailReason::Reason { failure_code: 0x4000 | 15, data: htlc_msat_height_data }
));
Expand Down Expand Up @@ -3779,12 +3789,18 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
pending_events.push(path_failure);
if let Some(ev) = full_failure_ev { pending_events.push(ev); }
},
HTLCSource::PreviousHopData(HTLCPreviousHopData { short_channel_id, htlc_id, incoming_packet_shared_secret, .. }) => {
HTLCSource::PreviousHopData(HTLCPreviousHopData { short_channel_id, htlc_id, incoming_packet_shared_secret, phantom_shared_secret, .. }) => {
let err_packet = match onion_error {
HTLCFailReason::Reason { failure_code, data } => {
log_trace!(self.logger, "Failing HTLC with payment_hash {} backwards from us with code {}", log_bytes!(payment_hash.0), failure_code);
let packet = onion_utils::build_failure_packet(&incoming_packet_shared_secret, failure_code, &data[..]).encode();
onion_utils::encrypt_failure_packet(&incoming_packet_shared_secret, &packet)
if let Some(phantom_ss) = phantom_shared_secret {
let phantom_packet = onion_utils::build_failure_packet(&phantom_ss, failure_code, &data[..]).encode();
let encrypted_phantom_packet = onion_utils::encrypt_failure_packet(&phantom_ss, &phantom_packet);
onion_utils::encrypt_failure_packet(&incoming_packet_shared_secret, &encrypted_phantom_packet.data[..])
} else {
let packet = onion_utils::build_failure_packet(&incoming_packet_shared_secret, failure_code, &data[..]).encode();
onion_utils::encrypt_failure_packet(&incoming_packet_shared_secret, &packet)
}
},
HTLCFailReason::LightningError { err } => {
log_trace!(self.logger, "Failing HTLC with payment_hash {} backwards with pre-built LightningError", log_bytes!(payment_hash.0));
Expand Down Expand Up @@ -4488,7 +4504,9 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
onion_utils::build_first_hop_failure_packet(incoming_shared_secret, error_code, &{
let mut res = Vec::with_capacity(8 + 128);
// TODO: underspecified, follow https://github.com/lightningnetwork/lightning-rfc/issues/791
res.extend_from_slice(&byte_utils::be16_to_array(0));
if error_code == 0x1000 | 20 {
res.extend_from_slice(&byte_utils::be16_to_array(0));
}
res.extend_from_slice(&upd.encode_with_len()[..]);
res
}[..])
Expand Down Expand Up @@ -5979,6 +5997,7 @@ impl_writeable_tlv_based_enum!(PendingHTLCRouting,
},
(1, Receive) => {
(0, payment_data, required),
(1, phantom_shared_secret, option),
(2, incoming_cltv_expiry, required),
},
(2, ReceiveKeysend) => {
Expand Down Expand Up @@ -6070,6 +6089,7 @@ impl_writeable_tlv_based_enum!(PendingHTLCStatus, ;

impl_writeable_tlv_based!(HTLCPreviousHopData, {
(0, short_channel_id, required),
(1, phantom_shared_secret, option),
(2, outpoint, required),
(4, htlc_id, required),
(6, incoming_packet_shared_secret, required)
Expand Down
7 changes: 0 additions & 7 deletions lightning/src/ln/msgs.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1291,10 +1291,6 @@ impl Readable for FinalOnionHopData {

impl Writeable for OnionHopData {
fn write<W: Writer>(&self, w: &mut W) -> Result<(), io::Error> {
// Note that this should never be reachable if Rust-Lightning generated the message, as we
// check values are sane long before we get here, though its possible in the future
// user-generated messages may hit this.
if self.amt_to_forward > MAX_VALUE_MSAT { panic!("We should never be sending infinite/overflow onion payments"); }
match self.format {
OnionHopDataFormat::Legacy { short_channel_id } => {
0u8.write(w)?;
Expand All @@ -1311,9 +1307,6 @@ impl Writeable for OnionHopData {
});
},
OnionHopDataFormat::FinalNode { ref payment_data, ref keysend_preimage } => {
if let Some(final_data) = payment_data {
if final_data.total_msat > MAX_VALUE_MSAT { panic!("We should never be sending infinite/overflow onion payments"); }
}
encode_varint_length_prefixed_tlv!(w, {
(2, HighZeroBytesDroppedVarInt(self.amt_to_forward), required),
(4, HighZeroBytesDroppedVarInt(self.outgoing_cltv_value), required),
Expand Down
Loading