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.
This commit is contained in:
Matt Corallo 2020-01-01 20:20:42 -05:00
parent f26e373396
commit b54817397d
3 changed files with 127 additions and 33 deletions

View file

@ -74,7 +74,9 @@ enum PendingHTLCRouting {
onion_packet: msgs::OnionPacket, onion_packet: msgs::OnionPacket,
short_channel_id: u64, // This should be NonZero<u64> eventually when we bump MSRV short_channel_id: u64, // This should be NonZero<u64> eventually when we bump MSRV
}, },
Receive {}, Receive {
payment_data: Option<msgs::FinalOnionHopData>,
},
} }
#[derive(Clone)] // See Channel::revoke_and_ack for why, tl;dr: Rust bug #[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], 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<msgs::FinalOnionHopData>,
}
/// Tracks the inbound corresponding to an outbound HTLC /// Tracks the inbound corresponding to an outbound HTLC
#[derive(Clone, PartialEq)] #[derive(Clone, PartialEq)]
pub(super) enum HTLCSource { pub(super) enum HTLCSource {
@ -276,12 +288,11 @@ pub(super) struct ChannelHolder<ChanSigner: ChannelKeys> {
/// guarantees are made about the existence of a channel with the short id here, nor the short /// guarantees are made about the existence of a channel with the short id here, nor the short
/// ids in the PendingHTLCInfo! /// ids in the PendingHTLCInfo!
pub(super) forward_htlcs: HashMap<u64, Vec<HTLCForwardInfo>>, pub(super) forward_htlcs: HashMap<u64, Vec<HTLCForwardInfo>>,
/// payment_hash -> Vec<(amount_received, htlc_source)> for tracking things that were to us and /// Tracks HTLCs that were to us and can be failed/claimed by the user
/// can be failed/claimed by the user
/// Note that while this is held in the same mutex as the channels themselves, no consistency /// 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 /// guarantees are made about the channels given here actually existing anymore by the time you
/// go to read them! /// go to read them!
pub(super) claimable_htlcs: HashMap<PaymentHash, Vec<(u64, HTLCPreviousHopData)>>, claimable_htlcs: HashMap<PaymentHash, Vec<ClaimableHTLC>>,
/// Messages to send to peers - pushed to in the same lock that they are generated in (except /// 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). /// for broadcast messages, where ordering isn't as strict).
pub(super) pending_msg_events: Vec<events::MessageSendEvent>, pub(super) pending_msg_events: Vec<events::MessageSendEvent>,
@ -1007,13 +1018,19 @@ impl<ChanSigner: ChannelKeys, M: Deref, T: Deref, K: Deref, F: Deref> ChannelMan
return_err!("Upstream node set CLTV to the wrong value", 18, &byte_utils::be32_to_array(msg.cltv_expiry)); 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 // 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 // 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 // instead we stay symmetric with the forwarding case, only responding (after a
// delay) once they've send us a commitment_signed! // delay) once they've send us a commitment_signed!
PendingHTLCStatus::Forward(PendingHTLCInfo { PendingHTLCStatus::Forward(PendingHTLCInfo {
routing: PendingHTLCRouting::Receive {}, routing: PendingHTLCRouting::Receive { payment_data },
payment_hash: msg.payment_hash.clone(), payment_hash: msg.payment_hash.clone(),
incoming_shared_secret: shared_secret, incoming_shared_secret: shared_secret,
amt_to_forward: next_hop_data.amt_to_forward, amt_to_forward: next_hop_data.amt_to_forward,
@ -1580,17 +1597,18 @@ impl<ChanSigner: ChannelKeys, M: Deref, T: Deref, K: Deref, F: Deref> ChannelMan
for forward_info in pending_forwards.drain(..) { for forward_info in pending_forwards.drain(..) {
match forward_info { match forward_info {
HTLCForwardInfo::AddHTLC { prev_short_channel_id, prev_htlc_id, forward_info: PendingHTLCInfo { 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, .. }, } => { incoming_shared_secret, payment_hash, amt_to_forward, .. }, } => {
let prev_hop_data = HTLCPreviousHopData { let prev_hop = HTLCPreviousHopData {
short_channel_id: prev_short_channel_id, short_channel_id: prev_short_channel_id,
htlc_id: prev_htlc_id, htlc_id: prev_htlc_id,
incoming_packet_shared_secret: incoming_shared_secret, incoming_packet_shared_secret: incoming_shared_secret,
}; };
match channel_state.claimable_htlcs.entry(payment_hash) { channel_state.claimable_htlcs.entry(payment_hash).or_insert(Vec::new()).push(ClaimableHTLC {
hash_map::Entry::Occupied(mut entry) => entry.get_mut().push((amt_to_forward, prev_hop_data)), prev_hop,
hash_map::Entry::Vacant(entry) => { entry.insert(vec![(amt_to_forward, prev_hop_data)]); }, value: amt_to_forward,
}; payment_data,
});
new_events.push(events::Event::PaymentReceived { new_events.push(events::Event::PaymentReceived {
payment_hash: payment_hash, payment_hash: payment_hash,
amt: amt_to_forward, amt: amt_to_forward,
@ -1660,11 +1678,11 @@ impl<ChanSigner: ChannelKeys, M: Deref, T: Deref, K: Deref, F: Deref> ChannelMan
let mut channel_state = Some(self.channel_state.lock().unwrap()); let mut channel_state = Some(self.channel_state.lock().unwrap());
let removed_source = channel_state.as_mut().unwrap().claimable_htlcs.remove(payment_hash); let removed_source = channel_state.as_mut().unwrap().claimable_htlcs.remove(payment_hash);
if let Some(mut sources) = removed_source { 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()); } if channel_state.is_none() { channel_state = Some(self.channel_state.lock().unwrap()); }
self.fail_htlc_backwards_internal(channel_state.take().unwrap(), 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: byte_utils::be64_to_array(recvd_value).to_vec() }); HTLCFailReason::Reason { failure_code: 0x4000 | 15, data: byte_utils::be64_to_array(htlc.value).to_vec() });
} }
true true
} else { false } } else { false }
@ -1788,17 +1806,17 @@ impl<ChanSigner: ChannelKeys, M: Deref, T: Deref, K: Deref, F: Deref> ChannelMan
let mut channel_state = Some(self.channel_state.lock().unwrap()); let mut channel_state = Some(self.channel_state.lock().unwrap());
let removed_source = channel_state.as_mut().unwrap().claimable_htlcs.remove(&payment_hash); let removed_source = channel_state.as_mut().unwrap().claimable_htlcs.remove(&payment_hash);
if let Some(mut sources) = removed_source { 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 channel_state.is_none() { channel_state = Some(self.channel_state.lock().unwrap()); }
if received_amount < expected_amount || received_amount > expected_amount * 2 { if htlc.value < expected_amount || htlc.value > expected_amount * 2 {
let mut htlc_msat_data = byte_utils::be64_to_array(received_amount).to_vec(); 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(); 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); htlc_msat_data.append(&mut height_data);
self.fail_htlc_backwards_internal(channel_state.take().unwrap(), 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 }); HTLCFailReason::Reason { failure_code: 0x4000|15, data: htlc_msat_data });
} else { } 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 true
@ -3098,8 +3116,9 @@ impl Writeable for PendingHTLCInfo {
onion_packet.write(writer)?; onion_packet.write(writer)?;
short_channel_id.write(writer)?; short_channel_id.write(writer)?;
}, },
&PendingHTLCRouting::Receive { } => { &PendingHTLCRouting::Receive { ref payment_data } => {
1u8.write(writer)?; 1u8.write(writer)?;
payment_data.write(writer)?;
}, },
} }
self.incoming_shared_secret.write(writer)?; self.incoming_shared_secret.write(writer)?;
@ -3118,7 +3137,9 @@ impl Readable for PendingHTLCInfo {
onion_packet: Readable::read(reader)?, onion_packet: Readable::read(reader)?,
short_channel_id: Readable::read(reader)?, short_channel_id: Readable::read(reader)?,
}, },
1u8 => PendingHTLCRouting::Receive { }, 1u8 => PendingHTLCRouting::Receive {
payment_data: Readable::read(reader)?,
},
_ => return Err(DecodeError::InvalidValue), _ => return Err(DecodeError::InvalidValue),
}, },
incoming_shared_secret: Readable::read(reader)?, incoming_shared_secret: Readable::read(reader)?,
@ -3187,6 +3208,12 @@ impl_writeable!(HTLCPreviousHopData, 0, {
incoming_packet_shared_secret incoming_packet_shared_secret
}); });
impl_writeable!(ClaimableHTLC, 0, {
prev_hop,
value,
payment_data
});
impl Writeable for HTLCSource { impl Writeable for HTLCSource {
fn write<W: Writer>(&self, writer: &mut W) -> Result<(), ::std::io::Error> { fn write<W: Writer>(&self, writer: &mut W) -> Result<(), ::std::io::Error> {
match self { match self {
@ -3328,9 +3355,8 @@ impl<ChanSigner: ChannelKeys + Writeable, M: Deref, T: Deref, K: Deref, F: Deref
for (payment_hash, previous_hops) in channel_state.claimable_htlcs.iter() { for (payment_hash, previous_hops) in channel_state.claimable_htlcs.iter() {
payment_hash.write(writer)?; payment_hash.write(writer)?;
(previous_hops.len() as u64).write(writer)?; (previous_hops.len() as u64).write(writer)?;
for &(recvd_amt, ref previous_hop) in previous_hops.iter() { for htlc in previous_hops.iter() {
recvd_amt.write(writer)?; htlc.write(writer)?;
previous_hop.write(writer)?;
} }
} }
@ -3507,7 +3533,7 @@ impl<'a, ChanSigner: ChannelKeys + Readable, M: Deref, T: Deref, K: Deref, F: De
let previous_hops_len: u64 = Readable::read(reader)?; let previous_hops_len: u64 = Readable::read(reader)?;
let mut previous_hops = Vec::with_capacity(cmp::min(previous_hops_len as usize, 2)); let mut previous_hops = Vec::with_capacity(cmp::min(previous_hops_len as usize, 2));
for _ in 0..previous_hops_len { for _ in 0..previous_hops_len {
previous_hops.push((Readable::read(reader)?, Readable::read(reader)?)); previous_hops.push(Readable::read(reader)?);
} }
claimable_htlcs.insert(payment_hash, previous_hops); claimable_htlcs.insert(payment_hash, previous_hops);
} }

View file

@ -614,6 +614,11 @@ pub trait RoutingMessageHandler : Send + Sync {
mod fuzzy_internal_msgs { mod fuzzy_internal_msgs {
// These types aren't intended to be pub, but are exposed for direct fuzzing (as we deserialize // These types aren't intended to be pub, but are exposed for direct fuzzing (as we deserialize
// them from untrusted input): // them from untrusted input):
#[derive(Clone)]
pub(crate) struct FinalOnionHopData {
pub(crate) payment_secret: [u8; 32],
pub(crate) total_msat: u64,
}
pub(crate) enum OnionHopDataFormat { pub(crate) enum OnionHopDataFormat {
Legacy { // aka Realm-0 Legacy { // aka Realm-0
@ -622,7 +627,9 @@ mod fuzzy_internal_msgs {
NonFinalNode { NonFinalNode {
short_channel_id: u64, short_channel_id: u64,
}, },
FinalNode, FinalNode {
payment_data: Option<FinalOnionHopData>,
},
} }
pub struct OnionHopData { pub struct OnionHopData {
@ -965,6 +972,22 @@ impl_writeable!(UpdateAddHTLC, 32+8+8+32+4+1366, {
onion_routing_packet onion_routing_packet
}); });
impl Writeable for FinalOnionHopData {
fn write<W: Writer>(&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: Read>(r: &mut R) -> Result<Self, DecodeError> {
let payment_secret = Readable::read(r)?;
let amt: HighZeroBytesDroppedVarInt<u64> = Readable::read(r)?;
Ok(Self { payment_secret, total_msat: amt.0 })
}
}
impl Writeable for OnionHopData { impl Writeable for OnionHopData {
fn write<W: Writer>(&self, w: &mut W) -> Result<(), ::std::io::Error> { fn write<W: Writer>(&self, w: &mut W) -> Result<(), ::std::io::Error> {
w.size_hint(33); w.size_hint(33);
@ -983,7 +1006,14 @@ impl Writeable for OnionHopData {
(6, short_channel_id) (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, { encode_varint_length_prefixed_tlv!(w, {
(2, HighZeroBytesDroppedVarInt(self.amt_to_forward)), (2, HighZeroBytesDroppedVarInt(self.amt_to_forward)),
(4, HighZeroBytesDroppedVarInt(self.outgoing_cltv_value)) (4, HighZeroBytesDroppedVarInt(self.outgoing_cltv_value))
@ -1008,19 +1038,24 @@ impl Readable for OnionHopData {
let mut amt = HighZeroBytesDroppedVarInt(0u64); let mut amt = HighZeroBytesDroppedVarInt(0u64);
let mut cltv_value = HighZeroBytesDroppedVarInt(0u32); let mut cltv_value = HighZeroBytesDroppedVarInt(0u32);
let mut short_id: Option<u64> = None; let mut short_id: Option<u64> = None;
let mut payment_data: Option<FinalOnionHopData> = None;
decode_tlv!(&mut rd, { decode_tlv!(&mut rd, {
(2, amt), (2, amt),
(4, cltv_value) (4, cltv_value)
}, { }, {
(6, short_id) (6, short_id),
(8, payment_data)
}); });
rd.eat_remaining().map_err(|_| DecodeError::ShortRead)?; rd.eat_remaining().map_err(|_| DecodeError::ShortRead)?;
let format = if let Some(short_channel_id) = short_id { let format = if let Some(short_channel_id) = short_id {
if payment_data.is_some() { return Err(DecodeError::InvalidValue); }
OnionHopDataFormat::NonFinalNode { OnionHopDataFormat::NonFinalNode {
short_channel_id, short_channel_id,
} }
} else { } else {
OnionHopDataFormat::FinalNode OnionHopDataFormat::FinalNode {
payment_data
}
}; };
(format, amt.0, cltv_value.0) (format, amt.0, cltv_value.0)
} else { } else {
@ -1305,7 +1340,7 @@ impl_writeable_len_match!(NodeAnnouncement, {
mod tests { mod tests {
use hex; use hex;
use ln::msgs; 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 ln::channelmanager::{PaymentPreimage, PaymentHash};
use util::ser::{Writeable, Readable}; use util::ser::{Writeable, Readable};
@ -1998,7 +2033,9 @@ mod tests {
#[test] #[test]
fn encoding_final_onion_hop_data() { fn encoding_final_onion_hop_data() {
let mut msg = msgs::OnionHopData { let mut msg = msgs::OnionHopData {
format: OnionHopDataFormat::FinalNode, format: OnionHopDataFormat::FinalNode {
payment_data: None,
},
amt_to_forward: 0x0badf00d01020304, amt_to_forward: 0x0badf00d01020304,
outgoing_cltv_value: 0xffffffff, outgoing_cltv_value: 0xffffffff,
}; };
@ -2006,7 +2043,36 @@ mod tests {
let target_value = hex::decode("1002080badf00d010203040404ffffffff").unwrap(); let target_value = hex::decode("1002080badf00d010203040404ffffffff").unwrap();
assert_eq!(encoded_value, target_value); assert_eq!(encoded_value, target_value);
msg = Readable::read(&mut Cursor::new(&target_value[..])).unwrap(); 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.amt_to_forward, 0x0badf00d01020304);
assert_eq!(msg.outgoing_cltv_value, 0xffffffff); assert_eq!(msg.outgoing_cltv_value, 0xffffffff);
} }

View file

@ -123,7 +123,9 @@ pub(super) fn build_onion_payloads(route: &Route, starting_htlc_offset: u32) ->
res.insert(0, msgs::OnionHopData { res.insert(0, msgs::OnionHopData {
format: if hop.node_features.supports_variable_length_onion() { format: if hop.node_features.supports_variable_length_onion() {
if idx == 0 { if idx == 0 {
msgs::OnionHopDataFormat::FinalNode msgs::OnionHopDataFormat::FinalNode {
payment_data: None,
}
} else { } else {
msgs::OnionHopDataFormat::NonFinalNode { msgs::OnionHopDataFormat::NonFinalNode {
short_channel_id: last_short_channel_id, short_channel_id: last_short_channel_id,