]> git.ipfire.org Git - thirdparty/suricata.git/commitdiff
systemd: reimplement sd_notify logic using UNIX socket 10790/head
authorVictor Julien <vjulien@oisf.net>
Wed, 3 Apr 2024 08:51:21 +0000 (10:51 +0200)
committerVictor Julien <victor@inliniac.net>
Tue, 9 Apr 2024 08:27:35 +0000 (10:27 +0200)
One of the lessons of the XZ backdoor story was that just linking to
libsystemd to call sd_notify is discouraged by the systemd project:

Lennart Poettering:
"PSA: In context of the xzpocalypse we now added an example reimplementation
of sd_notify() to our man page:

https://www.freedesktop.org/software/systemd/man/devel/sd_notify.html#Notes

It's pretty comprehensive (i.e. uses it for reload notification too), but
still relatively short.

In the past, I have been telling anyone who wanted to listen that if all you
want is sd_notify() then don't bother linking to libsystemd, since the
protocol is stable and should be considered the API, not our C wrapper
around it. After all, the protocol is so trivial"

From: https://mastodon.social/@pid_eins/112202687764571433

This commit takes the example code and uses it to reimplement the notify
logic.

The code is enabled if Linux is detected in configure. Since the code
won't do anything if the NOTIFY_SOCKET env var isn't set, this should
also work fine on systems w/o systemd.

Ticket: #6913.

.github/workflows/builds.yml
configure.ac
doc/userguide/configuration/systemd-notify.rst
src/Makefile.am
src/suricata.c
src/util-systemd.c [new file with mode: 0644]
src/util-systemd.h [new file with mode: 0644]

index a6e38dc851a1063714f2c509a61998a0be6e6822..4b29dd4fd9f26604196c97d50a70bc69ce490766 100644 (file)
@@ -864,7 +864,6 @@ jobs:
                 python \
                 python3-yaml \
                 sudo \
-                systemd-devel \
                 which \
                 zlib-devel
       - uses: actions/checkout@b4ffde65f46336ab88eb53be808477a3936bae11
@@ -908,7 +907,7 @@ jobs:
       - run: suricata-update -V
       - run: suricatasc -h
       # Check compilation against systemd
-      - run: ldd src/suricata | grep libsystemd &> /dev/null
+      - run: src/suricata --build-info | grep -E "Systemd support:\s+yes" &> /dev/null
 
   # Fedora 38 build using GCC.
   fedora-38-gcc:
@@ -1061,7 +1060,6 @@ jobs:
                 pkgconfig \
                 python3-yaml \
                 sudo \
-                systemd-devel \
                 which \
                 zlib-devel
       - uses: actions/checkout@b4ffde65f46336ab88eb53be808477a3936bae11
@@ -1100,7 +1098,7 @@ jobs:
       - run: suricata-update -V
       - run: suricatasc -h
       # Check compilation against systemd
-      - run: ldd src/suricata | grep libsystemd &> /dev/null
+      - run: src/suricata --build-info | grep -E "Systemd support:\s+yes" &> /dev/null
 
   # Fedora 39 build using GCC.
   fedora-39-gcc:
@@ -1196,7 +1194,7 @@ jobs:
   # issues only show up when not running as root, and by default all
   # jobs in GitHub actions are run as root inside the container.
   fedora-39-non-root:
-    name: Fedora 39 (non-root, debug, clang, asan, wshadow, rust-strict, systemd)
+    name: Fedora 39 (non-root, debug, clang, asan, wshadow, rust-strict)
     runs-on: ubuntu-latest
     container: fedora:39
     needs: [prepare-deps]
@@ -1238,7 +1236,6 @@ jobs:
                 pkgconfig \
                 python3-yaml \
                 sudo \
-                systemd-devel \
                 which \
                 zlib-devel
       - run: adduser suricata
index 88938e34a5ba47053210f8fe1115a814d099aa71..9e568bbfe2c5cb1a050247900509f6c9887243cf 100644 (file)
 
     RUST_SURICATA_LIBNAME="libsuricata_rust.a"
 
+    enable_systemd="no"
     e_magic_file=""
     e_magic_file_comment="#"
     case "$host" in
             CFLAGS="${CFLAGS} -fPIC"
             RUST_LDADD="-ldl -lrt -lm"
             can_build_shared_library="yes"
+            AC_DEFINE([SYSTEMD_NOTIFY], [1], [make Suricata notify systemd on startup])
+            enable_systemd="yes"
             ;;
         *-*-mingw32*|*-*-msys)
             CFLAGS="${CFLAGS} -DOS_WIN32"
         AC_MSG_RESULT(no)
     fi
 
