From 271c4803576bd69bb001bd08adcd524684cdd67c Mon Sep 17 00:00:00 2001 From: Thomas Eizinger Date: Tue, 5 Nov 2024 15:17:21 +1100 Subject: [PATCH] fix(connlib): don't attempt to encrypt too large packets (#7263) When encrypting packets, we need to reserve a buffer within which boringtun will encrypt the IP packet. Unfortunately, `boringtun` panics if that buffer is not big enough which essentially brings all of `connlib` down. Really, we should never see a packet that is too large and ideally, we enforce this at compile-time by creating different variants of `IpPacket` that are sized accordingly. That is a large refactoring so until then, we simply discard them instead of panicking. --------- Signed-off-by: Thomas Eizinger Co-authored-by: Jamil --- rust/connlib/snownet/src/node.rs | 13 +++++++++++++ rust/ip-packet/src/lib.rs | 2 +- website/src/components/Changelog/Android.tsx | 6 +++++- website/src/components/Changelog/Apple.tsx | 6 +++++- website/src/components/Changelog/GUI.tsx | 4 +++- website/src/components/Changelog/Gateway.tsx | 5 +++++ website/src/components/Changelog/Headless.tsx | 6 +++++- 7 files changed, 37 insertions(+), 5 deletions(-) diff --git a/rust/connlib/snownet/src/node.rs b/rust/connlib/snownet/src/node.rs index c3f76a679..5ba158284 100644 --- a/rust/connlib/snownet/src/node.rs +++ b/rust/connlib/snownet/src/node.rs @@ -439,6 +439,19 @@ where .get_established_mut(&connection) .ok_or(Error::NotConnected)?; + let packet_len = packet.packet().len(); + let max_len = if self.mode.is_client() { + ip_packet::PACKET_SIZE + } else { + ip_packet::PACKET_SIZE + ip_packet::NAT46_OVERHEAD + }; + + // TODO: This is a it of a hack, we should compile-time enforce this. + if packet_len > max_len { + tracing::warn!("Packet is too large; max={max_len}, actual={packet_len}"); + return Ok(None); + } + // Encode the packet with an offset of 4 bytes, in case we need to wrap it in a channel-data message. let Some(packet_len) = conn .encapsulate(packet.packet(), &mut buffer.inner[4..], now)? diff --git a/rust/ip-packet/src/lib.rs b/rust/ip-packet/src/lib.rs index 3ef0e4e6c..85082ac5c 100644 --- a/rust/ip-packet/src/lib.rs +++ b/rust/ip-packet/src/lib.rs @@ -37,7 +37,7 @@ pub const MAX_DATAGRAM_PAYLOAD: usize = /// Wireguard has a 32-byte overhead (4b message type + 4b receiver idx + 8b packet counter + 16b AEAD tag) const WG_OVERHEAD: usize = 32; /// In order to do NAT46 without copying, we need 20 extra byte in the buffer (IPv6 packets are 20 byte bigger than IPv4). -const NAT46_OVERHEAD: usize = 20; +pub const NAT46_OVERHEAD: usize = 20; /// TURN's data channels have a 4 byte overhead. const DATA_CHANNEL_OVERHEAD: usize = 4; diff --git a/website/src/components/Changelog/Android.tsx b/website/src/components/Changelog/Android.tsx index 9cee6d744..98a60fb82 100644 --- a/website/src/components/Changelog/Android.tsx +++ b/website/src/components/Changelog/Android.tsx @@ -11,7 +11,11 @@ export default function Android() { title="Android" > {/* When you cut a release, remove any solved issues from the "known issues" lists over in `client-apps`. This must not be done when the issue's PR merges. */} - + + + Mitigates a crash in case the maximum packet size is not respected. + + Handles DNS queries over TCP correctly. diff --git a/website/src/components/Changelog/Apple.tsx b/website/src/components/Changelog/Apple.tsx index f6a69c1d0..f470f1701 100644 --- a/website/src/components/Changelog/Apple.tsx +++ b/website/src/components/Changelog/Apple.tsx @@ -11,7 +11,11 @@ export default function Apple() { title="macOS / iOS" > {/* When you cut a release, remove any solved issues from the "known issues" lists over in `client-apps`. This must not be done when the issue's PR merges. */} - + + + Mitigates a crash in case the maximum packet size is not respected. + + Handles DNS queries over TCP correctly. diff --git a/website/src/components/Changelog/GUI.tsx b/website/src/components/Changelog/GUI.tsx index 260f11457..311ed2fb5 100644 --- a/website/src/components/Changelog/GUI.tsx +++ b/website/src/components/Changelog/GUI.tsx @@ -14,7 +14,9 @@ export default function GUI({ title }: { title: string }) { return ( {/* When you cut a release, remove any solved issues from the "known issues" lists over in `client-apps`. This must not be done when the issue's PR merges. */} - + + Mitigates a crash in case the maximum packet size is not respected. + Handles DNS queries over TCP correctly. diff --git a/website/src/components/Changelog/Gateway.tsx b/website/src/components/Changelog/Gateway.tsx index 5c0294202..f4937ffd4 100644 --- a/website/src/components/Changelog/Gateway.tsx +++ b/website/src/components/Changelog/Gateway.tsx @@ -10,6 +10,11 @@ export default function Gateway() { return ( + + + Mitigates a crash in case the maximum packet size is not respected. + + Separates traffic restrictions between DNS Resources CIDR Resources, diff --git a/website/src/components/Changelog/Headless.tsx b/website/src/components/Changelog/Headless.tsx index f981aef38..01a745766 100644 --- a/website/src/components/Changelog/Headless.tsx +++ b/website/src/components/Changelog/Headless.tsx @@ -11,7 +11,11 @@ export default function Headless() { return ( {/* When you cut a release, remove any solved issues from the "known issues" lists over in `client-apps`. This must not be done when the issue's PR merges. */} - + + + Mitigates a crash in case the maximum packet size is not respected. + + Handles DNS queries over TCP correctly.