From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by lists.proxmox.com (Postfix) with ESMTPS id DE510767BD for ; Fri, 16 Jul 2021 10:28:46 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id D601FDAC9 for ; Fri, 16 Jul 2021 10:28:46 +0200 (CEST) Received: from elsa.proxmox.com (unknown [94.136.29.99]) by firstgate.proxmox.com (Proxmox) with ESMTP id 37AD9DA4F for ; Fri, 16 Jul 2021 10:28:42 +0200 (CEST) Received: by elsa.proxmox.com (Postfix, from userid 0) id E4E98AE1D42; Fri, 16 Jul 2021 10:28:35 +0200 (CEST) From: Dietmar Maurer To: pbs-devel@lists.proxmox.com Date: Fri, 16 Jul 2021 10:28:33 +0200 Message-Id: <20210716082834.2354163-3-dietmar@proxmox.com> X-Mailer: git-send-email 2.30.2 In-Reply-To: <20210716082834.2354163-1-dietmar@proxmox.com> References: <20210716082834.2354163-1-dietmar@proxmox.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-SPAM-LEVEL: Spam detection results: 0 AWL 0.486 Adjusted score from AWL reputation of From: address BAYES_00 -1.9 Bayes spam probability is 0 to 1% KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment RDNS_NONE 0.793 Delivered to internal network by a host with no rDNS SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record Subject: [pbs-devel] [PATCH proxmox-backup 2/2] add helpers to write configuration files X-BeenThere: pbs-devel@lists.proxmox.com X-Mailman-Version: 2.1.29 Precedence: list List-Id: Proxmox Backup Server development discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 16 Jul 2021 08:28:46 -0000 --- src/backup/mod.rs | 41 ++++++++++++++++++++++++++++++ src/config/acl.rs | 14 +--------- src/config/acme/plugin.rs | 16 +----------- src/config/datastore.rs | 19 +------------- src/config/domains.rs | 19 +------------- src/config/drive.rs | 18 +------------ src/config/media_pool.rs | 19 +------------- src/config/mod.rs | 10 ++------ src/config/node.rs | 10 +------- src/config/remote.rs | 16 +----------- src/config/sync.rs | 16 +----------- src/config/tape_encryption_keys.rs | 33 +++--------------------- src/config/tape_job.rs | 16 +----------- src/config/user.rs | 14 +--------- src/config/verify.rs | 17 +------------ 15 files changed, 58 insertions(+), 220 deletions(-) diff --git a/src/backup/mod.rs b/src/backup/mod.rs index 1db61cf5..3337f3a6 100644 --- a/src/backup/mod.rs +++ b/src/backup/mod.rs @@ -134,3 +134,44 @@ pub fn open_backup_lockfile>( let file = proxmox::tools::fs::open_file_locked(&path, timeout, exclusive, options)?; Ok(BackupLockGuard(file)) } + +/// Atomically write data to file owned by "root:backup" with permission "0640" +/// +/// Only the superuser can write those files, but group 'backup' can read them. +pub fn replace_backup_config>( + path: P, + data: &[u8], +) -> Result<(), Error> { + let backup_user = backup_user()?; + let mode = nix::sys::stat::Mode::from_bits_truncate(0o0640); + // set the correct owner/group/permissions while saving file + // owner(rw) = root, group(r)= backup + let options = proxmox::tools::fs::CreateOptions::new() + .perm(mode) + .owner(nix::unistd::ROOT) + .group(backup_user.gid); + + proxmox::tools::fs::replace_file(path, data, options)?; + + Ok(()) +} + +/// Atomically write data to file owned by "root:root" with permission "0600" +/// +/// Only the superuser can read and write those files. +pub fn replace_secret_config>( + path: P, + data: &[u8], +) -> Result<(), Error> { + let mode = nix::sys::stat::Mode::from_bits_truncate(0o0600); + // set the correct owner/group/permissions while saving file + // owner(rw) = root, group(r)= root + let options = proxmox::tools::fs::CreateOptions::new() + .perm(mode) + .owner(nix::unistd::ROOT) + .group(nix::unistd::Gid::from_raw(0)); + + proxmox::tools::fs::replace_file(path, data, options)?; + + Ok(()) +} diff --git a/src/config/acl.rs b/src/config/acl.rs index b4b3510f..b7badb79 100644 --- a/src/config/acl.rs +++ b/src/config/acl.rs @@ -13,7 +13,6 @@ use serde::de::{value, IntoDeserializer}; use proxmox::api::{api, schema::*}; use proxmox::constnamedbitmap; -use proxmox::tools::{fs::replace_file, fs::CreateOptions}; use crate::api2::types::{Authid, Userid}; @@ -912,18 +911,7 @@ pub fn save_config(acl: &AclTree) -> Result<(), Error> { acl.write_config(&mut raw)?; - let backup_user = crate::backup::backup_user()?; - let mode = nix::sys::stat::Mode::from_bits_truncate(0o0640); - // set the correct owner/group/permissions while saving file - // owner(rw) = root, group(r)= backup - let options = CreateOptions::new() - .perm(mode) - .owner(nix::unistd::ROOT) - .group(backup_user.gid); - - replace_file(ACL_CFG_FILENAME, &raw, options)?; - - Ok(()) + crate::backup::replace_backup_config(ACL_CFG_FILENAME, &raw) } #[cfg(test)] diff --git a/src/config/acme/plugin.rs b/src/config/acme/plugin.rs index fde800e2..a4322fdd 100644 --- a/src/config/acme/plugin.rs +++ b/src/config/acme/plugin.rs @@ -9,8 +9,6 @@ use proxmox::api::{ section_config::{SectionConfig, SectionConfigData, SectionConfigPlugin}, }; -use proxmox::tools::{fs::replace_file, fs::CreateOptions}; - use crate::api2::types::PROXMOX_SAFE_ID_FORMAT; use crate::backup::{open_backup_lockfile, BackupLockGuard}; @@ -168,19 +166,7 @@ pub fn config() -> Result<(PluginData, [u8; 32]), Error> { pub fn save_config(config: &PluginData) -> Result<(), Error> { super::make_acme_dir()?; let raw = CONFIG.write(ACME_PLUGIN_CFG_FILENAME, &config.data)?; - - let backup_user = crate::backup::backup_user()?; - let mode = nix::sys::stat::Mode::from_bits_truncate(0o0640); - // set the correct owner/group/permissions while saving file - // owner(rw) = root, group(r)= backup - let options = CreateOptions::new() - .perm(mode) - .owner(nix::unistd::ROOT) - .group(backup_user.gid); - - replace_file(ACME_PLUGIN_CFG_FILENAME, raw.as_bytes(), options)?; - - Ok(()) + crate::backup::replace_backup_config(ACME_PLUGIN_CFG_FILENAME, raw.as_bytes()) } pub struct PluginData { diff --git a/src/config/datastore.rs b/src/config/datastore.rs index 9e37073d..46d28feb 100644 --- a/src/config/datastore.rs +++ b/src/config/datastore.rs @@ -13,11 +13,6 @@ use proxmox::api::{ } }; -use proxmox::tools::fs::{ - replace_file, - CreateOptions, -}; - use crate::api2::types::*; use crate::backup::{open_backup_lockfile, BackupLockGuard}; @@ -154,19 +149,7 @@ pub fn config() -> Result<(SectionConfigData, [u8;32]), Error> { pub fn save_config(config: &SectionConfigData) -> Result<(), Error> { let raw = CONFIG.write(DATASTORE_CFG_FILENAME, &config)?; - - let backup_user = crate::backup::backup_user()?; - let mode = nix::sys::stat::Mode::from_bits_truncate(0o0640); - // set the correct owner/group/permissions while saving file - // owner(rw) = root, group(r)= backup - let options = CreateOptions::new() - .perm(mode) - .owner(nix::unistd::ROOT) - .group(backup_user.gid); - - replace_file(DATASTORE_CFG_FILENAME, raw.as_bytes(), options)?; - - Ok(()) + crate::backup::replace_backup_config(DATASTORE_CFG_FILENAME, raw.as_bytes()) } // shell completion helper diff --git a/src/config/domains.rs b/src/config/domains.rs index 9f513a44..0d695777 100644 --- a/src/config/domains.rs +++ b/src/config/domains.rs @@ -13,11 +13,6 @@ use proxmox::api::{ } }; -use proxmox::tools::fs::{ - replace_file, - CreateOptions, -}; - use crate::api2::types::*; use crate::backup::{open_backup_lockfile, BackupLockGuard}; @@ -126,19 +121,7 @@ pub fn config() -> Result<(SectionConfigData, [u8;32]), Error> { pub fn save_config(config: &SectionConfigData) -> Result<(), Error> { let raw = CONFIG.write(DOMAINS_CFG_FILENAME, &config)?; - - let backup_user = crate::backup::backup_user()?; - let mode = nix::sys::stat::Mode::from_bits_truncate(0o0640); - // set the correct owner/group/permissions while saving file - // owner(rw) = root, group(r)= backup - let options = CreateOptions::new() - .perm(mode) - .owner(nix::unistd::ROOT) - .group(backup_user.gid); - - replace_file(DOMAINS_CFG_FILENAME, raw.as_bytes(), options)?; - - Ok(()) + crate::backup::replace_backup_config(DOMAINS_CFG_FILENAME, raw.as_bytes()) } // shell completion helper diff --git a/src/config/drive.rs b/src/config/drive.rs index 9c20051f..f86582ac 100644 --- a/src/config/drive.rs +++ b/src/config/drive.rs @@ -25,10 +25,6 @@ use proxmox::{ SectionConfigPlugin, }, }, - tools::fs::{ - replace_file, - CreateOptions, - }, }; use crate::{ @@ -97,19 +93,7 @@ pub fn config() -> Result<(SectionConfigData, [u8;32]), Error> { /// Save the configuration file pub fn save_config(config: &SectionConfigData) -> Result<(), Error> { let raw = CONFIG.write(DRIVE_CFG_FILENAME, &config)?; - - let backup_user = crate::backup::backup_user()?; - let mode = nix::sys::stat::Mode::from_bits_truncate(0o0640); - // set the correct owner/group/permissions while saving file - // owner(rw) = root, group(r)= backup - let options = CreateOptions::new() - .perm(mode) - .owner(nix::unistd::ROOT) - .group(backup_user.gid); - - replace_file(DRIVE_CFG_FILENAME, raw.as_bytes(), options)?; - - Ok(()) + crate::backup::replace_backup_config(DRIVE_CFG_FILENAME, raw.as_bytes()) } /// Check if the specified drive name exists in the config. diff --git a/src/config/media_pool.rs b/src/config/media_pool.rs index e50992d8..d9828e0f 100644 --- a/src/config/media_pool.rs +++ b/src/config/media_pool.rs @@ -20,10 +20,6 @@ use proxmox::{ SectionConfigPlugin, } }, - tools::fs::{ - replace_file, - CreateOptions, - }, }; use crate::{ @@ -57,7 +53,6 @@ pub const MEDIA_POOL_CFG_FILENAME: &str = "/etc/proxmox-backup/media-pool.cfg"; /// Lock file name (used to prevent concurrent access) pub const MEDIA_POOL_CFG_LOCKFILE: &str = "/etc/proxmox-backup/.media-pool.lck"; - /// Get exclusive lock pub fn lock() -> Result { open_backup_lockfile(MEDIA_POOL_CFG_LOCKFILE, None, true) @@ -77,19 +72,7 @@ pub fn config() -> Result<(SectionConfigData, [u8;32]), Error> { /// Save the configuration file pub fn save_config(config: &SectionConfigData) -> Result<(), Error> { let raw = CONFIG.write(MEDIA_POOL_CFG_FILENAME, &config)?; - - let backup_user = crate::backup::backup_user()?; - let mode = nix::sys::stat::Mode::from_bits_truncate(0o0640); - // set the correct owner/group/permissions while saving file - // owner(rw) = root, group(r)= backup - let options = CreateOptions::new() - .perm(mode) - .owner(nix::unistd::ROOT) - .group(backup_user.gid); - - replace_file(MEDIA_POOL_CFG_FILENAME, raw.as_bytes(), options)?; - - Ok(()) + crate::backup::replace_backup_config(MEDIA_POOL_CFG_FILENAME, raw.as_bytes()) } // shell completion helper diff --git a/src/config/mod.rs b/src/config/mod.rs index 014d184f..d820ee37 100644 --- a/src/config/mod.rs +++ b/src/config/mod.rs @@ -10,7 +10,6 @@ use openssl::rsa::{Rsa}; use openssl::x509::{X509Builder}; use openssl::pkey::PKey; -use proxmox::tools::fs::{CreateOptions, replace_file}; use proxmox::try_block; use pbs_buildcfg::{self, configdir}; @@ -194,18 +193,13 @@ pub fn update_self_signed_cert(force: bool) -> Result<(), Error> { } pub(crate) fn set_proxy_certificate(cert_pem: &[u8], key_pem: &[u8]) -> Result<(), Error> { - let backup_user = crate::backup::backup_user()?; - let options = CreateOptions::new() - .perm(Mode::from_bits_truncate(0o0640)) - .owner(nix::unistd::ROOT) - .group(backup_user.gid); let key_path = PathBuf::from(configdir!("/proxy.key")); let cert_path = PathBuf::from(configdir!("/proxy.pem")); create_configdir()?; - replace_file(&key_path, &key_pem, options.clone()) + crate::backup::replace_backup_config(&key_path, key_pem) .map_err(|err| format_err!("error writing certificate private key - {}", err))?; - replace_file(&cert_path, &cert_pem, options) + crate::backup::replace_backup_config(&cert_path, &cert_pem) .map_err(|err| format_err!("error writing certificate file - {}", err))?; Ok(()) diff --git a/src/config/node.rs b/src/config/node.rs index dc3eeeb0..0c1c9938 100644 --- a/src/config/node.rs +++ b/src/config/node.rs @@ -6,7 +6,6 @@ use serde::{Deserialize, Serialize}; use proxmox::api::api; use proxmox::api::schema::{ApiStringFormat, Updater}; -use proxmox::tools::fs::{replace_file, CreateOptions}; use proxmox_http::ProxyConfig; @@ -41,14 +40,7 @@ pub fn save_config(config: &NodeConfig) -> Result<(), Error> { config.validate()?; let raw = crate::tools::config::to_bytes(config, &NodeConfig::API_SCHEMA)?; - - let backup_user = crate::backup::backup_user()?; - let options = CreateOptions::new() - .perm(Mode::from_bits_truncate(0o0640)) - .owner(nix::unistd::ROOT) - .group(backup_user.gid); - - replace_file(CONF_FILE, &raw, options) + crate::backup::replace_backup_config(CONF_FILE, &raw) } #[api( diff --git a/src/config/remote.rs b/src/config/remote.rs index 0ef70677..86fe7b6e 100644 --- a/src/config/remote.rs +++ b/src/config/remote.rs @@ -13,8 +13,6 @@ use proxmox::api::{ } }; -use proxmox::tools::{fs::replace_file, fs::CreateOptions}; - use crate::api2::types::*; lazy_static! { @@ -102,19 +100,7 @@ pub fn config() -> Result<(SectionConfigData, [u8;32]), Error> { pub fn save_config(config: &SectionConfigData) -> Result<(), Error> { let raw = CONFIG.write(REMOTE_CFG_FILENAME, &config)?; - - let backup_user = crate::backup::backup_user()?; - let mode = nix::sys::stat::Mode::from_bits_truncate(0o0640); - // set the correct owner/group/permissions while saving file - // owner(rw) = root, group(r)= backup - let options = CreateOptions::new() - .perm(mode) - .owner(nix::unistd::ROOT) - .group(backup_user.gid); - - replace_file(REMOTE_CFG_FILENAME, raw.as_bytes(), options)?; - - Ok(()) + crate::backup::replace_backup_config(REMOTE_CFG_FILENAME, raw.as_bytes()) } // shell completion helper diff --git a/src/config/sync.rs b/src/config/sync.rs index 2fd3a2c1..5d5b2060 100644 --- a/src/config/sync.rs +++ b/src/config/sync.rs @@ -13,8 +13,6 @@ use proxmox::api::{ } }; -use proxmox::tools::{fs::replace_file, fs::CreateOptions}; - use crate::api2::types::*; lazy_static! { @@ -120,19 +118,7 @@ pub fn config() -> Result<(SectionConfigData, [u8;32]), Error> { pub fn save_config(config: &SectionConfigData) -> Result<(), Error> { let raw = CONFIG.write(SYNC_CFG_FILENAME, &config)?; - - let backup_user = crate::backup::backup_user()?; - let mode = nix::sys::stat::Mode::from_bits_truncate(0o0640); - // set the correct owner/group/permissions while saving file - // owner(rw) = root, group(r)= backup - let options = CreateOptions::new() - .perm(mode) - .owner(nix::unistd::ROOT) - .group(backup_user.gid); - - replace_file(SYNC_CFG_FILENAME, raw.as_bytes(), options)?; - - Ok(()) + crate::backup::replace_backup_config(SYNC_CFG_FILENAME, raw.as_bytes()) } // shell completion helper diff --git a/src/config/tape_encryption_keys.rs b/src/config/tape_encryption_keys.rs index 5ee0ac1f..6d4e91b9 100644 --- a/src/config/tape_encryption_keys.rs +++ b/src/config/tape_encryption_keys.rs @@ -15,11 +15,7 @@ use std::collections::HashMap; use anyhow::{bail, Error}; use serde::{Deserialize, Serialize}; -use proxmox::tools::fs::{ - file_read_optional_string, - replace_file, - CreateOptions, -}; +use proxmox::tools::fs::file_read_optional_string; use crate::{ backup::{ @@ -143,18 +139,7 @@ pub fn save_keys(map: HashMap) -> Result<(), Err } let raw = serde_json::to_string_pretty(&list)?; - - let mode = nix::sys::stat::Mode::from_bits_truncate(0o0600); - // set the correct owner/group/permissions while saving file - // owner(rw) = root, group(r)= root - let options = CreateOptions::new() - .perm(mode) - .owner(nix::unistd::ROOT) - .group(nix::unistd::Gid::from_raw(0)); - - replace_file(TAPE_KEYS_FILENAME, raw.as_bytes(), options)?; - - Ok(()) + crate::backup::replace_secret_config(TAPE_KEYS_FILENAME, raw.as_bytes()) } /// Store tape encryption key configurations (password protected keys) @@ -167,19 +152,7 @@ pub fn save_key_configs(map: HashMap) -> Result<(), Erro } let raw = serde_json::to_string_pretty(&list)?; - - let backup_user = crate::backup::backup_user()?; - let mode = nix::sys::stat::Mode::from_bits_truncate(0o0640); - // set the correct owner/group/permissions while saving file - // owner(rw) = root, group(r)= backup - let options = CreateOptions::new() - .perm(mode) - .owner(nix::unistd::ROOT) - .group(backup_user.gid); - - replace_file(TAPE_KEY_CONFIG_FILENAME, raw.as_bytes(), options)?; - - Ok(()) + crate::backup::replace_backup_config(TAPE_KEY_CONFIG_FILENAME, raw.as_bytes()) } /// Insert a new key diff --git a/src/config/tape_job.rs b/src/config/tape_job.rs index a5901e86..f09200fc 100644 --- a/src/config/tape_job.rs +++ b/src/config/tape_job.rs @@ -13,8 +13,6 @@ use proxmox::api::{ } }; -use proxmox::tools::{fs::replace_file, fs::CreateOptions}; - use crate::api2::types::{ Userid, JOB_ID_SCHEMA, @@ -159,19 +157,7 @@ pub fn config() -> Result<(SectionConfigData, [u8;32]), Error> { pub fn save_config(config: &SectionConfigData) -> Result<(), Error> { let raw = CONFIG.write(TAPE_JOB_CFG_FILENAME, &config)?; - - let backup_user = crate::backup::backup_user()?; - let mode = nix::sys::stat::Mode::from_bits_truncate(0o0640); - // set the correct owner/group/permissions while saving file - // owner(rw) = root, group(r)= backup - let options = CreateOptions::new() - .perm(mode) - .owner(nix::unistd::ROOT) - .group(backup_user.gid); - - replace_file(TAPE_JOB_CFG_FILENAME, raw.as_bytes(), options)?; - - Ok(()) + crate::backup::replace_backup_config(TAPE_JOB_CFG_FILENAME, raw.as_bytes()) } // shell completion helper diff --git a/src/config/user.rs b/src/config/user.rs index bdec5fc1..8d367ade 100644 --- a/src/config/user.rs +++ b/src/config/user.rs @@ -15,8 +15,6 @@ use proxmox::api::{ } }; -use proxmox::tools::{fs::replace_file, fs::CreateOptions}; - use crate::api2::types::*; use crate::tools::Memcom; @@ -259,17 +257,7 @@ pub fn cached_config() -> Result, Error> { pub fn save_config(config: &SectionConfigData) -> Result<(), Error> { let raw = CONFIG.write(USER_CFG_FILENAME, &config)?; - - let backup_user = crate::backup::backup_user()?; - let mode = nix::sys::stat::Mode::from_bits_truncate(0o0640); - // set the correct owner/group/permissions while saving file - // owner(rw) = root, group(r)= backup - let options = CreateOptions::new() - .perm(mode) - .owner(nix::unistd::ROOT) - .group(backup_user.gid); - - replace_file(USER_CFG_FILENAME, raw.as_bytes(), options)?; + crate::backup::replace_backup_config(USER_CFG_FILENAME, raw.as_bytes())?; // increase user cache generation // We use this in CachedUserInfo diff --git a/src/config/verify.rs b/src/config/verify.rs index 549f9801..9001fffc 100644 --- a/src/config/verify.rs +++ b/src/config/verify.rs @@ -13,8 +13,6 @@ use proxmox::api::{ } }; -use proxmox::tools::{fs::replace_file, fs::CreateOptions}; - use crate::api2::types::*; lazy_static! { @@ -118,20 +116,7 @@ pub fn config() -> Result<(SectionConfigData, [u8;32]), Error> { pub fn save_config(config: &SectionConfigData) -> Result<(), Error> { let raw = CONFIG.write(VERIFICATION_CFG_FILENAME, &config)?; - - let backup_user = crate::backup::backup_user()?; - let mode = nix::sys::stat::Mode::from_bits_truncate(0o0640); - // set the correct owner/group/permissions while saving file - // owner(rw) = root, group(r)= backup - - let options = CreateOptions::new() - .perm(mode) - .owner(nix::unistd::ROOT) - .group(backup_user.gid); - - replace_file(VERIFICATION_CFG_FILENAME, raw.as_bytes(), options)?; - - Ok(()) + crate::backup::replace_backup_config(VERIFICATION_CFG_FILENAME, raw.as_bytes()) } // shell completion helper -- 2.30.2