diff --git a/rust/connlib/tunnel/src/device_channel.rs b/rust/connlib/tunnel/src/device_channel.rs index 829fc4fd5..bb2c0bea0 100644 --- a/rust/connlib/tunnel/src/device_channel.rs +++ b/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:#}"), ) })?; diff --git a/rust/dns-over-tcp/src/stub_device.rs b/rust/dns-over-tcp/src/stub_device.rs index c099d37da..294b98b14 100644 --- a/rust/dns-over-tcp/src/stub_device.rs +++ b/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); diff --git a/rust/ip-packet/src/lib.rs b/rust/ip-packet/src/lib.rs index 446c12d22..5778af773 100644 --- a/rust/ip-packet/src/lib.rs +++ b/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 { + pub fn new(ip: IpPacketBuf, len: usize) -> Result { 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 { + pub fn new(ip: IpPacketBuf, len: usize) -> Result { 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 { } impl IpPacket { - pub fn new(buf: IpPacketBuf, len: usize) -> Option { - 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 { + 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 {