-  #systemd
-    AC_ARG_WITH(systemd_includes,
-            [  --with-systemd-includes=DIR  systemd include directory],
-            [with_systemd_includes="$withval"],[with_systemd_includes=no])
-    AC_ARG_WITH(systemd_libraries,
-            [  --with-systemd-libraries=DIR    systemd library directory],
-            [with_systemd_libraries="$withval"],[with_systemd_libraries="no"])
-
-    AC_CHECK_HEADER(systemd/sd-daemon.h, SYSTEMD="yes",SYSTEMD="no")
-    if test "$SYSTEMD" = "yes"; then
-        if test "$with_systemd_libraries" != "no"; then
-            LDFLAGS="${LDFLAGS} -L${with_systemd_libraries}"
-        fi
-
-        if test "$with_systemd_includes" != "no"; then
-            CPPFLAGS="${CPPFLAGS} -I${with_systems_includes}"
-        fi
-
-        # To prevent duping the lib link we reset LIBS after this check. Setting action-if-found to NULL doesn't seem to work
-        # see: http://blog.flameeyes.eu/2008/04/29/i-consider-ac_check_lib-harmful
-        SYSTEMD=""
-        TMPLIBS="${LIBS}"
-        AC_CHECK_LIB(systemd,sd_notify,,SYSTEMD="no")
-
-        if test "$SYSTEMD" != "no"; then
-            LIBS="${TMPLIBS} -lsystemd"
-        fi
-    fi
-  
   # libhs
     enable_hyperscan="no"
 
@@ -2691,6 +2665,7 @@ SURICATA_BUILD_CONF="Suricata Configuration:
   Libnet support:                          ${enable_libnet}
   liblz4 support:                          ${enable_liblz4}
   Landlock support:                        ${enable_landlock}
+  Systemd support:                         ${enable_systemd}
 
   Rust support:                            ${enable_rust}
   Rust strict mode:                        ${enable_rust_strict}
index e9fda83cfbbcc16287c7831369ec9e18b1a73661..52bf28507600b7c1770b8d7a46426dad486e7f16 100644 (file)
@@ -9,7 +9,9 @@ services/test frameworks that depend on a fully initialised Suricata .
 
 During the initialisation phase Suricata synchronises the initialisation thread with all active
 threads to ensure they are in a running state. Once synchronisation has been completed a ``READY=1``
-status notification is sent to the service manager using ``sd_notify()``.
+status notification is sent to the service manager using across the Systemd UNIX socket.
+
+The path of the UNIX socket is taken from the ``NOTIFY_SOCKET`` env var.
 
 Example
 -------
@@ -23,23 +25,11 @@ Requirements
 ------------
 This feature is only supported for distributions under the following conditions:
 
-1. Distribution contains ``libsystemd``
-2. Any distribution that runs under **systemd**
-3. Unit file configuration: ``Type=notify``
-4. Contains development files for systemd shared library
-
-To install development files:
-Fedora::
-
-    dnf -y install systemd-devel
-
-Ubuntu/Debian::
+1. Any distribution that runs under **systemd**
+2. Unit file configuration: ``Type=notify``
 
-    apt -y install systemd-dev
-
-This package shall be compile-time configured and therefore only built with distributions fulfilling
-requirements [1, 2]. For notification to the service manager the unit file must be configured as
-shown in requirement [3]. Upon all requirements being met the service manager will start and await
+For notification to the service manager the unit file must be configured as shown in requirement [2].
+Upon all requirements being met the service manager will start and await
 ``READY=1`` status from Suricata. Otherwise the service manager will treat the service unit as
 ``Type=simple`` and consider it started immediately after the main process ``ExecStart=`` has been
 forked.
@@ -50,8 +40,8 @@ To confirm the system is running under systemd::
 
     ps --no-headers -o comm 1
 
-See: https://man7.org/linux/man-pages/man3/sd_notify.3.html for a detailed description on
-``sd_notify``.
-
 See https://www.freedesktop.org/software/systemd/man/systemd.service.html for help
 writing systemd unit files.
+
+See https://www.freedesktop.org/software/systemd/man/devel/sd_notify.html#Notes for a discussion of
+the UNIX socket based notification.
index df37a5e9dc08ad418d75103ea0692b49750ca595..ec592ed5f6b35c3b105b8f1f29e6103d846865d9 100755 (executable)
@@ -621,6 +621,7 @@ noinst_HEADERS = \
        util-streaming-buffer.h \
        util-syslog.h \
        util-sysfs.h \
+       util-systemd.h \
        util-thash.h \
        util-threshold-config.h \
        util-time.h \
@@ -1235,6 +1236,7 @@ libsuricata_c_a_SOURCES = \
        util-strptime.c \
        util-syslog.c \
        util-sysfs.c \
+       util-systemd.c \
        util-thash.c \
        util-threshold-config.c \
        util-time.c \
index d59e0b8601660aa928803a2a87c3fbf47f16bbb8..6fdedf6c55eeb383dac517fd1d561c882975407e 100644 (file)
 #endif
 #endif
 
-#if HAVE_LIBSYSTEMD
-#include <systemd/sd-daemon.h>
-#endif
-
 #include "suricata.h"
 
 #include "conf.h"
 #include "util-time.h"
 #include "util-validate.h"
 #include "util-var-name.h"
