Skip to content

Commit

Permalink
systemd: reimplement sd_notify logic using UNIX socket
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
victorjulien committed Apr 9, 2024
1 parent 6d40517 commit 34f53f8
Show file tree
Hide file tree
Showing 7 changed files with 130 additions and 64 deletions.
9 changes: 3 additions & 6 deletions .github/workflows/builds.yml
Original file line number Diff line number Diff line change
Expand Up @@ -864,7 +864,6 @@ jobs:
python \
python3-yaml \
sudo \
systemd-devel \
which \
zlib-devel
- uses: actions/checkout@b4ffde65f46336ab88eb53be808477a3936bae11
Expand Down Expand Up @@ -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:
Expand Down Expand Up @@ -1061,7 +1060,6 @@ jobs:
pkgconfig \
python3-yaml \
sudo \
systemd-devel \
which \
zlib-devel
- uses: actions/checkout@b4ffde65f46336ab88eb53be808477a3936bae11
Expand Down Expand Up @@ -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:
Expand Down Expand Up @@ -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]
Expand Down Expand Up @@ -1238,7 +1236,6 @@ jobs:
pkgconfig \
python3-yaml \
sudo \
systemd-devel \
which \
zlib-devel
- run: adduser suricata
Expand Down
33 changes: 4 additions & 29 deletions configure.ac
Original file line number Diff line number Diff line change
Expand Up @@ -254,6 +254,7 @@

RUST_SURICATA_LIBNAME="libsuricata_rust.a"

enable_systemd="no"
e_magic_file=""
e_magic_file_comment="#"
case "$host" in
Expand Down Expand Up @@ -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"
Expand Down Expand Up @@ -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"

Expand Down Expand Up @@ -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}
Expand Down
30 changes: 10 additions & 20 deletions doc/userguide/configuration/systemd-notify.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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
-------
Expand All @@ -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.
Expand All @@ -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.
2 changes: 2 additions & 0 deletions src/Makefile.am
Original file line number Diff line number Diff line change
Expand Up @@ -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 \
Expand Down Expand Up @@ -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 \
Expand Down
14 changes: 5 additions & 9 deletions src/suricata.c
Original file line number Diff line number Diff line change
Expand Up @@ -37,10 +37,6 @@
#endif
#endif

#if HAVE_LIBSYSTEMD
#include <systemd/sd-daemon.h>
#endif

#include "suricata.h"

#include "conf.h"
Expand Down Expand Up @@ -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"
Expand Down Expand Up @@ -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
}
Expand Down
91 changes: 91 additions & 0 deletions src/util-systemd.c
Original file line number Diff line number Diff line change
@@ -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
15 changes: 15 additions & 0 deletions src/util-systemd.h
Original file line number Diff line number Diff line change
@@ -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 */

0 comments on commit 34f53f8

Please sign in to comment.