daemon: Add socket activation via /run/rpm-ostreed.socket

For historical reasons, the daemon ends up doing a lot of
initialization before even claiming the DBus name.  For example,
it calls `ostree_sysroot_load()`.  We also end up scanning
the RPM database, and actually parse all the GPG keys
in `/etc/pki/rpm-gpg` etc.

This causes two related problems:

- By doing all this work before claiming the bus name, we
  race against the (pretty low) DBus service timeout of 25s.
- If something hard fails at initialization, the client can't
  easily see the error because it just appears in the systemd
  journal.  The client will just see a service timeout.

The solution to this is to adopt systemd socket activation.
There's a new `rpm-ostreed.socket` and the daemon can be activated
that way.

The client (when run as root, the socket is only accessible to root
right now) connects, which will activate the daemon and attempt
initialization - without claiming the DBus name yet.

If something goes wrong here, the daemon will reply to the client
that activated it with the error, and then also exit with failure.

On success, everything continues as normal, including claiming
the DBus name.

Note that this also logically replaces the code that does explicit
`systemctl start rpm-ostreed` invocations.

After this patch:

```
$ systemctl stop rpm-ostreed
$ umount /boot
$ rpm-ostree status
error: Couldn't start daemon: Error setting up sysroot: loading sysroot: Unexpected state: /run/ostree-booted found, but no /boot/loader directory
```

Author: cgwalters

Cargo.toml

@@ -49,7 +49,7 @@ camino = "1.0.9"
 cap-std-ext = "0.26"
 cap-std = { version = "0.25", features = ["fs_utf8"] }
 # Explicitly force on libc
-rustix = { version = "0.35", features = ["use-libc"] }
+rustix = { version = "0.35", features = ["use-libc", "net"] }
 cap-primitives = "0.25.2"
 cap-tempfile = "0.25.2"
 chrono = { version = "0.4.21", features = ["serde"] }
@@ -123,6 +123,8 @@ lto = "thin"
 # Note: If you add a feature here, you also probably want to update utils.rs:get_features()
 fedora-integration = []
 rhsm = ["libdnf-sys/rhsm"]
+# Enable hard requirement on `rpm-ostreed.socket`; requires https://bugzilla.redhat.com/show_bug.cgi?id=2110012
+client-socket = []
 bin-unit-tests = []
 # ASAN+UBSAN
 sanitizers = []

Makefile-daemon.am

@@ -63,14 +63,18 @@ systemdunit_service_in_files = \
 	$(NULL)
 
 systemdunit_service_files = $(systemdunit_service_in_files:.service.in=.service)
-systemdunit_timer_files = \
+systemdunit_other_files = \
 	$(srcdir)/src/daemon/rpm-ostreed-automatic.timer \
 	$(srcdir)/src/daemon/rpm-ostree-countme.timer \
 	$(NULL)
 
+if CLIENT_SOCKET
+systemdunit_other_files += $(srcdir)/src/daemon/rpm-ostreed.socket
+endif
+
 systemdunit_DATA = \
 	$(systemdunit_service_files) \
-	$(systemdunit_timer_files) \
+	$(systemdunit_other_files) \
 	$(NULL)
 
 systemdunitdir       = $(prefix)/lib/systemd/system/
@@ -110,7 +114,7 @@ EXTRA_DIST += \
 	$(sysconf_DATA) \
 	$(service_in_files) \
 	$(systemdunit_service_in_files) \
-	$(systemdunit_timer_files) \
+	$(systemdunit_other_files) \
 	$(NULL)
 
 CLEANFILES += \

configure.ac

@@ -69,6 +69,14 @@ AC_ARG_ENABLE(featuresrs,
               AS_HELP_STRING([--enable-featuresrs],
                              [Rust features, see Cargo.toml for more information]),,
               [enable_featuresrs=])
+
+AC_ARG_ENABLE(client-socket,
+              AS_HELP_STRING([--enable-client-socket],
+                             [(default: no)]),,
+              [enable_client_socket=no])
+AS_IF([test x$enable_client_socket = xyes], [enable_featuresrs="$enable_featuresrs client-socket"])
+AM_CONDITIONAL(CLIENT_SOCKET, [echo $enable_featuresrs | grep -q 'client-socket'])
+
 AC_SUBST([RUST_FEATURES], $enable_featuresrs)
 
 # Initialize libtool

packaging/rpm-ostree.spec.in

@@ -34,6 +34,13 @@ BuildRequires: rust
 %bcond_with rhsm
 %endif
 
+# https://bugzilla.redhat.com/show_bug.cgi?id=2110012
+%if 0%{?fedora} >= 37
+%bcond_without client_socket
+%else
+%bcond_with client_socket
+%endif
+
 # RHEL (8,9) doesn't ship zchunk today.  Keep this in sync
 # with libdnf: https://gitlab.com/redhat/centos-stream/rpms/libdnf/-/blob/762f631e36d1e42c63a794882269d26c156b68c1/libdnf.spec#L45
 %if 0%{?rhel}
