Skip to content

Commit

Permalink
Fix reading only the needed TCP/UDP header bytes (#1730)
Browse files Browse the repository at this point in the history
* [windows_kext] Fix reading only the needed TCP/UDP header bytes

* [windows_kext] Disable debug mode

* [windows_kext] Block all fragment packets

* [windows_kext] Improve wording for compiler error
  • Loading branch information
vlabo authored Nov 7, 2024
1 parent 145f5e6 commit af3bb80
Show file tree
Hide file tree
Showing 6 changed files with 35 additions and 13 deletions.
5 changes: 5 additions & 0 deletions windows_kext/driver/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,11 @@ mod stream_callouts;

use wdk::allocator::WindowsAllocator;

// For consistent behavior during development and production only release mode should be used.
// Certain behavior of the compiler will change and this can result in errors and different behavior in debug and release mode.
#[cfg(debug_assertions)]
compile_error!("Must be built in release mode to ensure consistent behavior and prevent optimization-related issues. Use `cargo build --release`.");

#[cfg(not(test))]
use core::panic::PanicInfo;

Expand Down
5 changes: 1 addition & 4 deletions windows_kext/driver/src/logger.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,8 @@ use core::{
};
use protocol::info::{Info, Severity};

#[cfg(not(debug_assertions))]
pub const LOG_LEVEL: u8 = Severity::Warning as u8;

#[cfg(debug_assertions)]
pub const LOG_LEVEL: u8 = Severity::Trace as u8;
// pub const LOG_LEVEL: u8 = Severity::Trace as u8;

pub const MAX_LOG_LINE_SIZE: usize = 150;

Expand Down
7 changes: 7 additions & 0 deletions windows_kext/driver/src/packet_callouts.rs
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,13 @@ fn ip_packet_layer(
interface_index: u32,
sub_interface_index: u32,
) {
// Block all fragment data. No easy way to keep track of the origin and they are rarely used.
if data.is_fragment_data() {
data.action_block();
crate::err!("blocked fragment packet");
return;
}

let Some(device) = crate::entry::get_device() else {
return;
};
Expand Down
13 changes: 7 additions & 6 deletions windows_kext/driver/src/packet_util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -245,8 +245,6 @@ fn print_packet(packet: &[u8]) {
///
/// * `Ok(Key)` - A key containing the protocol, local and remote addresses and ports.
/// * `Err(String)` - An error message if the function fails to get net_buffer data.
const HEADERS_LEN: usize = smoltcp::wire::IPV4_HEADER_LEN + smoltcp::wire::TCP_HEADER_LEN;

fn get_ports(packet: &[u8], protocol: smoltcp::wire::IpProtocol) -> (u16, u16) {
match protocol {
smoltcp::wire::IpProtocol::Tcp => {
Expand All @@ -262,12 +260,13 @@ fn get_ports(packet: &[u8], protocol: smoltcp::wire::IpProtocol) -> (u16, u16) {
}

pub fn get_key_from_nbl_v4(nbl: &NetBufferList, direction: Direction) -> Result<Key, String> {
// Get bytes
let mut headers = [0; HEADERS_LEN];
// Get first bytes of the packet. IP header + src port (2 bytes) + dst port (2 bytes)
let mut headers = [0; smoltcp::wire::IPV4_HEADER_LEN + 4];
if nbl.read_bytes(&mut headers).is_err() {
return Err("failed to get net_buffer data".to_string());
}

// This will panic in debug mode, probably because of runtime checks.
// Parse packet
let ip_packet = Ipv4Packet::new_unchecked(&headers);
let (src_port, dst_port) = get_ports(
Expand Down Expand Up @@ -307,11 +306,13 @@ pub fn get_key_from_nbl_v4(nbl: &NetBufferList, direction: Direction) -> Result<
/// * `Ok(Key)` - A key containing the protocol, local and remote addresses and ports.
/// * `Err(String)` - An error message if the function fails to get net_buffer data.
pub fn get_key_from_nbl_v6(nbl: &NetBufferList, direction: Direction) -> Result<Key, String> {
// Get bytes
let mut headers = [0; smoltcp::wire::IPV6_HEADER_LEN + smoltcp::wire::TCP_HEADER_LEN];
// Get first bytes of the packet. IP header + src port (2 bytes) + dst port (2 bytes)
let mut headers = [0; smoltcp::wire::IPV6_HEADER_LEN + 4];
let Ok(()) = nbl.read_bytes(&mut headers) else {
return Err("failed to get net_buffer data".to_string());
};

// This will panic in debug mode, probably because of runtime checks.
// Parse packet
let ip_packet = Ipv6Packet::new_unchecked(&headers);
let (src_port, dst_port) = get_ports(
Expand Down
4 changes: 4 additions & 0 deletions windows_kext/wdk/src/filter_engine/callout_data.rs
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,10 @@ impl<'a> CalloutData<'a> {
}
}

pub fn is_fragment_data(&self) -> bool {
unsafe { (*self.metadata).is_fragment_data() }
}

pub fn pend_operation(
&mut self,
packet_list: Option<TransportPacketList>,
Expand Down
14 changes: 11 additions & 3 deletions windows_kext/wdk/src/filter_engine/metadata.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,9 @@ use windows_sys::Win32::{
NetworkManagement::{
IpHelper::IP_ADDRESS_PREFIX,
WindowsFilteringPlatform::{
FWPS_METADATA_FIELD_COMPLETION_HANDLE, FWPS_METADATA_FIELD_PROCESS_ID,
FWPS_METADATA_FIELD_PROCESS_PATH, FWPS_METADATA_FIELD_REMOTE_SCOPE_ID,
FWPS_METADATA_FIELD_TRANSPORT_CONTROL_DATA,
FWPS_METADATA_FIELD_COMPLETION_HANDLE, FWPS_METADATA_FIELD_FRAGMENT_DATA,
FWPS_METADATA_FIELD_PROCESS_ID, FWPS_METADATA_FIELD_PROCESS_PATH,
FWPS_METADATA_FIELD_REMOTE_SCOPE_ID, FWPS_METADATA_FIELD_TRANSPORT_CONTROL_DATA,
FWPS_METADATA_FIELD_TRANSPORT_ENDPOINT_HANDLE, FWP_BYTE_BLOB, FWP_DIRECTION,
},
},
Expand Down Expand Up @@ -137,6 +137,14 @@ impl FwpsIncomingMetadataValues {
None
}

pub(crate) fn is_fragment_data(&self) -> bool {
if self.has_field(FWPS_METADATA_FIELD_FRAGMENT_DATA) {
return self.fragment_metadata.fragment_offset != 0;
}

false
}

pub(crate) unsafe fn get_control_data(&self) -> Option<NonNull<[u8]>> {
if self.has_field(FWPS_METADATA_FIELD_TRANSPORT_CONTROL_DATA) {
if self.control_data.is_null() || self.control_data_length == 0 {
Expand Down

0 comments on commit af3bb80

Please sign in to comment.