From: Matt Corallo Date: Thu, 2 Jan 2020 01:20:42 +0000 (-0500) Subject: Support (de)serializing payment_data in onion TLVs and track them X-Git-Tag: v0.0.12~86^2~10 X-Git-Url: http://git.bitcoin.ninja/index.cgi?a=commitdiff_plain;h=b54817397d62817d806d2ac266617ef30709d1d3;p=rust-lightning Support (de)serializing payment_data in onion TLVs and track them This is the first step in Base AMP support, just tracking the relevant data in internal datastructures. --- diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index 84b595a2d..1a207bf91 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -74,7 +74,9 @@ enum PendingHTLCRouting { onion_packet: msgs::OnionPacket, short_channel_id: u64, // This should be NonZero eventually when we bump MSRV }, - Receive {}, + Receive { + payment_data: Option, + }, } #[derive(Clone)] // See Channel::revoke_and_ack for why, tl;dr: Rust bug @@ -119,6 +121,16 @@ pub(super) struct HTLCPreviousHopData { incoming_packet_shared_secret: [u8; 32], } +struct ClaimableHTLC { + prev_hop: HTLCPreviousHopData, + value: u64, + /// Filled in when the HTLC was received with a payment_secret packet, which contains a + /// total_msat (which may differ from value if this is a Multi-Path Payment) and a + /// payment_secret which prevents path-probing attacks and can associate different HTLCs which + /// are part of the same payment. + payment_data: Option, +} + /// Tracks the inbound corresponding to an outbound HTLC #[derive(Clone, PartialEq)] pub(super) enum HTLCSource { @@ -276,12 +288,11 @@ pub(super) struct ChannelHolder { /// guarantees are made about the existence of a channel with the short id here, nor the short /// ids in the PendingHTLCInfo! pub(super) forward_htlcs: HashMap>, - /// payment_hash -> Vec<(amount_received, htlc_source)> for tracking things that were to us and - /// can be failed/claimed by the user + /// Tracks HTLCs that were to us and can be failed/claimed by the user /// Note that while this is held in the same mutex as the channels themselves, no consistency /// guarantees are made about the channels given here actually existing anymore by the time you /// go to read them! - pub(super) claimable_htlcs: HashMap>, + claimable_htlcs: HashMap>, /// Messages to send to peers - pushed to in the same lock that they are generated in (except /// for broadcast messages, where ordering isn't as strict). pub(super) pending_msg_events: Vec, @@ -1007,13 +1018,19 @@ impl ChannelMan return_err!("Upstream node set CLTV to the wrong value", 18, &byte_utils::be32_to_array(msg.cltv_expiry)); } + let payment_data = match next_hop_data.format { + msgs::OnionHopDataFormat::Legacy { .. } => None, + msgs::OnionHopDataFormat::NonFinalNode { .. } => return_err!("Got non final data with an HMAC of 0", 0x4000 | 22, &[0;0]), + msgs::OnionHopDataFormat::FinalNode { payment_data } => payment_data, + }; + // 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 // instead we stay symmetric with the forwarding case, only responding (after a // delay) once they've send us a commitment_signed! PendingHTLCStatus::Forward(PendingHTLCInfo { - routing: PendingHTLCRouting::Receive {}, + routing: PendingHTLCRouting::Receive { payment_data }, payment_hash: msg.payment_hash.clone(), incoming_shared_secret: shared_secret, amt_to_forward: next_hop_data.amt_to_forward, @@ -1580,17 +1597,18 @@ impl ChannelMan for forward_info in pending_forwards.drain(..) { match forward_info { HTLCForwardInfo::AddHTLC { prev_short_channel_id, prev_htlc_id, forward_info: PendingHTLCInfo { - routing: PendingHTLCRouting::Receive { }, + routing: PendingHTLCRouting::Receive { payment_data }, incoming_shared_secret, payment_hash, amt_to_forward, .. }, } => { - let prev_hop_data = HTLCPreviousHopData { + let prev_hop = HTLCPreviousHopData { short_channel_id: prev_short_channel_id, htlc_id: prev_htlc_id, incoming_packet_shared_secret: incoming_shared_secret, }; - match channel_state.claimable_htlcs.entry(payment_hash) { - hash_map::Entry::Occupied(mut entry) => entry.get_mut().push((amt_to_forward, prev_hop_data)), - hash_map::Entry::Vacant(entry) => { entry.insert(vec![(amt_to_forward, prev_hop_data)]); }, - }; + channel_state.claimable_htlcs.entry(payment_hash).or_insert(Vec::new()).push(ClaimableHTLC { + prev_hop, + value: amt_to_forward, + payment_data, + }); new_events.push(events::Event::PaymentReceived { payment_hash: payment_hash, amt: amt_to_forward, @@ -1660,11 +1678,11 @@ impl ChannelMan let mut channel_state = Some(self.channel_state.lock().unwrap()); let removed_source = channel_state.as_mut().unwrap().claimable_htlcs.remove(payment_hash); if let Some(mut sources) = removed_source { - for (recvd_value, htlc_with_hash) in sources.drain(..) { + for htlc in sources.drain(..) { if channel_state.is_none() { channel_state = Some(self.channel_state.lock().unwrap()); } self.fail_htlc_backwards_internal(channel_state.take().unwrap(), - HTLCSource::PreviousHopData(htlc_with_hash), payment_hash, - HTLCFailReason::Reason { failure_code: 0x4000 | 15, data: byte_utils::be64_to_array(recvd_value).to_vec() }); + HTLCSource::PreviousHopData(htlc.prev_hop), payment_hash, + HTLCFailReason::Reason { failure_code: 0x4000 | 15, data: byte_utils::be64_to_array(htlc.value).to_vec() }); } true } else { false } @@ -1788,17 +1806,17 @@ impl ChannelMan let mut channel_state = Some(self.channel_state.lock().unwrap()); let removed_source = channel_state.as_mut().unwrap().claimable_htlcs.remove(&payment_hash); if let Some(mut sources) = removed_source { - for (received_amount, htlc_with_hash) in sources.drain(..) { + for htlc in sources.drain(..) { if channel_state.is_none() { channel_state = Some(self.channel_state.lock().unwrap()); } - if received_amount < expected_amount || received_amount > expected_amount * 2 { - let mut htlc_msat_data = byte_utils::be64_to_array(received_amount).to_vec(); + if htlc.value < expected_amount || htlc.value > expected_amount * 2 { + let mut htlc_msat_data = byte_utils::be64_to_array(htlc.value).to_vec(); let mut height_data = byte_utils::be32_to_array(self.latest_block_height.load(Ordering::Acquire) as u32).to_vec(); htlc_msat_data.append(&mut height_data); self.fail_htlc_backwards_internal(channel_state.take().unwrap(), - HTLCSource::PreviousHopData(htlc_with_hash), &payment_hash, + HTLCSource::PreviousHopData(htlc.prev_hop), &payment_hash, HTLCFailReason::Reason { failure_code: 0x4000|15, data: htlc_msat_data }); } else { - self.claim_funds_internal(channel_state.take().unwrap(), HTLCSource::PreviousHopData(htlc_with_hash), payment_preimage); + self.claim_funds_internal(channel_state.take().unwrap(), HTLCSource::PreviousHopData(htlc.prev_hop), payment_preimage); } } true @@ -3098,8 +3116,9 @@ impl Writeable for PendingHTLCInfo { onion_packet.write(writer)?; short_channel_id.write(writer)?; }, - &PendingHTLCRouting::Receive { } => { + &PendingHTLCRouting::Receive { ref payment_data } => { 1u8.write(writer)?; + payment_data.write(writer)?; }, } self.incoming_shared_secret.write(writer)?; @@ -3118,7 +3137,9 @@ impl Readable for PendingHTLCInfo { onion_packet: Readable::read(reader)?, short_channel_id: Readable::read(reader)?, }, - 1u8 => PendingHTLCRouting::Receive { }, + 1u8 => PendingHTLCRouting::Receive { + payment_data: Readable::read(reader)?, + }, _ => return Err(DecodeError::InvalidValue), }, incoming_shared_secret: Readable::read(reader)?, @@ -3187,6 +3208,12 @@ impl_writeable!(HTLCPreviousHopData, 0, { incoming_packet_shared_secret }); +impl_writeable!(ClaimableHTLC, 0, { + prev_hop, + value, + payment_data +}); + impl Writeable for HTLCSource { fn write(&self, writer: &mut W) -> Result<(), ::std::io::Error> { match self { @@ -3328,9 +3355,8 @@ impl, + }, } pub struct OnionHopData { @@ -965,6 +972,22 @@ impl_writeable!(UpdateAddHTLC, 32+8+8+32+4+1366, { onion_routing_packet }); +impl Writeable for FinalOnionHopData { + fn write(&self, w: &mut W) -> Result<(), ::std::io::Error> { + w.size_hint(32 + 8 - (self.total_msat.leading_zeros()/8) as usize); + self.payment_secret.write(w)?; + HighZeroBytesDroppedVarInt(self.total_msat).write(w) + } +} + +impl Readable for FinalOnionHopData { + fn read(r: &mut R) -> Result { + let payment_secret = Readable::read(r)?; + let amt: HighZeroBytesDroppedVarInt = Readable::read(r)?; + Ok(Self { payment_secret, total_msat: amt.0 }) + } +} + impl Writeable for OnionHopData { fn write(&self, w: &mut W) -> Result<(), ::std::io::Error> { w.size_hint(33); @@ -983,7 +1006,14 @@ impl Writeable for OnionHopData { (6, short_channel_id) }); }, - OnionHopDataFormat::FinalNode => { + OnionHopDataFormat::FinalNode { payment_data: Some(ref final_data) } => { + encode_varint_length_prefixed_tlv!(w, { + (2, HighZeroBytesDroppedVarInt(self.amt_to_forward)), + (4, HighZeroBytesDroppedVarInt(self.outgoing_cltv_value)), + (8, final_data) + }); + }, + OnionHopDataFormat::FinalNode { payment_data: None } => { encode_varint_length_prefixed_tlv!(w, { (2, HighZeroBytesDroppedVarInt(self.amt_to_forward)), (4, HighZeroBytesDroppedVarInt(self.outgoing_cltv_value)) @@ -1008,19 +1038,24 @@ impl Readable for OnionHopData { let mut amt = HighZeroBytesDroppedVarInt(0u64); let mut cltv_value = HighZeroBytesDroppedVarInt(0u32); let mut short_id: Option = None; + let mut payment_data: Option = None; decode_tlv!(&mut rd, { (2, amt), (4, cltv_value) }, { - (6, short_id) + (6, short_id), + (8, payment_data) }); rd.eat_remaining().map_err(|_| DecodeError::ShortRead)?; let format = if let Some(short_channel_id) = short_id { + if payment_data.is_some() { return Err(DecodeError::InvalidValue); } OnionHopDataFormat::NonFinalNode { short_channel_id, } } else { - OnionHopDataFormat::FinalNode + OnionHopDataFormat::FinalNode { + payment_data + } }; (format, amt.0, cltv_value.0) } else { @@ -1305,7 +1340,7 @@ impl_writeable_len_match!(NodeAnnouncement, { mod tests { use hex; use ln::msgs; - use ln::msgs::{ChannelFeatures, InitFeatures, NodeFeatures, OptionalField, OnionErrorPacket, OnionHopDataFormat}; + use ln::msgs::{ChannelFeatures, FinalOnionHopData, InitFeatures, NodeFeatures, OptionalField, OnionErrorPacket, OnionHopDataFormat}; use ln::channelmanager::{PaymentPreimage, PaymentHash}; use util::ser::{Writeable, Readable}; @@ -1998,7 +2033,9 @@ mod tests { #[test] fn encoding_final_onion_hop_data() { let mut msg = msgs::OnionHopData { - format: OnionHopDataFormat::FinalNode, + format: OnionHopDataFormat::FinalNode { + payment_data: None, + }, amt_to_forward: 0x0badf00d01020304, outgoing_cltv_value: 0xffffffff, }; @@ -2006,7 +2043,36 @@ mod tests { let target_value = hex::decode("1002080badf00d010203040404ffffffff").unwrap(); assert_eq!(encoded_value, target_value); msg = Readable::read(&mut Cursor::new(&target_value[..])).unwrap(); - if let OnionHopDataFormat::FinalNode = msg.format { } else { panic!(); } + if let OnionHopDataFormat::FinalNode { payment_data: None } = msg.format { } else { panic!(); } + assert_eq!(msg.amt_to_forward, 0x0badf00d01020304); + assert_eq!(msg.outgoing_cltv_value, 0xffffffff); + } + + #[test] + fn encoding_final_onion_hop_data_with_secret() { + let expected_payment_secret = [0x42u8; 32]; + let mut msg = msgs::OnionHopData { + format: OnionHopDataFormat::FinalNode { + payment_data: Some(FinalOnionHopData { + payment_secret: expected_payment_secret, + total_msat: 0x1badca1f + }), + }, + amt_to_forward: 0x0badf00d01020304, + outgoing_cltv_value: 0xffffffff, + }; + let encoded_value = msg.encode(); + let target_value = hex::decode("3602080badf00d010203040404ffffffff082442424242424242424242424242424242424242424242424242424242424242421badca1f").unwrap(); + assert_eq!(encoded_value, target_value); + msg = Readable::read(&mut Cursor::new(&target_value[..])).unwrap(); + if let OnionHopDataFormat::FinalNode { + payment_data: Some(FinalOnionHopData { + payment_secret, + total_msat: 0x1badca1f + }) + } = msg.format { + assert_eq!(payment_secret, expected_payment_secret); + } else { panic!(); } assert_eq!(msg.amt_to_forward, 0x0badf00d01020304); assert_eq!(msg.outgoing_cltv_value, 0xffffffff); } diff --git a/lightning/src/ln/onion_utils.rs b/lightning/src/ln/onion_utils.rs index c2835460e..a97621045 100644 --- a/lightning/src/ln/onion_utils.rs +++ b/lightning/src/ln/onion_utils.rs @@ -123,7 +123,9 @@ pub(super) fn build_onion_payloads(route: &Route, starting_htlc_offset: u32) -> res.insert(0, msgs::OnionHopData { format: if hop.node_features.supports_variable_length_onion() { if idx == 0 { - msgs::OnionHopDataFormat::FinalNode + msgs::OnionHopDataFormat::FinalNode { + payment_data: None, + } } else { msgs::OnionHopDataFormat::NonFinalNode { short_channel_id: last_short_channel_id,