split proxmox-file-restore into its own crate

This also moves a couple of required utilities such as
logrotate and some file descriptor methods to pbs-tools.

Note that the logrotate usage and run-dir handling should be
improved to work as a regular user as this *should* (IMHO)
be a regular unprivileged command (including running
qemu given the kvm privileges...)

Signed-off-by: Wolfgang Bumiller <w.bumiller@proxmox.com>

Cargo.toml

@@ -31,6 +31,7 @@ members = [
     "proxmox-backup-banner",
     "proxmox-backup-client",
     "proxmox-backup-debug",
+    "proxmox-file-restore",
     "pxar-bin",
 ]
 
@@ -83,7 +84,6 @@ url = "2.1"
 walkdir = "2"
 webauthn-rs = "0.2.5"
 xdg = "2.2"
-zstd = { version = "0.6", features = [ "bindgen" ] }
 nom = "5.1"
 crossbeam-channel = "0.5"
 

Makefile

@@ -43,6 +43,7 @@ SUBCRATES := \
 	proxmox-backup-banner \
 	proxmox-backup-client \
 	proxmox-backup-debug \
+	proxmox-file-restore \
 	pxar-bin
 
 ifeq ($(BUILD_MODE), release)
@@ -174,6 +175,8 @@ $(COMPILED_BINS) $(COMPILEDIR)/dump-catalog-shell-cli $(COMPILEDIR)/docgen: .do-
 	    --bin proxmox-backup-client \
 	    --package proxmox-backup-debug \
 	    --bin proxmox-backup-debug \
+	    --package proxmox-file-restore \
+	    --bin proxmox-file-restore \
 	    --package pxar-bin \
 	    --bin pxar \
 	    --package proxmox-backup \

pbs-buildcfg/src/lib.rs

@@ -14,6 +14,11 @@ pub const PROXMOX_PKG_REPOID: &str = env!("REPOID");
 pub const CONFIGDIR: &str = "/etc/proxmox-backup";
 pub const JS_DIR: &str = "/usr/share/javascript/proxmox-backup";
 
+/// Unix system user used by proxmox-backup-proxy
+pub const BACKUP_USER_NAME: &str = "backup";
+/// Unix system group used by proxmox-backup-proxy
+pub const BACKUP_GROUP_NAME: &str = "backup";
+
 #[macro_export]
 macro_rules! PROXMOX_BACKUP_RUN_DIR_M { () => ("/run/proxmox-backup") }
 

pbs-tools/Cargo.toml

@@ -28,6 +28,7 @@ serde_json = "1.0"
 tokio = { version = "1.6", features = [ "fs", "io-util", "rt", "rt-multi-thread", "sync" ] }
 url = "2.1"
 walkdir = "2"
+zstd = { version = "0.6", features = [ "bindgen" ] }
 
 proxmox = { version = "0.13.0", default-features = false, features = [ "tokio" ] }
 

pbs-tools/src/fd.rs

@@ -0,0 +1,14 @@
+//! Raw file descriptor related utilities.
+
+use std::os::unix::io::RawFd;
+
+use anyhow::Error;
+use nix::fcntl::{fcntl, FdFlag, F_GETFD, F_SETFD};
+
+/// Change the `O_CLOEXEC` flag of an existing file descriptor.
+pub fn fd_change_cloexec(fd: RawFd, on: bool) -> Result<(), Error> {
+    let mut flags = unsafe { FdFlag::from_bits_unchecked(fcntl(fd, F_GETFD)?) };
+    flags.set(FdFlag::FD_CLOEXEC, on);
+    fcntl(fd, F_SETFD(flags))?;
+    Ok(())
+}

pbs-tools/src/lib.rs

@@ -7,9 +7,11 @@ pub mod cert;
 pub mod cli;
 pub mod compression;
 pub mod format;
+pub mod fd;
 pub mod fs;
 pub mod io;
 pub mod json;
+pub mod logrotate;
 pub mod lru_cache;
 pub mod nom;
 pub mod ops;
