Implement ErrorMessage msg and ErrorAction::SendErrorMessage + fuzz test

Author: Antoine Riard

fuzz/fuzz_targets/channel_target.rs

@@ -118,6 +118,7 @@ pub fn do_test(data: &[u8]) {
 					msgs::DecodeError::UnknownRealmByte => return,
 					msgs::DecodeError::BadPublicKey => return,
 					msgs::DecodeError::BadSignature => return,
+					msgs::DecodeError::BadText => return,
 					msgs::DecodeError::ExtraAddressesPerType => return,
 					msgs::DecodeError::WrongLength => panic!("We picked the length..."),
 				}
@@ -138,6 +139,7 @@ pub fn do_test(data: &[u8]) {
 						msgs::DecodeError::UnknownRealmByte => return,
 						msgs::DecodeError::BadPublicKey => return,
 						msgs::DecodeError::BadSignature => return,
+						msgs::DecodeError::BadText => return,
 						msgs::DecodeError::ExtraAddressesPerType => return,
 						msgs::DecodeError::WrongLength => panic!("We picked the length..."),
 					}

fuzz/fuzz_targets/msg_targets/gen_target.sh

@@ -1,4 +1,4 @@
-for target in CommitmentSigned FundingCreated FundingLocked FundingSigned OpenChannel RevokeAndACK Shutdown UpdateAddHTLC UpdateFailHTLC UpdateFailMalformedHTLC UpdateFee UpdateFulfillHTLC AcceptChannel ClosingSigned ChannelReestablish; do
+for target in CommitmentSigned FundingCreated FundingLocked FundingSigned OpenChannel RevokeAndACK Shutdown UpdateAddHTLC UpdateFailHTLC UpdateFailMalformedHTLC UpdateFee UpdateFulfillHTLC AcceptChannel ClosingSigned ChannelReestablish ErrorMessage; do
 	tn=$(echo $target | 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

fuzz/fuzz_targets/msg_targets/msg_error_message_target.rs

@@ -0,0 +1,48 @@
+// 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::ErrorMessage, data);
+}
+
+#[cfg(feature = "afl")]
+extern crate afl;
+#[cfg(feature = "afl")]
+fn main() {
+	afl::read_stdio_bytes(|data| {
+		do_test(&data);
+	});
+}
+
+#[cfg(feature = "honggfuzz")]
+#[macro_use] extern crate honggfuzz;
+#[cfg(feature = "honggfuzz")]
+fn main() {
+	loop {
+		fuzz!(|data| {
+			do_test(data);
+		});
+	}
+}
+
+#[cfg(test)]
+mod tests {
+	use utils::extend_vec_from_hex;
+	#[test]
+	fn duplicate_crash() {
+		let mut a = Vec::new();
+		extend_vec_from_hex("00", &mut a);
+		super::do_test(&a);
+	}
+}

src/ln/msgs.rs

@@ -31,6 +31,8 @@ pub enum DecodeError {
 	BadPublicKey,
 	/// Failed to decode a signature (ie it's invalid)
 	BadSignature,
+	/// Value expected to be text wasn't decodable as text
+	BadText,
 	/// Buffer not of right length (either too short or too long)
 	WrongLength,
 	/// node_announcement included more than one address of a given type!
@@ -138,6 +140,11 @@ pub struct Init {
 	pub local_features: LocalFeatures,
 }
 
+pub struct ErrorMessage {
+	pub channel_id: [u8; 32],
+	pub data: String,
+}
+
 pub struct Ping {
 	pub ponglen: u16,
 	pub byteslen: u16,
@@ -375,6 +382,10 @@ pub enum ErrorAction {
 	DisconnectPeer,
 	/// The peer did something harmless that we weren't able to process, just log and ignore
 	IgnoreError,
+	/// The peer did something incorrect. Tell them.
+	SendErrorMessage {
+		msg: ErrorMessage
+	},
 }
 
 pub struct HandleError { //TODO: rename me
@@ -486,6 +497,7 @@ impl Error for DecodeError {
 			DecodeError::UnknownRealmByte => "Unknown realm byte in Onion packet",
 			DecodeError::BadPublicKey => "Invalid public key in packet",
 			DecodeError::BadSignature => "Invalid signature in packet",
+			DecodeError::BadText => "Invalid text in packet",
 			DecodeError::WrongLength => "Data was wrong length for packet",
 			DecodeError::ExtraAddressesPerType => "More than one address of a single type",
 		}
@@ -1600,6 +1612,37 @@ impl MsgEncodable for OnionErrorPacket {
 	}
 }
 
+impl MsgEncodable for ErrorMessage {
+	fn encode(&self) -> Vec<u8> {
+		let mut res = Vec::with_capacity(34 + self.data.len());
+		res.extend_from_slice(&self.channel_id);
+		res.extend_from_slice(&byte_utils::be16_to_array(self.data.len() as u16));
+		res.extend_from_slice(&self.data.as_bytes());
+		res
+	}
+}
+impl MsgDecodable for ErrorMessage {
+	fn decode(v: &[u8]) -> Result<Self,DecodeError> {
+		if v.len() < 34 {
+			return Err(DecodeError::WrongLength);
+		}
+		let len = byte_utils::slice_to_be16(&v[32..34]);
+		if v.len() < 34 + len as usize {
+			return Err(DecodeError::WrongLength);
+		}
+		let data = match String::from_utf8(v[34..34 + len as usize].to_vec()) {
+			Ok(s) => s,
+			Err(_) => return Err(DecodeError::BadText),
+		};
+		let mut channel_id = [0; 32];
+		channel_id[..].copy_from_slice(&v[0..32]);
+		Ok(Self {
+			channel_id,
+			data,
+		})
+	}
+}
+
 #[cfg(test)]
 mod tests {
 	use bitcoin::util::misc::hex_bytes;

src/ln/peer_handler.rs

@@ -302,6 +302,10 @@ impl<Descriptor: SocketDescriptor> PeerManager<Descriptor> {
 													msgs::ErrorAction::IgnoreError => {
 														continue;
 													},
+													msgs::ErrorAction::SendErrorMessage { msg } => {
+														encode_and_send_msg!(msg, 17);
+														continue;
+													},
 												}
 											} else {
 												return Err(PeerHandleError{ no_connection_possible: false });