chore(gui-client/windows): throw an error if the GUI runs as admin (#6176)

Closes #5878

It won't work properly as admin (deep links will all fail), and this
improves UX by making it obvious that admin powers are no longer needed
for the GUI.

```[tasklist]
- [x] Write up `SAFETY` comments
```
This commit is contained in:
Reactor Scram
2024-08-06 20:22:59 -05:00
committed by GitHub
parent 5b75e8714a
commit 0cb96d5e37
2 changed files with 65 additions and 7 deletions

View File

@@ -47,6 +47,9 @@ pub(crate) fn run() -> Result<()> {
match cli.command {
None => {
if cli.no_deep_links {
return run_gui(cli);
}
match elevation::is_normal_user() {
// Our elevation is correct (not elevated), just run the GUI
Ok(true) => run_gui(cli),
@@ -69,10 +72,7 @@ pub(crate) fn run() -> Result<()> {
Ok(())
}
Some(Cmd::SmokeTest) => {
if !elevation::is_normal_user()? {
anyhow::bail!("`smoke-test` must run as a normal user");
}
// Can't check elevation here because the Windows CI is always elevated
let settings = settings::load_advanced_settings().unwrap_or_default();
// Don't fix the log filter for smoke tests
let logging::Handles {
@@ -177,7 +177,7 @@ struct Cli {
/// If true, show a fake update notification that opens the Firezone release page when clicked
#[arg(long, hide = true)]
test_update_notification: bool,
/// Disable deep link registration and handling, for headless CI environments
/// For headless CI, disable deep links and allow the GUI to run as admin
#[arg(long, hide = true)]
no_deep_links: bool,
}

View File

@@ -47,6 +47,13 @@ mod imp {
#[cfg(target_os = "windows")]
mod imp {
use crate::client::gui::Error;
use anyhow::{Context as _, Result};
use std::{ffi::c_void, mem::size_of};
use windows::Win32::{
Foundation::{CloseHandle, HANDLE},
Security::{GetTokenInformation, TokenElevation, TOKEN_ELEVATION, TOKEN_QUERY},
System::Threading::{GetCurrentProcess, OpenProcessToken},
};
// Returns true on Windows
///
@@ -55,7 +62,58 @@ mod imp {
/// elevated, so we should warn users that it doesn't need elevation, but it's
/// not a show-stopper if they accidentally "Run as admin".
#[allow(clippy::unnecessary_wraps)]
pub(crate) fn is_normal_user() -> anyhow::Result<bool, Error> {
Ok(true)
pub(crate) fn is_normal_user() -> Result<bool, Error> {
let token = ProcessToken::our_process().map_err(Error::Other)?;
let elevated = token.is_elevated().map_err(Error::Other)?;
Ok(!elevated)
}
// https://stackoverflow.com/questions/8046097/how-to-check-if-a-process-has-the-administrative-rights/8196291#8196291
struct ProcessToken {
inner: HANDLE,
}
impl ProcessToken {
fn our_process() -> Result<Self> {
// SAFETY: Calling C APIs is unsafe
// `GetCurrentProcess` returns a pseudo-handle which does not need to be closed.
// Docs say nothing about thread safety. <https://learn.microsoft.com/en-us/windows/win32/api/processthreadsapi/nf-processthreadsapi-getcurrentprocess>
let our_proc = unsafe { GetCurrentProcess() };
let mut inner = HANDLE::default();
// SAFETY: We just created `inner`, and moving a `HANDLE` is safe.
// We assume that if `OpenProcessToken` fails, we don't need to close the `HANDLE`.
// Docs say nothing about threads or safety: <https://learn.microsoft.com/en-us/windows/win32/api/processthreadsapi/nf-processthreadsapi-openprocesstoken>
unsafe { OpenProcessToken(our_proc, TOKEN_QUERY, &mut inner) }
.context("`OpenProcessToken` failed")?;
Ok(Self { inner })
}
fn is_elevated(&self) -> Result<bool> {
let mut elevation = TOKEN_ELEVATION::default();
let token_elevation_sz = u32::try_from(size_of::<TOKEN_ELEVATION>())
.expect("`TOKEN_ELEVATION` size should fit into a u32");
let mut return_size = 0u32;
// SAFETY: Docs say nothing about threads or safety <https://learn.microsoft.com/en-us/windows/win32/api/securitybaseapi/nf-securitybaseapi-gettokeninformation>
// The type of `elevation` varies based on the 2nd parameter, but we hard-coded that.
// It should be fine.
unsafe {
GetTokenInformation(
self.inner,
TokenElevation,
Some(&mut elevation as *mut _ as *mut c_void),
token_elevation_sz,
&mut return_size as *mut _,
)
}?;
Ok(elevation.TokenIsElevated == 1)
}
}
impl Drop for ProcessToken {
fn drop(&mut self) {
// SAFETY: We got `inner` from `OpenProcessToken` and didn't mutate it after that.
unsafe { CloseHandle(self.inner) }.expect("`CloseHandle` should always succeed");
self.inner = HANDLE::default();
}
}
}