chore(connlib): better error reporting for invalid IP packets (#7320)

Currently, we don't report very detailed errors when we fail to parse
certain IP packets. With this patch, we use `Result` in more places and
also extend the validation of IP packets to:

a) enforce a length of at most 1280 bytes. This should already be the
case due to our MTU but bad things may happen if that is off for some
reason
b) validate the entire IP packet instead of just its header

rust/connlib/tunnel/src/device_channel.rs

@@ -47,10 +47,10 @@ impl Device {
             )));
         }
 
-        let packet = IpPacket::new(ip_packet, n).ok_or_else(|| {
+        let packet = IpPacket::new(ip_packet, n).map_err(|e| {
             io::Error::new(
                 io::ErrorKind::InvalidInput,
-                "received bytes are not an IP packet",
+                format!("Failed to parse IP packet: {e:#}"),
             )
         })?;
 

rust/dns-over-tcp/src/stub_device.rs

@@ -1,5 +1,6 @@
 use std::collections::VecDeque;
 
+use firezone_logging::anyhow_dyn_err;
 use ip_packet::{IpPacket, IpPacketBuf};
 
 /// A in-memory device for [`smoltcp`] that is entirely backed by buffers.
@@ -73,10 +74,14 @@ impl<'a> smoltcp::phy::TxToken for SmolTxToken<'a> {
         let mut ip_packet_buf = IpPacketBuf::new();
         let result = f(ip_packet_buf.buf());
 
-        let Some(mut ip_packet) = IpPacket::new(ip_packet_buf, len) else {
-            tracing::warn!("Received invalid IP packet");
-            return result;
+        let mut ip_packet = match IpPacket::new(ip_packet_buf, len) {
+            Ok(p) => p,
+            Err(e) => {
+                tracing::warn!(error = anyhow_dyn_err(&e), "Received invalid IP packet");
+                return result;
+            }
         };
+
         ip_packet.update_checksum();
         self.outbound_packets.push_back(ip_packet);
 

rust/ip-packet/src/lib.rs

@@ -21,7 +21,7 @@ pub use fz_p2p_control_slice::FzP2pControlSlice;
 #[cfg(all(test, feature = "proptest"))]
 mod proptests;
 
-use anyhow::{Context as _, Result};
+use anyhow::{bail, Context as _, Result};
 use icmpv4_header_slice_mut::Icmpv4HeaderSliceMut;
 use icmpv6_header_slice_mut::Icmpv6EchoHeaderSliceMut;
 use ipv4_header_slice_mut::Ipv4HeaderSliceMut;
@@ -162,15 +162,15 @@ pub struct ConvertibleIpv4Packet {
 }
 
 impl ConvertibleIpv4Packet {
-    pub fn new(ip: IpPacketBuf, len: usize) -> Option<ConvertibleIpv4Packet> {
+    pub fn new(ip: IpPacketBuf, len: usize) -> Result<ConvertibleIpv4Packet> {
         let this = Self {
             buf: ip.inner,
             start: NAT46_OVERHEAD,
             len,
         };
-        Ipv4HeaderSlice::from_slice(this.packet()).ok()?;
+        Ipv4Slice::from_slice(this.packet()).context("Invalid IPv4 packet")?;
 
-        Some(this)
+        Ok(this)
     }
 
     fn ip_header(&self) -> Ipv4HeaderSlice {
@@ -242,16 +242,16 @@ pub struct ConvertibleIpv6Packet {
 }
 
 impl ConvertibleIpv6Packet {
-    pub fn new(ip: IpPacketBuf, len: usize) -> Option<ConvertibleIpv6Packet> {
+    pub fn new(ip: IpPacketBuf, len: usize) -> Result<ConvertibleIpv6Packet> {
         let this = Self {
             buf: ip.inner,
             start: NAT46_OVERHEAD,
             len,
         };
 
-        Ipv6HeaderSlice::from_slice(this.packet()).ok()?;
+        Ipv6Slice::from_slice(this.packet()).context("Invalid IPv6 packet")?;
 
-        Some(this)
+        Ok(this)
     }
 
     fn header(&self) -> Ipv6HeaderSlice {
@@ -323,12 +323,14 @@ pub fn ipv6_translated(ip: Ipv6Addr) -> Option<Ipv4Addr> {
 }
 
 impl IpPacket {
-    pub fn new(buf: IpPacketBuf, len: usize) -> Option<Self> {
-        match buf.inner[NAT46_OVERHEAD] >> 4 {
-            4 => Some(IpPacket::Ipv4(ConvertibleIpv4Packet::new(buf, len)?)),
-            6 => Some(IpPacket::Ipv6(ConvertibleIpv6Packet::new(buf, len)?)),
-            _ => None,
-        }
+    pub fn new(buf: IpPacketBuf, len: usize) -> Result<Self> {
+        anyhow::ensure!(len <= PACKET_SIZE, "Packet too large (len: {len})");
+
+        Ok(match buf.inner[NAT46_OVERHEAD] >> 4 {
+            4 => IpPacket::Ipv4(ConvertibleIpv4Packet::new(buf, len)?),
+            6 => IpPacket::Ipv6(ConvertibleIpv6Packet::new(buf, len)?),
+            v => bail!("Invalid IP version: {v}"),
+        })
     }
 
     pub(crate) fn consume_to_ipv4(self, src: Ipv4Addr, dst: Ipv4Addr) -> Result<IpPacket> {