+#ifdef SYSTEMD_NOTIFY
+#include "util-systemd.h"
+#endif
 
 #ifdef WINDIVERT
 #include "decode-sll.h"
@@ -431,12 +430,9 @@ void GlobalsDestroy(void)
  */
 static void OnNotifyRunning(void)
 {
-#if HAVE_LIBSYSTEMD
-    if (sd_notify(0, "READY=1") < 0) {
+#ifdef SYSTEMD_NOTIFY
+    if (SystemDNotifyReady() < 0) {
         SCLogWarning("failed to notify systemd");
-        /* Please refer to:
-         * https://www.freedesktop.org/software/systemd/man/sd_notify.html#Return%20Value
-         * for discussion on why failure should not be considered an error */
     }
 #endif
 }
diff --git a/src/util-systemd.c b/src/util-systemd.c
new file mode 100644 (file)
index 0000000..ad2b0ae
--- /dev/null
@@ -0,0 +1,91 @@
+/* SPDX-License-Identifier: MIT-0 */
+
+/* Implement the systemd notify protocol without external dependencies.
+ * Supports both readiness notification on startup and on reloading,
+ * according to the protocol defined at:
+ * https://www.freedesktop.org/software/systemd/man/latest/sd_notify.html
+ * This protocol is guaranteed to be stable as per:
+ * https://systemd.io/PORTABILITY_AND_STABILITY/ */
+
+/* this file is copied from:
+ * https://github.com/systemd/systemd/blob/main/man/notify-selfcontained-example.c
+ * written by Luca Boccassi */
+
+#include "suricata-common.h"
+
+#if (defined SYSTEMD_NOTIFY) && (defined HAVE_SYS_UN_H) && (defined HAVE_SYS_STAT_H) &&            \
+        (defined HAVE_SYS_TYPES_H)
+#include <sys/un.h>
+#include <sys/stat.h>
+#include <sys/types.h>
+
+#include "util-systemd.h"
+
+#define _cleanup_(f) __attribute__((cleanup(f)))
+
+static void closep(int *fd)
+{
+    if (!fd || *fd < 0)
+        return;
+
+    close(*fd);
+    *fd = -1;
+}
+
+static int Notify(const char *message)
+{
+    union sockaddr_union {
+        struct sockaddr sa;
+        struct sockaddr_un sun;
+    } socket_addr = {
+        .sun.sun_family = AF_UNIX,
+    };
+    size_t path_length, message_length;
+    _cleanup_(closep) int fd = -1;
+    const char *socket_path;
+
+    socket_path = getenv("NOTIFY_SOCKET");
+    if (!socket_path)
+        return 0; /* Not running under systemd? Nothing to do */
+
+    if (!message)
+        return -EINVAL;
+
+    message_length = strlen(message);
+    if (message_length == 0)
+        return -EINVAL;
+
+    /* Only AF_UNIX is supported, with path or abstract sockets */
+    if (socket_path[0] != '/' && socket_path[0] != '@')
+        return -EAFNOSUPPORT;
+
+    path_length = strlen(socket_path);
+    /* Ensure there is room for NUL byte */
+    if (path_length >= sizeof(socket_addr.sun.sun_path))
+        return -E2BIG;
+
+    memcpy(socket_addr.sun.sun_path, socket_path, path_length);
+
+    /* Support for abstract socket */
+    if (socket_addr.sun.sun_path[0] == '@')
+        socket_addr.sun.sun_path[0] = 0;
+
+    fd = socket(AF_UNIX, SOCK_DGRAM | SOCK_CLOEXEC, 0);
+    if (fd < 0)
+        return -errno;
+
+    if (connect(fd, &socket_addr.sa, offsetof(struct sockaddr_un, sun_path) + path_length) != 0)
+        return -errno;
+
+    ssize_t written = write(fd, message, message_length);
+    if (written != (ssize_t)message_length)
+        return written < 0 ? -errno : -EPROTO;
+
+    return 1; /* Notified! */
+}
+
+int SystemDNotifyReady(void)
+{
+    return Notify("READY=1");
+}
+#endif
diff --git a/src/util-systemd.h b/src/util-systemd.h
new file mode 100644 (file)
index 0000000..1c72b34
--- /dev/null
@@ -0,0 +1,15 @@
+/* SPDX-License-Identifier: MIT-0 */
+
+/* Implement the systemd notify protocol without external dependencies.
+ * Supports both readiness notification on startup and on reloading,
+ * according to the protocol defined at:
+ * https://www.freedesktop.org/software/systemd/man/latest/sd_notify.html
+ * This protocol is guaranteed to be stable as per:
+ * https://systemd.io/PORTABILITY_AND_STABILITY/ */
+
+#ifndef SURICATA_UTIL_SYSTEMD_H
+#define SURICATA_UTIL_SYSTEMD_H
+
+int SystemDNotifyReady(void);
+
+#endif /* SURICATA_UTIL_SYSTEMD_H */