github-personal/firezone
1
0
Fork 0
mirror of https://github.com/outbackdingo/firezone.git synced 2026-01-27 10:18:54 +00:00
Code Issues Packages Projects Releases Wiki Activity

refactor(windows): de-duplicate Win32 error codes (#8071)

Browse Source 
The errors returned from Win32 API calls are currently duplicated in
several places. To makes it error-prone to handle them correctly. With
this PR, we de-duplicate this and add proper docs and links for further
reading to them.

We also fix a case where we would currently fail to set IP addresses for
our tunnel interface if the IP stack is not supported.
This commit is contained in:
Thomas Eizinger
2025-02-10 23:33:06 +00:00
committed by GitHub
parent e59aa0c93f
commit f48df7585c
2 changed files with 53 additions and 17 deletions
Download Patch File Download Diff File Expand all files Collapse all files

20
rust/bin-shared/src/tun_device_manager/windows.rs
View File

@@ -1,3 +1,4 @@
use crate::windows::error::{NOT_FOUND, NOT_SUPPORTED, OBJECT_EXISTS};
use crate::windows::TUNNEL_UUID;
use crate::TUNNEL_NAME;
use anyhow::{Context as _, Result};
@@ -28,7 +29,6 @@ use windows::Win32::{
},
Networking::WinSock::{ADDRESS_FAMILY, AF_INET, AF_INET6},
};
use windows_core::HRESULT;
use wintun::Adapter;
/// The ring buffer size used for Wintun.
@@ -78,9 +78,6 @@ impl TunDeviceManager {
.luid
.context("Cannot set IPs prior to creating an adapter")?;
tracing::debug!("Setting our IPv4 = {}", ipv4);
tracing::debug!("Setting our IPv6 = {}", ipv6);
// SAFETY: Both NET_LUID_LH unions should be the same. We're just copying out
// the u64 value and re-wrapping it, since wintun doesn't refer to the windows
// crate's version of NET_LUID_LH.
@@ -122,7 +119,6 @@ impl TunDeviceManager {
// It's okay if this blocks until the route is added in the OS.
fn add_route(route: IpNetwork, iface_idx: u32) {
const DUPLICATE_ERR: u32 = 0x80071392;
let entry = forward_entry(route, iface_idx);
// SAFETY: Windows shouldn't store the reference anywhere, it's just a way to pass lots of arguments at once. And no other thread sees this variable.
@@ -133,7 +129,7 @@ fn add_route(route: IpNetwork, iface_idx: u32) {
};
// We expect set_routes to call add_route with the same routes always making this error expected
if e.code().0 as u32 == DUPLICATE_ERR {
if e.code() == OBJECT_EXISTS {
return;
}
@@ -142,7 +138,6 @@ fn add_route(route: IpNetwork, iface_idx: u32) {
// It's okay if this blocks until the route is removed in the OS.
fn remove_route(route: IpNetwork, iface_idx: u32) {
const ELEMENT_NOT_FOUND: u32 = 0x80070490;
let entry = forward_entry(route, iface_idx);
// SAFETY: Windows shouldn't store the reference anywhere, it's just a way to pass lots of arguments at once. And no other thread sees this variable.
@@ -153,7 +148,7 @@ fn remove_route(route: IpNetwork, iface_idx: u32) {
return;
};
if e.code().0 as u32 == ELEMENT_NOT_FOUND {
if e.code() == NOT_FOUND {
return;
}
@@ -387,7 +382,7 @@ fn try_set_mtu(luid: NET_LUID_LH, family: ADDRESS_FAMILY, mtu: u32) -> Result<()
// SAFETY: TODO
if let Err(error) = unsafe { GetIpInterfaceEntry(&mut row) }.ok() {
if family == AF_INET6 && error.code() == windows_core::HRESULT::from_win32(0x80070490) {
if family == AF_INET6 && error.code() == NOT_FOUND {
tracing::debug!(?family, "Couldn't set MTU, maybe IPv6 is disabled.");
} else {
tracing::warn!(?family, "Couldn't set MTU: {}", err_with_src(&error));
@@ -406,7 +401,7 @@ fn try_set_mtu(luid: NET_LUID_LH, family: ADDRESS_FAMILY, mtu: u32) -> Result<()
}
fn try_set_ip(luid: NET_LUID_LH, ip: IpAddr) -> Result<()> {
const ERROR_OBJECT_ALREADY_EXISTS: HRESULT = HRESULT::from_win32(0x80071392);
tracing::debug!(%ip, "Setting tunnel interface IP");
// Safety: Docs don't mention anything in regards to safety of this function.
let mut row = unsafe {
@@ -435,7 +430,10 @@ fn try_set_ip(luid: NET_LUID_LH, ip: IpAddr) -> Result<()> {
// Safety: Docs don't mention anything about safety other than having to use `InitializeUnicastIpAddressEntry` and we did that.
match unsafe { CreateUnicastIpAddressEntry(&row) }.ok() {
Ok(()) => {}
Err(e) if e.code() == ERROR_OBJECT_ALREADY_EXISTS => {}
Err(e) if e.code() == NOT_SUPPORTED => {
tracing::debug!(%ip, "IP stack not supported");
}
Err(e) if e.code() == OBJECT_EXISTS => {}
Err(e) => {
return Err(anyhow::Error::new(e).context("Failed to create `UnicastIpAddressEntry`"))
}

50
rust/bin-shared/src/windows.rs
View File

@@ -36,6 +36,48 @@ pub const CREATE_NO_WINDOW: u32 = 0x08000000;
/// This ends up in registry keys and tunnel management.
pub const TUNNEL_UUID: Uuid = Uuid::from_u128(0xe924_5bc1_b8c1_44ca_ab1d_c6aa_d4f1_3b9c);
/// Error codes returned from Windows APIs.
///
/// For details, see the Windows Error reference: <https://learn.microsoft.com/en-us/openspecs/windows_protocols/ms-erref>.
///
/// ## tl;dr
///
/// - Windows _result_ codes are 32-bit numbers.
/// - The most-signficant bit indicates an error, if set. Hence all results here start with an `8`.
/// - We can ignore the next 4 bits.
/// - The "7" indicates the facility. In our case, this means Win32 APIs.
/// - The last 4 numbers (i.e. 16 bit) are the actual error code.
///
/// ## Adding new codes
///
/// We create the error codes using the [`windows_core::HRESULT::from_win32`] constructor which sets these bits correctly.
/// The doctests make sure we actually construct the error that we'll see in logs.
/// Being able to search for these with full-text search is important for maintenance.
pub mod error {
use windows_core::HRESULT;
/// Win32 error code objects that don't exist (like network adapters).
///
/// ```
/// assert_eq!(firezone_bin_shared::windows::error::NOT_FOUND.0 as u32, 0x80070490)
/// ```
pub const NOT_FOUND: HRESULT = HRESULT::from_win32(0x0490);
/// Win32 error code for objects that already exist (like routing table entries).
///
/// ```
/// assert_eq!(firezone_bin_shared::windows::error::OBJECT_EXISTS.0 as u32, 0x80071392)
/// ```
pub const OBJECT_EXISTS: HRESULT = HRESULT::from_win32(0x1392);
/// Win32 error code for unsupported operations (like setting an IPv6 address without an IPv6 stack).
///
/// ```
/// assert_eq!(firezone_bin_shared::windows::error::NOT_SUPPORTED.0 as u32, 0x80070032)
/// ```
pub const NOT_SUPPORTED: HRESULT = HRESULT::from_win32(0x0032);
}
#[derive(clap::ValueEnum, Clone, Copy, Debug)]
pub enum DnsControlMethod {
/// Explicitly disable DNS control.
@@ -160,8 +202,6 @@ struct RoutingTableEntry {
impl RoutingTableEntry {
/// Creates a new routing table entry by using the given prototype and overriding the route.
fn create(route: IpAddr, mut prototype: MIB_IPFORWARD_ROW2) -> io::Result<Self> {
const DUPLICATE_ERR: u32 = 0x80071392;
let prefix = &mut prototype.DestinationPrefix;
match route {
IpAddr::V4(x) => {
@@ -178,7 +218,7 @@ impl RoutingTableEntry {
unsafe { CreateIpForwardEntry2(&prototype) }
.ok()
.or_else(|e| {
if e.code().0 as u32 == DUPLICATE_ERR {
if e.code() == error::OBJECT_EXISTS {
Ok(())
} else {
Err(io::Error::other(e))
@@ -198,8 +238,6 @@ impl RoutingTableEntry {
impl Drop for RoutingTableEntry {
fn drop(&mut self) {
const NOT_FOUND: u32 = 0x80070490;
let iface_idx = self.entry.InterfaceIndex;
// Safety: The entry we stored is valid.
@@ -208,7 +246,7 @@ impl Drop for RoutingTableEntry {
return;
};
if e.code().0 as u32 == NOT_FOUND {
if e.code() == error::NOT_FOUND {
return;
}