summaryrefslogtreecommitdiffhomepage
diff options
context:
space:
mode:
-rw-r--r--Cargo.lock4
-rw-r--r--Cargo.toml1
-rw-r--r--mullvad-paths/Cargo.toml2
-rw-r--r--mullvad-paths/src/lib.rs4
-rw-r--r--mullvad-paths/src/windows.rs66
-rw-r--r--talpid-core/Cargo.toml2
-rw-r--r--talpid-routing/Cargo.toml2
-rw-r--r--talpid-wireguard/Cargo.toml2
-rw-r--r--test/Cargo.lock4
9 files changed, 43 insertions, 44 deletions
diff --git a/Cargo.lock b/Cargo.lock
index 1f818d4728..7e30a542e3 100644
--- a/Cargo.lock
+++ b/Cargo.lock
@@ -6659,9 +6659,9 @@ dependencies = [
[[package]]
name = "widestring"
-version = "1.1.0"
+version = "1.2.1"
source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "7219d36b6eac893fa81e84ebe06485e7dcbb616177469b142df14f1f4deb1311"
+checksum = "72069c3113ab32ab29e5584db3c6ec55d416895e60715417b5b883a357c3e471"
[[package]]
name = "winapi"
diff --git a/Cargo.toml b/Cargo.toml
index f71bab9b30..d86354da7d 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -95,6 +95,7 @@ clap = { version = "4.4.18", features = ["cargo", "derive"] }
once_cell = "1.16"
serde = "1.0.204"
serde_json = "1.0.122"
+widestring = "1.2"
windows = "0.61.0"
windows-sys = "0.61.0"
winreg = "0.55"
diff --git a/mullvad-paths/Cargo.toml b/mullvad-paths/Cargo.toml
index 4a62bedde7..fd4bf8a253 100644
--- a/mullvad-paths/Cargo.toml
+++ b/mullvad-paths/Cargo.toml
@@ -15,7 +15,7 @@ thiserror = { workspace = true }
log = { workspace = true }
[target.'cfg(windows)'.dependencies]
-widestring = "1.0"
+widestring = { workspace = true }
once_cell = { workspace = true }
[target.'cfg(windows)'.dependencies.windows-sys]
diff --git a/mullvad-paths/src/lib.rs b/mullvad-paths/src/lib.rs
index 508a3e8956..481cb06d32 100644
--- a/mullvad-paths/src/lib.rs
+++ b/mullvad-paths/src/lib.rs
@@ -42,6 +42,10 @@ pub enum Error {
#[error("Failed to create security attributes")]
GetSecurityAttributes(#[source] io::Error),
+ #[cfg(windows)]
+ #[error(transparent)]
+ ContainsNul(#[from] widestring::error::ContainsNul<u16>),
+
#[error("Device data directory has not been set")]
NoDataDir,
}
diff --git a/mullvad-paths/src/windows.rs b/mullvad-paths/src/windows.rs
index ae58891e3e..e4ffa97ebb 100644
--- a/mullvad-paths/src/windows.rs
+++ b/mullvad-paths/src/windows.rs
@@ -1,19 +1,12 @@
use crate::{Error, Result, UserPermissions};
use once_cell::sync::OnceCell;
use std::{
- ffi::OsStr,
io, mem,
- os::windows::{
- io::{
- AsHandle, AsRawHandle, BorrowedHandle, FromRawHandle, HandleOrNull, IntoRawHandle,
- OwnedHandle,
- },
- prelude::OsStrExt,
- },
+ os::windows::io::{AsHandle, AsRawHandle, BorrowedHandle, FromRawHandle, OwnedHandle},
path::{Path, PathBuf},
ptr,
};
-use widestring::{WideCStr, WideCString};
+use widestring::{U16CString, WideCStr, u16cstr};
use windows_sys::{
Win32::{
Foundation::{
@@ -78,15 +71,6 @@ pub fn create_dir(path: PathBuf, user_permissions: Option<UserPermissions>) -> R
Ok(path)
}
-fn get_wide_str<S: AsRef<OsStr>>(string: S) -> Vec<u16> {
- let wide_string: Vec<u16> = string.as_ref()
- .encode_wide()
- // Add null terminator
- .chain(std::iter::once(0))
- .collect();
- wide_string
-}
-
impl UserPermissions {
fn flags(self) -> u32 {
let mut flags = 0;
@@ -162,14 +146,15 @@ pub fn create_privileged_directory(path: &Path) -> Result<()> {
/// Sets security permissions for path such that admin has full ownership and access while
/// authenticated users only have read access.
fn set_security_permissions(path: &Path, user_permissions: UserPermissions) -> Result<()> {
- let wide_path = get_wide_str(path);
+ let wide_path = U16CString::from_os_str(path)?;
let security_information = Security::DACL_SECURITY_INFORMATION
| Security::PROTECTED_DACL_SECURITY_INFORMATION
| Security::GROUP_SECURITY_INFORMATION
| Security::OWNER_SECURITY_INFORMATION;
let mut admin_psid = [0u8; MAX_SID_SIZE as usize];
- let mut admin_psid_len = u32::try_from(admin_psid.len()).unwrap();
+ let mut admin_psid_len =
+ u32::try_from(admin_psid.len()).expect("admin_psid length fits in u32");
// SAFETY: The pointer to the PSID is valid for writes of `admin_psid_len` bytes
if unsafe {
@@ -203,7 +188,7 @@ fn set_security_permissions(path: &Path, user_permissions: UserPermissions) -> R
};
let mut au_psid = [0u8; MAX_SID_SIZE as usize];
- let mut au_psid_len = u32::try_from(au_psid.len()).unwrap();
+ let mut au_psid_len = u32::try_from(au_psid.len()).expect("au_psid length fits in u32");
// SAFETY: The pointer to the PSID is valid for writes of `au_psid_len` bytes
if unsafe {
@@ -243,7 +228,7 @@ fn set_security_permissions(path: &Path, user_permissions: UserPermissions) -> R
// `new_dacl` is a valid pointer to an ACL pointer
let result = unsafe {
SetEntriesInAclW(
- u32::try_from(ea_entries.len()).unwrap(),
+ u32::try_from(ea_entries.len()).expect("number of entries fits in u32"),
ea_entries.as_ptr(),
ptr::null(),
&mut new_dacl,
@@ -295,7 +280,7 @@ pub fn get_system_service_appdata() -> io::Result<PathBuf> {
.get_or_try_init(|| {
let join_handle = std::thread::spawn(|| {
impersonate_self(|| {
- let user_token = OwnedHandle::try_from(get_system_user_token()?).ok();
+ let user_token = get_system_user_token()?;
// SAFETY: `FOLDERID_LocalAppData` is a valid known folder ID
unsafe {
get_known_folder_path(
@@ -310,7 +295,7 @@ pub fn get_system_service_appdata() -> io::Result<PathBuf> {
infer_appdata_from_system_directory()
})
});
- join_handle.join().unwrap()
+ join_handle.join().expect("ImpersonateSelf should succeed")
})
.cloned()
}
@@ -319,16 +304,15 @@ pub fn get_system_service_appdata() -> io::Result<PathBuf> {
/// Useful for deducing the config path for the daemon on Windows when running as a user that
/// isn't the system service.
/// If the current user is system, this function succeeds and returns a NULL handle
-fn get_system_user_token() -> io::Result<HandleOrNull> {
+fn get_system_user_token() -> io::Result<Option<OwnedHandle>> {
let thread_token = get_current_thread_token()?;
if is_local_system_user_token(&thread_token)? {
- // SAFETY: It is safe to pass a null handle
- return Ok(unsafe { HandleOrNull::from_raw_handle(ptr::null_mut()) });
+ return Ok(None);
}
- let system_debug_priv = WideCString::from_str("SeDebugPrivilege").unwrap();
- adjust_token_privilege(&thread_token, &system_debug_priv, true)?;
+ let system_debug_priv = u16cstr!("SeDebugPrivilege");
+ adjust_token_privilege(&thread_token, system_debug_priv, true)?;
let find_result = find_process(|process_handle| {
let process_token = open_process_token(
@@ -343,12 +327,11 @@ fn get_system_user_token() -> io::Result<HandleOrNull> {
}
});
- if let Err(err) = adjust_token_privilege(&thread_token, &system_debug_priv, false) {
+ if let Err(err) = adjust_token_privilege(&thread_token, system_debug_priv, false) {
log::error!("Failed to drop SeDebugPrivilege: {}", err);
}
- // SAFETY: The handle is valid
- find_result.map(|h| unsafe { HandleOrNull::from_raw_handle(h.into_raw_handle()) })
+ find_result.map(Some)
}
fn open_process_token(process: &impl AsRawHandle, access: u32) -> io::Result<OwnedHandle> {
@@ -387,17 +370,26 @@ fn get_current_thread_token() -> std::io::Result<OwnedHandle> {
Ok(unsafe { OwnedHandle::from_raw_handle(token_handle) })
}
+/// Run provided closure in the security context of the calling process' impersonation token.
+///
+/// # Panics
+///
+/// If privileges can not be dropped after running `func`, the running process is shut down.
fn impersonate_self<T>(func: impl FnOnce() -> io::Result<T>) -> io::Result<T> {
- // SAFETY: Trivially safe
+ // SAFETY: SecurityImpersonation is a valid ImpersonationLevel.
if unsafe { ImpersonateSelf(SecurityImpersonation) } == 0 {
return Err(std::io::Error::last_os_error());
}
let result = func();
- // SAFETY: Trivially safe
+ // SAFETY: Must be called after a successful call to ImpersonateSelf.
if unsafe { RevertToSelf() } == 0 {
+ // The Windows documentation *strongly* suggest that the process should shut down if
+ // RevertToSelf fails. A failure to do so means that the current process keep running in
+ // an unintended context.
log::error!("RevertToSelf failed: {}", io::Error::last_os_error());
+ panic!("RevertToSelf failed. Aborting");
}
result
@@ -485,7 +477,8 @@ unsafe fn get_known_folder_path(
/// no more processes left. In the latter case, an error is returned.
fn find_process<T>(handle_process: impl Fn(BorrowedHandle<'_>) -> Option<T>) -> io::Result<T> {
let mut pid_buffer = vec![0u32; 2048];
- let mut num_procs: u32 = u32::try_from(pid_buffer.len()).unwrap();
+ let mut num_procs: u32 =
+ u32::try_from(pid_buffer.len()).expect("Number of processes IDs fit in u32");
let bytes_available = num_procs * (mem::size_of::<u32>() as u32);
let mut bytes_written = 0;
@@ -554,7 +547,8 @@ fn is_local_system_user_token(token: &impl AsRawHandle) -> io::Result<bool> {
let token_user = unsafe { &*(token_info.as_mut_ptr() as *const TOKEN_USER) };
let mut local_system_sid = [0u8; MAX_SID_SIZE as usize];
- let mut local_system_size = u32::try_from(local_system_sid.len()).unwrap();
+ let mut local_system_size =
+ u32::try_from(local_system_sid.len()).expect("local_system_sid length fits in u32");
// SAFETY: `local_system_sid` is valid for writes of `local_system_size` bytes
if unsafe {
diff --git a/talpid-core/Cargo.toml b/talpid-core/Cargo.toml
index 531b9fcff9..e7ba36d5e8 100644
--- a/talpid-core/Cargo.toml
+++ b/talpid-core/Cargo.toml
@@ -69,7 +69,7 @@ talpid-net = { path = "../talpid-net" }
[target.'cfg(windows)'.dependencies]
bitflags = "2.6"
csv = "1.3.1"
-widestring = "1.0"
+widestring = { workspace = true }
winreg = { workspace = true, features = ["transactions"] }
memoffset = "0.6"
once_cell = { workspace = true }
diff --git a/talpid-routing/Cargo.toml b/talpid-routing/Cargo.toml
index 4ba45beff2..f2e7a49eca 100644
--- a/talpid-routing/Cargo.toml
+++ b/talpid-routing/Cargo.toml
@@ -37,7 +37,7 @@ system-configuration = "0.5.1"
[target.'cfg(windows)'.dependencies]
talpid-windows = { path = "../talpid-windows" }
-widestring = "1.0"
+widestring = { workspace = true }
[target.'cfg(windows)'.dependencies.windows-sys]
workspace = true
diff --git a/talpid-wireguard/Cargo.toml b/talpid-wireguard/Cargo.toml
index 162bc7156e..5da612f577 100644
--- a/talpid-wireguard/Cargo.toml
+++ b/talpid-wireguard/Cargo.toml
@@ -65,7 +65,7 @@ talpid-dbus = { path = "../talpid-dbus" }
[target.'cfg(windows)'.dependencies]
bitflags = "1.2"
talpid-windows = { path = "../talpid-windows" }
-widestring = "1.0"
+widestring = { workspace = true }
rand_chacha = "0.3.1"
# TODO: Figure out which features are needed and which are not
diff --git a/test/Cargo.lock b/test/Cargo.lock
index 8d0bdfb29b..81a7640654 100644
--- a/test/Cargo.lock
+++ b/test/Cargo.lock
@@ -4463,9 +4463,9 @@ dependencies = [
[[package]]
name = "widestring"
-version = "1.1.0"
+version = "1.2.1"
source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "7219d36b6eac893fa81e84ebe06485e7dcbb616177469b142df14f1f4deb1311"
+checksum = "72069c3113ab32ab29e5584db3c6ec55d416895e60715417b5b883a357c3e471"
[[package]]
name = "winapi"