@@ -19,6 +21,7 @@ pub mod sha;
 pub mod str;
 pub mod stream;
 pub mod sync;
+pub mod sys;
 pub mod ticket;
 pub mod tokio;
 pub mod xattr;

pbs-tools/src/logrotate.rs

@@ -3,7 +3,7 @@ use std::fs::{File, rename};
 use std::os::unix::io::{FromRawFd, IntoRawFd};
 use std::io::Read;
 
-use anyhow::{bail, Error};
+use anyhow::{bail, format_err, Error};
 use nix::unistd;
 
 use proxmox::tools::fs::{CreateOptions, make_tmp_file};
@@ -12,18 +12,37 @@ use proxmox::tools::fs::{CreateOptions, make_tmp_file};
 pub struct LogRotate {
     base_path: PathBuf,
     compress: bool,
+
+    /// User logs should be reowned to.
+    owner: Option<String>,
 }
 
 impl LogRotate {
-    /// Creates a new instance if the path given is a valid file name
+    /// Creates a new instance if the path given is a valid file name (iow. does not end with ..)
-    /// (iow. does not end with ..)
+    /// 'compress' decides if compresses files will be created on rotation, and if it will search
-    /// 'compress' decides if compresses files will be created on
+    /// '.zst' files when iterating
-    /// rotation, and if it will search '.zst' files when iterating
+    ///
-    pub fn new<P: AsRef<Path>>(path: P, compress: bool) -> Option<Self> {
+    /// By default, newly created files will be owned by the backup user. See [`new_with_user`] for
+    /// a way to opt out of this behavior.
+    pub fn new<P: AsRef<Path>>(
+        path: P,
+        compress: bool,
+    ) -> Option<Self> {
+        Self::new_with_user(path, compress, Some(pbs_buildcfg::BACKUP_USER_NAME.to_owned()))
+    }
+
+    /// See [`new`]. Additionally this also takes a user which should by default be used to reown
+    /// new files to.
+    pub fn new_with_user<P: AsRef<Path>>(
+        path: P,
+        compress: bool,
+        owner: Option<String>,
+    ) -> Option<Self> {
         if path.as_ref().file_name().is_some() {
             Some(Self {
                 base_path: path.as_ref().to_path_buf(),
                 compress,
+                owner,
             })
         } else {
             None
@@ -133,10 +152,16 @@ impl LogRotate {
 
         let options = match options {
             Some(options) => options,
-            None => {
+            None => match self.owner.as_deref() {
-                let backup_user = crate::backup::backup_user()?;
+                Some(owner) => {
-                CreateOptions::new().owner(backup_user.uid).group(backup_user.gid)
+                    let user = crate::sys::query_user(owner)?
-            },
+                        .ok_or_else(|| {
+                            format_err!("failed to lookup owning user '{}' for logs", owner)
+                        })?;
+                    CreateOptions::new().owner(user.uid).group(user.gid)
+                }
+                None => CreateOptions::new(),
+            }
         };
 
         let metadata = match self.base_path.metadata() {

pbs-tools/src/sys.rs

@@ -0,0 +1,31 @@
+//! System level helpers.
+
+use nix::unistd::{Gid, Group, Uid, User};
+
+/// Query a user by name but only unless built with `#[cfg(test)]`.
+///
+/// This is to avoid having regression tests query the users of development machines which may
+/// not be compatible with PBS or privileged enough.
+pub fn query_user(user_name: &str) -> Result<Option<User>, nix::Error> {
+    if cfg!(test) {
+        Ok(Some(
+            User::from_uid(Uid::current())?.expect("current user does not exist"),
+        ))
+    } else {
+        User::from_name(user_name)
+    }
+}
+
+/// Query a group by name but only unless built with `#[cfg(test)]`.
+///
+/// This is to avoid having regression tests query the groups of development machines which may
+/// not be compatible with PBS or privileged enough.
+pub fn query_group(group_name: &str) -> Result<Option<Group>, nix::Error> {
+    if cfg!(test) {
+        Ok(Some(
+            Group::from_gid(Gid::current())?.expect("current group does not exist"),
+        ))
+    } else {
+        Group::from_name(group_name)
+    }
+}

proxmox-backup-debug/Cargo.toml

@@ -9,7 +9,7 @@ anyhow = "1.0"
 walkdir = "2"
 serde_json = "1.0"
 
-proxmox = { version = "0.13.0", features = [ "api-macro", "cli", "router" ] }
+proxmox = { version = "0.13.0", features = [ "api-macro", "cli" ] }
 
 pbs-client = { path = "../pbs-client" }
 pbs-datastore = { path = "../pbs-datastore" }

proxmox-file-restore/Cargo.toml

@@ -0,0 +1,27 @@
+[package]
+name = "proxmox-file-restore"
+version = "0.1.0"
+authors = ["Proxmox Support Team <support@proxmox.com>"]
+edition = "2018"
+
+[dependencies]
+anyhow = "1.0"
+base64 = "0.12"
+futures = "0.3"
+libc = "0.2"
+nix = "0.19.1"
+serde = { version = "1.0", features = ["derive"] }
+serde_json = "1.0"
+tokio = { version = "1.6", features = [ "io-std", "rt", "rt-multi-thread", "time" ] }
+
+pxar = { version = "0.10.1", features = [ "tokio-io" ] }
+
+proxmox = { version = "0.13.0", features = [ "api-macro", "cli" ] }
+
+pbs-api-types = { path = "../pbs-api-types" }
+pbs-buildcfg = { path = "../pbs-buildcfg" }
+pbs-client = { path = "../pbs-client" }
+pbs-datastore = { path = "../pbs-datastore" }
+pbs-runtime = { path = "../pbs-runtime" }
+pbs-systemd = { path = "../pbs-systemd" }
+pbs-tools = { path = "../pbs-tools" }

proxmox-file-restore/src/cpio.rs

@@ -3,8 +3,9 @@
 //! https://www.kernel.org/doc/html/latest/driver-api/early-userspace/buffer-format.html
 //! This does not provide full support for the format, only what is needed to include files in an
 //! initramfs intended for a linux kernel.
-use anyhow::{bail, Error};
 use std::ffi::{CString, CStr};
+
+use anyhow::{bail, Error};
 use tokio::io::{copy, AsyncRead, AsyncReadExt, AsyncWrite, AsyncWriteExt};
 
 /// Write a cpio file entry to an AsyncWrite.

proxmox-file-restore/src/main.rs

@@ -13,6 +13,7 @@ use proxmox::api::{
         run_cli_command, CliCommand, CliCommandMap, CliEnvironment, ColumnConfig, OUTPUT_FORMAT,
     },
 };
+use proxmox::tools::fs::{create_path, CreateOptions};
 use pxar::accessor::aio::Accessor;
 use pxar::decoder::aio::Decoder;
 
@@ -34,10 +35,13 @@ use pbs_client::tools::{
     REPO_URL_SCHEMA,
 };
 
-use proxmox_backup::tools;
+pub mod block_driver;
+pub use block_driver::*;
 
-mod proxmox_file_restore;
+pub mod cpio;
-use proxmox_file_restore::*;
+
+mod qemu_helper;
+mod block_driver_qemu;
 
 enum ExtractPath {
     ListArchives,
@@ -484,7 +488,24 @@ pub fn get_user_run_dir() -> Result<std::path::PathBuf, Error> {
     let uid = nix::unistd::Uid::current();
     let mut path: std::path::PathBuf = pbs_buildcfg::PROXMOX_BACKUP_RUN_DIR.into();
     path.push(uid.to_string());
-    tools::create_run_dir()?;
+    create_run_dir()?;
     std::fs::create_dir_all(&path)?;
     Ok(path)
 }
+
+/// FIXME: proxmox-file-restore should not depend on this!
+fn create_run_dir() -> Result<(), Error> {
+    let backup_user = backup_user()?;
+    let opts = CreateOptions::new()
+        .owner(backup_user.uid)
+        .group(backup_user.gid);
+    let _: bool = create_path(pbs_buildcfg::PROXMOX_BACKUP_RUN_DIR_M!(), None, Some(opts))?;
+    Ok(())
+}
+
+/// Return User info for the 'backup' user (``getpwnam_r(3)``)
+pub fn backup_user() -> Result<nix::unistd::User, Error> {
+    pbs_tools::sys::query_user(pbs_buildcfg::BACKUP_USER_NAME)?
+        .ok_or_else(|| format_err!("Unable to lookup '{}' user.", pbs_buildcfg::BACKUP_USER_NAME))
+}
+

proxmox-file-restore/src/qemu_helper.rs

@@ -15,9 +15,7 @@ use proxmox::tools::fs::{create_path, file_read_string, make_tmp_file, CreateOpt
 
 use pbs_client::{VsockClient, DEFAULT_VSOCK_PORT};
 
-use proxmox_backup::backup::backup_user;
+use crate::{cpio, backup_user};
-use proxmox_backup::tools;
-
 use super::SnapRestoreDetails;
 
 const PBS_VM_NAME: &str = "pbs-restore-vm";
@@ -88,7 +86,7 @@ async fn create_temp_initramfs(ticket: &str, debug: bool) -> Result<(File, Strin
     let (tmp_file, tmp_path) =
         make_tmp_file("/tmp/file-restore-qemu.initramfs.tmp", CreateOptions::new())?;
     nix::unistd::unlink(&tmp_path)?;
-    tools::fd_change_cloexec(tmp_file.as_raw_fd(), false)?;
+    pbs_tools::fd::fd_change_cloexec(tmp_file.as_raw_fd(), false)?;
 
     let initramfs = if debug {
         pbs_buildcfg::PROXMOX_BACKUP_INITRAMFS_DBG_FN
@@ -102,7 +100,7 @@ async fn create_temp_initramfs(ticket: &str, debug: bool) -> Result<(File, Strin
     tokio::io::copy(&mut base, &mut f).await?;
 
     let name = CString::new("ticket").unwrap();
-    tools::cpio::append_file(
+    cpio::append_file(
         &mut f,
         ticket.as_bytes(),
         &name,
@@ -114,7 +112,7 @@ async fn create_temp_initramfs(ticket: &str, debug: bool) -> Result<(File, Strin
         ticket.len() as u32,
     )
     .await?;
-    tools::cpio::append_trailer(&mut f).await?;
+    cpio::append_trailer(&mut f).await?;
 
     let tmp_file = f.into_std().await;
     let path = format!("/dev/fd/{}", &tmp_file.as_raw_fd());
@@ -144,13 +142,13 @@ pub async fn start_vm(
     let pid;
     let (mut pid_file, pid_path) = make_tmp_file("/tmp/file-restore-qemu.pid.tmp", CreateOptions::new())?;
     nix::unistd::unlink(&pid_path)?;
-    tools::fd_change_cloexec(pid_file.as_raw_fd(), false)?;
+    pbs_tools::fd::fd_change_cloexec(pid_file.as_raw_fd(), false)?;
 
     let (_ramfs_pid, ramfs_path) = create_temp_initramfs(ticket, debug).await?;
 
     let logpath = create_restore_log_dir()?;
     let logfile = &format!("{}/qemu.log", logpath);
-    let mut logrotate = tools::logrotate::LogRotate::new(logfile, false)
+    let mut logrotate = pbs_tools::logrotate::LogRotate::new(logfile, false)
         .ok_or_else(|| format_err!("could not get QEMU log file names"))?;
 
     if let Err(err) = logrotate.do_rotate(CreateOptions::default(), Some(16)) {
@@ -161,7 +159,7 @@ pub async fn start_vm(
         .append(true)
         .create_new(true)
         .open(logfile)?;
-    tools::fd_change_cloexec(logfd.as_raw_fd(), false)?;
+    pbs_tools::fd::fd_change_cloexec(logfd.as_raw_fd(), false)?;
 
     // preface log file with start timestamp so one can see how long QEMU took to start
     writeln!(logfd, "[{}] PBS file restore VM log", {

src/backup/mod.rs

@@ -1,39 +1,22 @@
 //! Server/client-specific parts for what's otherwise in pbs-datastore.
 
-use anyhow::{bail, Error};
+use anyhow::{format_err, Error};
 
 // Note: .pcat1 => Proxmox Catalog Format version 1
 pub const CATALOG_NAME: &str = "catalog.pcat1.didx";
 
-/// Unix system user used by proxmox-backup-proxy
+pub use pbs_buildcfg::{BACKUP_USER_NAME, BACKUP_GROUP_NAME};
-pub const BACKUP_USER_NAME: &str = "backup";
-/// Unix system group used by proxmox-backup-proxy
-pub const BACKUP_GROUP_NAME: &str = "backup";
 
 /// Return User info for the 'backup' user (``getpwnam_r(3)``)
 pub fn backup_user() -> Result<nix::unistd::User, Error> {
-    if cfg!(test) {
+    pbs_tools::sys::query_user(BACKUP_USER_NAME)?
-        // fix permission problems with regressions test (when run as non-root).
+        .ok_or_else(|| format_err!("Unable to lookup '{}' user.", BACKUP_USER_NAME))
-        Ok(nix::unistd::User::from_uid(nix::unistd::Uid::current())?.unwrap())
-    } else {
-        match nix::unistd::User::from_name(BACKUP_USER_NAME)? {
-            Some(user) => Ok(user),
-            None => bail!("Unable to lookup backup user."),
-        }
-    }
 }
 
 /// Return Group info for the 'backup' group (``getgrnam(3)``)
 pub fn backup_group() -> Result<nix::unistd::Group, Error> {
-    if cfg!(test) {
+    pbs_tools::sys::query_group(BACKUP_GROUP_NAME)?
-        // fix permission problems with regressions test (when run as non-root).
+        .ok_or_else(|| format_err!("Unable to lookup '{}' group.", BACKUP_GROUP_NAME))
-        Ok(nix::unistd::Group::from_gid(nix::unistd::Gid::current())?.unwrap())
-    } else {
-        match nix::unistd::Group::from_name(BACKUP_GROUP_NAME)? {
-            Some(group) => Ok(group),
-            None => bail!("Unable to lookup backup user."),
-        }
-    }
 }
 
 // Split

src/bin/proxmox-backup-api.rs

@@ -38,7 +38,7 @@ async fn run() -> Result<(), Error> {
 
     config::update_self_signed_cert(false)?;
 
-    proxmox_backup::tools::create_run_dir()?;
+    proxmox_backup::server::create_run_dir()?;
 
     proxmox_backup::rrd::create_rrdb_dir()?;
     proxmox_backup::server::jobstate::create_jobstate_dir()?;

src/bin/proxmox-backup-proxy.rs

@@ -30,6 +30,7 @@ use proxmox_backup::{
 
 use pbs_buildcfg::configdir;
 use pbs_systemd::time::{compute_next_event, parse_calendar_event};
+use pbs_tools::logrotate::LogRotate;
 
 use proxmox_backup::api2::types::Authid;
 use proxmox_backup::server;
@@ -42,7 +43,6 @@ use proxmox_backup::tools::{
         zfs_pool_stats,
         get_pool_from_dataset,
     },
-    logrotate::LogRotate,
 };
 
 use proxmox_backup::api2::pull::do_sync_job;

src/bin/proxmox_file_restore/mod.rs

@@ -1,6 +0,0 @@
-//! Block device drivers and tools for single file restore
-pub mod block_driver;
-pub use block_driver::*;
-
-mod qemu_helper;
-mod block_driver_qemu;

src/config/tfa.rs

@@ -497,7 +497,7 @@ impl TfaUserChallengeData {
     /// Load the user's current challenges with the intent to create a challenge (create the file
     /// if it does not exist), and keep a lock on the file.
     fn open(userid: &Userid) -> Result<Self, Error> {
-        crate::tools::create_run_dir()?;
+        crate::server::create_run_dir()?;
         let options = CreateOptions::new().perm(Mode::from_bits_truncate(0o0600));
         proxmox::tools::fs::create_path(CHALLENGE_DATA_PATH, Some(options.clone()), Some(options))
             .map_err(|err| {

src/server/mod.rs

@@ -10,6 +10,7 @@ use nix::unistd::Pid;
 use serde_json::Value;
 
 use proxmox::sys::linux::procfs::PidStat;
+use proxmox::tools::fs::{create_path, CreateOptions};
 
 use pbs_buildcfg;
 
@@ -28,8 +29,7 @@ pub fn pstart() -> u64 {
 
 pub fn write_pid(pid_fn: &str) -> Result<(), Error> {
     let pid_str = format!("{}\n", *PID);
-    let opts = proxmox::tools::fs::CreateOptions::new();
+    proxmox::tools::fs::replace_file(pid_fn, pid_str.as_bytes(), CreateOptions::new())
-    proxmox::tools::fs::replace_file(pid_fn, pid_str.as_bytes(), opts)
 }
 
 pub fn read_pid(pid_fn: &str) -> Result<i32, Error> {
@@ -110,3 +110,16 @@ pub(crate) async fn notify_datastore_removed() -> Result<(), Error> {
         .await?;
     Ok(())
 }
+
+/// Create the base run-directory.
+///
+/// This exists to fixate the permissions for the run *base* directory while allowing intermediate
+/// directories after it to have different permissions.
+pub fn create_run_dir() -> Result<(), Error> {
+    let backup_user = crate::backup::backup_user()?;
+    let opts = CreateOptions::new()
+        .owner(backup_user.uid)
+        .group(backup_user.gid);
+    let _: bool = create_path(pbs_buildcfg::PROXMOX_BACKUP_RUN_DIR_M!(), None, Some(opts))?;
+    Ok(())
+}

src/server/worker_task.rs

@@ -16,12 +16,12 @@ use proxmox::sys::linux::procfs;
 use proxmox::try_block;
 use proxmox::tools::fs::{create_path, replace_file, CreateOptions};
 
+use pbs_buildcfg;
+use pbs_tools::logrotate::{LogRotate, LogRotateFiles};
+
 use super::{UPID, UPIDExt};
 
-use pbs_buildcfg;
-
 use crate::server;
-use crate::tools::logrotate::{LogRotate, LogRotateFiles};
 use crate::tools::{FileLogger, FileLogOptions};
 use crate::api2::types::{Authid, TaskStateType};
 use crate::backup::{open_backup_lockfile, BackupLockGuard};

src/tools/mod.rs

@@ -8,7 +8,6 @@ use anyhow::{bail, format_err, Error};
 use openssl::hash::{hash, DigestBytes, MessageDigest};
 
 pub use proxmox::tools::fd::Fd;
-use proxmox::tools::fs::{create_path, CreateOptions};
 
 use proxmox_http::{
     client::SimpleHttp,
@@ -27,14 +26,12 @@ pub mod apt;
 pub mod async_io;
 pub mod compression;
 pub mod config;
-pub mod cpio;
 pub mod daemon;
 pub mod disks;
 
 mod memcom;
 pub use memcom::Memcom;
 
-pub mod logrotate;
 pub mod serde_filter;
 pub mod statistics;
 pub mod subscription;
@@ -211,16 +208,3 @@ pub fn setup_safe_path_env() {
         std::env::remove_var(name);
     }
 }
-
-/// Create the base run-directory.
-///
-/// This exists to fixate the permissions for the run *base* directory while allowing intermediate
-/// directories after it to have different permissions.
-pub fn create_run_dir() -> Result<(), Error> {
-    let backup_user = crate::backup::backup_user()?;
-    let opts = CreateOptions::new()
-        .owner(backup_user.uid)
-        .group(backup_user.gid);
-    let _: bool = create_path(pbs_buildcfg::PROXMOX_BACKUP_RUN_DIR_M!(), None, Some(opts))?;
-    Ok(())
-}