diff --git a/Cargo.lock b/Cargo.lock index 3d5d95cb58..e7f606981a 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3861,6 +3861,7 @@ dependencies = [ "nix", "path-clean", "tedge_config", + "tedge_utils", "tempfile", "uzers", "which", diff --git a/crates/common/tedge_utils/src/atomic.rs b/crates/common/tedge_utils/src/atomic.rs new file mode 100644 index 0000000000..8a8fe97a09 --- /dev/null +++ b/crates/common/tedge_utils/src/atomic.rs @@ -0,0 +1,125 @@ +//! Utilities for atomic writes. +//! +//! For deployment of configuration files, we need to create a file atomically because certain +//! programs might watch configuration file for changes, so if it's not written atomically, then +//! file might be only partially written and a program trying to read it may crash. +//! +//! Atomic write of a file consists of creating a temporary file in the same directory, filling it +//! with correct content and permissions, and only then renaming the temporary into the destination +//! filename. Because we're never actually writing into the file, we don't need to write permissions +//! for the destination file, even if it exists. Instead we need only write/execute permissions to +//! the directory file is located in unless the directory has a sticky bit set. Overwriting a file +//! will also change its uid/gid/mode, if writing process euid/egid is different from file's +//! uid/gid. To keep uid/gid the same, after the write we need to do `chown`, and to do it we need +//! sudo. + +use std::io::ErrorKind; +use std::io::Read; +use std::os::unix::fs::fchown; +use std::os::unix::fs::MetadataExt; +use std::os::unix::fs::PermissionsExt; +use std::path::Path; + +use anyhow::Context; + +/// Writes a file atomically and optionally sets its permissions. +/// +/// Setting ownership of a file is a privileged operation so it needs to be run as root. If any of +/// the filesystem operations fail due to not having permissions, the function will return an error. +/// +/// If the file already exists, its content will be overwritten but its permissions will remain +/// unchanged. +pub fn write_file_atomic_set_permissions_if_doesnt_exist( + mut src: impl Read, + dest: impl AsRef, + permissions: &MaybePermissions, +) -> anyhow::Result<()> { + let dest = dest.as_ref(); + + let target_permissions = target_permissions(dest, permissions) + .context("failed to compute target permissions of the file")?; + + // TODO: create tests to ensure writes we expect are atomic + let mut tempfile = tempfile::Builder::new() + .permissions(std::fs::Permissions::from_mode(0o600)) + .tempfile_in(dest.parent().context("invalid path")?) + .with_context(|| { + format!( + "could not create temporary file at '{}'", + dest.to_string_lossy() + ) + })?; + + std::io::copy(&mut src, &mut tempfile).context("failed to copy")?; + + tempfile + .as_file() + .set_permissions(std::fs::Permissions::from_mode(target_permissions.mode)) + .context("failed to set mode on the destination file")?; + + fchown( + tempfile.as_file(), + Some(target_permissions.uid), + Some(target_permissions.gid), + ) + .context("failed to change ownership of the destination file")?; + + tempfile.as_file().sync_all()?; + + tempfile + .persist(dest) + .context("failed to persist temporary file at destination")?; + + Ok(()) +} + +/// Computes target permissions for the file. +/// +/// - if file exists preserve current permissions +/// - if it doesn't exist apply permissions from `permissions` if they are defined +/// - set to root:root with default umask otherwise +/// +/// # Errors +/// - if desired user/group doesn't exist on the system +/// - no permission to read destination file +fn target_permissions(dest: &Path, permissions: &MaybePermissions) -> anyhow::Result { + let current_file_permissions = match std::fs::metadata(dest) { + Err(err) => match err.kind() { + ErrorKind::NotFound => None, + _ => return Err(err.into()), + }, + Ok(p) => Some(p), + }; + + let uid = current_file_permissions + .as_ref() + .map(|p| p.uid()) + .or(permissions.uid) + .unwrap_or(0); + + let gid = current_file_permissions + .as_ref() + .map(|p| p.gid()) + .or(permissions.gid) + .unwrap_or(0); + + let mode = current_file_permissions + .as_ref() + .map(|p| p.mode()) + .or(permissions.mode) + .unwrap_or(0o644); + + Ok(Permissions { uid, gid, mode }) +} + +pub struct MaybePermissions { + pub uid: Option, + pub gid: Option, + pub mode: Option, +} + +struct Permissions { + uid: u32, + gid: u32, + mode: u32, +} diff --git a/crates/common/tedge_utils/src/lib.rs b/crates/common/tedge_utils/src/lib.rs index a3e4acd151..3048ff7c46 100644 --- a/crates/common/tedge_utils/src/lib.rs +++ b/crates/common/tedge_utils/src/lib.rs @@ -1,3 +1,4 @@ +pub mod atomic; pub mod file; pub mod fs; pub mod paths; diff --git a/crates/core/tedge_write/Cargo.toml b/crates/core/tedge_write/Cargo.toml index e4ab181348..4e7ec1f8b2 100644 --- a/crates/core/tedge_write/Cargo.toml +++ b/crates/core/tedge_write/Cargo.toml @@ -18,6 +18,7 @@ clap.workspace = true nix.workspace = true path-clean.workspace = true tedge_config.workspace = true +tedge_utils.workspace = true uzers.workspace = true which.workspace = true diff --git a/crates/core/tedge_write/src/bin.rs b/crates/core/tedge_write/src/bin.rs index 0b7e21e82a..c1da3f9753 100644 --- a/crates/core/tedge_write/src/bin.rs +++ b/crates/core/tedge_write/src/bin.rs @@ -1,16 +1,10 @@ // TODO: force `destination_path` to be the first argument in clap -use std::fs; -use std::io; -use std::os::unix::fs::chown; -use std::os::unix::fs::MetadataExt; -use std::os::unix::fs::PermissionsExt; - use anyhow::bail; use anyhow::Context; -use camino::Utf8Path; use camino::Utf8PathBuf; use clap::Parser; +use tedge_utils::atomic::MaybePermissions; /// A binary used for writing to files which `tedge` user does not have write permissions for, using /// sudo. @@ -62,133 +56,34 @@ pub fn run(args: Args) -> anyhow::Result<()> { ); } - let file_existed_before_write = target_filepath.is_file(); - - write_stdin_to_file_atomic(&target_filepath)?; - - if file_existed_before_write { - return Ok(()); - } - - if args.user.is_some() || args.group.is_some() { - chown_by_user_and_group_name( - &target_filepath, - args.user.as_deref(), - args.group.as_deref(), - ) - .context("Changing ownership of destination file failed")?; - } - - if let Some(mode) = args.mode { - let mode = u32::from_str_radix(&mode, 8).context("Parsing mode failed")?; - let permissions = fs::Permissions::from_mode(mode); - fs::set_permissions(args.destination_path.as_std_path(), permissions) - .context("Could not set new permissions")?; - } - - Ok(()) -} + let mode = args + .mode + .map(|m| u32::from_str_radix(&m, 8).with_context(|| format!("invalid mode: {m}"))) + .transpose()?; -/// Writes contents of stdin into a file atomically. -/// -/// To write a file atomically, stdin is written into a temporary file, which is then renamed into the target file. -/// Because rename is only atomic if both source and destination are on the same filesystem, the temporary file is -/// located in the same directory as the target file. -/// -/// Using [`io::copy`], data should be copied using Linux-specific syscalls for copying between file descriptors, -/// without unnecessary copying to and from a userspace buffer. -fn write_stdin_to_file_atomic(target_filepath: &Utf8Path) -> anyhow::Result<()> { - let temp_filepath = { - let Some(temp_filename) = target_filepath.file_name().map(|f| format!("{f}.tmp")) else { - bail!("Destination path {target_filepath} does not name a valid filename"); - }; - target_filepath.with_file_name(temp_filename) - }; - - // can fail if no permissions or temporary file already exists - let mut temp_file = fs::OpenOptions::new() - .write(true) - .create_new(true) - .open(temp_filepath.as_std_path()) - .with_context(|| format!("Could not open temporary file `{temp_filepath}` for writing"))?; - - // If the target file already exists, use the same permissions and uid/gid - if target_filepath.is_file() { - let target_metadata = target_filepath.metadata().with_context(|| { - format!("Could not fetch metadata of target file {target_filepath}") - })?; - - let uid = target_metadata.uid(); - let gid = target_metadata.gid(); - chown(&temp_filepath, Some(uid), Some(gid)) - .context("Could not set destination owner/group")?; - - let target_permissions = target_metadata.permissions(); - temp_file - .set_permissions(target_permissions) - .with_context(|| { - format!("Could not set permissions for temporary file {temp_filepath}") - })?; - } - - let mut stdin = std::io::stdin().lock(); - io::copy(&mut stdin, &mut temp_file) - .with_context(|| format!("Could not write to the temporary file `{temp_filepath}`"))?; - - if let Err(e) = fs::rename(&temp_filepath, target_filepath) - .with_context(|| format!("Could not write to destination file `{target_filepath}`")) - { - let _ = fs::remove_file(&temp_filepath); - return Err(e); - } - Ok(()) -} + let uid = args + .user + .map(|u| uzers::get_user_by_name(&*u).with_context(|| format!("no such user: '{u}'"))) + .transpose()? + .map(|u| u.uid()); -fn chown_by_user_and_group_name( - filepath: &Utf8Path, - user: Option<&str>, - group: Option<&str>, -) -> anyhow::Result<()> { - // if user and group contain only digits, then they're ids - let user_id = user.and_then(|u| u.parse::().ok()); - let group_id = group.and_then(|g| g.parse::().ok()); + let gid = args + .group + .map(|g| uzers::get_group_by_name(&*g).with_context(|| format!("no such group: '{g}'"))) + .transpose()? + .map(|g| g.gid()); - let new_uid = match user { - Some(u) => { - if user_id.is_some() { - user_id - } else { - Some( - uzers::get_user_by_name(u) - .with_context(|| format!("User `{u}` does not exist"))? - .uid(), - ) - } - } - None => None, - }; + // what permissions we want to set if the file doesn't exist + let permissions = MaybePermissions { uid, gid, mode }; - let new_gid = match group { - Some(g) => { - if group_id.is_some() { - group_id - } else { - Some( - uzers::get_group_by_name(g) - .with_context(|| format!("Group `{g}` does not exist"))? - .gid(), - ) - } - } - None => None, - }; + let src = std::io::stdin().lock(); - nix::unistd::chown( - filepath.as_std_path(), - new_uid.map(nix::unistd::Uid::from_raw), - new_gid.map(nix::unistd::Gid::from_raw), + tedge_utils::atomic::write_file_atomic_set_permissions_if_doesnt_exist( + src, + &target_filepath, + &permissions, ) - .with_context(|| format!("chown failed for file `{filepath}`"))?; + .with_context(|| format!("failed to write to destination file '{target_filepath}'"))?; Ok(()) } diff --git a/crates/extensions/tedge_config_manager/src/actor.rs b/crates/extensions/tedge_config_manager/src/actor.rs index 051201f4a1..5e5a8e644c 100644 --- a/crates/extensions/tedge_config_manager/src/actor.rs +++ b/crates/extensions/tedge_config_manager/src/actor.rs @@ -6,9 +6,6 @@ use log::error; use log::info; use serde_json::json; use std::io::ErrorKind; -use std::os::unix::fs::fchown; -use std::os::unix::fs::MetadataExt; -use std::os::unix::fs::PermissionsExt; use std::sync::Arc; use tedge_actors::fan_in_message_type; use tedge_actors::Actor; @@ -34,9 +31,9 @@ use tedge_mqtt_ext::QoS; use tedge_mqtt_ext::Topic; use tedge_uploader_ext::UploadRequest; use tedge_uploader_ext::UploadResult; +use tedge_utils::atomic::MaybePermissions; use tedge_write::CopyOptions; -use crate::FileEntry; use crate::TedgeWriteStatus; use super::config::PluginConfig; @@ -386,9 +383,39 @@ impl ConfigManagerWorker { let to = Utf8PathBuf::from(&file_entry.path); - let Err(err) = move_file_atomic_set_permissions_if_doesnt_exist(from, file_entry) - .with_context(|| format!("failed to deploy config file from '{from}' to '{to}'")) - else { + let permissions = MaybePermissions { + uid: file_entry + .file_permissions + .user + .as_ref() + .map(|u| { + uzers::get_user_by_name(&u).with_context(|| format!("no such user: '{u}'")) + }) + .transpose()? + .map(|u| u.uid()), + + gid: file_entry + .file_permissions + .group + .as_ref() + .map(|g| { + uzers::get_group_by_name(&g).with_context(|| format!("no such group: '{g}'")) + }) + .transpose()? + .map(|g| g.gid()), + + mode: file_entry.file_permissions.mode, + }; + + let src = std::fs::File::open(from) + .with_context(|| format!("failed to open source temporary file '{from}'"))?; + + let Err(err) = tedge_utils::atomic::write_file_atomic_set_permissions_if_doesnt_exist( + src, + &to, + &permissions, + ) + .with_context(|| format!("failed to deploy config file from '{from}' to '{to}'")) else { return Ok(to); }; @@ -482,142 +509,6 @@ impl ConfigManagerWorker { } } -/// Writes a file atomically and optionally sets its permissions. -/// -/// Setting permissions (owner and group) of a file is a privileged operation so it needs to be run -/// as root. If the any of the filesystem operations fail due to not having permissions, the -/// function will return an error. -/// -/// If the file already exists, its content will be overwritten but its permissions will remain -/// unchanged. -/// -/// For deployment of configuration files, we need to create a file atomically because certain -/// programs might watch configuration file for changes, so if it's not written atomically, then -/// file might be only partially written and a program trying to read it may crash. -/// -/// Atomic write of a file consists of creating a temporary file in the same directory, filling it -/// with correct content and permissions, and only then renaming the temporary into the destination -/// filename. Because we're never actually writing into the file, we don't need to write permissions -/// for the destination file, even if it exists. Instead we need only write/execute permissions to -/// the directory file is located in unless the directory has a sticky bit set. Overwriting a file -/// will also change its uid/gid/mode, if writing process euid/egid is different from file's -/// uid/gid. To keep uid/gid the same, after the write we need to do `chown`, and to do it we need -/// sudo. -/// -/// # Errors -/// -/// - `src` doesn't exist -/// - user or group doesn't exist -/// - we have no write/execute permissions to the directory -fn move_file_atomic_set_permissions_if_doesnt_exist( - src: &Utf8Path, - file_entry: &FileEntry, -) -> anyhow::Result<()> { - let dest = Utf8Path::new(file_entry.path.as_str()); - - let target_permissions = config_file_target_permissions(file_entry, dest) - .context("failed to compute target permissions of the file")?; - - let mut src_file = std::fs::File::open(src) - .with_context(|| format!("failed to open temporary source file '{src}'"))?; - - // TODO: create tests to ensure writes we expect are atomic - let mut tempfile = tempfile::Builder::new() - .permissions(std::fs::Permissions::from_mode(0o600)) - .tempfile_in(dest.parent().context("invalid path")?) - .with_context(|| format!("could not create temporary file at '{dest}'"))?; - - std::io::copy(&mut src_file, &mut tempfile).context("failed to copy")?; - - tempfile - .as_file() - .set_permissions(std::fs::Permissions::from_mode(target_permissions.mode)) - .context("failed to set mode on the destination file")?; - - fchown( - tempfile.as_file(), - Some(target_permissions.uid), - Some(target_permissions.gid), - ) - .context("failed to change ownership of the destination file")?; - - tempfile.as_file().sync_all()?; - - tempfile - .persist(dest) - .context("failed to persist temporary file at destination")?; - - Ok(()) -} - -/// Computes target permissions for deployment of the config file. -/// -/// - if file exists preserve current permissions -/// - if it doesn't exist apply permissions from `permissions` if they are defined -/// - set to root:root with default umask otherwise -/// -/// # Errors -/// - if desired user/group doesn't exist on the system -/// - no permission to read destination file -fn config_file_target_permissions( - file_entry: &FileEntry, - dest: &Utf8Path, -) -> anyhow::Result { - let current_file_permissions = match std::fs::metadata(dest) { - Err(err) => match err.kind() { - ErrorKind::PermissionDenied => return Err(err).context("no permissions"), - ErrorKind::NotFound => None, - _ => return Err(err).context("unexpected IO error"), - }, - Ok(p) => Some(p), - }; - - let entry_uid = if let Some(ref u) = file_entry.file_permissions.user { - let uid = uzers::get_user_by_name(u) - .with_context(|| format!("no such user: '{u}'"))? - .uid(); - Some(uid) - } else { - None - }; - - let entry_gid = if let Some(ref g) = file_entry.file_permissions.group { - let gid = uzers::get_group_by_name(g) - .with_context(|| format!("no such group: '{g}'"))? - .gid(); - Some(gid) - } else { - None - }; - let entry_mode = file_entry.file_permissions.mode; - - let uid = current_file_permissions - .as_ref() - .map(|p| p.uid()) - .or(entry_uid) - .unwrap_or(0); - - let gid = current_file_permissions - .as_ref() - .map(|p| p.gid()) - .or(entry_gid) - .unwrap_or(0); - - let mode = current_file_permissions - .as_ref() - .map(|p| p.mode()) - .or(entry_mode) - .unwrap_or(0o644); - - Ok(Permissions { uid, gid, mode }) -} - -struct Permissions { - uid: u32, - gid: u32, - mode: u32, -} - #[derive(Debug, PartialEq, Eq, Clone)] pub enum ConfigOperation { Snapshot(Topic, ConfigSnapshotCmdPayload),