From e446047c666643148c50862a91a44fdba38a96cf Mon Sep 17 00:00:00 2001 From: FujiApple Date: Sat, 3 Feb 2024 18:49:11 +0800 Subject: [PATCH] fix(net): tracer panic for ICMP TimeExceeded packets with code 1 (#979) --- src/tracing/net/ipv4.rs | 75 ++++++++++++++++++++++++------- src/tracing/net/ipv6.rs | 86 +++++++++++++++++++++++++++++------- src/tracing/packet/icmpv4.rs | 21 +++++++++ src/tracing/packet/icmpv6.rs | 21 +++++++++ 4 files changed, 169 insertions(+), 34 deletions(-) diff --git a/src/tracing/net/ipv4.rs b/src/tracing/net/ipv4.rs index 8b1fa9d2..96f75c2e 100644 --- a/src/tracing/net/ipv4.rs +++ b/src/tracing/net/ipv4.rs @@ -9,7 +9,7 @@ use crate::tracing::packet::icmpv4::destination_unreachable::DestinationUnreacha use crate::tracing::packet::icmpv4::echo_reply::EchoReplyPacket; use crate::tracing::packet::icmpv4::echo_request::EchoRequestPacket; use crate::tracing::packet::icmpv4::time_exceeded::TimeExceededPacket; -use crate::tracing::packet::icmpv4::{IcmpCode, IcmpPacket, IcmpType}; +use crate::tracing::packet::icmpv4::{IcmpCode, IcmpPacket, IcmpTimeExceededCode, IcmpType}; use crate::tracing::packet::ipv4::Ipv4Packet; use crate::tracing::packet::tcp::TcpPacket; use crate::tracing::packet::udp::UdpPacket; @@ -352,23 +352,32 @@ fn extract_probe_resp( let recv = SystemTime::now(); let src = IpAddr::V4(ipv4.get_source()); let icmp_v4 = IcmpPacket::new_view(ipv4.payload())?; - Ok(match icmp_v4.get_icmp_type() { + let icmp_type = icmp_v4.get_icmp_type(); + let icmp_code = icmp_v4.get_icmp_code(); + Ok(match icmp_type { IcmpType::TimeExceeded => { - let packet = TimeExceededPacket::new_view(icmp_v4.packet())?; - let (nested_ipv4, extension) = match icmp_extension_mode { - IcmpExtensionParseMode::Enabled => { - let ipv4 = Ipv4Packet::new_view(packet.payload())?; - let ext = packet.extension().map(Extensions::try_from).transpose()?; - (ipv4, ext) - } - IcmpExtensionParseMode::Disabled => { - let ipv4 = Ipv4Packet::new_view(packet.payload_raw())?; - (ipv4, None) - } - }; - extract_probe_resp_seq(&nested_ipv4, protocol)?.map(|resp_seq| { - ProbeResponse::TimeExceeded(ProbeResponseData::new(recv, src, resp_seq), extension) - }) + if IcmpTimeExceededCode::from(icmp_code) == IcmpTimeExceededCode::TtlExpired { + let packet = TimeExceededPacket::new_view(icmp_v4.packet())?; + let (nested_ipv4, extension) = match icmp_extension_mode { + IcmpExtensionParseMode::Enabled => { + let ipv4 = Ipv4Packet::new_view(packet.payload())?; + let ext = packet.extension().map(Extensions::try_from).transpose()?; + (ipv4, ext) + } + IcmpExtensionParseMode::Disabled => { + let ipv4 = Ipv4Packet::new_view(packet.payload_raw())?; + (ipv4, None) + } + }; + extract_probe_resp_seq(&nested_ipv4, protocol)?.map(|resp_seq| { + ProbeResponse::TimeExceeded( + ProbeResponseData::new(recv, src, resp_seq), + extension, + ) + }) + } else { + None + } } IcmpType::DestinationUnreachable => { let packet = DestinationUnreachablePacket::new_view(icmp_v4.packet())?; @@ -473,3 +482,35 @@ fn extract_tcp_packet(ipv4: &Ipv4Packet<'_>) -> TraceResult<(u16, u16)> { Ok((tcp_packet.get_source(), tcp_packet.get_destination())) } } + +#[cfg(test)] +mod tests { + use super::*; + use crate::tracing::error::IoResult; + use crate::tracing::net::socket::MockSocket; + use crate::mocket_read; + + // This IPv4/ICMP TimeExceeded packet has code 1 ("Fragment reassembly + // time exceeded") and must be ignored. + // + // Note this is not real packet and so the length and checksum are not + // accurate. + #[test] + fn test_icmp_time_exceeded_fragment_reassembly_ignored() -> anyhow::Result<()> { + let expected_read_buf = hex_literal::hex!( + " + 45 20 2c 02 e4 5c 00 00 72 01 2e 04 67 4b 0b 34 + c0 a8 01 15 0b 01 1c 38 00 00 00 00 45 00 8c 05 + 85 4e 20 00 30 11 ab d6 c0 a8 01 15 67 4b 0b 34 + " + ); + let mut mocket = MockSocket::new(); + mocket + .expect_read() + .times(1) + .returning(mocket_read!(expected_read_buf)); + let resp = recv_icmp_probe(&mut mocket, Protocol::Udp, IcmpExtensionParseMode::Enabled)?; + assert!(resp.is_none()); + Ok(()) + } +} diff --git a/src/tracing/net/ipv6.rs b/src/tracing/net/ipv6.rs index 099b8428..7f01364f 100644 --- a/src/tracing/net/ipv6.rs +++ b/src/tracing/net/ipv6.rs @@ -9,7 +9,7 @@ use crate::tracing::packet::icmpv6::destination_unreachable::DestinationUnreacha use crate::tracing::packet::icmpv6::echo_reply::EchoReplyPacket; use crate::tracing::packet::icmpv6::echo_request::EchoRequestPacket; use crate::tracing::packet::icmpv6::time_exceeded::TimeExceededPacket; -use crate::tracing::packet::icmpv6::{IcmpCode, IcmpPacket, IcmpType}; +use crate::tracing::packet::icmpv6::{IcmpCode, IcmpPacket, IcmpTimeExceededCode, IcmpType}; use crate::tracing::packet::ipv6::Ipv6Packet; use crate::tracing::packet::tcp::TcpPacket; use crate::tracing::packet::udp::UdpPacket; @@ -302,23 +302,32 @@ fn extract_probe_resp( ) -> TraceResult> { let recv = SystemTime::now(); let ip = IpAddr::V6(src); - Ok(match icmp_v6.get_icmp_type() { + let icmp_type = icmp_v6.get_icmp_type(); + let icmp_code = icmp_v6.get_icmp_code(); + Ok(match icmp_type { IcmpType::TimeExceeded => { - let packet = TimeExceededPacket::new_view(icmp_v6.packet())?; - let (nested_ipv6, extension) = match icmp_extension_mode { - IcmpExtensionParseMode::Enabled => { - let ipv6 = Ipv6Packet::new_view(packet.payload())?; - let ext = packet.extension().map(Extensions::try_from).transpose()?; - (ipv6, ext) - } - IcmpExtensionParseMode::Disabled => { - let ipv6 = Ipv6Packet::new_view(packet.payload_raw())?; - (ipv6, None) - } - }; - extract_probe_resp_seq(&nested_ipv6, protocol)?.map(|resp_seq| { - ProbeResponse::TimeExceeded(ProbeResponseData::new(recv, ip, resp_seq), extension) - }) + if IcmpTimeExceededCode::from(icmp_code) == IcmpTimeExceededCode::TtlExpired { + let packet = TimeExceededPacket::new_view(icmp_v6.packet())?; + let (nested_ipv6, extension) = match icmp_extension_mode { + IcmpExtensionParseMode::Enabled => { + let ipv6 = Ipv6Packet::new_view(packet.payload())?; + let ext = packet.extension().map(Extensions::try_from).transpose()?; + (ipv6, ext) + } + IcmpExtensionParseMode::Disabled => { + let ipv6 = Ipv6Packet::new_view(packet.payload_raw())?; + (ipv6, None) + } + }; + extract_probe_resp_seq(&nested_ipv6, protocol)?.map(|resp_seq| { + ProbeResponse::TimeExceeded( + ProbeResponseData::new(recv, ip, resp_seq), + extension, + ) + }) + } else { + None + } } IcmpType::DestinationUnreachable => { let packet = DestinationUnreachablePacket::new_view(icmp_v6.packet())?; @@ -419,3 +428,46 @@ fn extract_tcp_packet(ipv6: &Ipv6Packet<'_>) -> TraceResult<(u16, u16)> { let tcp_packet = TcpPacket::new_view(ipv6.payload())?; Ok((tcp_packet.get_source(), tcp_packet.get_destination())) } + +#[cfg(test)] +mod tests { + use super::*; + use crate::mocket_recv_from; + use crate::tracing::error::IoResult; + use crate::tracing::net::socket::MockSocket; + use std::str::FromStr; + + // This ICMPv6 packet has code 1 ("Fragment reassembly time exceeded") + // and must be ignored. + // + // Note this is not real packet and so the length and checksum are not + // accurate. + #[test] + fn test_icmp_time_exceeded_fragment_reassembly_ignored() -> anyhow::Result<()> { + let expected_recv_from_buf = hex_literal::hex!( + " + 03 01 da 90 00 00 00 00 60 0f 02 00 00 2c 11 01 + fd 7a 11 5c a1 e0 ab 12 48 43 cd 96 62 63 08 2a + 2a 00 14 50 40 09 08 15 00 00 00 00 00 00 20 0e + 95 ce 81 24 00 2c 65 f5 00 00 00 00 00 00 00 00 + 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 + 00 00 00 00 00 00 00 00 00 00 00 00 + " + ); + let expected_recv_from_addr = SocketAddr::new( + IpAddr::V6(Ipv6Addr::from_str("2604:a880:ffff:6:1::41c").unwrap()), + 0, + ); + let mut mocket = MockSocket::new(); + mocket + .expect_recv_from() + .times(1) + .returning(mocket_recv_from!( + expected_recv_from_buf, + expected_recv_from_addr + )); + let resp = recv_icmp_probe(&mut mocket, Protocol::Udp, IcmpExtensionParseMode::Enabled)?; + assert!(resp.is_none()); + Ok(()) + } +} diff --git a/src/tracing/packet/icmpv4.rs b/src/tracing/packet/icmpv4.rs index 1efe94be..fe682b6d 100644 --- a/src/tracing/packet/icmpv4.rs +++ b/src/tracing/packet/icmpv4.rs @@ -47,6 +47,27 @@ impl From for IcmpCode { } } +/// The code for `TimeExceeded` ICMP packet type. +#[derive(Debug, Copy, Clone, Ord, PartialOrd, Eq, PartialEq)] +pub enum IcmpTimeExceededCode { + /// TTL expired in transit. + TtlExpired, + /// Fragment reassembly time exceeded. + FragmentReassembly, + /// An unknown code. + Unknown(u8), +} + +impl From for IcmpTimeExceededCode { + fn from(val: IcmpCode) -> Self { + match val { + IcmpCode(0) => Self::TtlExpired, + IcmpCode(1) => Self::FragmentReassembly, + IcmpCode(id) => Self::Unknown(id), + } + } +} + const TYPE_OFFSET: usize = 0; const CODE_OFFSET: usize = 1; const CHECKSUM_OFFSET: usize = 2; diff --git a/src/tracing/packet/icmpv6.rs b/src/tracing/packet/icmpv6.rs index 3e169f79..fa720f7f 100644 --- a/src/tracing/packet/icmpv6.rs +++ b/src/tracing/packet/icmpv6.rs @@ -47,6 +47,27 @@ impl From for IcmpCode { } } +/// The code for `TimeExceeded` `ICMPv6` packet type. +#[derive(Debug, Copy, Clone, Ord, PartialOrd, Eq, PartialEq)] +pub enum IcmpTimeExceededCode { + /// Hop limit exceeded in transit. + TtlExpired, + /// Fragment reassembly time exceeded. + FragmentReassembly, + /// An unknown code. + Unknown(u8), +} + +impl From for IcmpTimeExceededCode { + fn from(val: IcmpCode) -> Self { + match val { + IcmpCode(0) => Self::TtlExpired, + IcmpCode(1) => Self::FragmentReassembly, + IcmpCode(id) => Self::Unknown(id), + } + } +} + const TYPE_OFFSET: usize = 0; const CODE_OFFSET: usize = 1; const CHECKSUM_OFFSET: usize = 2;