diff --git a/configure.ac b/configure.ac index 5e89d469..303ebfde 100644 --- a/configure.ac +++ b/configure.ac @@ -136,6 +136,9 @@ LT_INIT AC_CHECK_HEADERS( \ [linux/magic.h] \ ) +AC_CHECK_HEADER([linux/bpf.h], [], + [AC_MSG_FAILURE([linux/bpf.h not found (install linux-libc-dev or kernel-headers)])] +) # # Checks for functions diff --git a/doc/conf.py b/doc/conf.py index 0a000166..05c6b530 100644 --- a/doc/conf.py +++ b/doc/conf.py @@ -53,23 +53,23 @@ domainrefs = { "linux:man1": { "text": "%s(1)", - "url": "https://linux.die.net/man/1/%s", + "url": "https://man7.org/linux/man-pages/man1/%s.1.html", }, "linux:man2": { "text": "%s(2)", - "url": "https://linux.die.net/man/2/%s", + "url": "https://man7.org/linux/man-pages/man2/%s.2.html", }, "linux:man3": { "text": "%s(3)", - "url": "https://linux.die.net/man/3/%s", + "url": "https://man7.org/linux/man-pages/man3/%s.3.html", }, "linux:man7": { "text": "%s(7)", - "url": "https://linux.die.net/man/7/%s", + "url": "https://man7.org/linux/man-pages/man7/%s.7.html", }, "linux:man8": { "text": "%s(8)", - "url": "https://linux.die.net/man/8/%s", + "url": "https://man7.org/linux/man-pages/man8/%s.8.html", }, "core:man1": { "text": "%s(1)", diff --git a/doc/guide/device-containment.rst b/doc/guide/device-containment.rst new file mode 100644 index 00000000..45ca4d27 --- /dev/null +++ b/doc/guide/device-containment.rst @@ -0,0 +1,210 @@ +.. _device-containment: + +################## +Device Containment +################## + +The IMP supports optional device containment for jobs using a BPF +cgroup device filter. When active, only explicitly allowed devices are +accessible to processes in the job's cgroup; all other device accesses +are denied. + +The motivating use case is GPU containment on systems where multiple +jobs share a node: each job should have access only to the GPUs +assigned to it, not to those assigned to other jobs. Device +containment falls to the IMP because it cannot currently be delegated +to the systemd user instance running as the flux user — applying a +cgroup device policy requires privilege that the user instance does not +hold. + +********* +IMP Input +********* + +The flux-core execution system sets the systemd ``DevicePolicy`` and +``DeviceAllow`` unit properties [5]_ when launching a job. These are +currently ignored by systemd since the user instance lacks the +privilege to enforce them. + +Per :doc:`RFC 15 ` [7]_, the IMP takes its input from a helper +program provided by flux-core, pointed to by :envvar:`FLUX_IMP_EXEC_HELPER`. +The helper reads the systemd properties and passes them to the IMP as-is +under an ``options`` key in the JSON object alongside the signed job +specification ``J``. For example: + +.. code-block:: json + + { + "J": "", + "options": { + "DevicePolicy": "closed", + "DeviceAllow": [ + ["/dev/nvidia0", "rw"], + ["char-pts", "rw"] + ] + } + } + +``DevicePolicy`` may be ``strict``, ``closed``, or ``auto`` (the default). +Each ``DeviceAllow`` entry is a two-element array of specifier and access +string, mirroring the systemd unit property format. The specifier may be +a path such as ``/dev/null``, or a device class such as ``char-pts`` or +``block-loop``. Access is a combination of ``r``, ``w``, and ``m`` for +:linux:man2:`read`, :linux:man2:`write`, and :linux:man2:`mknod`. + +********** +Data Flow +********** + +The policy travels through several IMP layers before reaching the kernel: + +#. The unprivileged IMP child reads JSON from the helper and parses + ``DevicePolicy`` and ``DeviceAllow`` from the ``options`` object. + It resolves each specifier — stat(2)-ing path-based entries and + scanning ``/proc/devices`` for class-based entries — into a flat + list of ``{type, major, minor, access}`` tuples in a + :c:struct:`device_allow`. Standard pseudo-devices are appended for + ``closed`` and ``auto`` policies. + +#. The unprivileged child encodes the resolved list into the privsep ``kv`` + struct using a compact per-entry format (e.g. ``c:195:0:rw``) and + sends it to the privileged parent. The ``kv`` encoding is defined + in :doc:`RFC 38 ` [6]_. + +#. The privileged parent decodes the ``kv`` struct back into a + :c:struct:`device_allow` and calls :c:func:`cgroup_device_apply`. + +#. :c:func:`cgroup_device_apply` builds a BPF instruction array, loads it + into the kernel with ``BPF_PROG_LOAD``, and attaches it to the job + cgroup fd with ``BPF_PROG_ATTACH``. + +The IMP enforces the policy by attaching a BPF [4]_ program of type +:c:macro:`BPF_PROG_TYPE_CGROUP_DEVICE` to the job's cgroup directory fd +before any job processes are created. + +*************** +Design Notes +*************** + +**No libbpf dependency.** The BPF program is loaded and attached using +:linux:man2:`bpf` [1]_ directly rather than through libbpf. Since the +IMP is a setuid binary, minimizing shared library dependencies reduces +the attack surface: a compromised or replaced ``libbpf.so`` could +otherwise be a privilege escalation vector. Using the syscall interface +directly also keeps the policy enforcement code self-contained and +auditable without reference to an external library. + +**Fail-closed error handling.** If device containment is requested but +cannot be applied — whether due to a kernel BPF error, a cgroup fd +problem, or any other failure — the IMP terminates before forking any +job processes. A job that escapes its intended device policy is treated +as worse than a job that does not start. + +**Normalization in the unprivileged child.** Policy interpretation — +resolving ``DevicePolicy`` rules and normalizing device specifiers to +``{type, major, minor, access}`` tuples — is performed by the +unprivileged IMP child before data crosses the privsep boundary. The +privileged IMP parent receives a pre-validated allowlist in a simple +numeric form, minimizing the complexity and attack surface of the code +that runs with elevated privilege. + +********************* +BPF Program Structure +********************* + +The BPF program is built as a flat instruction array [2]_ by +:c:func:`bpf_prog_build`. The program structure is: + +.. code-block:: none + + prologue: + r2 = ctx->major + r3 = ctx->minor + r4 = ctx->access_type + + for each allow entry: + if (r4 & 0xffff) != dev_type: skip to next entry + if r2 != major: skip to next entry + if minor != -1 and r3 != minor: skip to next entry + if r4 >> 16 has bits set outside allowed access: skip to next entry + r0 = 1; exit (allow) + + epilogue: + r0 = 0; exit (default deny) + +The ``access_type`` field of :c:struct:`bpf_cgroup_dev_ctx` packs the +device type into the lower 16 bits (:c:macro:`BPF_DEVCG_DEV_BLOCK`, +:c:macro:`BPF_DEVCG_DEV_CHAR`) and the requested access into the upper +16 bits (:c:macro:`BPF_DEVCG_ACC_MKNOD`, :c:macro:`BPF_DEVCG_ACC_READ`, +:c:macro:`BPF_DEVCG_ACC_WRITE`). + +Each allow entry emits 10 instructions without a minor check, or 11 +with one; the jump offsets within each entry are computed accordingly. + +************** +Error Handling +************** + +Any failure that would violate fail-closed error handling is fatal. +The IMP logs errors to stderr, which is captured in the job's standard +error output. On a fatality, it terminates before forking processes. + +.. list-table:: + :header-rows: 1 + :widths: 20 60 20 + + * - Failure class + - Description + - Strategy + * - Input parsing + - | The ``options`` object, ``DevicePolicy``, + | or ``DeviceAllow`` key is malformed. + - fatal + * - DeviceAllow entry + - | Device containment is requested but a + | ``DeviceAllow`` entry is malformed or + | could not be found. + - | Log warning, + | ignore entry + * - kv encoding/decoding + - Privsep boundary communication failed. + - fatal + * - Cgroup access + - Job cgroup directory cannot be opened. + - fatal + * - BPF program load + - The kernel BPF verifier [3]_ rejects program. + - fatal + * - BPF program attach + - The program cannot be attached to the job cgroup. + - fatal + +When ``DevicePolicy`` is absent or ``auto`` and ``DeviceAllow`` is absent +or empty, no containment is applied, and the job proceeds with full device +access. + +********** +References +********** + +.. [1] Linux man-pages project, :linux:man2:`bpf`, Linux Programmer's Manual. + +.. [2] Linux kernel contributors, `eBPF Instruction Set Specification + `_, + Linux kernel documentation. + +.. [3] Linux kernel contributors, `BPF Verifier + `_, + Linux kernel documentation. + +.. [4] Linux kernel contributors, `Classic BPF vs eBPF + `_, + Linux kernel documentation. + +.. [5] systemd contributors, `systemd.resource-control(5) + `_, + freedesktop.org. + +.. [6] flux-framework contributors, :doc:`rfc:spec_38`, flux-rfc. + +.. [7] flux-framework contributors, :doc:`rfc:spec_15`, flux-rfc. diff --git a/doc/guide/internals.rst b/doc/guide/internals.rst index 592d4529..695dfdcd 100644 --- a/doc/guide/internals.rst +++ b/doc/guide/internals.rst @@ -4,3 +4,4 @@ Resources for Flux Developers .. toctree:: :maxdepth: 1 + device-containment diff --git a/doc/test/spell.en.pws b/doc/test/spell.en.pws index ae2c6660..7b427c36 100644 --- a/doc/test/spell.en.pws +++ b/doc/test/spell.en.pws @@ -556,3 +556,41 @@ NOVERIFY auth localuser pam +ACC +allowlist +auditable +bpf +BPF +cgroup +Cgroup +CGROUP +designators +DEV +DEVCG +DeviceAllow +DevicePolicy +eBPF +envvar +fimm +foff +freedesktop +func +imm +JSON +kv +libbpf +mknod +MKNOD +preprocessor +privsep +prog +PROG +rfc +rw +syscall +verifier +Verifier +xffff +ing +nvidia +pts diff --git a/doc/test/spellcheck b/doc/test/spellcheck index 9026cbe4..2998ce28 100755 --- a/doc/test/spellcheck +++ b/doc/test/spellcheck @@ -1,7 +1,7 @@ #!/bin/bash if test $man_base_dir; then - set ${man_base_dir}/man*/*.rst + set ${man_base_dir}/man*/*.rst ${man_base_dir}/guide/*.rst fi if test $# == 0; then diff --git a/src/imp/Makefile.am b/src/imp/Makefile.am index 42d170fa..caac78db 100644 --- a/src/imp/Makefile.am +++ b/src/imp/Makefile.am @@ -62,12 +62,16 @@ IMP_SOURCES = \ signals.h \ cgroup.c \ cgroup.h \ + cgroup_device.c \ + cgroup_device.h \ run.c \ exec/user.h \ exec/user.c \ exec/exec.c \ exec/safe_popen.h \ - exec/safe_popen.c + exec/safe_popen.c \ + exec/device.h \ + exec/device.c if HAVE_PAM IMP_SOURCES += \ @@ -141,7 +145,8 @@ TESTS = \ test_impcmd.t \ test_passwd.t \ test_pidinfo.t \ - test_safe_popen.t + test_safe_popen.t \ + test_device.t check_PROGRAMS = \ $(TESTS) @@ -201,3 +206,12 @@ test_safe_popen_t_SOURCES = \ imp_log.c \ imp_log.h test_safe_popen_t_LDADD= $(test_ldadd) + +test_device_t_SOURCES = \ + test/device.c \ + exec/device.c \ + exec/device.h \ + imp_log.c \ + imp_log.h +test_device_t_CPPFLAGS = $(AM_CPPFLAGS) $(JANSSON_CFLAGS) +test_device_t_LDADD = $(test_ldadd) $(JANSSON_LIBS) diff --git a/src/imp/cgroup_device.c b/src/imp/cgroup_device.c new file mode 100644 index 00000000..c8e5550c --- /dev/null +++ b/src/imp/cgroup_device.c @@ -0,0 +1,248 @@ +/************************************************************\ + * Copyright 2026 Lawrence Livermore National Security, LLC + * (c.f. AUTHORS, NOTICE.LLNS, COPYING) + * + * This file is part of the Flux resource manager framework. + * For details, see https://github.com/flux-framework. + * + * SPDX-License-Identifier: LGPL-3.0 +\************************************************************/ + +#if HAVE_CONFIG_H +#include +#endif + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#include "cgroup_device.h" +#include "imp_log.h" + +/* + * BPF instruction building macros. + * Macro parameters are named foff/fimm (not off/imm) to avoid collision with + * struct bpf_insn field names in designated initializers — the preprocessor + * substitutes macro parameters before the compiler sees the field names. + * + * access_type field layout per struct bpf_cgroup_dev_ctx: + * upper 16 bits: BPF_DEVCG_ACC_* (MKNOD=1, READ=2, WRITE=4) + * lower 16 bits: BPF_DEVCG_DEV_* (BLOCK=1, CHAR=2) + */ +#define DEV_INSN_LDX_W(dst, src, foff) \ + ((struct bpf_insn){ \ + .code = BPF_LDX | BPF_W | BPF_MEM, \ + .dst_reg = (dst), \ + .src_reg = (src), \ + .off = (foff), \ + .imm = 0 }) +#define DEV_INSN_MOV32_REG(dst, src) \ + ((struct bpf_insn){ \ + .code = BPF_ALU | BPF_MOV | BPF_X, \ + .dst_reg = (dst), \ + .src_reg = (src), \ + .off = 0, \ + .imm = 0 }) +#define DEV_INSN_RSH32_IMM(dst, fimm) \ + ((struct bpf_insn){ \ + .code = BPF_ALU | BPF_RSH | BPF_K, \ + .dst_reg = (dst), \ + .src_reg = 0, \ + .off = 0, \ + .imm = (fimm) }) +#define DEV_INSN_AND32_IMM(dst, fimm) \ + ((struct bpf_insn){ \ + .code = BPF_ALU | BPF_AND | BPF_K, \ + .dst_reg = (dst), \ + .src_reg = 0, \ + .off = 0, \ + .imm = (fimm) }) +#define DEV_INSN_JNE_IMM(dst, fimm, foff) \ + ((struct bpf_insn){ \ + .code = BPF_JMP | BPF_JNE | BPF_K, \ + .dst_reg = (dst), \ + .src_reg = 0, \ + .off = (foff), \ + .imm = (fimm) }) +#define DEV_INSN_MOV64_IMM(dst, fimm) \ + ((struct bpf_insn){ \ + .code = BPF_ALU64 | BPF_MOV | BPF_K, \ + .dst_reg = (dst), \ + .src_reg = 0, \ + .off = 0, \ + .imm = (fimm) }) +#define DEV_INSN_EXIT() \ + ((struct bpf_insn){ \ + .code = BPF_JMP | BPF_EXIT, \ + .dst_reg = 0, \ + .src_reg = 0, \ + .off = 0, \ + .imm = 0 }) + +/* Number of BPF instructions emitted per device_allow_entry. + * Each entry is 10 instructions without a minor check, 11 with one. + */ +static int entry_insn_count (const struct device_allow_entry *e) +{ + return e->minor == -1 ? 10 : 11; +} + +/* Convert access string ("rwm") to BPF_DEVCG_ACC_* bitmask */ +static int access_to_mask (const char *access) +{ + int mask = 0; + for (const char *p = access; *p; p++) { + switch (*p) { + case 'r': mask |= BPF_DEVCG_ACC_READ; break; + case 'w': mask |= BPF_DEVCG_ACC_WRITE; break; + case 'm': mask |= BPF_DEVCG_ACC_MKNOD; break; + } + } + return mask; +} + +/* Build a BPF cgroup device filter program from a device_allow list. + * The program allows accesses matching any entry and denies all others. + * Returns allocated insn array with *countp set, or NULL with errno set. + * + * Program structure per entry: + * check device type (lower 16 bits of access_type) + * check major + * check minor (if not wildcard) + * check access (upper 16 bits of access_type) + * if all match: return 1 (allow) + * Default: return 0 (deny) + */ +static struct bpf_insn * +bpf_prog_build (struct device_allow *da, size_t *countp) +{ + size_t n_insns = 3 + 2; /* prologue (3) + epilogue (2) */ + struct bpf_insn *insns; + size_t pos = 0; + + for (int i = 0; i < da->count; i++) + n_insns += entry_insn_count (&da->entries[i]); + + if (!(insns = calloc (n_insns, sizeof (*insns)))) + return NULL; + + /* Prologue: load context fields into dedicated registers. + * r2 = major, r3 = minor, r4 = access_type + * Offsets match struct bpf_cgroup_dev_ctx field order. + */ + insns[pos++] = DEV_INSN_LDX_W (BPF_REG_2, BPF_REG_1, 4); /* major */ + insns[pos++] = DEV_INSN_LDX_W (BPF_REG_3, BPF_REG_1, 8); /* minor */ + insns[pos++] = DEV_INSN_LDX_W (BPF_REG_4, BPF_REG_1, 0); /* access_type */ + + for (int i = 0; i < da->count; i++) { + const struct device_allow_entry *e = &da->entries[i]; + int n = entry_insn_count (e); + int dev_type = (e->type == 'b') ? BPF_DEVCG_DEV_BLOCK + : BPF_DEVCG_DEV_CHAR; + int allowed = access_to_mask (e->access); + int deny_mask = ~allowed & 0x7; + + /* Check device type (lower 16 bits of access_type) */ + insns[pos++] = DEV_INSN_MOV32_REG (BPF_REG_5, BPF_REG_4); + insns[pos++] = DEV_INSN_AND32_IMM (BPF_REG_5, 0xffff); + insns[pos++] = DEV_INSN_JNE_IMM (BPF_REG_5, dev_type, n - 3); + /* Check major */ + insns[pos++] = DEV_INSN_JNE_IMM (BPF_REG_2, e->major, n - 4); + /* Check minor if not wildcard */ + if (e->minor != -1) + insns[pos++] = DEV_INSN_JNE_IMM (BPF_REG_3, e->minor, n - 5); + /* Check access: deny if requested bits exceed allowed */ + insns[pos++] = DEV_INSN_MOV32_REG (BPF_REG_5, BPF_REG_4); + insns[pos++] = DEV_INSN_RSH32_IMM (BPF_REG_5, 16); + insns[pos++] = DEV_INSN_AND32_IMM (BPF_REG_5, deny_mask); + insns[pos++] = DEV_INSN_JNE_IMM (BPF_REG_5, 0, 2); + /* Allow */ + insns[pos++] = DEV_INSN_MOV64_IMM (BPF_REG_0, 1); + insns[pos++] = DEV_INSN_EXIT (); + } + + /* Epilogue: default deny */ + insns[pos++] = DEV_INSN_MOV64_IMM (BPF_REG_0, 0); + insns[pos++] = DEV_INSN_EXIT (); + + assert (pos == n_insns); + *countp = n_insns; + return insns; +} + +static int cgroup_open (struct cgroup_info *cgroup) +{ + int fd = open (cgroup->path, O_RDONLY | O_DIRECTORY); + if (fd < 0) + return -1; + return fd; +} + +int cgroup_device_apply (struct cgroup_info *cgroup, + struct device_allow *da) +{ + int cgroup_fd = -1; + int prog_fd = -1; + int rc = -1; + struct bpf_insn *insns = NULL; + size_t n_insns; + if (!da) + return 0; + if (!cgroup) { + errno = EINVAL; + return -1; + } + if ((cgroup_fd = cgroup_open (cgroup)) < 0) { + imp_warn ("device: failed to open cgroup %s: %s", + cgroup->path, + strerror (errno)); + return -1; + } + if (!(insns = bpf_prog_build (da, &n_insns))) { + imp_warn ("device: failed to build BPF program: %s", strerror (errno)); + goto done; + } + + union bpf_attr load_attr = { + .prog_type = BPF_PROG_TYPE_CGROUP_DEVICE, + .insn_cnt = (__u32)n_insns, + .insns = (__u64)(uintptr_t)insns, + .license = (__u64)(uintptr_t)"LGPL", + }; + if ((prog_fd = syscall (SYS_bpf, + BPF_PROG_LOAD, + &load_attr, + sizeof (load_attr))) < 0) { + imp_warn ("device: failed to load BPF program: %s", strerror (errno)); + goto done; + } + + union bpf_attr attach_attr = { + .target_fd = cgroup_fd, + .attach_bpf_fd = prog_fd, + .attach_type = BPF_CGROUP_DEVICE, + }; + if (syscall (SYS_bpf, + BPF_PROG_ATTACH, + &attach_attr, + sizeof (attach_attr)) < 0) { + imp_warn ("device: failed to attach BPF program to cgroup: %s", + strerror (errno)); + goto done; + } + rc = 0; + +done: + free (insns); + if (prog_fd >= 0) + close (prog_fd); + close (cgroup_fd); + return rc; +} diff --git a/src/imp/cgroup_device.h b/src/imp/cgroup_device.h new file mode 100644 index 00000000..d3b335ba --- /dev/null +++ b/src/imp/cgroup_device.h @@ -0,0 +1,24 @@ +/************************************************************\ + * Copyright 2026 Lawrence Livermore National Security, LLC + * (c.f. AUTHORS, NOTICE.LLNS, COPYING) + * + * This file is part of the Flux resource manager framework. + * For details, see https://github.com/flux-framework. + * + * SPDX-License-Identifier: LGPL-3.0 +\************************************************************/ + +#ifndef HAVE_IMP_CGROUP_DEVICE_H +#define HAVE_IMP_CGROUP_DEVICE_H 1 + +#include "cgroup.h" +#include "exec/device.h" + +/* Attach a BPF device policy program to the job cgroup. + * Does nothing and returns 0 if da is NULL. + * Returns 0 on success, -1 on error with errno set. + */ +int cgroup_device_apply (struct cgroup_info *cgroup, + struct device_allow *da); + +#endif /* !HAVE_IMP_CGROUP_DEVICE_H */ diff --git a/src/imp/exec/device.c b/src/imp/exec/device.c new file mode 100644 index 00000000..00b8dc8e --- /dev/null +++ b/src/imp/exec/device.c @@ -0,0 +1,431 @@ +/************************************************************\ + * Copyright 2026 Lawrence Livermore National Security, LLC + * (c.f. AUTHORS, NOTICE.LLNS, COPYING) + * + * This file is part of the Flux resource manager framework. + * For details, see https://github.com/flux-framework. + * + * SPDX-License-Identifier: LGPL-3.0 +\************************************************************/ + +/* device.c - process DevicePolicy and DeviceAllow from input helper + * + * The IMP implements device containment on behalf of systemd because + * the systemd user instance doesn't have permission to load BPF programs. + * The IMP exec helper reads the following systemd properties and provides + * them to the IMP as input: + * + * DevicePolicy (string) may be set to + * strict - allow only DeviceAllow + * closed - allow DeviceAllow plus standard pseudo-devices + * auto (default) - closed, but allow all if DeviceAllow is missing/empty + * + * DeviceAllow (array) is a list of (specifier, access) tuples + * + * In a DeviceAllow entry: + * + * specifier (string) may be set to + * /dev/... - path to special file + * char-NAME - matches all char devices of the named class (/proc/devices) + * block-NAME - matches all block devices of the named class (/proc/devices) + * + * and access (string) may be set to a combination of + * r - read + * w - write + * m - mknod + * + * Error handling is fail-closed. + * + * If a DeviceAllow entry cannot be matched/processed, log a warning + * and continue on. This is consistent with fail-closed operation. + */ + +#if HAVE_CONFIG_H +#include +#endif +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#include "src/libutil/strlcpy.h" +#include "src/libutil/macros.h" +#include "imp_log.h" +#include "device.h" + +static bool access_is_valid (const char *access) +{ + int len = strlen (access); + + if (len < 1 || len > 3) + return false; + while (*access) { + if (*access != 'r' && *access != 'w' && *access != 'm') + return false; + access++; + } + return true; +} + +struct device_spec { + char *spec; + char *access; +}; + +/* This list initially mirrors systemd's bpf_devices_allow_list_static() + */ +static struct device_spec standard_devices[] = { + { "/dev/null", "rwm" }, + { "/dev/zero", "rwm" }, + { "/dev/full", "rwm" }, + { "/dev/random", "rwm" }, + { "/dev/urandom", "rwm" }, + { "/dev/tty", "rwm" }, + { "/dev/ptmx", "rwm" }, + { "char-pts", "rw" }, +}; + +static int device_entry_append (struct device_allow *da, + struct device_allow_entry entry) +{ + struct device_allow_entry *nentries; + size_t nsize = sizeof (da->entries[0]) * (da->count + 1); + + if (!(nentries = realloc (da->entries, nsize))) + return -1; + da->entries = nentries; + da->entries[da->count++] = entry; + return 0; +} + +static void strip_trailing_whitespace (char *s) +{ + for (int i = strlen(s) - 1; i >= 0; i--) { + if (!isspace (s[i])) + break; + s[i] = '\0'; + } +} + +static int device_class_append (struct device_allow *da, + const char *spec, + const char *access) +{ + const char *name; + char type; + FILE *fp; + char line[256]; + bool in_target_section = false; + int count = 0; + + if (strstarts (spec, "char-")) { + name = spec + 5; + type = 'c'; + } + else if (strstarts (spec, "block-")) { + name = spec + 6; + type = 'b'; + } + else { + imp_warn ("device: ignore %s (%s): invalid device node specifier", + spec, + access); + return 0; + } + if (!(fp = fopen ("/proc/devices", "r"))) { + imp_warn ("device: ignore %s (%s): /proc/devices: %s", + spec, + access, + strerror (errno)); + return 0; + } + while (fgets (line, sizeof (line), fp)) { + strip_trailing_whitespace (line); + if (streq (line, "Character devices:")) + in_target_section = type == 'c' ? true : false; + else if (streq (line, "Block devices:")) + in_target_section = type == 'b' ? true : false; + else if (in_target_section) { + int maj; + char devname[64]; + + if (sscanf (line, " %d %63s", &maj, devname) == 2 + && streq (devname, name)) { + struct device_allow_entry entry; + + entry.type = type; + entry.major = maj; + entry.minor = -1; + strlcpy (entry.access, access, sizeof (entry.access)); + if (device_entry_append (da, entry) < 0) + goto error; + count++; + } + } + } + if (count == 0) { + imp_warn ("device: ignore %s (%s): not found in /proc/devices", + spec, + access); + } + fclose (fp); + return count; +error: + ERRNO_SAFE_WRAP (fclose, fp); + return -1; +} + +static int device_path_append (struct device_allow *da, + const char *spec, + const char *access) +{ + struct stat st; + struct device_allow_entry entry; + + if (stat (spec, &st) < 0) { + imp_warn ("device: ignore %s (%s): %s", + spec, + access, + strerror (errno)); + return 0; + } + if (S_ISCHR (st.st_mode)) + entry.type = 'c'; + else if (S_ISBLK (st.st_mode)) + entry.type = 'b'; + else { + imp_warn ("device: ignore %s (%s): not a device", spec, access); + return 0; + } + entry.minor = minor (st.st_rdev); + entry.major = major (st.st_rdev); + strlcpy (entry.access, access, sizeof (entry.access)); + + if (device_entry_append (da, entry) < 0) + return -1; + return 1; +} + + +static int device_append (struct device_allow *da, + const char *spec, + const char *access) +{ + int count = 0; + + if (!access_is_valid (access)) { + imp_warn ("device: ignore %s (%s): invalid access key", + spec, + access); + } + else if (strstarts (spec, "char-") || strstarts (spec, "block-")) + count = device_class_append (da, spec, access); + else if (strstarts (spec, "/dev/")) + count = device_path_append (da, spec, access); + else + imp_warn ("device: ignore %s (%s): unknown specifier", spec, access); + + if (count > 0) + imp_debug ("device: allow %s (%s)", spec, access); + + return count; +} + +static int device_parse_options (json_t *options, + const char **policyp, + json_t **allowp) +{ + const char *policy = "auto"; // default + json_t *allow = NULL; + + if (json_unpack (options, + "{s?s s?o}", + "DevicePolicy", &policy, + "DeviceAllow", &allow) < 0) + goto error; + + if (!streq (policy, "auto") + && !streq (policy, "closed") + && !streq (policy, "strict")) + goto error; + + if (allow) { + if (!json_is_array (allow)) + goto error; + } + + *policyp = policy; + *allowp = allow; + return 0; +error: + errno = EINVAL; + return -1; +} + +int device_allow_from_options (json_t *options, struct device_allow **dap) +{ + const char *policy = "auto"; + json_t *allow; + struct device_allow *da = NULL; + + if (!dap) { + errno = EINVAL; + return -1; + } + if (!options) + goto allow_all; + if (device_parse_options (options, &policy, &allow) < 0) + return -1; + if (streq (policy, "auto") && (!allow || json_array_size (allow) == 0)) + goto allow_all; + if (!(da = calloc (1, sizeof (*da)))) + return -1; + imp_debug ("device: %s", policy); + if (!streq (policy, "strict")) { + for (size_t i = 0; i < ARRAY_SIZE (standard_devices); i++) { + if (device_append (da, + standard_devices[i].spec, + standard_devices[i].access) < 0) + goto error; + } + } + if (allow) { + size_t index; + json_t *entry; + + json_array_foreach (allow, index, entry) { + const char *spec; + const char *access; + + if (json_unpack (entry, "[ss]", &spec, &access) < 0) { + errno = EINVAL; + goto error; + } + if (device_append (da, spec, access) < 0) + goto error; + } + } + *dap = da; + return 0; +error: + device_allow_destroy (da); + return -1; +allow_all: + imp_debug ("device: allow all"); + *dap = NULL; + return 0; +} + +/* Encode one entry as "c:195:0:rw" */ +static int entry_encode (const struct device_allow_entry *e, + char *buf, + size_t size) +{ + int n = snprintf (buf, + size, + "%c:%d:%d:%s", + e->type, + e->major, + e->minor, + e->access); + if (n < 0 || (size_t)n >= size) { + errno = EOVERFLOW; + return -1; + } + return 0; +} + +static int entry_decode (const char *s, struct device_allow_entry *e) +{ + char type; + int major, minor; + char access[4]; + + if (sscanf (s, "%c:%d:%d:%3s", &type, &major, &minor, access) != 4 + || (type != 'c' && type != 'b') + || !access_is_valid (access)) { + errno = EINVAL; + return -1; + } + e->type = type; + e->major = major; + e->minor = minor; + strlcpy (e->access, access, sizeof (e->access)); + return 0; +} + +int device_allow_encode (const struct device_allow *da, struct kv *kv) +{ + char key[32]; + char val[32]; + + if (!da || !kv) { + errno = EINVAL; + return -1; + } + if (kv_put (kv, "device.count", KV_INT64, (int64_t)da->count) < 0) + return -1; + for (int i = 0; i < da->count; i++) { + snprintf (key, sizeof (key), "device.%d", i); + if (entry_encode (&da->entries[i], val, sizeof (val)) < 0) + return -1; + if (kv_put (kv, key, KV_STRING, val) < 0) + return -1; + } + return 0; +} + +int device_allow_decode (const struct kv *kv, struct device_allow **dap) +{ + struct device_allow *da = NULL; + int64_t count; + char key[32]; + + if (!kv || !dap) { + errno = EINVAL; + return -1; + } + if (kv_get (kv, "device.count", KV_INT64, &count) < 0) { + *dap = NULL; /* absent == no containment */ + return 0; + } + if (count < 0 || count > DEVICE_ALLOW_MAX_ENTRIES) { + errno = ERANGE; + return -1; + } + if (!(da = calloc (1, sizeof (*da)))) + return -1; + if (count > 0) { + if (!(da->entries = calloc (count, sizeof (*da->entries)))) + goto error; + for (int i = 0; i < (int)count; i++) { + const char *val; + snprintf (key, sizeof (key), "device.%d", i); + if (kv_get (kv, key, KV_STRING, &val) < 0) + goto error; + if (entry_decode (val, &da->entries[i]) < 0) + goto error; + } + } + da->count = (int)count; + *dap = da; + return 0; +error: + device_allow_destroy (da); + return -1; +} + +void device_allow_destroy (struct device_allow *da) +{ + if (da) { + free (da->entries); + free (da); + } +} + +// vi: ts=4 sw=4 expandtab diff --git a/src/imp/exec/device.h b/src/imp/exec/device.h new file mode 100644 index 00000000..d87753a3 --- /dev/null +++ b/src/imp/exec/device.h @@ -0,0 +1,58 @@ +/************************************************************\ + * Copyright 2026 Lawrence Livermore National Security, LLC + * (c.f. AUTHORS, NOTICE.LLNS, COPYING) + * + * This file is part of the Flux resource manager framework. + * For details, see https://github.com/flux-framework. + * + * SPDX-License-Identifier: LGPL-3.0 +\************************************************************/ + +#ifndef IMP_EXEC_DEVICE_H +#define IMP_EXEC_DEVICE_H + +#include +#include "src/libutil/kv.h" + +#define DEVICE_ALLOW_MAX_ENTRIES 512 + +/* A single resolved device allow entry. + * type is 'c' (char) or 'b' (block). + * major/minor are device numbers; minor == -1 means wildcard. + * access is a subset of "rwm" as a null-terminated string. + */ +struct device_allow_entry { + char type; + int major; + int minor; + char access[4]; +}; + +struct device_allow { + struct device_allow_entry *entries; + int count; +}; + +/* Parse device_allow from JSON options dict (received from IMP input). + * options may be NULL to indicate "no input options". + * Returns 0 with *dap set to NULL if no containment is configured. + * Returns 0 with *dap set on success. + * Returns -1 with errno set on parse error. + */ +int device_allow_from_options (json_t *options, struct device_allow **dap); + +/* Decode device_allow from a kv namespace. + * Returns 0 with *dap set to NULL if "device.count" is absent (no containment). + * Returns 0 with *dap set on success. + * Returns -1 with errno set on error. + */ +int device_allow_decode (const struct kv *kv, struct device_allow **dap); + +void device_allow_destroy (struct device_allow *da); + +/* Encode device_allow into kv namespace for privsep transfer. + * Returns 0 on success, -1 on error with errno set. + */ +int device_allow_encode (const struct device_allow *da, struct kv *kv); + +#endif /* IMP_EXEC_DEVICE_H */ diff --git a/src/imp/exec/exec.c b/src/imp/exec/exec.c index 1c910bd7..0df406c2 100644 --- a/src/imp/exec/exec.c +++ b/src/imp/exec/exec.c @@ -48,6 +48,8 @@ #include "user.h" #include "safe_popen.h" #include "signals.h" +#include "device.h" +#include "cgroup_device.h" #if HAVE_PAM #include "pam.h" @@ -67,6 +69,7 @@ struct imp_exec { struct kv *args; const void *spec; int specsz; + struct device_allow *da; }; extern const char *imp_get_security_config_pattern (void); @@ -116,6 +119,7 @@ static void imp_exec_destroy (struct imp_exec *exec) passwd_destroy (exec->user_pwd); passwd_destroy (exec->imp_pwd); kv_destroy (exec->args); + device_allow_destroy (exec->da); free (exec); } } @@ -170,6 +174,12 @@ static void imp_exec_init_kv (struct imp_exec *exec, struct kv *kv) if (!(exec->args = kv_split (kv, "args"))) imp_die (1, "exec: Failed to get job shell arguments"); + /* Optional device containment — absent means no containment */ + if (device_allow_decode (kv, &exec->da) < 0) + imp_die (1, + "exec: failed to decode device containment policy: %s", + strerror (errno)); + imp_exec_unwrap (exec, exec->J); } @@ -177,6 +187,7 @@ static void imp_exec_init_stream (struct imp_exec *exec, FILE *fp) { struct imp_state *imp; json_error_t err; + json_t *options = NULL; assert (exec != NULL && exec->imp != NULL && fp != NULL); @@ -196,11 +207,17 @@ static void imp_exec_init_stream (struct imp_exec *exec, FILE *fp) || json_unpack_ex (exec->input, &err, 0, - "{s:s}", - "J", &exec->J) < 0) + "{s:s s?o}", + "J", &exec->J, + "options", &options) < 0) imp_die (1, "exec: invalid json input: %s", err.text); imp_exec_unwrap (exec, exec->J); + + if (device_allow_from_options (options, &exec->da) < 0) + imp_die (1, + "exec: failed to parse device containment policy: %s", + strerror (errno)); } static void __attribute__((noreturn)) imp_exec (struct imp_exec *exec) @@ -264,6 +281,12 @@ int imp_exec_privileged (struct imp_state *imp, struct kv *kv) #endif /* HAVE_PAM */ } + /* Apply BPF device containment policy to job cgroup */ + if (cgroup_device_apply (imp->cgroup, exec->da) < 0) + imp_die (1, + "exec: failed to apply device containment policy: %s", + strerror (errno)); + /* Block signals so parent IMP isn't unduly terminated */ imp_sigblock_all (); @@ -353,6 +376,10 @@ static void imp_exec_put_kv (struct imp_exec *exec, imp_die (1, "exec: Failed to get job shell path"); if (kv_join (kv, exec->args, "args") < 0) imp_die (1, "exec: Failed to set job shell arguments"); + if (exec->da && device_allow_encode (exec->da, kv) < 0) + imp_die (1, + "exec: failed to encode device policy: %s", + strerror (errno)); } /* Read IMP input using a helper process diff --git a/src/imp/test/device.c b/src/imp/test/device.c new file mode 100644 index 00000000..ecaef9be --- /dev/null +++ b/src/imp/test/device.c @@ -0,0 +1,523 @@ +/************************************************************\ + * Copyright 2026 Lawrence Livermore National Security, LLC + * (c.f. AUTHORS, NOTICE.LLNS, COPYING) + * + * This file is part of the Flux resource manager framework. + * For details, see https://github.com/flux-framework. + * + * SPDX-License-Identifier: LGPL-3.0 +\************************************************************/ + +#include +#include +#include +#include +#include + +#include "exec/device.h" +#include "imp_log.h" +#include "src/libutil/kv.h" + +#include "src/libtap/tap.h" + +static int diag_output (int level, const char *msg, + void *arg __attribute__((unused))) +{ + diag ("%s: %s", imp_log_strlevel (level), msg); + return 0; +} + +/* Build an options object with optional DevicePolicy and DeviceAllow. + * policy may be NULL to omit. allow_arr may be NULL to omit. + * Caller owns the returned object. + */ +static json_t *make_options (const char *policy, json_t *allow_arr) +{ + json_t *obj = json_object (); + if (!obj) + return NULL; + if (policy) { + if (json_object_set_new (obj, "DevicePolicy", + json_string (policy)) < 0) + goto error; + } + if (allow_arr) { + if (json_object_set_new (obj, "DeviceAllow", allow_arr) < 0) + goto error; + } + return obj; +error: + json_decref (obj); + return NULL; +} + +/* Return true if da contains an entry matching type, major, minor, access. + * Pass -2 for major or minor to skip that field. + */ +static bool da_contains (const struct device_allow *da, + char type, + int major, + int minor, + const char *access) +{ + for (int i = 0; i < da->count; i++) { + const struct device_allow_entry *e = &da->entries[i]; + if (e->type == type + && (major == -2 || e->major == major) + && (minor == -2 || e->minor == minor) + && strcmp (e->access, access) == 0) + return true; + } + return false; +} + +/* ---- device_allow_from_options tests ---- */ + +static void test_null_options (void) +{ + struct device_allow *da = NULL; + + ok (device_allow_from_options (NULL, &da) == 0 && da == NULL, + "from_options: NULL options returns 0 with NULL da (no containment)"); +} + +static void test_null_dap (void) +{ + json_t *opts = make_options (NULL, NULL); + if (!opts) + BAIL_OUT ("make_options failed"); + + errno = 0; + ok (device_allow_from_options (opts, NULL) < 0 && errno == EINVAL, + "from_options: NULL dap fails with EINVAL"); + json_decref (opts); +} + +static void test_auto_no_allow (void) +{ + json_t *opts = make_options ("auto", NULL); + struct device_allow *da = NULL; + if (!opts) + BAIL_OUT ("make_options failed"); + + ok (device_allow_from_options (opts, &da) == 0 && da == NULL, + "from_options: auto + absent DeviceAllow returns NULL da"); + json_decref (opts); +} + +static void test_auto_empty_allow (void) +{ + json_t *opts = make_options ("auto", json_array ()); + struct device_allow *da = NULL; + if (!opts) + BAIL_OUT ("make_options failed"); + + ok (device_allow_from_options (opts, &da) == 0 && da == NULL, + "from_options: auto + empty DeviceAllow returns NULL da"); + json_decref (opts); +} + +static void test_invalid_policy (void) +{ + json_t *opts = make_options ("bogus", NULL); + struct device_allow *da = NULL; + if (!opts) + BAIL_OUT ("make_options failed"); + + errno = 0; + ok (device_allow_from_options (opts, &da) < 0 && errno == EINVAL, + "from_options: invalid DevicePolicy fails with EINVAL"); + device_allow_destroy (da); + json_decref (opts); +} + +static void test_path_entry (void) +{ + /* /dev/null is char major=1 minor=3 on all Linux systems */ + json_t *allow = json_pack ("[[ss]]", "/dev/null", "rw"); + json_t *opts = make_options ("strict", allow); + struct device_allow *da = NULL; + if (!opts) + BAIL_OUT ("make_options failed"); + + ok (device_allow_from_options (opts, &da) == 0 && da != NULL, + "from_options: strict /dev/null returns 0 with non-NULL da"); + ok (da && da_contains (da, 'c', 1, 3, "rw"), + "from_options: /dev/null resolved to {c, 1, 3, \"rw\"}"); + device_allow_destroy (da); + json_decref (opts); +} + +static void test_class_entry (void) +{ + /* char-pts should appear in /proc/devices on any system with a terminal */ + json_t *allow = json_pack ("[[ss]]", "char-pts", "rw"); + json_t *opts = make_options ("strict", allow); + struct device_allow *da = NULL; + if (!opts) + BAIL_OUT ("make_options failed"); + + ok (device_allow_from_options (opts, &da) == 0 && da != NULL, + "from_options: strict char-pts returns 0 with non-NULL da"); + ok (da && da->count > 0, + "from_options: char-pts resolved to at least one entry"); + ok (da && da_contains (da, 'c', -2, -1, "rw"), + "from_options: char-pts entry has type='c', minor=-1 (wildcard)"); + device_allow_destroy (da); + json_decref (opts); +} + +static void test_block_class_entry (void) +{ + /* block-loop (major 7) should appear in /proc/devices on any Linux system */ + json_t *allow = json_pack ("[[ss]]", "block-loop", "rw"); + json_t *opts = make_options ("strict", allow); + struct device_allow *da = NULL; + if (!opts) + BAIL_OUT ("make_options failed"); + + ok (device_allow_from_options (opts, &da) == 0 && da != NULL, + "from_options: strict block-loop returns 0 with non-NULL da"); + ok (da && da->count > 0, + "from_options: block-loop resolved to at least one entry"); + ok (da && da_contains (da, 'b', 7, -1, "rw"), + "from_options: block-loop entry has type='b', major=7, minor=-1 (wildcard)"); + device_allow_destroy (da); + json_decref (opts); +} + +static void test_nonexistent_path (void) +{ + /* Non-existent path is a warning, not an error (fail-closed: skip entry) */ + json_t *allow = json_pack ("[[ss]]", "/dev/nonexistent_flux_test", "rw"); + json_t *opts = make_options ("strict", allow); + struct device_allow *da = NULL; + if (!opts) + BAIL_OUT ("make_options failed"); + + ok (device_allow_from_options (opts, &da) == 0 && da != NULL, + "from_options: nonexistent path returns 0 (skip, not fatal)"); + ok (da && da->count == 0, + "from_options: nonexistent path produces empty allow list"); + device_allow_destroy (da); + json_decref (opts); +} + +static void test_invalid_access (void) +{ + /* Invalid access string is a warning, not an error */ + json_t *allow = json_pack ("[[ss]]", "/dev/null", "z"); + json_t *opts = make_options ("strict", allow); + struct device_allow *da = NULL; + if (!opts) + BAIL_OUT ("make_options failed"); + + ok (device_allow_from_options (opts, &da) == 0 && da != NULL, + "from_options: invalid access returns 0 (skip, not fatal)"); + ok (da && da->count == 0, + "from_options: invalid access produces empty allow list"); + device_allow_destroy (da); + json_decref (opts); +} + +static void test_unknown_specifier (void) +{ + /* Unknown specifier (not /dev/, char-, or block-) is warned and skipped */ + json_t *allow = json_pack ("[[ss]]", "unknownspec", "rw"); + json_t *opts = make_options ("strict", allow); + struct device_allow *da = NULL; + if (!opts) + BAIL_OUT ("make_options failed"); + + ok (device_allow_from_options (opts, &da) == 0 && da != NULL, + "from_options: unknown specifier returns 0 (skip, not fatal)"); + ok (da && da->count == 0, + "from_options: unknown specifier produces empty allow list"); + device_allow_destroy (da); + json_decref (opts); +} + +static void test_closed_standard_devices (void) +{ + /* closed policy with empty DeviceAllow should include standard devices */ + json_t *opts = make_options ("closed", json_array ()); + struct device_allow *da = NULL; + if (!opts) + BAIL_OUT ("make_options failed"); + + ok (device_allow_from_options (opts, &da) == 0 && da != NULL, + "from_options: closed + empty DeviceAllow returns non-NULL da"); + ok (da && da->count > 0, + "from_options: closed policy includes standard devices"); + ok (da && da_contains (da, 'c', 1, 3, "rwm"), + "from_options: closed policy includes /dev/null {c,1,3,rwm}"); + ok (da && da_contains (da, 'c', 1, 5, "rwm"), + "from_options: closed policy includes /dev/zero {c,1,5,rwm}"); + device_allow_destroy (da); + json_decref (opts); +} + +static void test_strict_no_standard_devices (void) +{ + /* strict policy should not add standard devices */ + json_t *allow = json_pack ("[[ss]]", "/dev/null", "rw"); + json_t *opts = make_options ("strict", allow); + struct device_allow *da = NULL; + if (!opts) + BAIL_OUT ("make_options failed"); + + ok (device_allow_from_options (opts, &da) == 0 && da != NULL, + "from_options: strict policy returns non-NULL da"); + ok (da && da->count == 1, + "from_options: strict policy has exactly the listed entry, no extras"); + device_allow_destroy (da); + json_decref (opts); +} + +static void test_closed_standard_devices_first (void) +{ + /* Standard devices are prepended before user entries */ + json_t *allow = json_pack ("[[ss]]", "/dev/null", "rw"); + json_t *opts = make_options ("closed", allow); + struct device_allow *da = NULL; + if (!opts) + BAIL_OUT ("make_options failed"); + + ok (device_allow_from_options (opts, &da) == 0 && da != NULL, + "from_options: closed + user entry returns non-NULL da"); + ok (da && da->count > 1, + "from_options: closed policy has user entry plus standard devices"); + ok (da && da->entries[0].major == 1 && da->entries[0].minor == 3, + "from_options: standard devices precede user entries"); + device_allow_destroy (da); + json_decref (opts); +} + +/* ---- encode/decode round-trip tests ---- */ + +static void test_encode_decode_roundtrip (void) +{ + struct device_allow *orig = calloc (1, sizeof (*orig)); + struct device_allow *decoded = NULL; + struct kv *kv; + + if (!orig) + BAIL_OUT ("calloc failed"); + orig->count = 2; + if (!(orig->entries = calloc (2, sizeof (orig->entries[0])))) + BAIL_OUT ("calloc failed"); + + orig->entries[0].type = 'c'; + orig->entries[0].major = 195; + orig->entries[0].minor = 0; + strcpy (orig->entries[0].access, "rw"); + + orig->entries[1].type = 'b'; + orig->entries[1].major = 8; + orig->entries[1].minor = -1; + strcpy (orig->entries[1].access, "rwm"); + + if (!(kv = kv_create ())) + BAIL_OUT ("kv_create failed"); + + ok (device_allow_encode (orig, kv) == 0, + "encode/decode: device_allow_encode succeeds"); + ok (device_allow_decode (kv, &decoded) == 0 && decoded != NULL, + "encode/decode: device_allow_decode returns non-NULL da"); + ok (decoded && decoded->count == 2, + "encode/decode: count == 2"); + ok (decoded && decoded->entries[0].type == 'c' + && decoded->entries[0].major == 195 + && decoded->entries[0].minor == 0 + && strcmp (decoded->entries[0].access, "rw") == 0, + "encode/decode: entry[0] round-trips correctly"); + ok (decoded && decoded->entries[1].type == 'b' + && decoded->entries[1].major == 8 + && decoded->entries[1].minor == -1 + && strcmp (decoded->entries[1].access, "rwm") == 0, + "encode/decode: entry[1] round-trips correctly (wildcard minor)"); + + device_allow_destroy (orig); + device_allow_destroy (decoded); + kv_destroy (kv); +} + +static void test_decode_absent (void) +{ + struct kv *kv = kv_create (); + struct device_allow *da = NULL; + + if (!kv) + BAIL_OUT ("kv_create failed"); + ok (device_allow_decode (kv, &da) == 0 && da == NULL, + "decode_absent: absent device.count returns 0 with NULL da"); + kv_destroy (kv); +} + +static void test_decode_negative_count (void) +{ + struct kv *kv = kv_create (); + struct device_allow *da = NULL; + + if (!kv) + BAIL_OUT ("kv_create failed"); + if (kv_put (kv, "device.count", KV_INT64, (int64_t)-1) < 0) + BAIL_OUT ("kv_put failed"); + errno = 0; + ok (device_allow_decode (kv, &da) < 0 && errno == ERANGE, + "decode_negative: negative count fails with ERANGE"); + kv_destroy (kv); +} + +static void test_decode_count_too_large (void) +{ + struct kv *kv = kv_create (); + struct device_allow *da = NULL; + + if (!kv) + BAIL_OUT ("kv_create failed"); + if (kv_put (kv, "device.count", KV_INT64, + (int64_t)(DEVICE_ALLOW_MAX_ENTRIES + 1)) < 0) + BAIL_OUT ("kv_put failed"); + errno = 0; + ok (device_allow_decode (kv, &da) < 0 && errno == ERANGE, + "decode_too_large: count > %d fails with ERANGE", + DEVICE_ALLOW_MAX_ENTRIES); + kv_destroy (kv); +} + +static void test_options_allow_not_array (void) +{ + json_t *opts = json_pack ("{s:s}", "DeviceAllow", "notanarray"); + struct device_allow *da = NULL; + if (!opts) + BAIL_OUT ("json_pack failed"); + + errno = 0; + ok (device_allow_from_options (opts, &da) < 0 && errno == EINVAL, + "from_options: DeviceAllow not-an-array fails with EINVAL"); + device_allow_destroy (da); + json_decref (opts); +} + +static void test_options_policy_not_string (void) +{ + json_t *opts = json_pack ("{s:i}", "DevicePolicy", 42); + struct device_allow *da = NULL; + if (!opts) + BAIL_OUT ("json_pack failed"); + + errno = 0; + ok (device_allow_from_options (opts, &da) < 0 && errno == EINVAL, + "from_options: DevicePolicy not-a-string fails with EINVAL"); + device_allow_destroy (da); + json_decref (opts); +} + +static void test_null_encode_decode_params (void) +{ + struct kv *kv = kv_create (); + struct device_allow empty = { .count = 0, .entries = NULL }; + struct device_allow *da = NULL; + + if (!kv) + BAIL_OUT ("kv_create failed"); + + errno = 0; + ok (device_allow_encode (NULL, kv) < 0 && errno == EINVAL, + "encode: NULL da fails with EINVAL"); + errno = 0; + ok (device_allow_encode (&empty, NULL) < 0 && errno == EINVAL, + "encode: NULL kv fails with EINVAL"); + errno = 0; + ok (device_allow_decode (NULL, &da) < 0 && errno == EINVAL, + "decode: NULL kv fails with EINVAL"); + errno = 0; + ok (device_allow_decode (kv, NULL) < 0 && errno == EINVAL, + "decode: NULL dap fails with EINVAL"); + + kv_destroy (kv); +} + +/* Encode an entry with type='x' (invalid). entry_encode does not + * validate, so encode succeeds; decode must reject the invalid type. + */ +static void test_encode_invalid_entry (void) +{ + struct device_allow *da = calloc (1, sizeof (*da)); + struct kv *kv = kv_create (); + struct device_allow *decoded = NULL; + + if (!da || !kv) + BAIL_OUT ("allocation failed"); + da->count = 1; + if (!(da->entries = calloc (1, sizeof (da->entries[0])))) + BAIL_OUT ("calloc failed"); + da->entries[0].type = 'x'; + da->entries[0].major = 1; + da->entries[0].minor = 0; + strcpy (da->entries[0].access, "rw"); + + ok (device_allow_encode (da, kv) == 0, + "encode: invalid entry type='x' encodes without error"); + errno = 0; + ok (device_allow_decode (kv, &decoded) < 0 && errno == EINVAL, + "decode: kv with invalid entry type fails with EINVAL"); + + device_allow_destroy (da); + device_allow_destroy (decoded); + kv_destroy (kv); +} + +static void test_decode_entry_not_string (void) +{ + struct kv *kv = kv_create (); + struct device_allow *da = NULL; + + if (!kv) + BAIL_OUT ("kv_create failed"); + if (kv_put (kv, "device.count", KV_INT64, (int64_t)1) < 0) + BAIL_OUT ("kv_put failed"); + if (kv_put (kv, "device.0", KV_INT64, (int64_t)42) < 0) + BAIL_OUT ("kv_put failed"); + ok (device_allow_decode (kv, &da) < 0, + "decode: entry stored as INT64 instead of STRING fails"); + device_allow_destroy (da); + kv_destroy (kv); +} + +int main (void) +{ + plan (NO_PLAN); + + imp_openlog (); + imp_log_add ("tap", IMP_LOG_DEBUG, diag_output, NULL); + imp_log_set_level (NULL, IMP_LOG_DEBUG); + + test_null_options (); + test_null_dap (); + test_auto_no_allow (); + test_auto_empty_allow (); + test_invalid_policy (); + test_path_entry (); + test_class_entry (); + test_block_class_entry (); + test_nonexistent_path (); + test_invalid_access (); + test_unknown_specifier (); + test_closed_standard_devices (); + test_strict_no_standard_devices (); + test_closed_standard_devices_first (); + test_encode_decode_roundtrip (); + test_decode_absent (); + test_decode_negative_count (); + test_decode_count_too_large (); + test_options_allow_not_array (); + test_options_policy_not_string (); + test_null_encode_decode_params (); + test_encode_invalid_entry (); + test_decode_entry_not_string (); + + done_testing (); + return 0; +} diff --git a/src/libutil/macros.h b/src/libutil/macros.h index 6d80b416..12d4452c 100644 --- a/src/libutil/macros.h +++ b/src/libutil/macros.h @@ -20,4 +20,25 @@ */ #define BASE64_DECODE_SIZE(x) ((((x) + 3) / 4) * 3) + +#ifndef streq +#define streq(x,y) (strcmp((x),(y)) == 0) +#endif + +#ifndef strstarts +#define strstarts(x,y) (strncmp((x),(y),strlen(y)) == 0) +#endif + +#ifndef ARRAY_SIZE +#define ARRAY_SIZE(x) (sizeof((x)) / sizeof((x)[0])) +#endif + +#ifndef ERRNO_SAFE_WRAP +#define ERRNO_SAFE_WRAP(fun, ...) do { \ + int saved_errno = errno; \ + (fun)(__VA_ARGS__); \ + errno = saved_errno; \ +} while (0) +#endif + #endif diff --git a/t/Makefile.am b/t/Makefile.am index 36845bd3..8f115825 100644 --- a/t/Makefile.am +++ b/t/Makefile.am @@ -22,7 +22,8 @@ TESTSCRIPTS = \ t1003-sign-curve.t \ t2000-imp-exec.t \ t2002-imp-run.t \ - t2003-imp-exec-pam.t + t2003-imp-exec-pam.t \ + t2004-imp-exec-device.t TESTS = \ $(TESTSCRIPTS) @@ -40,7 +41,8 @@ check_PROGRAMS = \ src/xsign_munge \ src/xsign_curve \ src/uidlookup \ - src/sanitizers-enabled + src/sanitizers-enabled \ + src/bpf_cgroup_probe check_LTLIBRARIES = \ src/getpwuid.la @@ -102,6 +104,8 @@ src_uidlookup_SOURCES = src/uidlookup.c src_uidlookup_CPPFLAGS = $(test_cppflags) src_uidlookup_LDADD = $(test_ldadd) +src_bpf_cgroup_probe_SOURCES = src/bpf_cgroup_probe.c + EXTRA_DIST= \ sharness.sh \ sharness.d \ diff --git a/t/src/bpf_cgroup_probe.c b/t/src/bpf_cgroup_probe.c new file mode 100644 index 00000000..41ac7f2a --- /dev/null +++ b/t/src/bpf_cgroup_probe.c @@ -0,0 +1,40 @@ +/************************************************************\ + * Copyright 2026 Lawrence Livermore National Security, LLC + * (c.f. AUTHORS, NOTICE.LLNS, COPYING) + * + * This file is part of the Flux resource manager framework. + * For details, see https://github.com/flux-framework. + * + * SPDX-License-Identifier: LGPL-3.0 +\************************************************************/ + +/* Exit 0 if BPF_PROG_TYPE_CGROUP_DEVICE programs can be loaded, + * nonzero otherwise. Used as a prereq probe in the test suite. + */ + +#include +#include +#include +#include + +int main (void) +{ + struct bpf_insn prog[] = { + { .code = BPF_ALU64 | BPF_MOV | BPF_K, .dst_reg = BPF_REG_0, .imm = 1 }, + { .code = BPF_JMP | BPF_EXIT }, + }; + union bpf_attr attr = { + .prog_type = BPF_PROG_TYPE_CGROUP_DEVICE, + .insn_cnt = 2, + .insns = (__u64)(uintptr_t)prog, + .license = (__u64)(uintptr_t)"GPL", + }; + int fd = (int)syscall (SYS_bpf, BPF_PROG_LOAD, &attr, sizeof (attr)); + if (fd < 0) + return 1; + close (fd); + return 0; +} + +/* vi: ts=4 sw=4 expandtab + */ diff --git a/t/t2004-imp-exec-device.t b/t/t2004-imp-exec-device.t new file mode 100755 index 00000000..154bdb64 --- /dev/null +++ b/t/t2004-imp-exec-device.t @@ -0,0 +1,155 @@ +#!/bin/sh +# + +test_description='IMP exec device containment tests + +Tests for DevicePolicy/DeviceAllow parsing and BPF device containment. +Unprivileged tests check JSON parsing and policy resolution. +Privileged tests check that BPF programs are loaded and attached. +' + +# Append --logfile option if FLUX_TESTS_LOGFILE is set in environment: +test -n "$FLUX_TESTS_LOGFILE" && set -- "$@" --logfile +. `dirname $0`/sharness.sh + +flux_imp=${SHARNESS_BUILD_DIRECTORY}/src/imp/flux-imp +sign=${SHARNESS_BUILD_DIRECTORY}/t/src/sign +bpf_cgroup_probe=${SHARNESS_BUILD_DIRECTORY}/t/src/bpf_cgroup_probe + +echo "# Using ${flux_imp}" + +CGROUP_MOUNT=$(awk '$3 == "cgroup2" {print $2}' /proc/self/mounts) +test -n "$CGROUP_MOUNT" || bail_out "Failed to get cgroup2 mount dir!" +CURRENT_CGROUP_PATH=$(cat /proc/self/cgroup | sed -n s/^0:://p) +CGROUP_PATH="${CGROUP_MOUNT}${CURRENT_CGROUP_PATH}/imp-shell.$$" +echo "# using CGROUP_PATH=$CGROUP_PATH" + +# Produce IMP exec JSON with no options (baseline, no containment). +fake_J() { + echo foo | env FLUX_IMP_CONFIG_PATTERN=sign-none.toml $sign +} + +no_device_input() { + printf '{"J":"%s"}' $(fake_J) +} + +# Produce IMP exec JSON +# Usage: device_input DevicePolicy DeviceAllow +device_input() { + printf '{"J":"%s","options":{"DevicePolicy":"%s","DeviceAllow":%s}}' \ + $(fake_J) "$1" "${2:-[]}" +} + +cat <<'EOF' >run-in-cgroup.sh +#!/bin/sh +path=$1 +shift +test -d $path || mkdir -p $path && +echo $$ >${path}/cgroup.procs && +exec "$@" +EOF +chmod +x run-in-cgroup.sh + +cat <sign-none.toml +allow-sudo = true +[sign] +max-ttl = 30 +default-type = "none" +allowed-types = [ "none" ] +[exec] +allowed-users = [ "$(whoami)" ] +allowed-shells = [ "echo", "cat", "true" ] +allow-unprivileged-exec = true +EOF +export FLUX_IMP_CONFIG_PATTERN=sign-none.toml + +if test_have_prereq SUDO; then + if $SUDO mkdir $CGROUP_PATH; then + test_set_prereq CGROUPFS + cleanup "$SUDO rmdir $CGROUP_PATH" + fi + if $SUDO ./run-in-cgroup.sh $CGROUP_PATH cat /proc/self/cgroup; then + test_set_prereq CGROUP_WRITABLE + fi + if test_have_prereq CGROUPFS,CGROUP_WRITABLE; then + if $SUDO $bpf_cgroup_probe; then + test_set_prereq BPF_CGROUP + fi + fi +fi + + +# ---- Unprivileged tests: policy parsing only, no BPF applied ---- + +test_expect_success 'absent options key: exec succeeds with no containment' ' + no_device_input | $flux_imp exec echo ok >absent.out && + grep -q ok absent.out +' +test_expect_success 'invalid DevicePolicy: exec fails with diagnostic' ' + device_input bogus | + test_must_fail $flux_imp exec echo ok >bad-device.out 2>&1 && + test_debug "cat bad-device.out" && + grep "device containment policy" bad-device.out +' +test_expect_success 'auto policy with empty DeviceAllow: exec succeeds' ' + device_input auto | + $flux_imp exec echo ok >auto-empty.out && + grep -q ok auto-empty.out +' +test_expect_success 'strict policy with /dev/null: exec succeeds' ' + device_input strict "[[\"/dev/null\",\"rw\"]]" | + $flux_imp exec echo ok >strict-null.out && + grep -q ok strict-null.out +' +test_expect_success 'closed policy with empty DeviceAllow: exec succeeds' ' + device_input closed | + $flux_imp exec echo ok >closed-empty.out && + grep -q ok closed-empty.out +' +test_expect_success 'block- class specifier: exec succeeds' ' + device_input strict "[[\"block-loop\",\"rw\"]]" | + $flux_imp exec echo ok >block-loop.out && + grep -q ok block-loop.out +' +test_expect_success 'unknown specifier: exec succeeds (entry skipped, no containment failure)' ' + device_input strict "[[\"unknownspec\",\"rw\"]]" | + $flux_imp exec echo ok >unknown-spec.out && + grep -q ok unknown-spec.out +' + +# ---- Privileged tests: BPF program loaded and attached to cgroup ---- + +test_expect_success BPF_CGROUP \ + 'closed policy: BPF program loads and job runs successfully' ' + device_input closed | + $SUDO FLUX_IMP_CONFIG_PATTERN=sign-none.toml \ + ./run-in-cgroup.sh "$CGROUP_PATH" \ + $flux_imp exec echo ok >closed-bpf.out && + grep -q ok closed-bpf.out +' +test_expect_success BPF_CGROUP \ + 'strict policy with /dev/null: BPF program loads and job runs successfully' ' + device_input strict "[[\"/dev/null\",\"rwm\"]]" | + $SUDO FLUX_IMP_CONFIG_PATTERN=sign-none.toml \ + ./run-in-cgroup.sh "$CGROUP_PATH" \ + $flux_imp exec echo ok >strict-bpf.out && + grep -q ok strict-bpf.out +' +test_expect_success BPF_CGROUP \ + 'strict policy with empty DeviceAllow denies access to /dev/null' ' + device_input strict "[]" | + test_must_fail $SUDO FLUX_IMP_CONFIG_PATTERN=sign-none.toml \ + ./run-in-cgroup.sh "$CGROUP_PATH" \ + $flux_imp exec cat /dev/null >deny-null.out 2>&1 && + test_debug "cat deny-null.out" && + grep -q "Operation not permitted" deny-null.out +' +test_expect_success BPF_CGROUP \ + 'strict policy allowing /dev/null permits access' ' + device_input strict "[[\"/dev/null\",\"rw\"]]" | + $SUDO FLUX_IMP_CONFIG_PATTERN=sign-none.toml \ + ./run-in-cgroup.sh "$CGROUP_PATH" \ + $flux_imp exec cat /dev/null +' + +test_done