Merge pull request #3778 from lucab/ups/intercept-groupadd

core: wrap and intercept `groupadd` calls in scriptlets

rust/src/builtins/mod.rs

@@ -3,4 +3,5 @@
 
 pub(crate) mod apply_live;
 pub(crate) mod compose;
+pub mod scriptlet_intercept;
 pub mod usroverlay;

rust/src/builtins/scriptlet_intercept/groupadd.rs

@@ -0,0 +1,162 @@
+//! CLI handler for intercepted `groupadd`.
+
+// SPDX-License-Identifier: Apache-2.0 OR MIT
+
+use anyhow::{anyhow, Result};
+use cap_std::fs::{Dir, Permissions};
+use cap_std_ext::prelude::CapStdExtDirExt;
+use clap::{Arg, Command};
+use std::io::Write;
+use std::os::unix::prelude::PermissionsExt;
+
+/// Entrypoint for (the rpm-ostree implementation of) `groupadd`.
+pub(crate) fn entrypoint(args: &[&str]) -> Result<()> {
+    fail::fail_point!("intercept_groupadd_ok", |_| Ok(()));
+
+    // This parses the same CLI surface as the real `groupadd`,
+    // but in the end we only extract the group name and (if
+    // present) the static GID.
+    let matches = cli_cmd().get_matches_from(args);
+    let gid = matches
+        .value_of("gid")
+        .map(|s| s.parse::<u32>())
+        .transpose()?;
+    let groupname = matches
+        .value_of("groupname")
+        .ok_or_else(|| anyhow!("missing required groupname argument"))?;
+    if !matches.is_present("system") {
+        eprintln!("Trying to create non-system group '{groupname}'; this will become an error in the future.");
+    }
+
+    let rootdir = Dir::open_ambient_dir("/", cap_std::ambient_authority())?;
+    generate_sysusers_fragment(&rootdir, groupname, gid)?;
+
+    Ok(())
+}
+
+/// CLI parser, matches <https://linux.die.net/man/8/groupadd>.
+fn cli_cmd() -> Command<'static> {
+    let name = "groupadd";
+    Command::new(name)
+        .bin_name(name)
+        .about("create a new group")
+        .arg(Arg::new("force").short('f').long("force"))
+        .arg(
+            Arg::new("help")
+                .short('h')
+                .long("help")
+                .action(clap::ArgAction::Help),
+        )
+        .arg(Arg::new("gid").short('g').long("gid").takes_value(true))
+        .arg(Arg::new("key").short('K').long("key").takes_value(true))
+        .arg(Arg::new("allow_duplicates").short('o').long("non-unique"))
+        .arg(
+            Arg::new("password")
+                .short('p')
+                .long("password")
+                .takes_value(true),
+        )
+        .arg(Arg::new("system").short('r').long("system"))
+        .arg(
+            Arg::new("chroot_dir")
+                .short('R')
+                .long("root")
+                .takes_value(true),
+        )
+        .arg(
+            Arg::new("prefix_dir")
+                .short('P')
+                .long("prefix")
+                .takes_value(true),
+        )
+        .arg(Arg::new("users").short('U').long("users").takes_value(true))
+        .arg(Arg::new("groupname").required(true))
+}
+
+/// Write a sysusers.d configuration fragment for the given group.
+///
+/// This returns whether a new fragment has been actually written
+/// to disk.
+fn generate_sysusers_fragment(rootdir: &Dir, groupname: &str, gid: Option<u32>) -> Result<bool> {
+    static SYSUSERS_DIR: &str = "usr/lib/sysusers.d";
+
+    // The filename of the configuration fragment is in fact a public
+    // API, because users may have masked it in /etc. Do not change this.
+    let filename = format!("30-pkg-group-{groupname}.conf");
+
+    rootdir.create_dir_all(SYSUSERS_DIR)?;
+    let conf_dir = rootdir.open_dir(SYSUSERS_DIR)?;
+    if conf_dir.exists(&filename) {
+        return Ok(false);
+    }
+
+    let gid_value = gid
+        .map(|id| id.to_string())
+        .unwrap_or_else(|| "-".to_string());
+    conf_dir.atomic_replace_with(&filename, |fragment| -> Result<()> {
+        let perms = Permissions::from_mode(0o644);
+        fragment.get_mut().as_file_mut().set_permissions(perms)?;
+
+        fragment.write_all(b"# Generated by rpm-ostree\n")?;
+        let entry = format!("g {groupname} {gid_value}\n");
+        fragment.write_all(entry.as_bytes())?;
+
+        Ok(())
+    })?;
+
+    Ok(true)
+}
+
+#[cfg(test)]
+mod test {
+    use super::*;
+    use std::io::Read;
+
+    #[test]
+    fn test_clap_cmd() {
+        cli_cmd().debug_assert();
+
+        let cmd = cli_cmd();
+        let static_gid = ["/usr/sbin/groupadd", "-g", "23", "squid"];
+        let matches = cmd.try_get_matches_from(static_gid).unwrap();
+        assert_eq!(matches.value_of("gid"), Some("23"));
+        assert_eq!(matches.value_of("groupname"), Some("squid"));
+
+        let cmd = cli_cmd();
+        let dynamic_gid = ["/usr/sbin/groupadd", "-r", "chrony"];
+        let matches = cmd.try_get_matches_from(dynamic_gid).unwrap();
+        assert!(matches.contains_id("system"));
+        assert_eq!(matches.value_of("gid"), None);
+        assert_eq!(matches.value_of("groupname"), Some("chrony"));
+
+        let err_cases = [vec!["/usr/sbin/groupadd"]];
+        for input in err_cases {
+            let cmd = cli_cmd();
+            cmd.try_get_matches_from(input).unwrap_err();
+        }
+    }
+
+    #[test]
+    fn test_fragment_generation() {
+        let tmpdir = cap_tempfile::tempdir(cap_tempfile::ambient_authority()).unwrap();
+
+        let groups = [
+            ("foo", Some(42), true, "42"),
+            ("foo", None, false, "42"),
+            ("bar", None, true, "-"),
+        ];
+        for entry in groups {
+            let generated = generate_sysusers_fragment(&tmpdir, entry.0, entry.1).unwrap();
+            assert_eq!(generated, entry.2, "{:?}", entry);
+
+            let path = format!("usr/lib/sysusers.d/30-pkg-group-{}.conf", entry.0);
+            assert!(tmpdir.is_file(&path));
+
+            let mut fragment = tmpdir.open(&path).unwrap();
+            let mut content = String::new();
+            fragment.read_to_string(&mut content).unwrap();
+            let expected = format!("# Generated by rpm-ostree\ng {} {}\n", entry.0, entry.3);
+            assert_eq!(content, expected)
+        }
+    }
+}

