From 34f53f85bcf3ccc168fd3845fbc8a18a5c96e350 Mon Sep 17 00:00:00 2001 From: Victor Julien Date: Wed, 3 Apr 2024 10:51:21 +0200 Subject: [PATCH] systemd: reimplement sd_notify logic using UNIX socket 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 | 9 +- configure.ac | 33 +------ .../configuration/systemd-notify.rst | 30 ++---- src/Makefile.am | 2 + src/suricata.c | 14 +-- src/util-systemd.c | 91 +++++++++++++++++++ src/util-systemd.h | 15 +++ 7 files changed, 130 insertions(+), 64 deletions(-) create mode 100644 src/util-systemd.c create mode 100644 src/util-systemd.h diff --git a/.github/workflows/builds.yml b/.github/workflows/builds.yml index a6e38dc851..4b29dd4fd9 100644 --- a/.github/workflows/builds.yml +++ b/.github/workflows/builds.yml @@ -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 diff --git a/configure.ac b/configure.ac index 88938e34a5..9e568bbfe2 100644 --- a/configure.ac +++ b/configure.ac @@ -254,6 +254,7 @@ RUST_SURICATA_LIBNAME="libsuricata_rust.a" + enable_systemd="no" e_magic_file="" e_magic_file_comment="#" case "$host" in @@ -281,6 +282,8 @@ 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" @@ -659,35 +662,6 @@ 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} diff --git a/doc/userguide/configuration/systemd-notify.rst b/doc/userguide/configuration/systemd-notify.rst index e9fda83cfb..52bf285076 100644 --- a/doc/userguide/configuration/systemd-notify.rst +++ b/doc/userguide/configuration/systemd-notify.rst @@ -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. diff --git a/src/Makefile.am b/src/Makefile.am index df37a5e9dc..ec592ed5f6 100755 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -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 \ diff --git a/src/suricata.c b/src/suricata.c index d59e0b8601..6fdedf6c55 100644 --- a/src/suricata.c +++ b/src/suricata.c @@ -37,10 +37,6 @@ #endif #endif -#if HAVE_LIBSYSTEMD -#include -#endif - #include "suricata.h" #include "conf.h" @@ -145,6 +141,9 @@ #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 index 0000000000..ad2b0ae4fa --- /dev/null +++ b/src/util-systemd.c @@ -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 +#include +#include + +#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 index 0000000000..1c72b34d2c --- /dev/null +++ b/src/util-systemd.h @@ -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 */