From c43e535bc03404bad16e3d30e5b2fc7215e6ca15 Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Wed, 19 Sep 2018 13:06:35 -0400 Subject: [PATCH 1/1] Simplify DecodeError enum by removing some useless distinctions --- fuzz/fuzz_targets/router_target.rs | 7 ++----- src/ln/channelmanager.rs | 2 +- src/ln/msgs.rs | 27 ++++++++++----------------- src/ln/peer_handler.rs | 7 ++----- src/util/ser.rs | 4 ++-- 5 files changed, 17 insertions(+), 30 deletions(-) diff --git a/fuzz/fuzz_targets/router_target.rs b/fuzz/fuzz_targets/router_target.rs index 4ccd32746..52f9a235f 100644 --- a/fuzz/fuzz_targets/router_target.rs +++ b/fuzz/fuzz_targets/router_target.rs @@ -125,15 +125,12 @@ pub fn do_test(data: &[u8]) { match <($MsgType)>::read(&mut reader) { Ok(msg) => msg, Err(e) => match e { - msgs::DecodeError::UnknownRealmByte => return, + msgs::DecodeError::UnknownVersion => return, msgs::DecodeError::UnknownRequiredFeature => return, - msgs::DecodeError::BadPublicKey => return, - msgs::DecodeError::BadSignature => return, - msgs::DecodeError::BadText => return, + msgs::DecodeError::InvalidValue => return, msgs::DecodeError::ExtraAddressesPerType => return, msgs::DecodeError::BadLengthDescriptor => return, msgs::DecodeError::ShortRead => panic!("We picked the length..."), - msgs::DecodeError::InvalidValue => panic!("Should not happen with p2p message decoding"), msgs::DecodeError::Io(e) => panic!(format!("{}", e)), } } diff --git a/src/ln/channelmanager.rs b/src/ln/channelmanager.rs index 9b3f7713c..01faec671 100644 --- a/src/ln/channelmanager.rs +++ b/src/ln/channelmanager.rs @@ -806,7 +806,7 @@ impl ChannelManager { match msgs::OnionHopData::read(&mut Cursor::new(&decoded[..])) { Err(err) => { let error_code = match err { - msgs::DecodeError::UnknownRealmByte => 0x4000 | 1, + msgs::DecodeError::UnknownVersion => 0x4000 | 1, // unknown realm byte _ => 0x2000 | 2, // Should never happen }; return_err!("Unable to decode our hop data", error_code, &[0;0]); diff --git a/src/ln/msgs.rs b/src/ln/msgs.rs index 5dd3165bf..f87e3722f 100644 --- a/src/ln/msgs.rs +++ b/src/ln/msgs.rs @@ -30,16 +30,14 @@ use util::ser::{Readable, Writeable, Writer}; /// An error in decoding a message or struct. #[derive(Debug)] pub enum DecodeError { - /// Unknown realm byte in an OnionHopData packet - UnknownRealmByte, + /// A version byte specified something we don't know how to handle. + /// Includes unknown realm byte in an OnionHopData packet + UnknownVersion, /// 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) - BadSignature, - /// Value expected to be text wasn't decodable as text - BadText, + /// Value was invalid, eg a byte which was supposed to be a bool was something other than a 0 + /// or 1, a public key/private key/signature was invalid, text wasn't UTF-8, etc + InvalidValue, /// Buffer too short ShortRead, /// node_announcement included more than one address of a given type! @@ -49,8 +47,6 @@ pub enum DecodeError { BadLengthDescriptor, /// Error from std::io Io(::std::io::Error), - /// 1 or 0 is not found for boolean value - InvalidValue, } /// Tracks localfeatures which are only in init messages @@ -614,16 +610,13 @@ pub(crate) struct OnionErrorPacket { impl Error for DecodeError { fn description(&self) -> &str { match *self { - DecodeError::UnknownRealmByte => "Unknown realm byte in Onion packet", + DecodeError::UnknownVersion => "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", + DecodeError::InvalidValue => "Nonsense bytes didn't map to the type they were interpreted as", DecodeError::ShortRead => "Packet extended beyond the provided bytes", DecodeError::ExtraAddressesPerType => "More than one address of a single type", DecodeError::BadLengthDescriptor => "A length descriptor in the packet didn't describe the later data correctly", DecodeError::Io(ref e) => e.description(), - DecodeError::InvalidValue => "0 or 1 is not found for boolean", } } } @@ -919,7 +912,7 @@ impl Readable for OnionHopData { realm: { let r: u8 = Readable::read(r)?; if r != 0 { - return Err(DecodeError::UnknownRealmByte); + return Err(DecodeError::UnknownVersion); } r }, @@ -1087,7 +1080,7 @@ impl Readable for ErrorMessage { sz = cmp::min(data_len, sz); match String::from_utf8(data[..sz as usize].to_vec()) { Ok(s) => s, - Err(_) => return Err(DecodeError::BadText), + Err(_) => return Err(DecodeError::InvalidValue), } } }) diff --git a/src/ln/peer_handler.rs b/src/ln/peer_handler.rs index 6c2b4d798..797c55191 100644 --- a/src/ln/peer_handler.rs +++ b/src/ln/peer_handler.rs @@ -374,14 +374,12 @@ impl PeerManager { Ok(x) => x, Err(e) => { match e { - msgs::DecodeError::UnknownRealmByte => return Err(PeerHandleError{ no_connection_possible: false }), + msgs::DecodeError::UnknownVersion => 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::InvalidValue => 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"); @@ -389,7 +387,6 @@ impl PeerManager { }, msgs::DecodeError::BadLengthDescriptor => return Err(PeerHandleError{ no_connection_possible: false }), msgs::DecodeError::Io(_) => return Err(PeerHandleError{ no_connection_possible: false }), - msgs::DecodeError::InvalidValue => panic!("should not happen with message decoding"), } } }; diff --git a/src/util/ser.rs b/src/util/ser.rs index 8c713dfee..92cc66b81 100644 --- a/src/util/ser.rs +++ b/src/util/ser.rs @@ -295,7 +295,7 @@ impl Readable for PublicKey { let buf: [u8; 33] = Readable::read(r)?; match PublicKey::from_slice(&Secp256k1::without_caps(), &buf) { Ok(key) => Ok(key), - Err(_) => return Err(DecodeError::BadPublicKey), + Err(_) => return Err(DecodeError::InvalidValue), } } } @@ -324,7 +324,7 @@ impl Readable for Signature { let buf: [u8; 64] = Readable::read(r)?; match Signature::from_compact(&Secp256k1::without_caps(), &buf) { Ok(sig) => Ok(sig), - Err(_) => return Err(DecodeError::BadSignature), + Err(_) => return Err(DecodeError::InvalidValue), } } } -- 2.39.5