rust/src/builtins/scriptlet_intercept/mod.rs

@@ -0,0 +1,49 @@
+//! CLI handler for `rpm-ostree scriplet-intercept`.
+
+// SPDX-License-Identifier: Apache-2.0 OR MIT
+
+mod groupadd;
+use anyhow::{bail, Result};
+
+/// Entrypoint for `rpm-ostree scriplet-intercept`.
+pub fn entrypoint(args: &[&str]) -> Result<()> {
+    // Here we expect arguments that look like
+    // `rpm-ostree scriptlet-intercept <command> -- <rest>`
+    if args.len() < 4 || args[3] != "--" {
+        bail!("Invalid arguments");
+    }
+
+    let orig_command = args[2];
+    let rest = &args[4..];
+    match orig_command {
+        "groupadd" => groupadd::entrypoint(rest),
+        x => bail!("Unable to intercept command '{}'", x),
+    }
+}
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    #[test]
+    fn test_entrypoint_args() {
+        // Short-circuit groupadd logic, this test is only meant to check CLI parsing.
+        let _guard = fail::FailScenario::setup();
+        fail::cfg("intercept_groupadd_ok", "return").unwrap();
+
+        let err_cases = [
+            vec![],
+            vec!["rpm-ostree", "install"],
+            vec!["rpm-ostree", "scriptlet-intercept", "groupadd"],
+            vec!["rpm-ostree", "scriptlet-intercept", "foo", "--"],
+        ];
+        for input in &err_cases {
+            entrypoint(input).unwrap_err();
+        }
+
+        let ok_cases = [vec!["rpm-ostree", "scriptlet-intercept", "groupadd", "--"]];
+        for input in &ok_cases {
+            entrypoint(input).unwrap();
+        }
+    }
+}

