diff --git a/fuzz/Cargo.toml b/fuzz/Cargo.toml index 48c9064c649..3cd249ea821 100644 --- a/fuzz/Cargo.toml +++ b/fuzz/Cargo.toml @@ -62,11 +62,11 @@ path = "fuzz_targets/msg_pong_target.rs" [[bin]] name = "msg_error_message_target" -path = "fuzz_targets/msg_error_message_target.rs" +path = "fuzz_targets/msg_targets/msg_error_message_target.rs" [[bin]] name = "msg_update_add_htlc_target" -path = "fuzz_targets/msg_update_add_htlc_target.rs" +path = "fuzz_targets/msg_targets/msg_update_add_htlc_target.rs" [[bin]] name = "msg_accept_channel_target" @@ -123,3 +123,31 @@ path = "fuzz_targets/msg_targets/msg_update_fail_htlc_target.rs" [[bin]] name = "msg_channel_reestablish_target" path = "fuzz_targets/msg_targets/msg_channel_reestablish_target.rs" + +[[bin]] +name = "msg_announcement_signatures_target" +path = "fuzz_targets/msg_targets/msg_announcement_signatures_target.rs" + +[[bin]] +name = "msg_channel_announcement_target" +path = "fuzz_targets/msg_targets/msg_channel_announcement_target.rs" + +[[bin]] +name = "msg_channel_update_target" +path = "fuzz_targets/msg_targets/msg_channel_update_target.rs" + +[[bin]] +name = "msg_decoded_onion_error_packet_target" +path = "fuzz_targets/msg_targets/msg_decoded_onion_error_packet_target.rs" + +[[bin]] +name = "msg_init_target" +path = "fuzz_targets/msg_targets/msg_init_target.rs" + +[[bin]] +name = "msg_node_announcement_target" +path = "fuzz_targets/msg_targets/msg_node_announcement_target.rs" + +[[bin]] +name = "msg_onion_hop_data_target" +path = "fuzz_targets/msg_targets/msg_onion_hop_data_target.rs" diff --git a/fuzz/fuzz_targets/channel_target.rs b/fuzz/fuzz_targets/channel_target.rs index ed5fa13b09c..37a67183696 100644 --- a/fuzz/fuzz_targets/channel_target.rs +++ b/fuzz/fuzz_targets/channel_target.rs @@ -124,6 +124,7 @@ pub fn do_test(data: &[u8]) { Ok(msg) => msg, Err(e) => match e { msgs::DecodeError::UnknownRealmByte => return, + msgs::DecodeError::UnknownRequiredFeature => return, msgs::DecodeError::BadPublicKey => return, msgs::DecodeError::BadSignature => return, msgs::DecodeError::BadText => return, @@ -146,6 +147,7 @@ pub fn do_test(data: &[u8]) { Ok(msg) => msg, Err(e) => match e { msgs::DecodeError::UnknownRealmByte => return, + msgs::DecodeError::UnknownRequiredFeature => return, msgs::DecodeError::BadPublicKey => return, msgs::DecodeError::BadSignature => return, msgs::DecodeError::BadText => return, diff --git a/fuzz/fuzz_targets/msg_targets/gen_target.sh b/fuzz/fuzz_targets/msg_targets/gen_target.sh index 249c0ce1f34..32f071e85cd 100755 --- a/fuzz/fuzz_targets/msg_targets/gen_target.sh +++ b/fuzz/fuzz_targets/msg_targets/gen_target.sh @@ -1,5 +1,33 @@ -for target in CommitmentSigned FundingCreated FundingLocked FundingSigned OpenChannel RevokeAndACK Shutdown UpdateFailHTLC UpdateFailMalformedHTLC UpdateFee UpdateFulfillHTLC AcceptChannel ClosingSigned ChannelReestablish; do - tn=$(echo $target | sed 's/\([a-z0-9]\)\([A-Z]\)/\1_\2/g') +#!/bin/sh + +GEN_TEST() { + tn=$(echo $1 | sed 's/\([a-z0-9]\)\([A-Z]\)/\1_\2/g') fn=msg_$(echo $tn | tr '[:upper:]' '[:lower:]')_target.rs - cat msg_target_template.txt | sed s/MSG_TARGET/$target/ > $fn -done + cat msg_target_template.txt | sed s/MSG_TARGET/$1/ | sed "s/TEST_MSG/$2/" | sed "s/EXTRA_ARGS/$3/" > $fn +} + +GEN_TEST AcceptChannel test_msg "" +GEN_TEST AnnouncementSignatures test_msg "" +GEN_TEST ChannelReestablish test_msg "" +GEN_TEST ClosingSigned test_msg "" +GEN_TEST CommitmentSigned test_msg "" +GEN_TEST DecodedOnionErrorPacket test_msg "" +GEN_TEST FundingCreated test_msg "" +GEN_TEST FundingLocked test_msg "" +GEN_TEST FundingSigned test_msg "" +GEN_TEST Init test_msg "" +GEN_TEST OpenChannel test_msg "" +GEN_TEST RevokeAndACK test_msg "" +GEN_TEST Shutdown test_msg "" +GEN_TEST UpdateFailHTLC test_msg "" +GEN_TEST UpdateFailMalformedHTLC test_msg "" +GEN_TEST UpdateFee test_msg "" +GEN_TEST UpdateFulfillHTLC test_msg "" + +GEN_TEST ChannelAnnouncement test_msg_exact "" +GEN_TEST ChannelUpdate test_msg_exact "" +GEN_TEST NodeAnnouncement test_msg_exact "" + +GEN_TEST UpdateAddHTLC test_msg_hole ", 85, 33" +GEN_TEST ErrorMessage test_msg_hole ", 32, 2" +GEN_TEST OnionHopData test_msg_hole ", 1+8+8+4, 12" diff --git a/fuzz/fuzz_targets/msg_targets/msg_announcement_signatures_target.rs b/fuzz/fuzz_targets/msg_targets/msg_announcement_signatures_target.rs new file mode 100644 index 00000000000..33c52b33e9e --- /dev/null +++ b/fuzz/fuzz_targets/msg_targets/msg_announcement_signatures_target.rs @@ -0,0 +1,46 @@ +// This file is auto-generated by gen_target.sh based on msg_target_template.txt +// To modify it, modify msg_target_template.txt and run gen_target.sh instead. + +extern crate lightning; + +use lightning::ln::msgs; +use lightning::util::reset_rng_state; + +use lightning::ln::msgs::{MsgEncodable, MsgDecodable}; + +mod utils; + +#[inline] +pub fn do_test(data: &[u8]) { + reset_rng_state(); + test_msg!(msgs::AnnouncementSignatures, data); +} + +#[cfg(feature = "afl")] +#[macro_use] extern crate afl; +#[cfg(feature = "afl")] +fn main() { + fuzz!(|data| { + do_test(data); + }); +} + +#[cfg(feature = "honggfuzz")] +#[macro_use] extern crate honggfuzz; +#[cfg(feature = "honggfuzz")] +fn main() { + loop { + fuzz!(|data| { + do_test(data); + }); + } +} + +extern crate hex; +#[cfg(test)] +mod tests { + #[test] + fn duplicate_crash() { + super::do_test(&::hex::decode("00").unwrap()); + } +} diff --git a/fuzz/fuzz_targets/msg_targets/msg_channel_announcement_target.rs b/fuzz/fuzz_targets/msg_targets/msg_channel_announcement_target.rs new file mode 100644 index 00000000000..1252cfe79de --- /dev/null +++ b/fuzz/fuzz_targets/msg_targets/msg_channel_announcement_target.rs @@ -0,0 +1,46 @@ +// This file is auto-generated by gen_target.sh based on msg_target_template.txt +// To modify it, modify msg_target_template.txt and run gen_target.sh instead. + +extern crate lightning; + +use lightning::ln::msgs; +use lightning::util::reset_rng_state; + +use lightning::ln::msgs::{MsgEncodable, MsgDecodable}; + +mod utils; + +#[inline] +pub fn do_test(data: &[u8]) { + reset_rng_state(); + test_msg_exact!(msgs::ChannelAnnouncement, data); +} + +#[cfg(feature = "afl")] +#[macro_use] extern crate afl; +#[cfg(feature = "afl")] +fn main() { + fuzz!(|data| { + do_test(data); + }); +} + +#[cfg(feature = "honggfuzz")] +#[macro_use] extern crate honggfuzz; +#[cfg(feature = "honggfuzz")] +fn main() { + loop { + fuzz!(|data| { + do_test(data); + }); + } +} + +extern crate hex; +#[cfg(test)] +mod tests { + #[test] + fn duplicate_crash() { + super::do_test(&::hex::decode("00").unwrap()); + } +} diff --git a/fuzz/fuzz_targets/msg_update_add_htlc_target.rs b/fuzz/fuzz_targets/msg_targets/msg_channel_update_target.rs similarity index 74% rename from fuzz/fuzz_targets/msg_update_add_htlc_target.rs rename to fuzz/fuzz_targets/msg_targets/msg_channel_update_target.rs index 5616047c25e..c488fb6da97 100644 --- a/fuzz/fuzz_targets/msg_update_add_htlc_target.rs +++ b/fuzz/fuzz_targets/msg_targets/msg_channel_update_target.rs @@ -1,3 +1,6 @@ +// This file is auto-generated by gen_target.sh based on msg_target_template.txt +// To modify it, modify msg_target_template.txt and run gen_target.sh instead. + extern crate lightning; use lightning::ln::msgs; @@ -5,14 +8,12 @@ use lightning::util::reset_rng_state; use lightning::ln::msgs::{MsgEncodable, MsgDecodable}; +mod utils; + #[inline] pub fn do_test(data: &[u8]) { reset_rng_state(); - if let Ok(msg) = msgs::UpdateAddHTLC::decode(data){ - let enc = msg.encode(); - assert_eq!(&data[0..85], &enc[0..85]); - assert_eq!(&data[85+33..enc.len()], &enc[85+33..]); - } + test_msg_exact!(msgs::ChannelUpdate, data); } #[cfg(feature = "afl")] diff --git a/fuzz/fuzz_targets/msg_targets/msg_decoded_onion_error_packet_target.rs b/fuzz/fuzz_targets/msg_targets/msg_decoded_onion_error_packet_target.rs new file mode 100644 index 00000000000..56a5fb9f676 --- /dev/null +++ b/fuzz/fuzz_targets/msg_targets/msg_decoded_onion_error_packet_target.rs @@ -0,0 +1,46 @@ +// This file is auto-generated by gen_target.sh based on msg_target_template.txt +// To modify it, modify msg_target_template.txt and run gen_target.sh instead. + +extern crate lightning; + +use lightning::ln::msgs; +use lightning::util::reset_rng_state; + +use lightning::ln::msgs::{MsgEncodable, MsgDecodable}; + +mod utils; + +#[inline] +pub fn do_test(data: &[u8]) { + reset_rng_state(); + test_msg!(msgs::DecodedOnionErrorPacket, data); +} + +#[cfg(feature = "afl")] +#[macro_use] extern crate afl; +#[cfg(feature = "afl")] +fn main() { + fuzz!(|data| { + do_test(data); + }); +} + +#[cfg(feature = "honggfuzz")] +#[macro_use] extern crate honggfuzz; +#[cfg(feature = "honggfuzz")] +fn main() { + loop { + fuzz!(|data| { + do_test(data); + }); + } +} + +extern crate hex; +#[cfg(test)] +mod tests { + #[test] + fn duplicate_crash() { + super::do_test(&::hex::decode("00").unwrap()); + } +} diff --git a/fuzz/fuzz_targets/msg_targets/msg_error_message_target.rs b/fuzz/fuzz_targets/msg_targets/msg_error_message_target.rs new file mode 100644 index 00000000000..97e15c3c4b7 --- /dev/null +++ b/fuzz/fuzz_targets/msg_targets/msg_error_message_target.rs @@ -0,0 +1,46 @@ +// This file is auto-generated by gen_target.sh based on msg_target_template.txt +// To modify it, modify msg_target_template.txt and run gen_target.sh instead. + +extern crate lightning; + +use lightning::ln::msgs; +use lightning::util::reset_rng_state; + +use lightning::ln::msgs::{MsgEncodable, MsgDecodable}; + +mod utils; + +#[inline] +pub fn do_test(data: &[u8]) { + reset_rng_state(); + test_msg_hole!(msgs::ErrorMessage, data, 32, 2); +} + +#[cfg(feature = "afl")] +#[macro_use] extern crate afl; +#[cfg(feature = "afl")] +fn main() { + fuzz!(|data| { + do_test(data); + }); +} + +#[cfg(feature = "honggfuzz")] +#[macro_use] extern crate honggfuzz; +#[cfg(feature = "honggfuzz")] +fn main() { + loop { + fuzz!(|data| { + do_test(data); + }); + } +} + +extern crate hex; +#[cfg(test)] +mod tests { + #[test] + fn duplicate_crash() { + super::do_test(&::hex::decode("00").unwrap()); + } +} diff --git a/fuzz/fuzz_targets/msg_error_message_target.rs b/fuzz/fuzz_targets/msg_targets/msg_init_target.rs similarity index 75% rename from fuzz/fuzz_targets/msg_error_message_target.rs rename to fuzz/fuzz_targets/msg_targets/msg_init_target.rs index ff3719559b2..8a5ee76f5b5 100644 --- a/fuzz/fuzz_targets/msg_error_message_target.rs +++ b/fuzz/fuzz_targets/msg_targets/msg_init_target.rs @@ -1,3 +1,6 @@ +// This file is auto-generated by gen_target.sh based on msg_target_template.txt +// To modify it, modify msg_target_template.txt and run gen_target.sh instead. + extern crate lightning; use lightning::ln::msgs; @@ -5,14 +8,12 @@ use lightning::util::reset_rng_state; use lightning::ln::msgs::{MsgEncodable, MsgDecodable}; +mod utils; + #[inline] pub fn do_test(data: &[u8]) { reset_rng_state(); - if let Ok(msg) = msgs::ErrorMessage::decode(data){ - let enc = msg.encode(); - assert_eq!(&data[0..32], &enc[0..32]); - assert_eq!(&data[34..enc.len()], &enc[34..]); - } + test_msg!(msgs::Init, data); } #[cfg(feature = "afl")] diff --git a/fuzz/fuzz_targets/msg_targets/msg_node_announcement_target.rs b/fuzz/fuzz_targets/msg_targets/msg_node_announcement_target.rs new file mode 100644 index 00000000000..54b9cb684b5 --- /dev/null +++ b/fuzz/fuzz_targets/msg_targets/msg_node_announcement_target.rs @@ -0,0 +1,46 @@ +// This file is auto-generated by gen_target.sh based on msg_target_template.txt +// To modify it, modify msg_target_template.txt and run gen_target.sh instead. + +extern crate lightning; + +use lightning::ln::msgs; +use lightning::util::reset_rng_state; + +use lightning::ln::msgs::{MsgEncodable, MsgDecodable}; + +mod utils; + +#[inline] +pub fn do_test(data: &[u8]) { + reset_rng_state(); + test_msg_exact!(msgs::NodeAnnouncement, data); +} + +#[cfg(feature = "afl")] +#[macro_use] extern crate afl; +#[cfg(feature = "afl")] +fn main() { + fuzz!(|data| { + do_test(data); + }); +} + +#[cfg(feature = "honggfuzz")] +#[macro_use] extern crate honggfuzz; +#[cfg(feature = "honggfuzz")] +fn main() { + loop { + fuzz!(|data| { + do_test(data); + }); + } +} + +extern crate hex; +#[cfg(test)] +mod tests { + #[test] + fn duplicate_crash() { + super::do_test(&::hex::decode("00").unwrap()); + } +} diff --git a/fuzz/fuzz_targets/msg_targets/msg_onion_hop_data_target.rs b/fuzz/fuzz_targets/msg_targets/msg_onion_hop_data_target.rs new file mode 100644 index 00000000000..70849c18981 --- /dev/null +++ b/fuzz/fuzz_targets/msg_targets/msg_onion_hop_data_target.rs @@ -0,0 +1,46 @@ +// This file is auto-generated by gen_target.sh based on msg_target_template.txt +// To modify it, modify msg_target_template.txt and run gen_target.sh instead. + +extern crate lightning; + +use lightning::ln::msgs; +use lightning::util::reset_rng_state; + +use lightning::ln::msgs::{MsgEncodable, MsgDecodable}; + +mod utils; + +#[inline] +pub fn do_test(data: &[u8]) { + reset_rng_state(); + test_msg_hole!(msgs::OnionHopData, data, 1+8+8+4, 12); +} + +#[cfg(feature = "afl")] +#[macro_use] extern crate afl; +#[cfg(feature = "afl")] +fn main() { + fuzz!(|data| { + do_test(data); + }); +} + +#[cfg(feature = "honggfuzz")] +#[macro_use] extern crate honggfuzz; +#[cfg(feature = "honggfuzz")] +fn main() { + loop { + fuzz!(|data| { + do_test(data); + }); + } +} + +extern crate hex; +#[cfg(test)] +mod tests { + #[test] + fn duplicate_crash() { + super::do_test(&::hex::decode("00").unwrap()); + } +} diff --git a/fuzz/fuzz_targets/msg_targets/msg_target_template.txt b/fuzz/fuzz_targets/msg_targets/msg_target_template.txt index 599a4f7225b..6053c041cf7 100644 --- a/fuzz/fuzz_targets/msg_targets/msg_target_template.txt +++ b/fuzz/fuzz_targets/msg_targets/msg_target_template.txt @@ -13,7 +13,7 @@ mod utils; #[inline] pub fn do_test(data: &[u8]) { reset_rng_state(); - test_msg!(msgs::MSG_TARGET, data); + TEST_MSG!(msgs::MSG_TARGET, dataEXTRA_ARGS); } #[cfg(feature = "afl")] diff --git a/fuzz/fuzz_targets/msg_targets/msg_update_add_htlc_target.rs b/fuzz/fuzz_targets/msg_targets/msg_update_add_htlc_target.rs new file mode 100644 index 00000000000..64806f20fa4 --- /dev/null +++ b/fuzz/fuzz_targets/msg_targets/msg_update_add_htlc_target.rs @@ -0,0 +1,46 @@ +// This file is auto-generated by gen_target.sh based on msg_target_template.txt +// To modify it, modify msg_target_template.txt and run gen_target.sh instead. + +extern crate lightning; + +use lightning::ln::msgs; +use lightning::util::reset_rng_state; + +use lightning::ln::msgs::{MsgEncodable, MsgDecodable}; + +mod utils; + +#[inline] +pub fn do_test(data: &[u8]) { + reset_rng_state(); + test_msg_hole!(msgs::UpdateAddHTLC, data, 85, 33); +} + +#[cfg(feature = "afl")] +#[macro_use] extern crate afl; +#[cfg(feature = "afl")] +fn main() { + fuzz!(|data| { + do_test(data); + }); +} + +#[cfg(feature = "honggfuzz")] +#[macro_use] extern crate honggfuzz; +#[cfg(feature = "honggfuzz")] +fn main() { + loop { + fuzz!(|data| { + do_test(data); + }); + } +} + +extern crate hex; +#[cfg(test)] +mod tests { + #[test] + fn duplicate_crash() { + super::do_test(&::hex::decode("00").unwrap()); + } +} diff --git a/fuzz/fuzz_targets/msg_targets/utils.rs b/fuzz/fuzz_targets/msg_targets/utils.rs index 27de871dcb6..64bfd89326b 100644 --- a/fuzz/fuzz_targets/msg_targets/utils.rs +++ b/fuzz/fuzz_targets/msg_targets/utils.rs @@ -11,3 +11,28 @@ macro_rules! test_msg { } } } + +#[macro_export] +macro_rules! test_msg_exact { + ($MsgType: path, $data: ident) => { + { + if let Ok(msg) = <$MsgType as MsgDecodable>::decode($data){ + let enc = msg.encode(); + assert_eq!(&$data[..], &enc[..]); + } + } + } +} + +#[macro_export] +macro_rules! test_msg_hole { + ($MsgType: path, $data: ident, $hole: expr, $hole_len: expr) => { + { + if let Ok(msg) = <$MsgType as MsgDecodable>::decode($data){ + let enc = msg.encode(); + assert_eq!(&$data[..$hole], &enc[..$hole]); + assert_eq!(&$data[$hole + $hole_len..enc.len()], &enc[$hole + $hole_len..]); + } + } + } +} diff --git a/fuzz/fuzz_targets/router_target.rs b/fuzz/fuzz_targets/router_target.rs index 452369e5308..469db990ffc 100644 --- a/fuzz/fuzz_targets/router_target.rs +++ b/fuzz/fuzz_targets/router_target.rs @@ -75,6 +75,7 @@ pub fn do_test(data: &[u8]) { Ok(msg) => msg, Err(e) => match e { msgs::DecodeError::UnknownRealmByte => return, + msgs::DecodeError::UnknownRequiredFeature => return, msgs::DecodeError::BadPublicKey => return, msgs::DecodeError::BadSignature => return, msgs::DecodeError::BadText => return, diff --git a/src/ln/channel.rs b/src/ln/channel.rs index b59379aa74f..f890f039689 100644 --- a/src/ln/channel.rs +++ b/src/ln/channel.rs @@ -2303,6 +2303,7 @@ impl Channel { node_id_2: if were_node_one { self.get_their_node_id() } else { our_node_id }, bitcoin_key_1: if were_node_one { our_bitcoin_key } else { self.their_funding_pubkey.unwrap() }, bitcoin_key_2: if were_node_one { self.their_funding_pubkey.unwrap() } else { our_bitcoin_key }, + excess_data: Vec::new(), }; let msghash = Message::from_slice(&Sha256dHash::from_data(&msg.encode()[..])[..]).unwrap(); diff --git a/src/ln/channelmanager.rs b/src/ln/channelmanager.rs index 073ba792ac1..dc7ac2a8161 100644 --- a/src/ln/channelmanager.rs +++ b/src/ln/channelmanager.rs @@ -876,6 +876,7 @@ impl ChannelManager { htlc_minimum_msat: chan.get_our_htlc_minimum_msat(), fee_base_msat: chan.get_our_fee_base_msat(&*self.fee_estimator), fee_proportional_millionths: self.fee_proportional_millionths, + excess_data: Vec::new(), }; let msg_hash = Sha256dHash::from_data(&unsigned.encode()[..]); diff --git a/src/ln/msgs.rs b/src/ln/msgs.rs index f696b024fd9..034a580c018 100644 --- a/src/ln/msgs.rs +++ b/src/ln/msgs.rs @@ -28,6 +28,8 @@ pub trait MsgEncodable { pub enum DecodeError { /// Unknown realm byte in an OnionHopData packet UnknownRealmByte, + /// Unknown feature mandating we fail to parse message + UnknownRequiredFeature, /// Failed to decode a public key (ie it's invalid) BadPublicKey, /// Failed to decode a signature (ie it's invalid) @@ -337,6 +339,8 @@ pub struct UnsignedNodeAnnouncement { /// List of addresses on which this node is reachable. Note that you may only have up to one /// address of each type, if you have more, they may be silently discarded or we may panic! pub addresses: Vec, + pub excess_address_data: Vec, + pub excess_data: Vec, } pub struct NodeAnnouncement { pub signature: Signature, @@ -352,6 +356,7 @@ pub struct UnsignedChannelAnnouncement { pub node_id_2: PublicKey, pub bitcoin_key_1: PublicKey, pub bitcoin_key_2: PublicKey, + pub excess_data: Vec, } #[derive(PartialEq, Clone)] pub struct ChannelAnnouncement { @@ -372,6 +377,7 @@ pub struct UnsignedChannelUpdate { pub htlc_minimum_msat: u64, pub fee_base_msat: u32, pub fee_proportional_millionths: u32, + pub excess_data: Vec, } #[derive(PartialEq, Clone)] pub struct ChannelUpdate { @@ -456,11 +462,11 @@ pub trait ChannelMessageHandler : events::EventsProvider + Send + Sync { } pub trait RoutingMessageHandler : Send + Sync { - fn handle_node_announcement(&self, msg: &NodeAnnouncement) -> Result<(), HandleError>; + fn handle_node_announcement(&self, msg: &NodeAnnouncement) -> Result; /// Handle a channel_announcement message, returning true if it should be forwarded on, false /// or returning an Err otherwise. fn handle_channel_announcement(&self, msg: &ChannelAnnouncement) -> Result; - fn handle_channel_update(&self, msg: &ChannelUpdate) -> Result<(), HandleError>; + fn handle_channel_update(&self, msg: &ChannelUpdate) -> Result; fn handle_htlc_fail_channel_update(&self, update: &HTLCFailChannelUpdate); } @@ -506,6 +512,7 @@ impl Error for DecodeError { fn description(&self) -> &str { match *self { DecodeError::UnknownRealmByte => "Unknown realm byte in Onion packet", + DecodeError::UnknownRequiredFeature => "Unknown required feature preventing decode", DecodeError::BadPublicKey => "Invalid public key in packet", DecodeError::BadSignature => "Invalid signature in packet", DecodeError::BadText => "Invalid text in packet", @@ -1193,6 +1200,10 @@ impl MsgEncodable for AnnouncementSignatures { impl MsgDecodable for UnsignedNodeAnnouncement { fn decode(v: &[u8]) -> Result { let features = GlobalFeatures::decode(&v[..])?; + if features.requires_unknown_bits() { + return Err(DecodeError::UnknownRequiredFeature); + } + if v.len() < features.encoded_len() + 4 + 33 + 3 + 32 + 2 { return Err(DecodeError::ShortRead); } @@ -1215,7 +1226,6 @@ impl MsgDecodable for UnsignedNodeAnnouncement { loop { if addr_read_limit <= read_pos { break; } match v[read_pos] { - 0 => { read_pos += 1; }, 1 => { if addresses.len() > 0 { return Err(DecodeError::ExtraAddressesPerType); @@ -1282,6 +1292,15 @@ impl MsgDecodable for UnsignedNodeAnnouncement { } } + let excess_address_data = if read_pos < addr_read_limit { + let mut excess_address_data = Vec::with_capacity(addr_read_limit - read_pos); + excess_address_data.extend_from_slice(&v[read_pos..addr_read_limit]); + excess_address_data + } else { Vec::new() }; + + let mut excess_data = Vec::with_capacity(v.len() - addr_read_limit); + excess_data.extend_from_slice(&v[addr_read_limit..]); + let secp_ctx = Secp256k1::without_caps(); Ok(Self { features, @@ -1290,13 +1309,15 @@ impl MsgDecodable for UnsignedNodeAnnouncement { rgb, alias, addresses, + excess_address_data, + excess_data, }) } } impl MsgEncodable for UnsignedNodeAnnouncement { fn encode(&self) -> Vec { let features = self.features.encode(); - let mut res = Vec::with_capacity(74 + features.len() + self.addresses.len()); + let mut res = Vec::with_capacity(74 + features.len() + self.addresses.len()*7 + self.excess_address_data.len() + self.excess_data.len()); res.extend_from_slice(&features[..]); res.extend_from_slice(&byte_utils::be32_to_array(self.timestamp)); res.extend_from_slice(&self.node_id.serialize()); @@ -1332,8 +1353,10 @@ impl MsgEncodable for UnsignedNodeAnnouncement { }, } } - res.extend_from_slice(&byte_utils::be16_to_array(addr_slice.len() as u16)); + res.extend_from_slice(&byte_utils::be16_to_array((addr_slice.len() + self.excess_address_data.len()) as u16)); res.extend_from_slice(&addr_slice[..]); + res.extend_from_slice(&self.excess_address_data[..]); + res.extend_from_slice(&self.excess_data[..]); res } } @@ -1364,11 +1387,16 @@ impl MsgEncodable for NodeAnnouncement { impl MsgDecodable for UnsignedChannelAnnouncement { fn decode(v: &[u8]) -> Result { let features = GlobalFeatures::decode(&v[..])?; + if features.requires_unknown_bits() { + return Err(DecodeError::UnknownRequiredFeature); + } if v.len() < features.encoded_len() + 32 + 8 + 33*4 { return Err(DecodeError::ShortRead); } let start = features.encoded_len(); let secp_ctx = Secp256k1::without_caps(); + let mut excess_data = Vec::with_capacity(v.len() - start - 172); + excess_data.extend_from_slice(&v[start + 172..]); Ok(Self { features, chain_hash: deserialize(&v[start..start + 32]).unwrap(), @@ -1377,13 +1405,14 @@ impl MsgDecodable for UnsignedChannelAnnouncement { node_id_2: secp_pubkey!(&secp_ctx, &v[start + 73..start + 106]), bitcoin_key_1: secp_pubkey!(&secp_ctx, &v[start + 106..start + 139]), bitcoin_key_2: secp_pubkey!(&secp_ctx, &v[start + 139..start + 172]), + excess_data, }) } } impl MsgEncodable for UnsignedChannelAnnouncement { fn encode(&self) -> Vec { let features = self.features.encode(); - let mut res = Vec::with_capacity(172 + features.len()); + let mut res = Vec::with_capacity(172 + features.len() + self.excess_data.len()); res.extend_from_slice(&features[..]); res.extend_from_slice(&self.chain_hash[..]); res.extend_from_slice(&byte_utils::be64_to_array(self.short_channel_id)); @@ -1391,6 +1420,7 @@ impl MsgEncodable for UnsignedChannelAnnouncement { res.extend_from_slice(&self.node_id_2.serialize()); res.extend_from_slice(&self.bitcoin_key_1.serialize()); res.extend_from_slice(&self.bitcoin_key_2.serialize()); + res.extend_from_slice(&self.excess_data[..]); res } } @@ -1429,6 +1459,8 @@ impl MsgDecodable for UnsignedChannelUpdate { if v.len() < 32+8+4+2+2+8+4+4 { return Err(DecodeError::ShortRead); } + let mut excess_data = Vec::with_capacity(v.len() - 64); + excess_data.extend_from_slice(&v[64..]); Ok(Self { chain_hash: deserialize(&v[0..32]).unwrap(), short_channel_id: byte_utils::slice_to_be64(&v[32..40]), @@ -1438,12 +1470,13 @@ impl MsgDecodable for UnsignedChannelUpdate { htlc_minimum_msat: byte_utils::slice_to_be64(&v[48..56]), fee_base_msat: byte_utils::slice_to_be32(&v[56..60]), fee_proportional_millionths: byte_utils::slice_to_be32(&v[60..64]), + excess_data }) } } impl MsgEncodable for UnsignedChannelUpdate { fn encode(&self) -> Vec { - let mut res = Vec::with_capacity(64); + let mut res = Vec::with_capacity(64 + self.excess_data.len()); res.extend_from_slice(&self.chain_hash[..]); res.extend_from_slice(&byte_utils::be64_to_array(self.short_channel_id)); res.extend_from_slice(&byte_utils::be32_to_array(self.timestamp)); @@ -1452,6 +1485,7 @@ impl MsgEncodable for UnsignedChannelUpdate { res.extend_from_slice(&byte_utils::be64_to_array(self.htlc_minimum_msat)); res.extend_from_slice(&byte_utils::be32_to_array(self.fee_base_msat)); res.extend_from_slice(&byte_utils::be32_to_array(self.fee_proportional_millionths)); + res.extend_from_slice(&self.excess_data[..]); res } } @@ -1657,11 +1691,6 @@ mod tests { #[test] fn encoding_channel_reestablish_no_secret() { - let public_key = { - let secp_ctx = Secp256k1::new(); - PublicKey::from_secret_key(&secp_ctx, &SecretKey::from_slice(&secp_ctx, &hex::decode("0101010101010101010101010101010101010101010101010101010101010101").unwrap()[..]).unwrap()) - }; - let cr = msgs::ChannelReestablish { channel_id: [4, 0, 0, 0, 0, 0, 0, 0, 5, 0, 0, 0, 0, 0, 0, 0, 6, 0, 0, 0, 0, 0, 0, 0, 7, 0, 0, 0, 0, 0, 0, 0], next_local_commitment_number: 3, diff --git a/src/ln/peer_handler.rs b/src/ln/peer_handler.rs index 9315f56902d..3122e572e57 100644 --- a/src/ln/peer_handler.rs +++ b/src/ln/peer_handler.rs @@ -331,21 +331,23 @@ impl PeerManager { ($thing: expr) => { match $thing { Ok(x) => x, - Err(_e) => { - //TODO: Handle e? - return Err(PeerHandleError{ no_connection_possible: false }); - } - }; - } - } - - macro_rules! try_ignore_potential_decodeerror { - ($thing: expr) => { - match $thing { - Ok(x) => x, - Err(_e) => { - log_debug!(self, "Error decoding message, ignoring due to lnd spec incompatibility. See https://github.com/lightningnetwork/lnd/issues/1407"); - continue; + Err(e) => { + match e { + msgs::DecodeError::UnknownRealmByte => return Err(PeerHandleError{ no_connection_possible: false }), + msgs::DecodeError::UnknownRequiredFeature => { + log_debug!(self, "Got a channel/node announcement with an known required feature flag, you may want to udpate!"); + continue; + }, + msgs::DecodeError::BadPublicKey => return Err(PeerHandleError{ no_connection_possible: false }), + msgs::DecodeError::BadSignature => return Err(PeerHandleError{ no_connection_possible: false }), + msgs::DecodeError::BadText => return Err(PeerHandleError{ no_connection_possible: false }), + msgs::DecodeError::ShortRead => return Err(PeerHandleError{ no_connection_possible: false }), + msgs::DecodeError::ExtraAddressesPerType => { + log_debug!(self, "Error decoding message, ignoring due to lnd spec incompatibility. See https://github.com/lightningnetwork/lnd/issues/1407"); + continue; + }, + msgs::DecodeError::BadLengthDescriptor => return Err(PeerHandleError{ no_connection_possible: false }), + } } }; } @@ -576,12 +578,20 @@ impl PeerManager { } }, 257 => { - let msg = try_ignore_potential_decodeerror!(msgs::NodeAnnouncement::decode(&msg_data[2..])); - try_potential_handleerror!(self.message_handler.route_handler.handle_node_announcement(&msg)); + let msg = try_potential_decodeerror!(msgs::NodeAnnouncement::decode(&msg_data[2..])); + let should_forward = try_potential_handleerror!(self.message_handler.route_handler.handle_node_announcement(&msg)); + + if should_forward { + // TODO: forward msg along to all our other peers! + } }, 258 => { let msg = try_potential_decodeerror!(msgs::ChannelUpdate::decode(&msg_data[2..])); - try_potential_handleerror!(self.message_handler.route_handler.handle_channel_update(&msg)); + let should_forward = try_potential_handleerror!(self.message_handler.route_handler.handle_channel_update(&msg)); + + if should_forward { + // TODO: forward msg along to all our other peers! + } }, _ => { if (msg_type & 1) == 0 { diff --git a/src/ln/router.rs b/src/ln/router.rs index 1ca55fa2600..a71eb913ce5 100644 --- a/src/ln/router.rs +++ b/src/ln/router.rs @@ -168,10 +168,14 @@ macro_rules! secp_verify_sig { } impl RoutingMessageHandler for Router { - fn handle_node_announcement(&self, msg: &msgs::NodeAnnouncement) -> Result<(), HandleError> { + fn handle_node_announcement(&self, msg: &msgs::NodeAnnouncement) -> Result { let msg_hash = Message::from_slice(&Sha256dHash::from_data(&msg.contents.encode()[..])[..]).unwrap(); secp_verify_sig!(self.secp_ctx, &msg_hash, &msg.signature, &msg.contents.node_id); + if msg.contents.features.requires_unknown_bits() { + panic!("Unknown-required-features NodeAnnouncements should never deserialize!"); + } + let mut network = self.network_map.write().unwrap(); match network.nodes.get_mut(&msg.contents.node_id) { None => Err(HandleError{err: "No existing channels for node_announcement", action: Some(ErrorAction::IgnoreError)}), @@ -185,7 +189,7 @@ impl RoutingMessageHandler for Router { node.rgb = msg.contents.rgb; node.alias = msg.contents.alias; node.addresses = msg.contents.addresses.clone(); - Ok(()) + Ok(msg.contents.excess_data.is_empty() && msg.contents.excess_address_data.is_empty() && !msg.contents.features.supports_unknown_bits()) } } } @@ -201,7 +205,7 @@ impl RoutingMessageHandler for Router { //TODO: Only allow bitcoin chain_hash if msg.contents.features.requires_unknown_bits() { - return Err(HandleError{err: "Channel announcement required unknown feature flags", action: None}); + panic!("Unknown-required-features ChannelAnnouncements should never deserialize!"); } let mut network = self.network_map.write().unwrap(); @@ -263,7 +267,7 @@ impl RoutingMessageHandler for Router { add_channel_to_node!(msg.contents.node_id_1); add_channel_to_node!(msg.contents.node_id_2); - Ok(!msg.contents.features.supports_unknown_bits()) + Ok(msg.contents.excess_data.is_empty() && !msg.contents.features.supports_unknown_bits()) } fn handle_htlc_fail_channel_update(&self, update: &msgs::HTLCFailChannelUpdate) { @@ -285,7 +289,7 @@ impl RoutingMessageHandler for Router { } } - fn handle_channel_update(&self, msg: &msgs::ChannelUpdate) -> Result<(), HandleError> { + fn handle_channel_update(&self, msg: &msgs::ChannelUpdate) -> Result { let mut network = self.network_map.write().unwrap(); let dest_node_id; let chan_enabled = msg.contents.flags & (1 << 1) != (1 << 1); @@ -351,7 +355,7 @@ impl RoutingMessageHandler for Router { mut_node.lowest_inbound_channel_fee_proportional_millionths = lowest_inbound_channel_fee_proportional_millionths; } - Ok(()) + Ok(msg.contents.excess_data.is_empty()) } } diff --git a/src/util/test_utils.rs b/src/util/test_utils.rs index dc158230fad..52e44f92abc 100644 --- a/src/util/test_utils.rs +++ b/src/util/test_utils.rs @@ -136,13 +136,13 @@ impl TestRoutingMessageHandler { } impl msgs::RoutingMessageHandler for TestRoutingMessageHandler { - fn handle_node_announcement(&self, _msg: &msgs::NodeAnnouncement) -> Result<(), HandleError> { + fn handle_node_announcement(&self, _msg: &msgs::NodeAnnouncement) -> Result { Err(HandleError { err: "", action: None }) } fn handle_channel_announcement(&self, _msg: &msgs::ChannelAnnouncement) -> Result { Err(HandleError { err: "", action: None }) } - fn handle_channel_update(&self, _msg: &msgs::ChannelUpdate) -> Result<(), HandleError> { + fn handle_channel_update(&self, _msg: &msgs::ChannelUpdate) -> Result { Err(HandleError { err: "", action: None }) } fn handle_htlc_fail_channel_update(&self, _update: &msgs::HTLCFailChannelUpdate) {}