@@ -179,7 +186,7 @@ env NOCONFIGURE=1 ./autogen.sh
 export RUSTFLAGS="%{build_rustflags}"
 %endif
 %configure --disable-silent-rules --enable-gtk-doc %{?rpmdb_default} %{?with_sanitizers:--enable-sanitizers}  %{?with_bin_unit_tests:--enable-bin-unit-tests} \
-  %{?with_rhsm:--enable-featuresrs=rhsm}
+  %{?with_rhsm:--enable-featuresrs=rhsm} %{?with_client_socket:--enable-client-socket} 
 
 %make_build
 

rpmostree-cxxrs.cxx

@@ -8,6 +8,7 @@
 #include "rpmostree-package-variants.h"
 #include "rpmostree-rpm-util.h"
 #include "rpmostree-util.h"
+#include "rpmostreed-daemon.hpp"
 #include "rpmostreemain.h"
 #include "src/libpriv/rpmostree-cxxrs-prelude.h"
 #include <algorithm>
@@ -2167,6 +2168,10 @@ extern "C"
                                                ::rust::Str opt_deploy_id, ::rust::Str opt_os_name,
                                                ::rpmostreecxx::OstreeDeployment **return$) noexcept;
 
+  ::rust::repr::PtrLen rpmostreecxx$cxxbridge1$daemon_main (bool debug) noexcept;
+
+  void rpmostreecxx$cxxbridge1$daemon_terminate () noexcept;
+
   ::rust::repr::PtrLen rpmostreecxx$cxxbridge1$daemon_sanitycheck_environment (
       const ::rpmostreecxx::OstreeSysroot &sysroot) noexcept;
 
@@ -2933,6 +2938,40 @@ extern "C"
     return throw$;
   }
 
+  ::rust::repr::PtrLen
+  rpmostreecxx$cxxbridge1$daemon_init_inner (bool debug) noexcept
+  {
+    void (*daemon_init_inner$) (bool) = ::rpmostreecxx::daemon_init_inner;
+    ::rust::repr::PtrLen throw$;
+    ::rust::behavior::trycatch (
+        [&] {
+          daemon_init_inner$ (debug);
+          throw$.ptr = nullptr;
+        },
+        [&] (const char *catch$) noexcept {
+          throw$.len = ::std::strlen (catch$);
+          throw$.ptr = const_cast<char *> (::cxxbridge1$exception (catch$, throw$.len));
+        });
+    return throw$;
+  }
+
+  ::rust::repr::PtrLen
+  rpmostreecxx$cxxbridge1$daemon_main_inner () noexcept
+  {
+    void (*daemon_main_inner$) () = ::rpmostreecxx::daemon_main_inner;
+    ::rust::repr::PtrLen throw$;
+    ::rust::behavior::trycatch (
+        [&] {
+          daemon_main_inner$ ();
+          throw$.ptr = nullptr;
+        },
+        [&] (const char *catch$) noexcept {
+          throw$.len = ::std::strlen (catch$);
+          throw$.ptr = const_cast<char *> (::cxxbridge1$exception (catch$, throw$.len));
+        });
+    return throw$;
+  }
+
   ::rust::repr::PtrLen
   rpmostreecxx$cxxbridge1$client_require_root () noexcept
   {
@@ -3960,6 +3999,22 @@ deployment_get_base (::rpmostreecxx::OstreeSysroot &sysroot, ::rust::Str opt_dep
   return ::std::move (return$.value);
 }
 
+void
+daemon_main (bool debug)
+{
+  ::rust::repr::PtrLen error$ = rpmostreecxx$cxxbridge1$daemon_main (debug);
+  if (error$.ptr)
+    {
+      throw ::rust::impl< ::rust::Error>::error (error$);
+    }
+}
+
+void
+daemon_terminate () noexcept
+{
+  rpmostreecxx$cxxbridge1$daemon_terminate ();
+}
+
 void
 daemon_sanitycheck_environment (const ::rpmostreecxx::OstreeSysroot &sysroot)
 {

rpmostree-cxxrs.h

@@ -8,6 +8,7 @@
 #include "rpmostree-package-variants.h"
 #include "rpmostree-rpm-util.h"
 #include "rpmostree-util.h"
+#include "rpmostreed-daemon.hpp"
 #include "rpmostreemain.h"
 #include "src/libpriv/rpmostree-cxxrs-prelude.h"
 #include <algorithm>
@@ -1823,6 +1824,10 @@ ::rpmostreecxx::OstreeDeployment *deployment_get_base (::rpmostreecxx::OstreeSys
                                                        ::rust::Str opt_deploy_id,
                                                        ::rust::Str opt_os_name);
 
+void daemon_main (bool debug);
+
+void daemon_terminate () noexcept;
+
 void daemon_sanitycheck_environment (const ::rpmostreecxx::OstreeSysroot &sysroot);
 
 ::rust::String deployment_generate_id (const ::rpmostreecxx::OstreeDeployment &deployment) noexcept;

rust/src/client.rs

@@ -6,13 +6,12 @@ use crate::core::OSTREE_BOOTED;
 use crate::cxxrsutil::*;
 use crate::ffi::SystemHostType;
 use crate::utils;
-use anyhow::{anyhow, Result};
+use anyhow::{anyhow, Context, Result};
 use cap_std_ext::rustix;
 use gio::prelude::*;
 use ostree_ext::{gio, glib};
 use std::io::{BufRead, Write};
 use std::os::unix::io::IntoRawFd;
-use std::process::Command;
 
 /// The well-known bus name.
 const BUS_NAME: &str = "org.projectatomic.rpmostree1";
@@ -49,7 +48,8 @@ impl ClientConnection {
             SYSROOT_PATH,
             "org.projectatomic.rpmostree1.Sysroot",
             gio::NONE_CANCELLABLE,
-        )?;
+        )
+        .context("Initializing sysroot proxy")?;
         // Today the daemon mode requires running inside a booted deployment.
         let booted = sysroot_proxy
             .cached_property("Booted")
@@ -155,6 +155,45 @@ pub(crate) fn client_handle_fd_argument(
     }
 }
 