rust/src/core.rs

@@ -40,6 +40,10 @@ const SSS_CACHE_PATH: &str = "usr/sbin/sss_cache";
 const SYSTEMCTL_PATH: &str = "usr/bin/systemctl";
 const SYSTEMCTL_WRAPPER: &[u8] = include_bytes!("../../src/libpriv/systemctl-wrapper.sh");
 
+// Intercept `groupadd` for automatic sysusers.d fragment generation.
+const GROUPADD_PATH: &str = "usr/sbin/groupadd";
+const GROUPADD_WRAPPER: &[u8] = include_bytes!("../../src/libpriv/groupadd-wrapper.sh");
+
 const RPMOSTREE_CORE_STAGED_RPMS_DIR: &str = "rpm-ostree/staged-rpms";
 
 pub(crate) const OSTREE_BOOTED: &str = "/run/ostree-booted";
@@ -132,8 +136,10 @@ pub(crate) fn log_treefile(tf: &crate::treefile::Treefile) {
 impl FilesystemScriptPrep {
     /// Filesystem paths that we rename out of the way if present
     const OPTIONAL_PATHS: &'static [&'static str] = &[SSS_CACHE_PATH];
-    const REPLACE_OPTIONAL_PATHS: &'static [(&'static str, &'static [u8])] =
-        &[(SYSTEMCTL_PATH, SYSTEMCTL_WRAPPER)];
+    const REPLACE_OPTIONAL_PATHS: &'static [(&'static str, &'static [u8])] = &[
+        (GROUPADD_PATH, GROUPADD_WRAPPER),
+        (SYSTEMCTL_PATH, SYSTEMCTL_WRAPPER),
+    ];
 
     fn saved_name(name: &str) -> String {
         format!("{}.rpmostreesave", name)
@@ -360,19 +366,39 @@ mod test {
         d.ensure_dir_with("usr/bin", &db)?;
         d.ensure_dir_with("usr/sbin", &db)?;
         d.atomic_write_with_perms(super::SSS_CACHE_PATH, "sss binary", mode.clone())?;
-        let original_systemctl = "original systemctl";
-        d.atomic_write_with_perms(super::SYSTEMCTL_PATH, original_systemctl, mode.clone())?;
-        // Replaced systemctl
+        // Neutered sss_cache.
         {
             assert!(d.try_exists(super::SSS_CACHE_PATH)?);
             let g = super::prepare_filesystem_script_prep(d.as_raw_fd())?;
             assert!(!d.try_exists(super::SSS_CACHE_PATH)?);
+            g.undo()?;
+            assert!(d.try_exists(super::SSS_CACHE_PATH)?);
+        }
+        // Replaced systemctl.
+        {
+            let original_systemctl = "original systemctl";
+            d.atomic_write_with_perms(super::SYSTEMCTL_PATH, original_systemctl, mode.clone())?;
+            let contents = d.read_to_string(super::SYSTEMCTL_PATH)?;
+            assert_eq!(contents, original_systemctl);
+            let g = super::prepare_filesystem_script_prep(d.as_raw_fd())?;
             let contents = d.read_to_string(super::SYSTEMCTL_PATH)?;
             assert_eq!(contents.as_bytes(), super::SYSTEMCTL_WRAPPER);
             g.undo()?;
             let contents = d.read_to_string(super::SYSTEMCTL_PATH)?;
             assert_eq!(contents, original_systemctl);
-            assert!(d.try_exists(super::SSS_CACHE_PATH)?);
+        }
+        // Replaced groupadd.
+        {
+            let original_groupadd = "original groupadd";
+            d.atomic_write_with_perms(super::GROUPADD_PATH, original_groupadd, mode.clone())?;
+            let contents = d.read_to_string(super::GROUPADD_PATH)?;
+            assert_eq!(contents, original_groupadd);
+            let g = super::prepare_filesystem_script_prep(d.as_raw_fd())?;
+            let contents = d.read_to_string(super::GROUPADD_PATH)?;
+            assert_eq!(contents.as_bytes(), super::GROUPADD_WRAPPER);
+            g.undo()?;
+            let contents = d.read_to_string(super::GROUPADD_PATH)?;
+            assert_eq!(contents, original_groupadd);
         }
         Ok(())
     }

rust/src/main.rs

@@ -46,6 +46,8 @@ async fn inner_async_main(args: Vec<String>) -> Result<i32> {
                 "cliwrap" => rpmostree_rust::cliwrap::entrypoint(args).map(|_| 0),
                 // The `unlock` is a hidden alias for "ostree CLI compatibility"
                 "usroverlay" | "unlock" => builtins::usroverlay::entrypoint(args).map(|_| 0),
+                // A hidden wrapper to intercept some binaries in RPM scriptlets.
+                "scriptlet-intercept" => builtins::scriptlet_intercept::entrypoint(args).map(|_| 0),
                 // C++ main
                 _ => Ok(rpmostree_rust::ffi::rpmostree_main(args)?),
             }

src/app/libmain.cxx

@@ -84,6 +84,8 @@ static RpmOstreeCommand commands[] = {
   /* Rust-implemented commands; they're here so that they show up in `rpm-ostree
    * --help` alongside the other commands, but the command itself is fully
    *  handled Rust side. */
+  { "scriptlet-intercept", static_cast<RpmOstreeBuiltinFlags> (RPM_OSTREE_BUILTIN_FLAG_HIDDEN),
+    "Intercept some commands used by RPM scriptlets", NULL },
   { "usroverlay", static_cast<RpmOstreeBuiltinFlags> (RPM_OSTREE_BUILTIN_FLAG_REQUIRES_ROOT),
     "Apply a transient overlayfs to /usr", NULL },
   /* Legacy aliases */

src/libpriv/groupadd-wrapper.sh

@@ -0,0 +1,13 @@
+#!/usr/bin/bash
+# Used by rpmostree-core.c to intercept `groupadd` calls.
+# We want to learn about group creation and distinguish between
+# static and dynamic GIDs, in order to auto-generate relevant
+# `sysusers.d` fragments.
+# See also https://github.com/coreos/rpm-ostree/issues/3762
+
+if test -v RPMOSTREE_EXP_BRIDGE_SYSUSERS; then
+    rpm-ostree scriptlet-intercept groupadd -- "$0" "$@"
+fi
+
+# Forward to the real `groupadd` for group creation.
+exec /usr/sbin/groupadd.rpmostreesave "$@"

src/libpriv/rpmostree-scripts.cxx

@@ -366,6 +366,10 @@ rpmostree_run_script_in_bwrap_container (int rootfs_fd, GLnxTmpDir *var_lib_rpm_
 
   gboolean debugging_script = g_strcmp0 (g_getenv ("RPMOSTREE_SCRIPT_DEBUG"), pkg_script) == 0;
 
+  const char *bridge_sysusers = g_getenv ("RPMOSTREE_EXP_BRIDGE_SYSUSERS");
+  if (bridge_sysusers != NULL)
+    bwrap->setenv ("RPMOSTREE_EXP_BRIDGE_SYSUSERS", rust::String (bridge_sysusers));
+
   /* https://github.com/systemd/systemd/pull/7631 AKA
    * "systemctl,verbs: Introduce SYSTEMD_OFFLINE environment variable"
    * https://github.com/systemd/systemd/commit/f38951a62837a00a0b1ff42d007e9396b347742d