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

fix(gui-client/windows): allow GUI to run as admin again (#6308)

Browse Source 
Closes #6305 too

I couldn't find the ticket for this so I'm not sure which customers are
affected.

---------

Signed-off-by: Reactor Scram <ReactorScram@users.noreply.github.com>
Co-authored-by: Thomas Eizinger <thomas@eizinger.io>
This commit is contained in:
Reactor Scram
2024-08-15 17:36:18 -05:00
committed by GitHub
parent c67cbfad08
commit 4ddec81f28
7 changed files with 119 additions and 93 deletions
Download Patch File Download Diff File Expand all files Collapse all files

16
rust/gui-client/docs/manual_testing.md
View File

@@ -34,9 +34,19 @@ If the client stops running while signed in, then the token may be stored in Win
# Permissions
- [ ] Given a production exe, when you run it normally, then it will ask to escalate to Admin privileges ([#2751](https://github.com/firezone/firezone/issues/2751))
- [ ] Given the client is running, when you authenticate in the browser, then the client will not ask for privileges again ([#2751](https://github.com/firezone/firezone/issues/2751))
- (Running as an unprivileged user is not supported yet) ([#2751](https://github.com/firezone/firezone/issues/2751))
## Linux Permissions
- [ ] The IPC service with `run-debug` can NOT run as a normal user
- [ ] The IPC service with `run-debug` can run with `sudo`
- [ ] The GUI can run as a normal user
- [ ] The GUI can NOT run with `sudo`
## Windows Permissions
- [ ] The IPC service with `run-debug` can NOT run as a normal user
- [ ] The IPC service with `run-debug` can run as admin
- [ ] The GUI can run as a normal user
- [ ] The GUI can run as admin
# Auth flow

2
rust/gui-client/src-tauri/src/client.rs
View File

@@ -41,7 +41,7 @@ pub(crate) fn run() -> Result<()> {
if cli.no_deep_links {
return run_gui(cli);
}
match elevation::is_normal_user() {
match elevation::gui_check() {
// Our elevation is correct (not elevated), just run the GUI
Ok(true) => run_gui(cli),
Ok(false) => bail!("The GUI should run as a normal user, not elevated"),

98
rust/gui-client/src-tauri/src/client/elevation.rs
View File

@@ -1,18 +1,17 @@
pub(crate) use imp::is_normal_user;
pub(crate) use platform::gui_check;
#[cfg(target_os = "linux")]
mod imp {
mod platform {
use crate::client::gui::Error;
use anyhow::{Context as _, Result};
use firezone_headless_client::FIREZONE_GROUP;
/// Returns true if we're running without root privileges
/// Returns true if all permissions are correct for the GUI to run
///
/// Everything that needs root / admin powers happens in the IPC services,
/// so for security and practicality reasons the GUIs must be non-root.
/// (In Linux by default a root GUI app barely works at all)
pub(crate) fn is_normal_user() -> Result<bool, Error> {
// Must use `eprintln` here because `tracing` won't be initialized yet.
pub(crate) fn gui_check() -> Result<bool, Error> {
let user = std::env::var("USER").context("USER env var should be set")?;
if user == "root" {
return Ok(false);
@@ -35,97 +34,26 @@ mod imp {
}
}
// Stub only
#[cfg(target_os = "macos")]
mod imp {
/// Placeholder for cargo check on macOS
pub(crate) fn is_normal_user() -> anyhow::Result<bool, crate::client::gui::Error> {
Ok(true)
}
}
#[cfg(target_os = "windows")]
mod imp {
mod platform {
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},
};
use anyhow::Result;
// Returns true on Windows
///
/// On Windows, checking for elevation is complicated,
/// so it just always returns true. The Windows GUI does work correctly even if
/// 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".
/// On Windows, some users will run as admin, and the GUI does work correctly,
/// unlike on Linux where most distros don't like to mix root GUI apps with X11 / Wayland.
#[allow(clippy::unnecessary_wraps)]
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.
// Closing a pseudo-handle is a harmless no-op, though this is a real handle.
// <https://learn.microsoft.com/en-us/windows/win32/api/processthreadsapi/nf-processthreadsapi-getcurrentprocess>
// > The pseudo handle need not be closed when it is no longer needed. Calling the CloseHandle function with a pseudo handle has no effect. If the pseudo handle is duplicated by DuplicateHandle, the duplicate handle must be closed.
unsafe { CloseHandle(self.inner) }.expect("`CloseHandle` should always succeed");
self.inner = HANDLE::default();
}
pub(crate) fn gui_check() -> Result<bool, Error> {
Ok(true)
}
}
#[cfg(test)]
mod tests {
// Make sure it doesn't crash
// Make sure it doesn't panic
#[test]
fn is_normal_user() {
super::is_normal_user().unwrap();
fn gui_check_no_panic() {
super::gui_check().ok();
}
}

8
rust/headless-client/src/ipc_service.rs
View File

@@ -2,7 +2,7 @@ use crate::{
device_id, dns_control::DnsController, known_dirs, signals, CallbackHandler, CliCommon,
ConnlibMsg, IpcServerMsg,
};
use anyhow::{Context as _, Result};
use anyhow::{bail, Context as _, Result};
use clap::Parser;
use connlib_client_shared::{keypair, ConnectArgs, LoginUrl, Session};
use firezone_bin_shared::{
@@ -106,6 +106,9 @@ fn run_debug_ipc_service(cli: Cli) -> Result<()> {
git_version = firezone_bin_shared::GIT_VERSION,
system_uptime_seconds = crate::uptime::get().map(|dur| dur.as_secs()),
);
if !platform::elevation_check()? {
bail!("IPC service failed its elevation check, try running as admin / root");
}
let rt = tokio::runtime::Runtime::new()?;
let _guard = rt.enter();
let mut signals = signals::Terminate::new()?;
@@ -124,6 +127,9 @@ fn run_smoke_test() -> Result<()> {
#[cfg(debug_assertions)]
fn run_smoke_test() -> Result<()> {
crate::setup_stdout_logging()?;
if !platform::elevation_check()? {
bail!("IPC service failed its elevation check, try running as admin / root");
}
let rt = tokio::runtime::Runtime::new()?;
let _guard = rt.enter();
let mut dns_controller = DnsController {

11
rust/headless-client/src/ipc_service/linux.rs
View File

@@ -7,8 +7,8 @@ use anyhow::{bail, Result};
/// Linux uses the CLI args from here, Windows does not
pub(crate) fn run_ipc_service(cli: CliCommon) -> Result<()> {
let _handle = super::setup_logging(cli.log_dir)?;
if !nix::unistd::getuid().is_root() {
anyhow::bail!("This is the IPC service binary, it's not meant to run interactively.");
if !elevation_check()? {
bail!("IPC service failed its elevation check, try running as admin / root");
}
let rt = tokio::runtime::Runtime::new()?;
let _guard = rt.enter();
@@ -17,6 +17,13 @@ pub(crate) fn run_ipc_service(cli: CliCommon) -> Result<()> {
rt.block_on(super::ipc_listen(cli.dns_control, &mut signals))
}
/// Returns true if the IPC service can run properly
// Fallible on Windows
#[allow(clippy::unnecessary_wraps)]
pub(crate) fn elevation_check() -> Result<bool> {
Ok(nix::unistd::getuid().is_root())
}
pub(crate) fn install_ipc_service() -> Result<()> {
bail!("`install_ipc_service` not implemented and not needed on Linux")
}

74
rust/headless-client/src/ipc_service/windows.rs
View File

@@ -2,8 +2,18 @@ use crate::CliCommon;
use anyhow::{bail, Context as _, Result};
use firezone_bin_shared::platform::DnsControlMethod;
use futures::future::{self, Either};
use std::{ffi::OsString, pin::pin, time::Duration};
use std::{
ffi::{c_void, OsString},
mem::size_of,
pin::pin,
time::Duration,
};
use tokio::sync::mpsc;
use windows::Win32::{
Foundation::{CloseHandle, HANDLE},
Security::{GetTokenInformation, TokenElevation, TOKEN_ELEVATION, TOKEN_QUERY},
System::Threading::{GetCurrentProcess, OpenProcessToken},
};
use windows_service::{
service::{
ServiceAccess, ServiceControl, ServiceControlAccept, ServiceErrorControl, ServiceExitCode,
@@ -16,6 +26,65 @@ use windows_service::{
const SERVICE_NAME: &str = "firezone_client_ipc";
const SERVICE_TYPE: ServiceType = ServiceType::OWN_PROCESS;
/// Returns true if the IPC service can run properly
pub(crate) fn elevation_check() -> Result<bool> {
let token = ProcessToken::our_process()?;
let elevated = token.is_elevated()?;
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.
// Closing a pseudo-handle is a harmless no-op, though this is a real handle.
// <https://learn.microsoft.com/en-us/windows/win32/api/processthreadsapi/nf-processthreadsapi-getcurrentprocess>
// > The pseudo handle need not be closed when it is no longer needed. Calling the CloseHandle function with a pseudo handle has no effect. If the pseudo handle is duplicated by DuplicateHandle, the duplicate handle must be closed.
unsafe { CloseHandle(self.inner) }.expect("`CloseHandle` should always succeed");
self.inner = HANDLE::default();
}
}
pub(crate) fn install_ipc_service() -> Result<()> {
let manager_access = ServiceManagerAccess::CONNECT | ServiceManagerAccess::CREATE_SERVICE;
let service_manager = ServiceManager::local_computer(None::<&str>, manager_access)?;
@@ -76,6 +145,9 @@ fn fallible_service_run(
logging_handle: firezone_logging::file::Handle,
) -> Result<()> {
tracing::info!(?arguments, "fallible_windows_service_run");
if !elevation_check()? {
bail!("IPC service failed its elevation check, try running as admin / root");
}
let rt = tokio::runtime::Runtime::new()?;
let (shutdown_tx, shutdown_rx) = mpsc::channel(1);

3
website/src/components/Changelog/GUI.tsx
View File

@@ -22,6 +22,9 @@ export default function GUI({ title }: { title: string }) {
<ChangeItem enable={title === "Windows"} pull="6280">
Fixes a bug where the "Clear Logs" button did not clear the IPC service logs.
</ChangeItem>
<ChangeItem enable={title === "Windows"} pull="6308">
Fixes a bug where the GUI could not run if the user is Administrator
</ChangeItem>
</ul>
</Entry>
*/}