+/// Connect to the client socket and ensure the daemon is initialized;
+/// this avoids DBus and ensures that we get any early startup errors
+/// returned cleanly.
+#[cfg(feature = "client-socket")]
+fn start_daemon_via_socket() -> Result<()> {
+    use cap_std::io_lifetimes::IntoSocketlike;
+
+    let address = sockaddr()?;
+    let socket = rustix::net::socket(
+        rustix::net::AddressFamily::UNIX,
+        rustix::net::SocketType::SEQPACKET,
+        rustix::net::Protocol::from_raw(0),
+    )?;
+    let addr = crate::client::sockaddr()?;
+    tracing::debug!("Starting daemon via {address:?}");
+    rustix::net::connect_unix(&socket, &addr)
+        .with_context(|| anyhow!("Failed to connect to {address:?}"))?;
+    let socket = socket.into_socketlike();
+    crate::daemon::write_message(
+        &socket,
+        crate::daemon::SocketMessage::ClientHello {
+            selfid: crate::core::self_id()?,
+        },
+    )?;
+    let resp = crate::daemon::recv_message(&socket)?;
+    match resp {
+        crate::daemon::SocketMessage::ServerOk => Ok(()),
+        crate::daemon::SocketMessage::ServerError { msg } => {
+            Err(anyhow!("server error: {msg}").into())
+        }
+        o => Err(anyhow!("unexpected message: {o:?}").into()),
+    }
+}
+
+/// Returns the address of the client socket.
+pub(crate) fn sockaddr() -> Result<rustix::net::SocketAddrUnix> {
+    rustix::net::SocketAddrUnix::new("/run/rpm-ostree/client.sock").map_err(anyhow::Error::msg)
+}
+
 /// Explicitly ensure the daemon is started via systemd, if possible.
 ///
 /// This works around bugs from DBus activation, see
@@ -165,12 +204,16 @@ pub(crate) fn client_handle_fd_argument(
 /// to systemd directly and use its client tools to scrape errors.
 ///
 /// What we really should do probably is use native socket activation.
-pub(crate) fn client_start_daemon() -> CxxResult<()> {
+#[cfg(not(feature = "client-socket"))]
+fn start_daemon_via_systemctl() -> Result<()> {
+    use std::process::Command;
+
     let service = "rpm-ostreed.service";
     // Assume non-root can't use systemd right now.
     if rustix::process::getuid().as_raw() != 0 {
         return Ok(());
     }
+
     // Unfortunately, RHEL8 systemd will count "systemctl start"
     // invocations against the restart limit, so query the status
     // first.
@@ -197,6 +240,18 @@ pub(crate) fn client_start_daemon() -> CxxResult<()> {
     Ok(())
 }
 
+pub(crate) fn client_start_daemon() -> CxxResult<()> {
+    // systemctl and socket paths only work for root right now; in the future
+    // the socket may be opened up.
+    if rustix::process::getuid().as_raw() != 0 {
+        return Ok(());
+    }
+    #[cfg(feature = "client-socket")]
+    return start_daemon_via_socket().map_err(Into::into);
+    #[cfg(not(feature = "client-socket"))]
+    return start_daemon_via_systemctl().map_err(Into::into);
+}
+
 /// Convert the GVariant parameters from the DownloadProgress DBus API to a human-readable English string.
 pub(crate) fn client_render_download_progress(progress: &crate::ffi::GVariant) -> String {
     let progress = progress

rust/src/core.rs

@@ -332,6 +332,14 @@ fn stage_container_rpm_files(rpms: Vec<File>) -> CxxResult<Vec<String>> {
     Ok(r)
 }
 
+/// Return an opaque identifier for the current executing binary.  This can
+/// be passed via IPC to verify that client and server are running the same code.
+pub(crate) fn self_id() -> Result<String> {
+    use std::os::unix::fs::MetadataExt;
+    let metadata = std::fs::metadata("/proc/self/exe").context("Failed to read /proc/self/exe")?;
+    Ok(format!("dev={};inode={}", metadata.dev(), metadata.ino()))
+}
+
 #[cfg(test)]
 mod test {
     use super::*;