From 485ee209e298a9856591b5145b16683df9d94729 Mon Sep 17 00:00:00 2001 From: Vicki Pfau Date: Tue, 25 Feb 2025 01:00:49 -0800 Subject: [PATCH] manager/user: Check some validity of config before creating interfaces --- src/manager/user.rs | 34 ++++++++++++++--- src/platform.rs | 89 ++++++++++++++++++++++++++++++++++++++++++++- 2 files changed, 116 insertions(+), 7 deletions(-) diff --git a/src/manager/user.rs b/src/manager/user.rs index 9e43db1..2f30f93 100644 --- a/src/manager/user.rs +++ b/src/manager/user.rs @@ -602,12 +602,24 @@ async fn create_config_interfaces( object_server.at(MANAGER_PATH, storage).await?; } - if config.update_bios.is_some() { - object_server.at(MANAGER_PATH, update_bios).await?; + if let Some(config) = config.update_bios.as_ref() { + match config.is_valid().await { + Ok(true) => { + object_server.at(MANAGER_PATH, update_bios).await?; + } + Ok(false) => (), + Err(e) => error!("Failed to verify if BIOS update config is valid: {e}"), + } } - if config.update_dock.is_some() { - object_server.at(MANAGER_PATH, update_dock).await?; + if let Some(config) = config.update_dock.as_ref() { + match config.is_valid().await { + Ok(true) => { + object_server.at(MANAGER_PATH, update_dock).await?; + } + Ok(false) => (), + Err(e) => error!("Failed to verify if dock update config is valid: {e}"), + } } Ok(()) @@ -734,9 +746,11 @@ mod test { ServiceConfig, StorageConfig, }; use crate::systemd::test::{MockManager, MockUnit}; - use crate::{power, testing}; + use crate::{path, power, testing}; + use std::os::unix::fs::PermissionsExt; use std::time::Duration; + use tokio::fs::{set_permissions, write}; use tokio::sync::mpsc::unbounded_channel; use tokio::time::sleep; use zbus::object_server::Interface; @@ -766,11 +780,15 @@ mod test { }) } - async fn start(platform_config: Option) -> Result { + async fn start(mut platform_config: Option) -> Result { let mut handle = testing::start(); let (tx_ctx, _rx_ctx) = channel::(); let (tx_job, _rx_job) = unbounded_channel::(); + if let Some(ref mut config) = platform_config { + config.set_test_paths(); + } + handle.test.platform_config.replace(platform_config); let connection = handle.new_dbus().await?; connection.request_name("org.freedesktop.systemd1").await?; @@ -791,6 +809,10 @@ mod test { .await?; } + let exe_path = path("exe"); + write(&exe_path, "").await?; + set_permissions(&exe_path, PermissionsExt::from_mode(0o700)).await?; + fake_model(HardwareVariant::Galileo).await?; handle .test diff --git a/src/platform.rs b/src/platform.rs index f389fcc..f3ab244 100644 --- a/src/platform.rs +++ b/src/platform.rs @@ -6,11 +6,14 @@ */ use anyhow::Result; +use nix::errno::Errno; +use nix::unistd::{access, AccessFlags}; use serde::Deserialize; use std::path::PathBuf; -use tokio::fs::read_to_string; +use tokio::fs::{metadata, read_to_string}; #[cfg(not(test))] use tokio::sync::OnceCell; +use tokio::task::spawn_blocking; #[cfg(not(test))] use crate::hardware::is_deck; @@ -46,6 +49,25 @@ pub(crate) struct ScriptConfig { pub script_args: Vec, } +impl ScriptConfig { + pub(crate) async fn is_valid(&self) -> Result { + let script = self.script.clone(); + if !spawn_blocking(move || match access(&script, AccessFlags::X_OK) { + Ok(()) => Ok(true), + Err(Errno::ENOENT | Errno::EACCES) => Ok(false), + Err(e) => Err(e), + }) + .await?? + { + return Ok(false); + } + if !metadata(&self.script).await?.is_file() { + return Ok(false); + } + Ok(true) + } +} + #[derive(Clone, Default, Deserialize, Debug)] pub(crate) struct ResetConfig { pub all: ScriptConfig, @@ -110,6 +132,20 @@ impl PlatformConfig { let config = read_to_string("/usr/share/steamos-manager/platforms/jupiter.toml").await?; Ok(Some(toml::from_str(config.as_ref())?)) } + + #[cfg(test)] + pub(crate) fn set_test_paths(&mut self) { + if let Some(ref mut update_bios) = self.update_bios { + if update_bios.script.as_os_str().is_empty() { + update_bios.script = crate::path("exe"); + } + } + if let Some(ref mut update_dock) = self.update_dock { + if update_dock.script.as_os_str().is_empty() { + update_dock.script = crate::path("exe"); + } + } + } } #[cfg(not(test))] @@ -127,6 +163,57 @@ pub(crate) async fn platform_config() -> Result> { #[cfg(test)] mod test { use super::*; + use crate::{path, testing}; + use std::os::unix::fs::PermissionsExt; + use tokio::fs::{set_permissions, write}; + + #[tokio::test] + async fn script_config_valid_no_path() { + assert!(!ScriptConfig::default().is_valid().await.unwrap()); + } + + #[tokio::test] + async fn script_config_valid_directory() { + assert!(!ScriptConfig { + script: PathBuf::from("/"), + script_args: Vec::new(), + } + .is_valid() + .await + .unwrap()); + } + + #[tokio::test] + async fn script_config_valid_noexec() { + let _handle = testing::start(); + let exe_path = path("exe"); + write(&exe_path, "").await.unwrap(); + set_permissions(&exe_path, PermissionsExt::from_mode(0o600)).await.unwrap(); + + assert!(!ScriptConfig { + script: exe_path, + script_args: Vec::new(), + } + .is_valid() + .await + .unwrap()); + } + + #[tokio::test] + async fn script_config_valid() { + let _handle = testing::start(); + let exe_path = path("exe"); + write(&exe_path, "").await.unwrap(); + set_permissions(&exe_path, PermissionsExt::from_mode(0o700)).await.unwrap(); + + assert!(ScriptConfig { + script: exe_path, + script_args: Vec::new(), + } + .is_valid() + .await + .unwrap()); + } #[tokio::test] async fn jupiter_valid() {