Merge pull request #2657 from cgwalters/live-tmpfiles

rust: Introduce systemd-run based isolation mod, use in live

rust/src/isolation.rs

@@ -0,0 +1,52 @@
+//! APIs for multi-process isolation
+// SPDX-License-Identifier: Apache-2.0 OR MIT
+
+use anyhow::{anyhow, Result};
+use fn_error_context::context;
+use std::process::Command;
+
+const SELF_UNIT: &str = "rpm-ostreed.service";
+/// Run as a child process, synchronously.
+const BASE_ARGS: &[&str] = &["--wait", "--pipe", "--no-ask-password", "--quiet"];
+
+/// Configuration for transient unit.
+pub(crate) struct UnitConfig<'a> {
+    /// If provided, will be used as the name of the unit
+    pub(crate) name: Option<&'a str>,
+    /// Unit/Service properties, e.g. DynamicUser=yes
+    pub(crate) properties: &'a [&'a str],
+    /// The command to execute
+    pub(crate) exec_args: &'a [&'a str],
+}
+
+/// Create a child process via `systemd-run` and synchronously wait
+/// for its completion.  This runs in `--pipe` mode, so e.g. stdout/stderr
+/// will go to the parent process.
+/// Use this for isolation, as well as to escape the parent rpm-ostreed.service
+/// isolation like `ProtectHome=true`.
+#[context("Running systemd worker")]
+pub(crate) fn run_systemd_worker_sync(cfg: &UnitConfig) -> Result<()> {
+    if !systemd::daemon::booted()? {
+        return Err(anyhow!("Not running under systemd"));
+    }
+    let mut cmd = Command::new("systemd-run");
+    cmd.args(BASE_ARGS);
+    if let Some(name) = cfg.name {
+        cmd.arg("--unit");
+        cmd.arg(name);
+    }
+    for prop in cfg.properties.iter() {
+        cmd.arg("--property");
+        cmd.arg(prop);
+    }
+    // This ensures that this unit won't escape our process.
+    cmd.arg(format!("--property=BindsTo={}", SELF_UNIT));
+    cmd.arg(format!("--property=After={}", SELF_UNIT));
+    cmd.arg("--");
+    cmd.args(cfg.exec_args);
+    let status = cmd.status()?;
+    if !status.success() {
+        return Err(anyhow!("{}", status));
+    }
+    Ok(())
+}

rust/src/lib.rs

@@ -413,6 +413,7 @@ pub(crate) use extensions::*;
 mod fedora_integration;
 mod history;
 pub use self::history::*;
+mod isolation;
 mod journal;
 pub(crate) use self::journal::*;
 mod initramfs;

rust/src/live.rs

@@ -8,8 +8,10 @@
  */
 
 use crate::ffi::LiveApplyState;
+use crate::isolation;
 use crate::{cxxrsutil::*, variant_utils};
-use anyhow::{anyhow, bail, Context, Result};
+use anyhow::{anyhow, Context, Result};
+use fn_error_context::context;
 use nix::sys::statvfs;
 use openat_ext::OpenatDirExt;
 use ostree::DeploymentUnlockedState;
@@ -18,7 +20,6 @@ use std::borrow::Cow;
 use std::os::unix::io::AsRawFd;
 use std::path::{Path, PathBuf};
 use std::pin::Pin;
-use std::process::Command;
 use variant_utils::{variant_dict_lookup_bool, variant_dict_lookup_str};
 
 /// GVariant `s`: Choose a specific commit
@@ -301,50 +302,34 @@ fn update_etc(
 // we actually need to escape our mount namespace and affect
 // the "main" mount namespace so that other processes will
 // see the overlayfs.
+#[context("Creating overlayfs")]
 fn unlock_transient(sysroot: &ostree::Sysroot) -> Result<()> {
     // Temporarily drop the lock
     sysroot.unlock();
-    let status = Command::new("systemd-run")
+    isolation::run_systemd_worker_sync(&isolation::UnitConfig {
-        .args(&[
+        name: Some("rpm-ostree-unlock"),
-            "-u",
+        properties: &[],
-            "rpm-ostree-unlock",
+        exec_args: &["ostree", "admin", "unlock", "--transient"],
-            "--wait",
+    })?;
-            "--",
-            "ostree",
-            "admin",
-            "unlock",
-            "--transient",
-        ])
-        .status();
-    sysroot.lock()?;
-    let status = status?;
-    if !status.success() {
-        bail!("Failed to unlock --transient");
-    }
     Ok(())
 }
 
-/// Run `systemd-tmpfiles` via `systemd-run` so we escape our mount namespace.
+/// Run `systemd-tmpfiles` as a separate systemd unit to escape
-/// This allows our `ProtectHome=` in the unit file to work.
+/// our mount namespace.
+/// This allows our `ProtectHome=` in the unit file to work
+/// for example.  Longer term I'd like to protect even more of `/var`.
+#[context("Running tmpfiles for /run and /var")]
 fn rerun_tmpfiles() -> Result<()> {
-    for prefix in &["/run", "/var"] {
+    isolation::run_systemd_worker_sync(&isolation::UnitConfig {
-        let status = Command::new("systemd-run")
+        name: Some("rpm-ostree-tmpfiles"),
-            .args(&[
+        properties: &[],
-                "-u",
+        exec_args: &[
-                "rpm-ostree-tmpfiles",
+            "systemd-tmpfiles",
-                "--wait",
+            "--create",
-                "--",
+            "--prefix=/run",
-                "systemd-tmpfiles",
+            "--prefix=/var",
-                "--create",
+        ],
-                "--prefix",
+    })
-                prefix,
-            ])
-            .status()?;
-        if !status.success() {
-            bail!("Failed to invoke systemd-tmpfiles");
-        }
-    }
-    Ok(())
 }
 
 fn get_required_booted_deployment(sysroot: &ostree::Sysroot) -> Result<ostree::Deployment> {
@@ -508,7 +493,7 @@ pub(crate) fn transaction_apply_live(
         &openat::Dir::open("/etc")?,
     )?;
     std::mem::drop(task);
-    let task = crate::ffi::progress_begin_task("Running systemd-tmpfiles for /var");
+    let task = crate::ffi::progress_begin_task("Running systemd-tmpfiles for /run and /var");
     rerun_tmpfiles()?;
     std::mem::drop(task);