fix(net): tracer panic for ICMP TimeExceeded packets with code 1 (#979)

fujiapple852 · Feb 4, 2024 · b98dd10 · b98dd10
1 parent a642d25
commit b98dd10
Show file tree

Hide file tree

Showing 4 changed files with 122 additions and 34 deletions.
diff --git 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,31 @@ 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 mut mocket = MockSocket::new();
+        mocket.expect_read().times(1).returning(mocket_read!(
+            "
+           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 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
@@ -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<Option<ProbeResponse>> {
     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())?;

diff --git a/src/tracing/packet/icmpv4.rs b/src/tracing/packet/icmpv4.rs
@@ -47,6 +47,27 @@ impl From<u8> 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<IcmpCode> 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
@@ -47,6 +47,27 @@ impl From<u8> 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<IcmpCode> 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;