From 02a4a2f893153a8325ed2a932dc9c9fa90333700 Mon Sep 17 00:00:00 2001 From: Young Joon Lee Date: Tue, 20 Oct 2026 19:53:16 +0900 Subject: [PATCH 1/3] feat(monitoring): manage the exporter firewall guard and keep exporters up across docker restarts Embed the hand-installed rootfiles-exporter-firewall helper (same paths, chains and owner marker, so existing guards are adopted in place) behind modules.monitoring.exporter_firewall. Exporter drop-ins now order after the guard and verify their rules in ExecStartPre instead of Requires=, the guard unit only Wants docker, and the DCGM exporter and hub units are wanted by docker.service. Doctor reports coverage from the guard check and names an unmanaged guard. A runner scenario covers adoption, filtering, guard and docker restarts and fail-closed starts. Closes #83 Claude-Session: https://claude.ai/code/session_0177Zk7to4EYohgM59v4Fcj7 --- .github/workflows/test.yaml | 15 + README.md | 7 +- docs/releases/v0.18.0.md | 13 + internal/config/config.go | 17 +- internal/config/monitoring_firewall_test.go | 42 ++ internal/config/monitoring_validate.go | 16 + internal/module/assets/exporter-firewall.sh | 581 ++++++++++++++++++ internal/module/doctor.go | 23 +- internal/module/monitoring.go | 8 +- internal/module/monitoring_exporters.go | 16 +- internal/module/monitoring_guard.go | 146 +++++ internal/module/monitoring_guard_test.go | 261 ++++++++ internal/module/monitoring_hub.go | 10 +- internal/module/monitoring_hub_test.go | 5 +- internal/module/monitoring_test.go | 15 + tests/scenarios/exporter-firewall.sh | 118 ++++ .../fixtures/exporter-firewall-v1.sh | 551 +++++++++++++++++ 17 files changed, 1829 insertions(+), 15 deletions(-) create mode 100644 docs/releases/v0.18.0.md create mode 100644 internal/config/monitoring_firewall_test.go create mode 100644 internal/module/assets/exporter-firewall.sh create mode 100644 internal/module/monitoring_guard.go create mode 100644 internal/module/monitoring_guard_test.go create mode 100755 tests/scenarios/exporter-firewall.sh create mode 100644 tests/scenarios/fixtures/exporter-firewall-v1.sh diff --git a/.github/workflows/test.yaml b/.github/workflows/test.yaml index c1a3a17..22ef1aa 100644 --- a/.github/workflows/test.yaml +++ b/.github/workflows/test.yaml @@ -321,6 +321,21 @@ jobs: chmod +x rootfiles ROOTFILES_TEST_BINARY="$PWD/rootfiles" bash tests/scenarios/tenant-lifecycle.sh + exporter-firewall-scenario: + needs: unit + runs-on: ubuntu-latest + timeout-minutes: 20 + steps: + - uses: actions/checkout@v4 + - uses: actions/download-artifact@v4 + with: + name: rootfiles-binary + path: . + - name: Exporter firewall guard on the runner's Docker, iptables and systemd + run: | + chmod +x rootfiles + ROOTFILES_TEST_BINARY="$PWD/rootfiles" bash tests/scenarios/exporter-firewall.sh + rocky: needs: unit runs-on: ubuntu-latest diff --git a/README.md b/README.md index 20dd75e..9d6acdb 100644 --- a/README.md +++ b/README.md @@ -650,10 +650,15 @@ modules: allow_from: - 10.0.0.5/32 perimeter_firewall: false + exporter_firewall: false ``` DCGM exporter requires an already working NVIDIA GPU, driver, Docker daemon, and NVIDIA container runtime. A missing prerequisite shows as `blocked` in `rootfiles check` and fails `apply --dry-run`; `apply` still sets up node exporter and the rest of the module, then fails with the DCGM error and the fix (for a missing runtime: `sudo nvidia-ctk runtime configure --runtime=docker && sudo systemctl reload docker`). If UFW is active, `allow_from` only applies when its incoming default is deny/reject and there are no broader allow rules for either exporter port. rootfiles reports conflicts and leaves existing firewall rules unchanged. It does not enable UFW. +`exporter_firewall: true` installs an iptables guard that admits the exporter ports only from `allow_from`, for hosts without UFW whose segment is not closed by a perimeter firewall. It needs an IPv4 `listen_address`, exactly one IPv4 `/32` in `allow_from`, the default ports 9100 and 9400, Docker running (the guard filters DCGM in `DOCKER-USER`) and python3; an active firewalld is reported as blocked. rootfiles writes `/usr/local/sbin/rootfiles-exporter-firewall` and `/etc/rootfiles/exporter-firewall.conf` and runs the helper's `setup`, which adds owner-marked chains (`RFILES_EXPORTER_IN` on INPUT, `RFILES_DCGM_DU` on DOCKER-USER), `rootfiles-exporter-firewall.service` and drop-ins that reapply the rules when Docker or UFW starts. An exporter starts only while its rules are present (`ExecStartPre` runs the helper's `verify-host`/`verify-docker`); stopping the guard unit or Docker leaves the rules and node exporter in place. On a hub host the hub's Compose bridge is admitted so its Prometheus can scrape the host. A guard installed by hand with the same helper is adopted without rule changes. Setting the option back to `false` leaves the guard in place; `rootfiles doctor` reports it as unmanaged, and `sudo rootfiles-exporter-firewall rollback` removes it. + +The DCGM exporter and monitoring hub units are wanted by `docker.service`, so they start again whenever Docker starts. + The default DCGM image is pinned to `nvcr.io/nvidia/k8s/dcgm-exporter:3.3.8-3.6.0-ubuntu22.04` for the fleet's older R535 driver baseline. Blackwell and newer driver combinations have not been qualified with that old DCGM release. Select a pinned `dcgm_exporter_image` only after checking NVIDIA's supported GPU, driver, and paired DCGM/exporter versions and validating the endpoint on that host. NVIDIA documents that driver 580.126.16 requires DCGM 4.3.x or newer, so the default image must not be assumed compatible with R580 hosts. The suggested 4.6.1-4.8.4 distroless override needs real GPU validation before rollout. The managed collector file selects only temperature, utilization, framebuffer used/free, XID, and volatile double-bit ECC fields shared by the legacy and newer image versions. ECC series may be absent on GPUs or configurations that do not expose ECC counters. The DCGM 3.3.8 XID gauge reports the last observed XID and may stay stale after recovery. @@ -738,7 +743,7 @@ hosts: config: sites/gpu01.yaml ``` -For an exporter host with existing Docker and NVIDIA runtime, enable only monitoring and bind the exporters to its private address and allow the hub address through the host firewall. If a perimeter firewall already restricts the segment, set `perimeter_firewall: true` instead of adding host firewall rules. +For an exporter host with existing Docker and NVIDIA runtime, enable only monitoring and bind the exporters to its private address and allow the hub address through the host firewall. If a perimeter firewall already restricts the segment, set `perimeter_firewall: true` instead of adding host firewall rules; without UFW, `exporter_firewall: true` restricts the exporters on the host itself. ```yaml modules: diff --git a/docs/releases/v0.18.0.md b/docs/releases/v0.18.0.md new file mode 100644 index 0000000..0dced15 --- /dev/null +++ b/docs/releases/v0.18.0.md @@ -0,0 +1,13 @@ +rootfiles can now manage the exporter firewall guard, and node exporter, the DCGM exporter and the monitoring hub stay up when Docker restarts. + +### New +- `modules.monitoring.exporter_firewall: true` installs and checks an iptables guard that admits the exporter ports only from `allow_from` (one IPv4 /32, default ports). A guard installed by hand with the same `rootfiles-exporter-firewall` helper is adopted without changing its rules. `rootfiles doctor` counts the exporters as covered only while the guard's check passes, and names a guard that rootfiles does not manage. (#84) + +### Fixed +- After Docker stopped and started again (a package upgrade or a manual restart), node exporter on hosts with the exporter firewall guard stayed down until the next `rootfiles apply`, and the DCGM exporter and the monitoring hub did not come back by themselves. Exporters now keep running while the guard unit or Docker stops, refuse to start only while their firewall rules are missing, and the DCGM exporter and hub start again with Docker. (#84) + +### Action needed +- On hosts with a hand-installed exporter firewall guard, set `exporter_firewall: true` under `modules.monitoring` (and `perimeter_firewall: false` unless a network firewall closes the segment), then run `sudo rootfiles apply --module monitoring` or `rootfiles fleet apply`. The apply replaces the guard's systemd drop-ins and restarts the DCGM exporter once; the firewall rules stay in place. (#84) +- Hosts with the DCGM exporter or the monitoring hub need one `rootfiles apply` after updating so their units start with Docker. (#84) + +**Full changelog**: https://github.com/STAIxBWLB/rootfiles/compare/v0.17.1...v0.18.0 diff --git a/internal/config/config.go b/internal/config/config.go index 2e1f25f..6b01146 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -137,13 +137,16 @@ type MonitoringConfig struct { // NodeExporter installs prometheus-node-exporter; rootfiles' scheduled // report also publishes rootfiles_* metrics through its textfile // collector. - NodeExporter bool `yaml:"node_exporter"` - DCGMExporter bool `yaml:"dcgm_exporter"` - NodeExporterPort int `yaml:"node_exporter_port,omitempty"` - DCGMExporterPort int `yaml:"dcgm_exporter_port,omitempty"` - ListenAddress string `yaml:"listen_address,omitempty"` - AllowFrom []string `yaml:"allow_from,omitempty"` - PerimeterFirewall bool `yaml:"perimeter_firewall"` + NodeExporter bool `yaml:"node_exporter"` + DCGMExporter bool `yaml:"dcgm_exporter"` + NodeExporterPort int `yaml:"node_exporter_port,omitempty"` + DCGMExporterPort int `yaml:"dcgm_exporter_port,omitempty"` + ListenAddress string `yaml:"listen_address,omitempty"` + AllowFrom []string `yaml:"allow_from,omitempty"` + PerimeterFirewall bool `yaml:"perimeter_firewall"` + // ExporterFirewall installs the iptables guard that admits exporter + // traffic only from allow_from (IPv4, one /32, default ports). + ExporterFirewall bool `yaml:"exporter_firewall"` DCGMExporterImage string `yaml:"dcgm_exporter_image,omitempty"` Hub MonitoringHubConfig `yaml:"hub"` } diff --git a/internal/config/monitoring_firewall_test.go b/internal/config/monitoring_firewall_test.go new file mode 100644 index 0000000..4d6db96 --- /dev/null +++ b/internal/config/monitoring_firewall_test.go @@ -0,0 +1,42 @@ +package config + +import ( + "strings" + "testing" +) + +func TestValidateExporterFirewall(t *testing.T) { + valid := MonitoringConfig{Enabled: true, NodeExporter: true, ListenAddress: "192.0.2.5", AllowFrom: []string{"198.51.100.200/32"}, ExporterFirewall: true} + errFor := func(m MonitoringConfig) string { + var msgs []string + c := &Config{} + c.Modules.Monitoring = m + c.validateMonitoring(func(f string, a ...any) { msgs = append(msgs, f) }) + return strings.Join(msgs, "; ") + } + if got := errFor(valid); got != "" { + t.Fatalf("valid config rejected: %s", got) + } + for name, mutate := range map[string]func(*MonitoringConfig){ + "no listen_address": func(m *MonitoringConfig) { m.ListenAddress = "" }, + "IPv6 listen_address": func(m *MonitoringConfig) { m.ListenAddress = "2001:db8::5" }, + "two sources": func(m *MonitoringConfig) { m.AllowFrom = append(m.AllowFrom, "198.51.100.201/32") }, + "no source": func(m *MonitoringConfig) { m.AllowFrom = nil }, + "subnet source": func(m *MonitoringConfig) { m.AllowFrom = []string{"198.51.100.0/24"} }, + "IPv6 source": func(m *MonitoringConfig) { m.AllowFrom = []string{"2001:db8::1/128"} }, + "no exporter": func(m *MonitoringConfig) { m.NodeExporter = false }, + "custom port": func(m *MonitoringConfig) { m.NodeExporterPort = 9101 }, + } { + m := valid + m.AllowFrom = append([]string(nil), valid.AllowFrom...) + mutate(&m) + if got := errFor(m); !strings.Contains(got, "exporter_firewall") { + t.Errorf("%s: want an exporter_firewall error, got %q", name, got) + } + } + off := valid + off.ExporterFirewall, off.AllowFrom = false, []string{"198.51.100.0/24", "198.51.100.201/32"} + if got := errFor(off); got != "" { + t.Fatalf("exporter_firewall off must not restrict allow_from: %s", got) + } +} diff --git a/internal/config/monitoring_validate.go b/internal/config/monitoring_validate.go index 32c0560..9d4ff67 100644 --- a/internal/config/monitoring_validate.go +++ b/internal/config/monitoring_validate.go @@ -115,6 +115,22 @@ func (c *Config) validateMonitoring(add func(string, ...any)) { if m.DCGMExporter { image("dcgm_exporter_image", m.ExporterImage()) } + if m.ExporterFirewall { + addr, err := netip.ParseAddr(m.ListenAddress) + switch { + case err != nil || !addr.Is4(): + add("modules.monitoring.exporter_firewall: listen_address must be an IPv4 address") + case !m.NodeExporter && !m.DCGMExporter: + add("modules.monitoring.exporter_firewall: enable node_exporter or dcgm_exporter") + case m.NodePort() != 9100 || m.DCGMPort() != 9400: + add("modules.monitoring.exporter_firewall: requires the default exporter ports 9100 and 9400") + } + if len(m.AllowFrom) != 1 { + add("modules.monitoring.exporter_firewall: allow_from must list exactly one address") + } else if p, err := netip.ParsePrefix(m.AllowFrom[0]); err != nil || !p.Addr().Is4() || p.Bits() != 32 || p.String() != m.AllowFrom[0] { + add("modules.monitoring.exporter_firewall: allow_from must be one IPv4 /32 such as 192.0.2.10/32") + } + } if m.Hub.Enabled { h := m.Hub.WithDefaults() ip("hub.listen_address", h.ListenAddress) diff --git a/internal/module/assets/exporter-firewall.sh b/internal/module/assets/exporter-firewall.sh new file mode 100644 index 0000000..2f30c5d --- /dev/null +++ b/internal/module/assets/exporter-firewall.sh @@ -0,0 +1,581 @@ +#!/usr/bin/env bash +# rootfiles exporter firewall guard (installed by the rootfiles monitoring +# module when modules.monitoring.exporter_firewall is true; do not edit). +# +# rootfiles writes this file to /usr/local/sbin/rootfiles-exporter-firewall +# (root:root, 0750) and /etc/rootfiles/exporter-firewall.conf (root:root, 0600): +# BIND_IP=192.0.2.10 +# ALLOW_FROM=198.51.100.200/32 +# WEB_INTERFACE=br-0123456789ab # monitoring hub Compose bridge, hub hosts only +# IPv4 iptables only. Setup writes rules and units but does not start or +# restart Docker, UFW, exporter, or application units. +# +# Actions: plan (read-only), setup, apply, apply-host, apply-docker, check, +# verify-host, verify-docker (read-only rule checks run by the exporters' +# ExecStartPre), rollback. +# The Docker guard matches only forwarded traffic with conntrack original +# destination BIND_IP:9400. It changes no Docker containers, global firewall +# policies, or other ports. Host-originated Docker checks do not traverse these +# DOCKER-USER original-destination rules. +set -euo pipefail + +readonly OWNER="rootfiles-exporter-firewall:v1" +readonly CONFIG="/etc/rootfiles/exporter-firewall.conf" +readonly HELPER="/usr/local/sbin/rootfiles-exporter-firewall" +readonly SERVICE="rootfiles-exporter-firewall.service" +readonly SERVICE_FILE="/etc/systemd/system/rootfiles-exporter-firewall.service" +readonly DOCKER_DROPIN="/etc/systemd/system/docker.service.d/90-rootfiles-exporter-firewall.conf" +readonly UFW_DROPIN="/etc/systemd/system/ufw.service.d/90-rootfiles-exporter-firewall.conf" +readonly NODE_DROPIN="/etc/systemd/system/prometheus-node-exporter.service.d/90-rootfiles-exporter-firewall.conf" +readonly DCGM_DROPIN="/etc/systemd/system/rootfiles-dcgm-exporter.service.d/90-rootfiles-exporter-firewall.conf" +readonly HUB_DROPIN="/etc/systemd/system/rootfiles-monitoring-hub.service.d/90-rootfiles-exporter-firewall.conf" +readonly INPUT_CHAIN="RFILES_EXPORTER_IN" +readonly DOCKER_CHAIN="RFILES_DCGM_DU" +readonly NODE_PORT=9100 +readonly DCGM_PORT=9400 +KEEP_GUARDS=0 + +ACTION=plan +if [[ "$#" -gt 0 ]]; then ACTION="$1"; fi +BIND_IP="" +ALLOW_FROM="" +WEB_INTERFACE="" + +die() { echo "ERROR: $*" >&2; exit 1; } +note() { echo "rootfiles exporter firewall: $*"; } +warn() { echo "WARNING: $*" >&2; } +require_root() { [[ "$EUID" -eq 0 ]] || die "run as root"; } +ipt() { iptables -w 5 "$@"; } + +validate_config() { + require_root + mode_is_trusted /etc/rootfiles || die "/etc/rootfiles must be root-owned and not group/world writable" + load_config +} + +require_helper_installed() { + local current owner mode + mode_is_trusted /usr/local/sbin || die "/usr/local/sbin must be a trusted root-owned directory" + current="$(readlink -f -- "$0")" + [[ "$current" == "$HELPER" && -f "$HELPER" && ! -L "$HELPER" ]] || die "install this script as $HELPER before setup" + owner="$(stat -c '%u:%g' "$HELPER")" + [[ "$owner" == "0:0" ]] || die "$HELPER must be root:root owned" + mode="$(stat -c '%a' "$HELPER")" + (( (0$mode & 022) == 0 )) || die "$HELPER must not be group/world writable" + grep -Fq "$OWNER" "$HELPER" || die "$HELPER has no matching owner marker" +} + +mode_is_trusted() { + local path="$1" owner mode + [[ -d "$path" && ! -L "$path" ]] || return 1 + owner="$(stat -c '%u:%g' "$path")" + [[ "$owner" == "0:0" ]] || return 1 + mode="$(stat -c '%a' "$path")" + (( (0$mode & 022) == 0 )) +} + +ensure_trusted_dir() { + local path="$1" parent + if [[ "$path" == "/" ]]; then + mode_is_trusted / || die "untrusted root directory" + return + fi + parent="$(dirname "$path")" + ensure_trusted_dir "$parent" + [[ ! -L "$path" ]] || die "refusing symlink directory: $path" + if [[ -e "$path" ]]; then + mode_is_trusted "$path" || die "untrusted directory: $path" + else + mode_is_trusted "$parent" || die "untrusted parent directory: $parent" + install -d -o root -g root -m 0755 "$path" + mode_is_trusted "$path" || die "created directory is not trusted: $path" + fi +} + +file_is_owned_or_absent() { + local path="$1" + [[ ! -L "$path" ]] || die "refusing symlink: $path" + if [[ -e "$path" ]]; then + [[ -f "$path" ]] || die "not a regular file: $path" + [[ "$(stat -c '%u:%g' "$path")" == "0:0" ]] || die "file is not root-owned: $path" + grep -Fq "$OWNER" "$path" || die "unowned file collision: $path" + fi +} + +write_owned_file() { + local path="$1" content="$2" parent tmp + parent="$(dirname "$path")" + ensure_trusted_dir "$parent" + file_is_owned_or_absent "$path" + tmp="$(mktemp "$parent/.rootfiles-exporter-fw.XXXXXX")" + trap 'rm -f -- "$tmp"' EXIT + printf '%s' "$content" > "$tmp" + chown root:root "$tmp" + chmod 0644 "$tmp" + mv -f -- "$tmp" "$path" + trap - EXIT +} + +remove_owned_file() { + local path="$1" + [[ -e "$path" || -L "$path" ]] || return 0 + [[ ! -L "$path" && -f "$path" ]] || die "refusing to remove non-regular managed path: $path" + [[ "$(stat -c '%u:%g' "$path")" == "0:0" ]] || die "refusing to remove non-root-owned file: $path" + grep -Fq "$OWNER" "$path" || die "refusing to remove unowned file: $path" + rm -f -- "$path" +} + +load_config() { + local key value seen_bind=0 seen_allow=0 seen_if=0 owner mode + [[ -f "$CONFIG" && ! -L "$CONFIG" ]] || die "missing regular config file: $CONFIG" + owner="$(stat -c '%u:%g' "$CONFIG")" + [[ "$owner" == "0:0" ]] || die "config must be root:root owned" + mode="$(stat -c '%a' "$CONFIG")" + (( (0$mode & 022) == 0 )) || die "config must not be group/world writable" + while IFS='=' read -r key value || [[ -n "$key$value" ]]; do + case "$key" in + ""|\#*) continue ;; + BIND_IP) + (( seen_bind == 0 )) || die "duplicate BIND_IP" + seen_bind=1 + BIND_IP="$value" + ;; + ALLOW_FROM) + (( seen_allow == 0 )) || die "duplicate ALLOW_FROM" + seen_allow=1 + ALLOW_FROM="$value" + ;; + WEB_INTERFACE) + (( seen_if == 0 )) || die "duplicate WEB_INTERFACE" + seen_if=1 + WEB_INTERFACE="$value" + ;; + *) die "unsupported config key: $key" ;; + esac + done < "$CONFIG" + [[ "$seen_bind" -eq 1 && "$seen_allow" -eq 1 ]] || die "config requires BIND_IP and ALLOW_FROM" + [[ "$BIND_IP" =~ ^[0-9.]+$ ]] || die "BIND_IP must be an IPv4 address" + [[ "$ALLOW_FROM" =~ ^[0-9./]+$ && "$ALLOW_FROM" != *,* ]] || die "ALLOW_FROM must be one IPv4 CIDR" + if [[ -n "$WEB_INTERFACE" && ! "$WEB_INTERFACE" =~ ^br-[0-9a-fA-F]{12}$ ]]; then + die "WEB_INTERFACE must be a Docker bridge name like br-0123456789ab" + fi + command -v python3 >/dev/null || die "python3 is required for strict IPv4/CIDR validation" + python3 -c 'import ipaddress,sys; ipaddress.IPv4Address(sys.argv[1]); n=ipaddress.IPv4Network(sys.argv[2], strict=True); assert n.prefixlen == 32, "exporter source must be one host /32"' "$BIND_IP" "$ALLOW_FROM" || + die "BIND_IP or ALLOW_FROM is not a valid canonical IPv4 value" + command -v iptables >/dev/null || die "iptables is required" + if command -v firewall-cmd >/dev/null && firewall-cmd --state >/dev/null 2>&1; then + die "active firewalld is unsupported by this iptables/UFW template" + fi +} + +ufw_unit_loaded() { + [[ "$(systemctl show -p LoadState --value ufw.service 2>/dev/null || true)" == "loaded" ]] +} + +ufw_active() { + command -v ufw >/dev/null && ufw status 2>/dev/null | grep -qi '^Status: active$' +} + +warn_legacy_publisher() { + command -v docker >/dev/null || return 0 + local name + while IFS= read -r name; do + [[ -n "$name" && "$name" != "rootfiles-dcgm-exporter" ]] && + warn "container '$name' already publishes $DCGM_PORT; it is untouched but gets this source restriction on BIND_IP:$DCGM_PORT" + done < <(docker ps --filter "publish=$DCGM_PORT" --format '{{.Names}}' 2>/dev/null || true) + return 0 +} + +chain_exists() { ipt -S "$1" >/dev/null 2>&1; } +chain_owned() { + local chain="$1" family="$2" line marker_found=0 + while IFS= read -r line; do + [[ "$line" == *"-A $chain "* ]] || continue + [[ "$line" == *"$OWNER:$family:"* ]] || return 1 + [[ "$line" == *"$OWNER:$family:owner"* ]] && marker_found=1 + done < <(ipt -S "$chain" 2>/dev/null) + (( marker_found == 1 )) +} + +check_parent_references() { + local parent="$1" chain="$2" jump_mark="$3" line + while IFS= read -r line; do + [[ "$line" == *"-j $chain"* ]] || continue + [[ "$line" == *"$jump_mark"* ]] || die "unowned jump references $chain from $parent: $line" + done < <(ipt -S "$parent" 2>/dev/null) +} + +check_all_chain_references() { + local parent="$1" chain="$2" jump_mark="$3" line + while IFS= read -r line; do + [[ "$line" == *"-j $chain"* ]] || continue + [[ "$line" == "-A $parent "* && "$line" == *"$jump_mark"* ]] || + die "unowned filter-table reference to $chain: $line" + done < <(iptables-save -t filter) +} + +preflight_chains() { + ipt -S INPUT >/dev/null || die "INPUT chain is unavailable" + ipt -S DOCKER-USER >/dev/null || die "DOCKER-USER is missing; start Docker before setup" + if chain_exists "$INPUT_CHAIN" && ! chain_owned "$INPUT_CHAIN" input; then + die "chain collision: $INPUT_CHAIN exists without this template's owner marker" + fi + if chain_exists "$DOCKER_CHAIN" && ! chain_owned "$DOCKER_CHAIN" docker; then + die "chain collision: $DOCKER_CHAIN exists without this template's owner marker" + fi + check_parent_references INPUT "$INPUT_CHAIN" "$OWNER:jump:input" + check_parent_references DOCKER-USER "$DOCKER_CHAIN" "$OWNER:jump:docker" + check_all_chain_references INPUT "$INPUT_CHAIN" "$OWNER:jump:input" + check_all_chain_references DOCKER-USER "$DOCKER_CHAIN" "$OWNER:jump:docker" + check_guard_config INPUT "$OWNER:guard:input" "-p tcp -d $BIND_IP --dport $NODE_PORT" + check_guard_config DOCKER-USER "$OWNER:guard:docker" "-p tcp -m conntrack --ctdir ORIGINAL --ctorigdst $BIND_IP --ctorigdstport $DCGM_PORT" +} + +preflight_managed_files() { + mode_is_trusted /etc/systemd/system || die "untrusted systemd unit directory" + for dir in /etc/systemd/system/docker.service.d /etc/systemd/system/ufw.service.d /etc/systemd/system/prometheus-node-exporter.service.d /etc/systemd/system/rootfiles-dcgm-exporter.service.d /etc/systemd/system/rootfiles-monitoring-hub.service.d; do + if [[ -e "$dir" || -L "$dir" ]]; then mode_is_trusted "$dir" || die "untrusted systemd drop-in directory: $dir"; fi + done + file_is_owned_or_absent "$SERVICE_FILE" + file_is_owned_or_absent "$DOCKER_DROPIN" + file_is_owned_or_absent "$UFW_DROPIN" + file_is_owned_or_absent "$NODE_DROPIN" + file_is_owned_or_absent "$DCGM_DROPIN" + file_is_owned_or_absent "$HUB_DROPIN" + if ! ufw_unit_loaded && ufw_active; then + die "UFW is active but ufw.service is unavailable; cannot persist INPUT rules across UFW reloads" + fi +} + +ensure_owned_chain() { + local chain="$1" family="$2" + if chain_exists "$chain"; then + chain_owned "$chain" "$family" || die "refusing to modify unowned chain: $chain" + else + ipt -N "$chain" + fi +} + +remove_owned_jumps() { + local parent="$1" chain="$2" marker="$3" + while ipt -C "$parent" -m comment --comment "$marker" -j "$chain" >/dev/null 2>&1; do + ipt -D "$parent" -m comment --comment "$marker" -j "$chain" + done +} + +check_guard_config() { + local parent="$1" marker="$2" exact="$3" line + while IFS= read -r line; do + [[ "$line" == *"$marker"* ]] || continue + ipt -C "$parent" $exact -m comment --comment "$marker" -j DROP >/dev/null 2>&1 || + die "owned fail-closed guard has stale or unexpected parameters in $parent; inspect it before changing config" + done < <(ipt -S "$parent" 2>/dev/null) +} + +ensure_input_guard() { + local marker="$OWNER:guard:input" + check_guard_config INPUT "$marker" "-p tcp -d $BIND_IP --dport $NODE_PORT" + ipt -C INPUT -p tcp -d "$BIND_IP" --dport "$NODE_PORT" -m comment --comment "$marker" -j DROP >/dev/null 2>&1 || + ipt -I INPUT 1 -p tcp -d "$BIND_IP" --dport "$NODE_PORT" -m comment --comment "$marker" -j DROP +} + +ensure_docker_guard() { + local marker="$OWNER:guard:docker" + check_guard_config DOCKER-USER "$marker" "-p tcp -m conntrack --ctdir ORIGINAL --ctorigdst $BIND_IP --ctorigdstport $DCGM_PORT" + ipt -C DOCKER-USER -p tcp -m conntrack --ctdir ORIGINAL --ctorigdst "$BIND_IP" --ctorigdstport "$DCGM_PORT" -m comment --comment "$marker" -j DROP >/dev/null 2>&1 || + ipt -I DOCKER-USER 1 -p tcp -m conntrack --ctdir ORIGINAL --ctorigdst "$BIND_IP" --ctorigdstport "$DCGM_PORT" -m comment --comment "$marker" -j DROP +} + +remove_input_guard() { + while ipt -C INPUT -p tcp -d "$BIND_IP" --dport "$NODE_PORT" -m comment --comment "$OWNER:guard:input" -j DROP >/dev/null 2>&1; do + ipt -D INPUT -p tcp -d "$BIND_IP" --dport "$NODE_PORT" -m comment --comment "$OWNER:guard:input" -j DROP + done +} + +remove_docker_guard() { + while ipt -C DOCKER-USER -p tcp -m conntrack --ctdir ORIGINAL --ctorigdst "$BIND_IP" --ctorigdstport "$DCGM_PORT" -m comment --comment "$OWNER:guard:docker" -j DROP >/dev/null 2>&1; do + ipt -D DOCKER-USER -p tcp -m conntrack --ctdir ORIGINAL --ctorigdst "$BIND_IP" --ctorigdstport "$DCGM_PORT" -m comment --comment "$OWNER:guard:docker" -j DROP + done +} + +apply_host_rules() { + local jump="$OWNER:jump:input" + check_parent_references INPUT "$INPUT_CHAIN" "$jump" + check_all_chain_references INPUT "$INPUT_CHAIN" "$jump" + check_guard_config INPUT "$OWNER:guard:input" "-p tcp -d $BIND_IP --dport $NODE_PORT" + ensure_input_guard + ensure_owned_chain "$INPUT_CHAIN" input + remove_owned_jumps INPUT "$INPUT_CHAIN" "$jump" + ipt -F "$INPUT_CHAIN" + # Leave the owner marker in place even if the process is interrupted. The + # parent DROP guard keeps this endpoint closed until the rebuilt chain is + # attached and complete. + ipt -A "$INPUT_CHAIN" -m comment --comment "$OWNER:input:owner" -j RETURN + ipt -I "$INPUT_CHAIN" 1 -p tcp -d "$BIND_IP" --dport "$NODE_PORT" -m comment --comment "$OWNER:input:node-drop" -j DROP + ipt -I "$INPUT_CHAIN" 1 -p tcp -s "$ALLOW_FROM" -d "$BIND_IP" --dport "$NODE_PORT" -m conntrack --ctstate NEW,ESTABLISHED -m comment --comment "$OWNER:input:web" -j ACCEPT + if [[ -n "$WEB_INTERFACE" ]]; then + ipt -I "$INPUT_CHAIN" 1 -i "$WEB_INTERFACE" -p tcp -d "$BIND_IP" --dport "$NODE_PORT" -m comment --comment "$OWNER:input:web-bridge" -j ACCEPT + fi + ipt -I "$INPUT_CHAIN" 1 -i lo -p tcp -d "$BIND_IP" --dport "$NODE_PORT" -m comment --comment "$OWNER:input:loopback" -j ACCEPT + ipt -I INPUT 1 -m comment --comment "$jump" -j "$INPUT_CHAIN" + if [[ "$KEEP_GUARDS" -eq 0 ]]; then remove_input_guard; fi +} + +apply_docker_rules() { + local jump="$OWNER:jump:docker" + ipt -S DOCKER-USER >/dev/null || die "DOCKER-USER is missing; start Docker before applying DCGM protection" + check_parent_references DOCKER-USER "$DOCKER_CHAIN" "$jump" + check_all_chain_references DOCKER-USER "$DOCKER_CHAIN" "$jump" + check_guard_config DOCKER-USER "$OWNER:guard:docker" "-p tcp -m conntrack --ctdir ORIGINAL --ctorigdst $BIND_IP --ctorigdstport $DCGM_PORT" + ensure_docker_guard + ensure_owned_chain "$DOCKER_CHAIN" docker + remove_owned_jumps DOCKER-USER "$DOCKER_CHAIN" "$jump" + ipt -F "$DOCKER_CHAIN" + ipt -A "$DOCKER_CHAIN" -m comment --comment "$OWNER:docker:owner" -j RETURN + ipt -I "$DOCKER_CHAIN" 1 -p tcp -m conntrack --ctdir ORIGINAL --ctorigdst "$BIND_IP" --ctorigdstport "$DCGM_PORT" -m comment --comment "$OWNER:docker:dcgm-drop" -j DROP + ipt -I "$DOCKER_CHAIN" 1 -p tcp -s "$ALLOW_FROM" -m conntrack --ctstate NEW,ESTABLISHED --ctdir ORIGINAL --ctorigdst "$BIND_IP" --ctorigdstport "$DCGM_PORT" -m comment --comment "$OWNER:docker:web" -j ACCEPT + ipt -I "$DOCKER_CHAIN" 1 -p tcp -m conntrack --ctstate ESTABLISHED,RELATED --ctdir REPLY --ctorigdst "$BIND_IP" --ctorigdstport "$DCGM_PORT" -m comment --comment "$OWNER:docker:reply" -j RETURN + ipt -I DOCKER-USER 1 -m comment --comment "$jump" -j "$DOCKER_CHAIN" + if [[ "$KEEP_GUARDS" -eq 0 ]]; then remove_docker_guard; fi +} + +check_rules() { + check_host_rules && check_docker_rules +} + +check_host_rules() { + ipt -S INPUT | grep -Fq -- "$OWNER:guard:input" && return 1 + ipt -C INPUT -m comment --comment "$OWNER:jump:input" -j "$INPUT_CHAIN" || return 1 + ipt -C "$INPUT_CHAIN" -i lo -p tcp -d "$BIND_IP" --dport "$NODE_PORT" -m comment --comment "$OWNER:input:loopback" -j ACCEPT || return 1 + if [[ -n "$WEB_INTERFACE" ]]; then + ipt -C "$INPUT_CHAIN" -i "$WEB_INTERFACE" -p tcp -d "$BIND_IP" --dport "$NODE_PORT" -m comment --comment "$OWNER:input:web-bridge" -j ACCEPT || return 1 + fi + ipt -C "$INPUT_CHAIN" -p tcp -s "$ALLOW_FROM" -d "$BIND_IP" --dport "$NODE_PORT" -m conntrack --ctstate NEW,ESTABLISHED -m comment --comment "$OWNER:input:web" -j ACCEPT || return 1 + ipt -C "$INPUT_CHAIN" -p tcp -d "$BIND_IP" --dport "$NODE_PORT" -m comment --comment "$OWNER:input:node-drop" -j DROP || return 1 + return 0 +} + +check_docker_rules() { + ipt -S DOCKER-USER 2>/dev/null | grep -Fq -- "$OWNER:guard:docker" && return 1 + ipt -C DOCKER-USER -m comment --comment "$OWNER:jump:docker" -j "$DOCKER_CHAIN" || return 1 + ipt -C "$DOCKER_CHAIN" -p tcp -s "$ALLOW_FROM" -m conntrack --ctstate NEW,ESTABLISHED --ctdir ORIGINAL --ctorigdst "$BIND_IP" --ctorigdstport "$DCGM_PORT" -m comment --comment "$OWNER:docker:web" -j ACCEPT || return 1 + ipt -C "$DOCKER_CHAIN" -p tcp -m conntrack --ctdir ORIGINAL --ctorigdst "$BIND_IP" --ctorigdstport "$DCGM_PORT" -m comment --comment "$OWNER:docker:dcgm-drop" -j DROP || return 1 + return 0 +} + +remove_owned_chain() { + local parent="$1" chain="$2" family="$3" jump="$4" line + chain_exists "$chain" || return 0 + chain_owned "$chain" "$family" || die "refusing to remove unowned chain: $chain" + check_parent_references "$parent" "$chain" "$jump" + remove_owned_jumps "$parent" "$chain" "$jump" + while IFS= read -r line; do + [[ "$line" == *"-j $chain"* ]] && die "chain $chain still has an external reference: $line" + done < <(iptables-save -t filter) + ipt -F "$chain" + ipt -X "$chain" +} + +preflight_rollback() { + if chain_exists "$INPUT_CHAIN" && ! chain_owned "$INPUT_CHAIN" input; then die "refusing unowned chain $INPUT_CHAIN"; fi + if chain_exists "$DOCKER_CHAIN" && ! chain_owned "$DOCKER_CHAIN" docker; then die "refusing unowned chain $DOCKER_CHAIN"; fi + check_parent_references INPUT "$INPUT_CHAIN" "$OWNER:jump:input" + if ipt -S DOCKER-USER >/dev/null 2>&1; then check_parent_references DOCKER-USER "$DOCKER_CHAIN" "$OWNER:jump:docker"; fi + check_all_chain_references INPUT "$INPUT_CHAIN" "$OWNER:jump:input" + if ipt -S DOCKER-USER >/dev/null 2>&1; then check_all_chain_references DOCKER-USER "$DOCKER_CHAIN" "$OWNER:jump:docker"; fi + check_guard_config INPUT "$OWNER:guard:input" "-p tcp -d $BIND_IP --dport $NODE_PORT" + check_guard_config DOCKER-USER "$OWNER:guard:docker" "-p tcp -m conntrack --ctdir ORIGINAL --ctorigdst $BIND_IP --ctorigdstport $DCGM_PORT" +} + +service_content() { + printf '%s' "# $OWNER +[Unit] +Description=Rootfiles exporter firewall guard +After=network-online.target ufw.service docker.service +Wants=network-online.target docker.service + +[Service] +Type=oneshot +RemainAfterExit=yes +ExecStart=$HELPER apply + +[Install] +WantedBy=multi-user.target +" +} + +docker_content() { + printf '%s' "# $OWNER +[Unit] +After=ufw.service + +[Service] +ExecStartPost=$HELPER apply-docker +" +} + +ufw_content() { + printf '%s' "# $OWNER +[Service] +ExecStartPost=$HELPER apply-host +" +} + +# Exporters start only while their rules are in place, but do not stop or +# fail when the guard unit or Docker stops: the rules outlive both. +exporter_content() { + local verify="${1:-}" + printf '%s' "# $OWNER +[Unit] +After=$SERVICE +" + [[ -z "$verify" ]] || printf '%s' " +[Service] +ExecStartPre=+$HELPER $verify +" +} + +managed_files() { + printf '%s\n' "$SERVICE_FILE service_content" "$DOCKER_DROPIN docker_content" \ + "$NODE_DROPIN exporter_content verify-host" "$DCGM_DROPIN exporter_content verify-docker" \ + "$HUB_DROPIN exporter_content" + if ufw_unit_loaded; then printf '%s\n' "$UFW_DROPIN ufw_content"; fi +} + +install_systemd() { + preflight_managed_files + local path fn arg + while read -r path fn arg; do + ensure_trusted_dir "$(dirname "$path")" + write_owned_file "$path" "$($fn $arg)"$'\n' + done < <(managed_files) + systemctl daemon-reload +} + +check_managed_files() { + local path fn arg + while read -r path fn arg; do + [[ -f "$path" && "$(cat -- "$path")" == "$($fn $arg)" ]] || return 1 + done < <(managed_files) +} + +legacy_warning() { + local name + command -v docker >/dev/null || return 0 + while IFS= read -r name; do + [[ -n "$name" && "$name" != "rootfiles-dcgm-exporter" ]] && + warn "container '$name' already publishes $DCGM_PORT; it is untouched but gets this source restriction on BIND_IP:$DCGM_PORT" + done < <(docker ps --filter "publish=$DCGM_PORT" --format '{{.Names}}' 2>/dev/null || true) + return 0 +} + +setup_firewall() { + validate_config + preflight_chains + require_helper_installed + preflight_managed_files + legacy_warning + install_systemd + systemctl enable "$SERVICE" >/dev/null + apply_all + note "installed and applied INPUT $BIND_IP:$NODE_PORT and DOCKER-USER original destination $BIND_IP:$DCGM_PORT" +} + +apply_all() { + validate_config + require_helper_installed + preflight_chains + ensure_input_guard + ensure_docker_guard + KEEP_GUARDS=1 + apply_host_rules + apply_docker_rules + KEEP_GUARDS=0 + remove_input_guard + remove_docker_guard + legacy_warning + note "rules reapplied" +} + +check_mode() { + validate_config + if check_rules; then + systemctl is-enabled --quiet "$SERVICE" || die "$SERVICE is not enabled" + check_managed_files || die "guard unit or drop-ins are missing or outdated; run setup" + note "PASS: INPUT limits node exporter and DOCKER-USER limits only the $BIND_IP:$DCGM_PORT published destination" + legacy_warning + else + die "required owner-marked firewall rules are missing or drifted" + fi +} + +rollback_mode() { + require_root + validate_config + preflight_managed_files + preflight_rollback + systemctl disable --now "$SERVICE" >/dev/null 2>&1 || true + remove_owned_file "$DOCKER_DROPIN" + remove_owned_file "$UFW_DROPIN" + remove_owned_file "$NODE_DROPIN" + remove_owned_file "$DCGM_DROPIN" + remove_owned_file "$HUB_DROPIN" + remove_owned_file "$SERVICE_FILE" + systemctl daemon-reload + remove_owned_chain INPUT "$INPUT_CHAIN" input "$OWNER:jump:input" + if ipt -S DOCKER-USER >/dev/null 2>&1; then + remove_owned_chain DOCKER-USER "$DOCKER_CHAIN" docker "$OWNER:jump:docker" + fi + remove_input_guard + if ipt -S DOCKER-USER >/dev/null 2>&1; then remove_docker_guard; fi + note "removed only this template's marked hooks, jumps, and chains; config/helper retained" +} + +main() { + require_root + case "$ACTION" in + plan) + validate_config + note "would allow WEB CIDR $ALLOW_FROM to host INPUT $BIND_IP:$NODE_PORT" + [[ -n "$WEB_INTERFACE" ]] && note "would allow local scrape traffic on interface $WEB_INTERFACE to INPUT $BIND_IP:$NODE_PORT" + [[ -n "$WEB_INTERFACE" ]] || warn "WEB_INTERFACE is unset; same-host Prometheus scraping is not admitted by this template" + note "would restrict only forwarded original destination $BIND_IP:$DCGM_PORT in DOCKER-USER" + note "host-originated and container-local Docker checks are outside the DOCKER-USER rule" + legacy_warning + ;; + setup) + setup_firewall + ;; + apply) + apply_all + ;; + apply-host) + validate_config + apply_host_rules + ;; + apply-docker) + validate_config + apply_docker_rules + ;; + check) + check_mode + ;; + verify-host) + validate_config + check_host_rules || die "INPUT rules for $BIND_IP:$NODE_PORT are missing; refusing to start the exporter" + ;; + verify-docker) + validate_config + check_docker_rules || die "DOCKER-USER rules for $BIND_IP:$DCGM_PORT are missing; refusing to start the exporter" + ;; + rollback) + rollback_mode + ;; + *) + echo "Usage: $0 {plan|setup|apply|apply-host|apply-docker|check|verify-host|verify-docker|rollback}" >&2 + exit 2 + ;; + esac +} + +main "$@" diff --git a/internal/module/doctor.go b/internal/module/doctor.go index cf74e69..3e9ae63 100644 --- a/internal/module/doctor.go +++ b/internal/module/doctor.go @@ -53,8 +53,27 @@ func Doctor(ctx context.Context, rc *RunContext) []Finding { func doctorExporterExposure(ctx context.Context, rc *RunContext) []Finding { cfg := rc.Config.Modules.Monitoring + var out []Finding + if !cfg.ExporterFirewall && rc.Runner.FileExists(exporterGuardHelperPath) { + out = append(out, Finding{Check: "exporter firewall", Level: LevelWarn, + Detail: exporterGuardHelperPath + " is installed but not managed by rootfiles", + Hint: "set modules.monitoring.exporter_firewall: true, or remove it with `sudo " + exporterGuardHelperPath + " rollback`"}) + } if !cfg.NodeExporter && !cfg.DCGMExporter { - return nil + return out + } + return append(out, doctorExporterListener(ctx, rc)...) +} + +func doctorExporterListener(ctx context.Context, rc *RunContext) []Finding { + cfg := rc.Config.Modules.Monitoring + if cfg.ExporterFirewall { + if _, err := rc.Runner.Query(ctx, exporterGuardHelperPath, "check"); err != nil { + return []Finding{{Check: "exporter exposure", Level: LevelWarn, + Detail: "exporter firewall check fails: " + guardError(err), + Hint: "sudo rootfiles apply --module monitoring"}} + } + return []Finding{{Check: "exporter exposure", Level: LevelOK, Detail: "exporters admit only " + strings.Join(cfg.AllowFrom, ", ") + " (rootfiles exporter firewall)"}} } address := cfg.ListenAddress ports := exporterPorts(cfg) @@ -83,7 +102,7 @@ func doctorExporterExposure(ctx context.Context, rc *RunContext) []Finding { } return []Finding{{Check: "exporter exposure", Level: LevelWarn, Detail: fmt.Sprintf("exporter listener %s is not covered by a host or perimeter firewall", listener), - Hint: "set a private listen_address, configure modules.monitoring.allow_from with an active UFW firewall, or set perimeter_firewall: true"}} + Hint: "set a private listen_address, set exporter_firewall: true, configure modules.monitoring.allow_from with an active UFW firewall, or set perimeter_firewall: true"}} } func isPrivateListenAddress(address string) bool { diff --git a/internal/module/monitoring.go b/internal/module/monitoring.go index 66ccb18..82efe34 100644 --- a/internal/module/monitoring.go +++ b/internal/module/monitoring.go @@ -39,6 +39,7 @@ func (m *MonitoringModule) Check(ctx context.Context, rc *RunContext) (*CheckRes return nil, err } changes = append(changes, hubChanges...) + changes = append(changes, checkExporterGuard(ctx, rc)...) return &CheckResult{Satisfied: len(changes) == 0, Changes: changes}, nil } @@ -62,7 +63,12 @@ func (m *MonitoringModule) Apply(ctx context.Context, rc *RunContext) (*ApplyRes if err != nil { return nil, err } - return mergeApplyResults(exporters, hub), exportErr + // After the hub, so a first apply admits the bridge the hub just created. + guard, err := applyExporterGuard(ctx, rc) + if err != nil { + return nil, err + } + return mergeApplyResults(mergeApplyResults(exporters, hub), guard), exportErr } func mergeApplyResults(a, b *ApplyResult) *ApplyResult { diff --git a/internal/module/monitoring_exporters.go b/internal/module/monitoring_exporters.go index 12a115a..5efb6ba 100644 --- a/internal/module/monitoring_exporters.go +++ b/internal/module/monitoring_exporters.go @@ -71,6 +71,8 @@ func checkMonitoringExporters(ctx context.Context, rc *RunContext) ([]Change, er got, err := rc.Runner.ReadFile(dcgmUnitPath) if err != nil || string(got) != want { changes = append(changes, Change{Description: "Install or update DCGM exporter systemd unit", Command: "write " + dcgmUnitPath}) + } else if !rc.Runner.FileExists(dockerWantsLink(dcgmExporterUnit)) { + changes = append(changes, Change{Description: "Start the DCGM exporter whenever Docker starts", Command: "systemctl reenable " + dcgmExporterUnit}) } if !serviceActive(ctx, rc, dcgmExporterUnit) || !serviceEnabled(ctx, rc, dcgmExporterUnit) { changes = append(changes, Change{Description: "Start DCGM exporter", Command: "systemctl enable --now " + dcgmExporterUnit}) @@ -173,6 +175,12 @@ func applyMonitoringExporters(ctx context.Context, rc *RunContext) (*ApplyResult return nil, err } } + if unitChanged || !rc.Runner.FileExists(dockerWantsLink(dcgmExporterUnit)) { + if _, err := rc.Runner.Run(ctx, "systemctl", "reenable", dcgmExporterUnit); err != nil { + return nil, fmt.Errorf("enabling DCGM exporter: %w", err) + } + out.Changed = true + } if !serviceActive(ctx, rc, dcgmExporterUnit) || !serviceEnabled(ctx, rc, dcgmExporterUnit) || unitChanged || collectorsChanged { args := []string{"enable", "--now", dcgmExporterUnit} if serviceActive(ctx, rc, dcgmExporterUnit) && (unitChanged || collectorsChanged) { @@ -280,7 +288,7 @@ func renderDCGMUnit(cfg config.MonitoringConfig) string { address = "0.0.0.0" } publish := net.JoinHostPort(address, strconv.Itoa(cfg.DCGMPort())) + ":9400" - return fmt.Sprintf("[Unit]\nDescription=rootfiles NVIDIA DCGM exporter\nAfter=docker.service\nRequires=docker.service\n\n[Service]\nRestart=always\nRestartSec=5\nExecStartPre=-/usr/bin/docker rm -f rootfiles-dcgm-exporter\nExecStart=/usr/bin/docker run --rm --name rootfiles-dcgm-exporter --runtime=nvidia --gpus all -p %s --mount type=bind,src=%s,dst=%s,readonly %s -f %s\nExecStop=/usr/bin/docker stop rootfiles-dcgm-exporter\n\n[Install]\nWantedBy=multi-user.target\n", publish, dcgmCollectorsPath, dcgmCollectorsContainer, cfg.ExporterImage(), dcgmCollectorsContainer) + return fmt.Sprintf("[Unit]\nDescription=rootfiles NVIDIA DCGM exporter\nAfter=docker.service\nRequires=docker.service\n\n[Service]\nRestart=always\nRestartSec=5\nExecStartPre=-/usr/bin/docker rm -f rootfiles-dcgm-exporter\nExecStart=/usr/bin/docker run --rm --name rootfiles-dcgm-exporter --runtime=nvidia --gpus all -p %s --mount type=bind,src=%s,dst=%s,readonly %s -f %s\nExecStop=/usr/bin/docker stop rootfiles-dcgm-exporter\n\n[Install]\nWantedBy=multi-user.target docker.service\n", publish, dcgmCollectorsPath, dcgmCollectorsContainer, cfg.ExporterImage(), dcgmCollectorsContainer) } // DCGM's collectors file replaces image defaults. Keep only fields shared by @@ -296,6 +304,12 @@ DCGM_FI_DEV_ECC_DBE_VOL_TOTAL, counter, Total number of double-bit volatile ECC DCGM_FI_DEV_XID_ERRORS, gauge, Value of the last XID error encountered. ` +// dockerWantsDir holds the links `systemctl enable` makes for units with +// WantedBy=docker.service, which start again whenever Docker starts. +var dockerWantsDir = monitoringSystemdDir + "/docker.service.wants" + +func dockerWantsLink(unit string) string { return filepath.Join(dockerWantsDir, unit) } + func exporterPorts(cfg config.MonitoringConfig) []int { var ports []int if cfg.NodeExporter { diff --git a/internal/module/monitoring_guard.go b/internal/module/monitoring_guard.go new file mode 100644 index 0000000..a97dd4c --- /dev/null +++ b/internal/module/monitoring_guard.go @@ -0,0 +1,146 @@ +package module + +import ( + "bytes" + "context" + _ "embed" + "fmt" + "os" + "path/filepath" + "strings" +) + +// The exporter firewall guard admits exporter traffic only from allow_from. +// The helper keeps the paths, chains and owner marker of the hand-installed +// v1 template, so hosts that already run it are adopted in place. +// +//go:embed assets/exporter-firewall.sh +var exporterGuardHelper []byte + +const exporterGuardUnit = "rootfiles-exporter-firewall.service" + +// Overridable in tests. +var ( + exporterGuardHelperPath = "/usr/local/sbin/rootfiles-exporter-firewall" + exporterGuardConfPath = "/etc/rootfiles/exporter-firewall.conf" +) + +// exporterGuardConf renders the helper config. On a hub host the hub's +// Compose bridge is admitted too, so its Prometheus can scrape this host. +func exporterGuardConf(ctx context.Context, rc *RunContext) string { + cfg := rc.Config.Modules.Monitoring + s := "# Managed by rootfiles\nBIND_IP=" + cfg.ListenAddress + "\nALLOW_FROM=" + cfg.AllowFrom[0] + "\n" + if cfg.Hub.Enabled { + network := filepath.Base(filepath.Dir(monitoringHubCompose)) + "_default" + if res, err := rc.Runner.Query(ctx, "docker", "network", "inspect", "--format", "{{.Id}}", network); err == nil { + if id := strings.TrimSpace(res.Stdout); len(id) >= 12 { + s += "WEB_INTERFACE=br-" + id[:12] + "\n" + } + } + } + return s +} + +// exporterGuardBlock names what keeps the guard from being installed. wait +// is set when the docker module of this run may still provide DOCKER-USER. +func exporterGuardBlock(ctx context.Context, rc *RunContext) (block string, wait bool) { + if rc.Runner.CommandExists("firewall-cmd") { + if _, err := rc.Runner.Query(ctx, "firewall-cmd", "--state"); err == nil { + return "exporter firewall: firewalld is active; the guard supports iptables and UFW only", false + } + } + for _, cmd := range []string{"iptables", "iptables-save", "python3"} { + if !rc.Runner.CommandExists(cmd) { + return "exporter firewall: " + cmd + " is missing", false + } + } + if _, err := rc.Runner.Query(ctx, "iptables", "-w", "5", "-S", "DOCKER-USER"); err != nil { + return "exporter firewall: Docker's DOCKER-USER chain is missing; start Docker first", rc.moduleSucceeding("docker") + } + return "", false +} + +func checkExporterGuard(ctx context.Context, rc *RunContext) []Change { + if !rc.Config.Modules.Monitoring.ExporterFirewall { + return nil + } + if block, wait := exporterGuardBlock(ctx, rc); block != "" { + if wait { + return []Change{{Description: block + " (waits for the docker module)"}} + } + return []Change{{Description: block, State: ChangeBlocked}} + } + setup := exporterGuardHelperPath + " setup" + var changes []Change + if cur, err := rc.Runner.ReadFile(exporterGuardHelperPath); err != nil || !bytes.Equal(cur, exporterGuardHelper) { + changes = append(changes, Change{Description: "Install the exporter firewall helper", Command: "write " + exporterGuardHelperPath + " && " + setup}) + } + if cur, err := rc.Runner.ReadFile(exporterGuardConfPath); err != nil || string(cur) != exporterGuardConf(ctx, rc) { + changes = append(changes, Change{Description: "Write the exporter firewall config", Command: "write " + exporterGuardConfPath + " && " + setup}) + } + if len(changes) == 0 { + if _, err := rc.Runner.Query(ctx, exporterGuardHelperPath, "check"); err != nil { + changes = append(changes, Change{Description: "Restore exporter firewall rules and units: " + guardError(err), Command: setup}) + } + } + return changes +} + +func applyExporterGuard(ctx context.Context, rc *RunContext) (*ApplyResult, error) { + out := &ApplyResult{} + if !rc.Config.Modules.Monitoring.ExporterFirewall { + return out, nil + } + if block, _ := exporterGuardBlock(ctx, rc); block != "" { + return nil, fmt.Errorf("%s", block) + } + write := func(path string, content []byte, mode os.FileMode) (bool, error) { + if cur, err := rc.Runner.ReadFile(path); err == nil && bytes.Equal(cur, content) { + return false, nil + } + if err := rc.Runner.MkdirAll(filepath.Dir(path), 0o755); err != nil { + return false, err + } + return true, rc.Runner.WriteFile(path, content, mode) + } + helperChanged, err := write(exporterGuardHelperPath, exporterGuardHelper, 0o750) + if err != nil { + return nil, err + } + confChanged, err := write(exporterGuardConfPath, []byte(exporterGuardConf(ctx, rc)), 0o600) + if err != nil { + return nil, err + } + if !helperChanged && !confChanged { + if _, err := rc.Runner.Query(ctx, exporterGuardHelperPath, "check"); err == nil { + return out, nil + } + } + // setup rebuilds the owned chains behind a DROP guard, so the exporters + // stay closed while it runs. + if _, err := rc.Runner.Run(ctx, exporterGuardHelperPath, "setup"); err != nil { + return nil, fmt.Errorf("exporter firewall setup: %s", guardError(err)) + } + out.Changed = true + out.Messages = append(out.Messages, "exporter firewall guard installed and applied") + // An exporter refused to start while the rules were missing (its + // ExecStartPre verify failed) starts now that they are back. + cfg := rc.Config.Modules.Monitoring + for unit, on := range map[string]bool{nodeExporterUnit: cfg.NodeExporter, dcgmExporterUnit: cfg.DCGMExporter} { + if on && !serviceActive(ctx, rc, unit) { + if _, err := rc.Runner.Run(ctx, "systemctl", "start", unit); err != nil { + out.Warnings = append(out.Warnings, "starting "+unit+": "+firstLine(err.Error())) + } + } + } + return out, nil +} + +// guardError keeps the helper's own message, which it prints to stderr. +func guardError(err error) string { + msg := err.Error() + if i := strings.LastIndex(msg, "ERROR: "); i >= 0 { + return firstLine(msg[i+len("ERROR: "):]) + } + return firstLine(msg) +} diff --git a/internal/module/monitoring_guard_test.go b/internal/module/monitoring_guard_test.go new file mode 100644 index 0000000..4992d60 --- /dev/null +++ b/internal/module/monitoring_guard_test.go @@ -0,0 +1,261 @@ +package module + +import ( + "context" + "os" + "path/filepath" + "strings" + "testing" + + "github.com/STAIxBWLB/rootfiles/internal/config" +) + +// fakeGuardHost points the guard at a temp tree and swaps the embedded +// helper for a stand-in: `check` passes while dir/rules-ok exists and +// `setup` creates it. iptables answers as a host with Docker running. +// active steers `systemctl is-active`. It returns the temp dir and call log. +func fakeGuardHost(t *testing.T, active bool) (string, string) { + t.Helper() + dir := t.TempDir() + calls := filepath.Join(dir, "calls.log") + oldHelper, oldConf, oldBytes := exporterGuardHelperPath, exporterGuardConfPath, exporterGuardHelper + t.Cleanup(func() { + exporterGuardHelperPath, exporterGuardConfPath, exporterGuardHelper = oldHelper, oldConf, oldBytes + }) + exporterGuardHelperPath = filepath.Join(dir, "sbin", "rootfiles-exporter-firewall") + exporterGuardConfPath = filepath.Join(dir, "etc", "exporter-firewall.conf") + exporterGuardHelper = []byte("#!/bin/sh\necho \"helper $*\" >> " + calls + "\ncase $1 in\ncheck) test -f " + dir + "/rules-ok || { echo 'ERROR: required owner-marked firewall rules are missing or drifted' >&2; exit 1; } ;;\nsetup) touch " + dir + "/rules-ok ;;\nesac\n") + state := "inactive" + if active { + state = "active" + } + fakeMonitoringCommand(t, "iptables", "#!/bin/sh\nexit 0\n") + fakeMonitoringCommand(t, "iptables-save", "#!/bin/sh\nexit 0\n") + fakeMonitoringCommand(t, "python3", "#!/bin/sh\nexit 0\n") + fakeMonitoringCommand(t, "systemctl", "#!/bin/sh\necho \"systemctl $*\" >> "+calls+"\ncase $1 in is-active) echo "+state+";; esac\n") + return dir, calls +} + +func guardRC(t *testing.T, dryRun bool) *RunContext { + rc := newRealRC(t) + rc.Runner.DryRun, rc.DryRun = dryRun, dryRun + rc.Config.Modules.Monitoring = config.MonitoringConfig{ + NodeExporter: true, ListenAddress: "192.0.2.5", AllowFrom: []string{"198.51.100.200/32"}, ExporterFirewall: true, + } + return rc +} + +func readCalls(t *testing.T, calls string) string { + t.Helper() + b, _ := os.ReadFile(calls) + return string(b) +} + +func TestExporterGuardInstallsAndConverges(t *testing.T) { + dir, calls := fakeGuardHost(t, true) + rc := guardRC(t, false) + ctx := context.Background() + + if changes := checkExporterGuard(ctx, rc); len(changes) != 2 { + t.Fatalf("fresh host: want helper and config changes, got %+v", changes) + } + res, err := applyExporterGuard(ctx, rc) + if err != nil || !res.Changed { + t.Fatalf("apply = %+v, %v", res, err) + } + helper, err := os.Stat(exporterGuardHelperPath) + if err != nil || helper.Mode().Perm() != 0o750 { + t.Fatalf("helper mode: %v %v", helper, err) + } + conf, err := os.ReadFile(exporterGuardConfPath) + if err != nil || string(conf) != "# Managed by rootfiles\nBIND_IP=192.0.2.5\nALLOW_FROM=198.51.100.200/32\n" { + t.Fatalf("config = %q, %v", conf, err) + } + if fi, _ := os.Stat(exporterGuardConfPath); fi.Mode().Perm() != 0o600 { + t.Fatalf("config mode %v", fi.Mode()) + } + if !strings.Contains(readCalls(t, calls), "helper setup") { + t.Fatalf("setup not run: %s", readCalls(t, calls)) + } + if changes := checkExporterGuard(ctx, rc); len(changes) != 0 { + t.Fatalf("check after apply: %+v", changes) + } + if res, err := applyExporterGuard(ctx, rc); err != nil || res.Changed { + t.Fatalf("second apply = %+v, %v; want unchanged", res, err) + } + + // Rules flushed behind rootfiles' back: check names it, apply restores. + if err := os.Remove(filepath.Join(dir, "rules-ok")); err != nil { + t.Fatal(err) + } + changes := checkExporterGuard(ctx, rc) + if len(changes) != 1 || !strings.Contains(changes[0].Description, "rules are missing") { + t.Fatalf("drift: %+v", changes) + } + if res, err := applyExporterGuard(ctx, rc); err != nil || !res.Changed { + t.Fatalf("restore = %+v, %v", res, err) + } + if changes := checkExporterGuard(ctx, rc); len(changes) != 0 { + t.Fatalf("check after restore: %+v", changes) + } +} + +func TestExporterGuardStartsExporterRefusedWithoutRules(t *testing.T) { + _, calls := fakeGuardHost(t, false) + rc := guardRC(t, false) + if _, err := applyExporterGuard(context.Background(), rc); err != nil { + t.Fatal(err) + } + if !strings.Contains(readCalls(t, calls), "systemctl start "+nodeExporterUnit) { + t.Fatalf("inactive exporter not started after setup: %s", readCalls(t, calls)) + } + if strings.Contains(readCalls(t, calls), dcgmExporterUnit) { + t.Fatalf("disabled DCGM exporter must not be started: %s", readCalls(t, calls)) + } +} + +func TestExporterGuardDryRunChangesNothing(t *testing.T) { + _, calls := fakeGuardHost(t, true) + rc := guardRC(t, true) + if _, err := applyExporterGuard(context.Background(), rc); err != nil { + t.Fatal(err) + } + if _, err := os.Stat(exporterGuardHelperPath); !os.IsNotExist(err) { + t.Fatalf("dry run wrote the helper: %v", err) + } + if _, err := os.Stat(exporterGuardConfPath); !os.IsNotExist(err) { + t.Fatalf("dry run wrote the config: %v", err) + } + if strings.Contains(readCalls(t, calls), "helper") { + t.Fatalf("dry run ran the helper: %s", readCalls(t, calls)) + } +} + +func TestExporterGuardOffLeavesExistingGuardAlone(t *testing.T) { + _, calls := fakeGuardHost(t, true) + if err := os.MkdirAll(filepath.Dir(exporterGuardHelperPath), 0o755); err != nil { + t.Fatal(err) + } + old := []byte("#!/bin/sh\n# rootfiles-exporter-firewall:v1 hand-installed\n") + if err := os.WriteFile(exporterGuardHelperPath, old, 0o750); err != nil { + t.Fatal(err) + } + rc := guardRC(t, false) + rc.Config.Modules.Monitoring.ExporterFirewall = false + if changes := checkExporterGuard(context.Background(), rc); len(changes) != 0 { + t.Fatalf("off: want no changes, got %+v", changes) + } + if res, err := applyExporterGuard(context.Background(), rc); err != nil || res.Changed { + t.Fatalf("off: apply = %+v, %v", res, err) + } + if got, _ := os.ReadFile(exporterGuardHelperPath); string(got) != string(old) { + t.Fatal("off: helper was rewritten") + } + if strings.Contains(readCalls(t, calls), "helper") { + t.Fatalf("off: helper was run: %s", readCalls(t, calls)) + } + found := false + for _, f := range doctorExporterExposure(context.Background(), rc) { + if f.Check == "exporter firewall" && f.Level == LevelWarn && strings.Contains(f.Detail, "not managed") { + found = true + } + } + if !found { + t.Fatalf("doctor must name the unmanaged guard: %+v", doctorExporterExposure(context.Background(), rc)) + } +} + +func TestExporterGuardBlocked(t *testing.T) { + t.Run("firewalld active", func(t *testing.T) { + fakeGuardHost(t, true) + fakeMonitoringCommand(t, "firewall-cmd", "#!/bin/sh\necho running\n") + rc := guardRC(t, false) + changes := checkExporterGuard(context.Background(), rc) + if len(changes) != 1 || changes[0].State != ChangeBlocked || !strings.Contains(changes[0].Description, "firewalld") { + t.Fatalf("want blocked by firewalld, got %+v", changes) + } + if _, err := applyExporterGuard(context.Background(), rc); err == nil { + t.Fatal("apply must refuse while firewalld is active") + } + if _, err := os.Stat(exporterGuardHelperPath); !os.IsNotExist(err) { + t.Fatal("blocked apply wrote the helper") + } + }) + t.Run("docker not running", func(t *testing.T) { + fakeGuardHost(t, true) + fakeMonitoringCommand(t, "iptables", "#!/bin/sh\necho 'No chain/target/match by that name.' >&2; exit 1\n") + rc := guardRC(t, false) + changes := checkExporterGuard(context.Background(), rc) + if len(changes) != 1 || changes[0].State != ChangeBlocked || !strings.Contains(changes[0].Description, "DOCKER-USER") { + t.Fatalf("want blocked on DOCKER-USER, got %+v", changes) + } + rc.Config.Modules.Docker.Enabled = true + changes = checkExporterGuard(context.Background(), rc) + if len(changes) != 1 || changes[0].State == ChangeBlocked { + t.Fatalf("docker module in this run: want pending, got %+v", changes) + } + }) +} + +func TestExporterGuardAdmitsHubBridge(t *testing.T) { + fakeGuardHost(t, true) + fakeMonitoringCommand(t, "docker", "#!/bin/sh\n[ \"$1 $2 $5\" = 'network inspect monitoring_default' ] && echo 8209a3f1a15e44f4f9d1f6a517e78995130119f37f60b93f4d39baacf6f0fe61\n") + rc := guardRC(t, false) + rc.Config.Modules.Monitoring.Hub.Enabled = true + if got := exporterGuardConf(context.Background(), rc); !strings.HasSuffix(got, "\nWEB_INTERFACE=br-8209a3f1a15e\n") { + t.Fatalf("hub bridge missing from config: %q", got) + } +} + +func TestDoctorExporterGuardCoverage(t *testing.T) { + dir, _ := fakeGuardHost(t, true) + rc := guardRC(t, false) + if _, err := applyExporterGuard(context.Background(), rc); err != nil { + t.Fatal(err) + } + got := doctorExporterExposure(context.Background(), rc) + if len(got) != 1 || got[0].Level != LevelOK || !strings.Contains(got[0].Detail, "198.51.100.200/32") { + t.Fatalf("passing guard should cover the exporters: %+v", got) + } + if err := os.Remove(filepath.Join(dir, "rules-ok")); err != nil { + t.Fatal(err) + } + got = doctorExporterExposure(context.Background(), rc) + if len(got) != 1 || got[0].Level != LevelWarn || !strings.Contains(got[0].Detail, "rules are missing") { + t.Fatalf("failing guard check should warn: %+v", got) + } +} + +// The embedded helper must keep the v1 names so hand-installed guards are +// adopted in place, and must not tie exporters to the guard unit's state. +func TestEmbeddedExporterGuardHelper(t *testing.T) { + s := string(exporterGuardHelper) + for _, want := range []string{ + `readonly OWNER="rootfiles-exporter-firewall:v1"`, `readonly INPUT_CHAIN="RFILES_EXPORTER_IN"`, `readonly DOCKER_CHAIN="RFILES_DCGM_DU"`, + `readonly CONFIG="/etc/rootfiles/exporter-firewall.conf"`, `readonly HELPER="/usr/local/sbin/rootfiles-exporter-firewall"`, + "Wants=network-online.target docker.service", "ExecStartPre=+$HELPER $verify", + `"$NODE_DROPIN exporter_content verify-host"`, `"$DCGM_DROPIN exporter_content verify-docker"`, + "verify-host)", "verify-docker)", + } { + if !strings.Contains(s, want) { + t.Errorf("helper missing %q", want) + } + } + if strings.Contains(s, "Requires=") { + t.Error("helper must not write Requires= dependencies") + } + if exporterGuardHelperPath != "/usr/local/sbin/rootfiles-exporter-firewall" || exporterGuardConfPath != "/etc/rootfiles/exporter-firewall.conf" { + t.Error("Go paths must match the helper's") + } +} + +func TestDockerDependentUnitsStartWithDocker(t *testing.T) { + for name, unit := range map[string]string{ + "dcgm": renderDCGMUnit(config.MonitoringConfig{DCGMExporter: true}), + "hub": monitoringHubSystemdUnit("/etc/rootfiles/monitoring/compose.yaml"), + } { + if !strings.Contains(unit, "\nWantedBy=multi-user.target docker.service\n") { + t.Errorf("%s unit must be wanted by docker.service:\n%s", name, unit) + } + } +} diff --git a/internal/module/monitoring_hub.go b/internal/module/monitoring_hub.go index 7d39b9d..477266e 100644 --- a/internal/module/monitoring_hub.go +++ b/internal/module/monitoring_hub.go @@ -100,7 +100,7 @@ func checkMonitoringHub(ctx context.Context, rc *RunContext) ([]Change, error) { } } unit, err := rc.Runner.ReadFile(monitoringHubUnitPath) - if err != nil || string(unit) != monitoringHubSystemdUnit(monitoringHubCompose) { + if err != nil || string(unit) != monitoringHubSystemdUnit(monitoringHubCompose) || !rc.Runner.FileExists(dockerWantsLink(monitoringHubUnit)) { configDrift = true } if configDrift { @@ -254,6 +254,12 @@ func applyMonitoringHub(ctx context.Context, rc *RunContext) (*ApplyResult, erro return nil, fmt.Errorf("reloading systemd: %w", err) } } + if unitChanged || !rc.Runner.FileExists(dockerWantsLink(monitoringHubUnit)) { + if _, err := rc.Runner.Run(ctx, "systemctl", "reenable", monitoringHubUnit); err != nil { + return nil, fmt.Errorf("enabling monitoring hub unit: %w", err) + } + changed = true + } active := unitActive(ctx, rc) if changed && active { if _, err := rc.Runner.Run(ctx, "docker", "compose", "-f", monitoringHubCompose, "up", "-d"); err != nil { @@ -794,7 +800,7 @@ func monitoringAlertmanagerVolumes(h config.MonitoringHubConfig) []monitoringHub } func monitoringHubSystemdUnit(composePath string) string { - return "[Unit]\nDescription=rootfiles monitoring hub\nRequires=docker.service\nAfter=docker.service\n\n[Service]\nType=oneshot\nRemainAfterExit=yes\nWorkingDirectory=" + monitoringHubConfigDir + "\nExecStart=/usr/bin/docker compose -f " + composePath + " up -d\nExecStop=/usr/bin/docker compose -f " + composePath + " down\nTimeoutStartSec=0\n\n[Install]\nWantedBy=multi-user.target\n" + return "[Unit]\nDescription=rootfiles monitoring hub\nRequires=docker.service\nAfter=docker.service\n\n[Service]\nType=oneshot\nRemainAfterExit=yes\nWorkingDirectory=" + monitoringHubConfigDir + "\nExecStart=/usr/bin/docker compose -f " + composePath + " up -d\nExecStop=/usr/bin/docker compose -f " + composePath + " down\nTimeoutStartSec=0\n\n[Install]\nWantedBy=multi-user.target docker.service\n" } const monitoringHubDefaultAlertConfig = "route:\n receiver: default\nreceivers:\n - name: default\n" diff --git a/internal/module/monitoring_hub_test.go b/internal/module/monitoring_hub_test.go index 49a5209..e230d35 100644 --- a/internal/module/monitoring_hub_test.go +++ b/internal/module/monitoring_hub_test.go @@ -607,7 +607,10 @@ func fakeMonitoringHub(t *testing.T) (tmp, stateFile, stoppedFile, failFile, cal calls = filepath.Join(tmp, "calls.log") dockerScript := "#!/bin/sh\nprintf 'docker %s\\n' \"$*\" >> '" + calls + "'\ncase \"$4\" in\nps) if test -f '" + failFile + "'; then printf '[]\\n'; elif test -f '" + stoppedFile + "'; then printf '[{\"Service\":\"prometheus\",\"State\":\"running\"},{\"Service\":\"alertmanager\",\"State\":\"running\"}]\\n'; elif test -f '" + stateFile + "'; then printf '[{\"Service\":\"prometheus\",\"State\":\"running\"},{\"Service\":\"alertmanager\",\"State\":\"running\"},{\"Service\":\"grafana\",\"State\":\"running\"}]\\n'; else printf '[]\\n'; fi ;;\nup) rm -f '" + stoppedFile + "' ;;\nesac\nexit 0\n" fakeMonitoringCommand(t, "docker", dockerScript) - fakeMonitoringCommand(t, "systemctl", "#!/bin/sh\nprintf 'systemctl %s\\n' \"$*\" >> '"+calls+"'\ncase \"$1\" in\nis-active) if test -f '"+stateFile+"'; then exit 0; else exit 3; fi ;;\nenable) docker compose -f '"+monitoringHubCompose+"' up -d || exit $?; touch '"+stateFile+"' ;;\nesac\nexit 0\n") + oldWants := dockerWantsDir + dockerWantsDir = filepath.Join(tmp, "docker.service.wants") + t.Cleanup(func() { dockerWantsDir = oldWants }) + fakeMonitoringCommand(t, "systemctl", "#!/bin/sh\nprintf 'systemctl %s\\n' \"$*\" >> '"+calls+"'\ncase \"$1\" in\nis-active) if test -f '"+stateFile+"'; then exit 0; else exit 3; fi ;;\nenable) docker compose -f '"+monitoringHubCompose+"' up -d || exit $?; touch '"+stateFile+"' ;;\nreenable) mkdir -p '"+dockerWantsDir+"' && touch '"+dockerWantsDir+"'/\"$2\" ;;\nesac\nexit 0\n") return tmp, stateFile, stoppedFile, failFile, calls } diff --git a/internal/module/monitoring_test.go b/internal/module/monitoring_test.go index 4fbb791..38180d5 100644 --- a/internal/module/monitoring_test.go +++ b/internal/module/monitoring_test.go @@ -159,6 +159,7 @@ func TestDCGMInstalledPrerequisitesDoNotRequireEnabledModules(t *testing.T) { fakeMonitoringCommand(t, "nvidia-smi", "#!/bin/sh\necho 'GPU 0: Test GPU'\n") fakeMonitoringCommand(t, "systemctl", "#!/bin/sh\ncase \"$1\" in is-active) echo active;; is-enabled) echo enabled;; esac\n") fakeMonitoringCommand(t, "ufw", "#!/bin/sh\necho 'Status: inactive'\n") + withDockerWants(t, dcgmExporterUnit) rc := newRealRC(t) // Both lifecycle modules are disabled so their existing host configuration // remains untouched, while the installed runtime and GPU are usable. @@ -369,3 +370,17 @@ func TestDCGMRuntimeFromNvidiaModuleIsPendingNotBlocked(t *testing.T) { } } } + +// withDockerWants points the Docker wants directory at a temp dir holding +// links for units. +func withDockerWants(t *testing.T, units ...string) { + t.Helper() + old := dockerWantsDir + dockerWantsDir = t.TempDir() + t.Cleanup(func() { dockerWantsDir = old }) + for _, u := range units { + if err := os.WriteFile(filepath.Join(dockerWantsDir, u), nil, 0o644); err != nil { + t.Fatal(err) + } + } +} diff --git a/tests/scenarios/exporter-firewall.sh b/tests/scenarios/exporter-firewall.sh new file mode 100755 index 0000000..aede824 --- /dev/null +++ b/tests/scenarios/exporter-firewall.sh @@ -0,0 +1,118 @@ +#!/usr/bin/env bash +# Host-run acceptance scenario for modules.monitoring.exporter_firewall on a +# disposable Docker + iptables + systemd host (the CI runner): config +# validation and dry run, adoption of a hand-installed v1 guard without rule +# changes, source filtering, exporters surviving guard and Docker restarts, +# and fail-closed exporter starts. Docker containers stand in for the allowed +# scraper and for another host in the segment. +set -euo pipefail + +REPO_ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")/../.." && pwd)" +source "$REPO_ROOT/tests/integration/assert.sh" + +BIN="${ROOTFILES_TEST_BINARY:?ROOTFILES_TEST_BINARY must name a Linux rootfiles binary}" +sudo install -m 0755 -o root -g root "$BIN" /usr/local/bin/rootfiles +RF="sudo /usr/local/bin/rootfiles" +HELPER=/usr/local/sbin/rootfiles-exporter-firewall +CFG=/etc/rootfiles-test/monitoring.yaml +CURL=curlimages/curl:8.10.1 +BIND="$(ip -4 -o route get 1.1.1.1 | sed -n 's/.* src \([0-9.]*\).*/\1/p')" +ALLOWED=172.30.9.10 +OTHER=172.30.9.11 + +cleanup() { + sudo "$HELPER" rollback >/dev/null 2>&1 || true + docker network rm efw-test >/dev/null 2>&1 || true +} +trap cleanup EXIT + +check() { # check DESCRIPTION COMMAND... + local desc=$1; shift + if "$@"; then pass "$desc"; else fail "$desc"; fi +} +refused() { ! "$@" >/dev/null 2>&1; } +scrape() { # scrape SOURCE_IP: fetch node exporter metrics from that address + docker run --rm --network efw-test --ip "$1" "$CURL" -fsS --max-time 5 "http://$BIND:9100/metrics" 2>/dev/null | grep -q '^node_exporter_build_info' +} +rules() { # the owned rules, without counters + { sudo iptables -S RFILES_EXPORTER_IN; sudo iptables -S RFILES_DCGM_DU + sudo iptables -S INPUT | grep rootfiles-exporter-firewall || true + sudo iptables -S DOCKER-USER | grep rootfiles-exporter-firewall || true; } 2>&1 +} +write_config() { # write_config ALLOW_FROM_YAML + sudo tee "$CFG" >/dev/null </dev/null +docker pull -q "$CURL" >/dev/null + +echo "--- AC5: invalid config and dry run change nothing ---" +write_config "[$ALLOWED/32, $OTHER/32]" +check "two sources rejected" refused $RF apply --config "$CFG" --module monitoring --yes +write_config "[$ALLOWED/32]" +$RF apply --config "$CFG" --module monitoring --dry-run --yes >/dev/null +check "dry run wrote no helper" sudo test ! -e "$HELPER" +check "dry run wrote no config" sudo test ! -e /etc/rootfiles/exporter-firewall.conf + +echo "--- AC1: adopt a hand-installed v1 guard ---" +sudo install -m 0750 -o root -g root "$REPO_ROOT/tests/scenarios/fixtures/exporter-firewall-v1.sh" "$HELPER" +sudo install -d -m 0755 /etc/rootfiles +printf 'BIND_IP=%s\nALLOW_FROM=%s/32\n' "$BIND" "$ALLOWED" | sudo tee /etc/rootfiles/exporter-firewall.conf >/dev/null +sudo chmod 0600 /etc/rootfiles/exporter-firewall.conf +sudo "$HELPER" setup +before="$(rules)" +check "v1 node exporter drop-in has Requires=" sudo grep -q '^Requires=rootfiles-exporter-firewall.service' /etc/systemd/system/prometheus-node-exporter.service.d/90-rootfiles-exporter-firewall.conf +$RF apply --config "$CFG" --module monitoring --yes +after="$(rules)" +check "owned rules unchanged by adoption" [ "$before" = "$after" ] +[ "$before" = "$after" ] || diff <(echo "$before") <(echo "$after") || true +check "monitoring satisfied after apply" $RF check --config "$CFG" --module monitoring +check "helper check passes" sudo "$HELPER" check +check "exporter drop-in no longer requires the guard unit" bash -c '! grep -q "^Requires=" /etc/systemd/system/prometheus-node-exporter.service.d/90-rootfiles-exporter-firewall.conf' +check "guard unit only wants docker" bash -c 'grep -q "^Wants=network-online.target docker.service" /etc/systemd/system/rootfiles-exporter-firewall.service && ! grep -q "^Requires=" /etc/systemd/system/rootfiles-exporter-firewall.service' +check "node exporter active" systemctl is-active --quiet prometheus-node-exporter + +echo "--- AC2: only allow_from reaches the exporter ---" +check "allowed source scrapes" scrape "$ALLOWED" +check "other source is refused" refused scrape "$OTHER" + +echo "--- AC4: guard unit stop does not stop the exporter; missing rules block its start ---" +sudo systemctl stop rootfiles-exporter-firewall +check "node exporter survives guard unit stop" systemctl is-active --quiet prometheus-node-exporter +check "rules outlive the guard unit" sudo "$HELPER" verify-host +sudo systemctl start rootfiles-exporter-firewall +sudo iptables -D INPUT -m comment --comment rootfiles-exporter-firewall:v1:jump:input -j RFILES_EXPORTER_IN +check "restart refused without INPUT rules" refused sudo systemctl restart prometheus-node-exporter +check "exporter not running without rules" refused systemctl is-active --quiet prometheus-node-exporter +check "other source still refused" refused scrape "$OTHER" +check "check reports the drift" refused $RF check --config "$CFG" --module monitoring +$RF apply --config "$CFG" --module monitoring --yes +check "apply restores the rules" sudo "$HELPER" check +check "apply starts the refused exporter" systemctl is-active --quiet prometheus-node-exporter +check "allowed source scrapes after restore" scrape "$ALLOWED" + +echo "--- AC3: Docker stop and start ---" +sudo systemctl stop docker.socket docker.service +check "node exporter survives docker stop" systemctl is-active --quiet prometheus-node-exporter +check "guard unit survives docker stop" systemctl is-active --quiet rootfiles-exporter-firewall +sudo systemctl start docker.service +for _ in $(seq 1 30); do sudo iptables -S DOCKER-USER 2>/dev/null | grep -q jump:docker && break; sleep 1; done +check "node exporter active after docker start" systemctl is-active --quiet prometheus-node-exporter +check "DOCKER-USER rules back after docker start" sudo "$HELPER" check +check "monitoring satisfied after docker restart" $RF check --config "$CFG" --module monitoring +check "allowed source scrapes after docker restart" scrape "$ALLOWED" +check "other source refused after docker restart" refused scrape "$OTHER" + +report diff --git a/tests/scenarios/fixtures/exporter-firewall-v1.sh b/tests/scenarios/fixtures/exporter-firewall-v1.sh new file mode 100644 index 0000000..0c1a44f --- /dev/null +++ b/tests/scenarios/fixtures/exporter-firewall-v1.sh @@ -0,0 +1,551 @@ +#!/usr/bin/env bash +# Operator template for a narrowly scoped exporter firewall guard. +# +# Install this file as /usr/local/sbin/rootfiles-exporter-firewall (root:root, +# mode 0750), then create /etc/rootfiles/exporter-firewall.conf (root:root, +# mode 0600). Config format: +# BIND_IP=192.0.2.10 +# ALLOW_FROM=198.51.100.200/32 +# WEB_INTERFACE=br-0123456789ab # optional local WEB scrape bridge only +# BIND_IP must match modules.monitoring.listen_address and ALLOW_FROM must be +# the single configured modules.monitoring.allow_from CIDR. Do not substitute a +# broad Docker subnet for the local WEB_INTERFACE exception. +# This template supports IPv4 iptables only. Do not set perimeter_firewall=true +# until `check` passes. On the WEB host, obtain WEB_INTERFACE from the running +# monitoring Compose network after startup, add it to the config, then rerun +# setup. Setup writes rules and enables this helper but does not start/restart +# Docker, UFW, exporter, or application units. +# +# Run plan, setup, check, apply, or rollback. "plan" is read-only. +# The Docker guard matches only forwarded traffic with conntrack original +# destination BIND_IP:9400. It changes no Docker containers, global firewall +# policies, or other ports. Host-originated Docker checks do not traverse these +# DOCKER-USER original-destination rules. +set -euo pipefail + +readonly OWNER="rootfiles-exporter-firewall:v1" +readonly CONFIG="/etc/rootfiles/exporter-firewall.conf" +readonly HELPER="/usr/local/sbin/rootfiles-exporter-firewall" +readonly SERVICE="rootfiles-exporter-firewall.service" +readonly SERVICE_FILE="/etc/systemd/system/rootfiles-exporter-firewall.service" +readonly DOCKER_DROPIN="/etc/systemd/system/docker.service.d/90-rootfiles-exporter-firewall.conf" +readonly UFW_DROPIN="/etc/systemd/system/ufw.service.d/90-rootfiles-exporter-firewall.conf" +readonly NODE_DROPIN="/etc/systemd/system/prometheus-node-exporter.service.d/90-rootfiles-exporter-firewall.conf" +readonly DCGM_DROPIN="/etc/systemd/system/rootfiles-dcgm-exporter.service.d/90-rootfiles-exporter-firewall.conf" +readonly HUB_DROPIN="/etc/systemd/system/rootfiles-monitoring-hub.service.d/90-rootfiles-exporter-firewall.conf" +readonly INPUT_CHAIN="RFILES_EXPORTER_IN" +readonly DOCKER_CHAIN="RFILES_DCGM_DU" +readonly NODE_PORT=9100 +readonly DCGM_PORT=9400 +KEEP_GUARDS=0 + +ACTION=plan +if [[ "$#" -gt 0 ]]; then ACTION="$1"; fi +BIND_IP="" +ALLOW_FROM="" +WEB_INTERFACE="" + +die() { echo "ERROR: $*" >&2; exit 1; } +note() { echo "rootfiles exporter firewall: $*"; } +warn() { echo "WARNING: $*" >&2; } +require_root() { [[ "$EUID" -eq 0 ]] || die "run as root"; } +ipt() { iptables -w 5 "$@"; } + +validate_config() { + require_root + mode_is_trusted /etc/rootfiles || die "/etc/rootfiles must be root-owned and not group/world writable" + load_config +} + +require_helper_installed() { + local current owner mode + mode_is_trusted /usr/local/sbin || die "/usr/local/sbin must be a trusted root-owned directory" + current="$(readlink -f -- "$0")" + [[ "$current" == "$HELPER" && -f "$HELPER" && ! -L "$HELPER" ]] || die "install this script as $HELPER before setup" + owner="$(stat -c '%u:%g' "$HELPER")" + [[ "$owner" == "0:0" ]] || die "$HELPER must be root:root owned" + mode="$(stat -c '%a' "$HELPER")" + (( (0$mode & 022) == 0 )) || die "$HELPER must not be group/world writable" + grep -Fq "$OWNER" "$HELPER" || die "$HELPER has no matching owner marker" +} + +mode_is_trusted() { + local path="$1" owner mode + [[ -d "$path" && ! -L "$path" ]] || return 1 + owner="$(stat -c '%u:%g' "$path")" + [[ "$owner" == "0:0" ]] || return 1 + mode="$(stat -c '%a' "$path")" + (( (0$mode & 022) == 0 )) +} + +ensure_trusted_dir() { + local path="$1" parent + if [[ "$path" == "/" ]]; then + mode_is_trusted / || die "untrusted root directory" + return + fi + parent="$(dirname "$path")" + ensure_trusted_dir "$parent" + [[ ! -L "$path" ]] || die "refusing symlink directory: $path" + if [[ -e "$path" ]]; then + mode_is_trusted "$path" || die "untrusted directory: $path" + else + mode_is_trusted "$parent" || die "untrusted parent directory: $parent" + install -d -o root -g root -m 0755 "$path" + mode_is_trusted "$path" || die "created directory is not trusted: $path" + fi +} + +file_is_owned_or_absent() { + local path="$1" + [[ ! -L "$path" ]] || die "refusing symlink: $path" + if [[ -e "$path" ]]; then + [[ -f "$path" ]] || die "not a regular file: $path" + [[ "$(stat -c '%u:%g' "$path")" == "0:0" ]] || die "file is not root-owned: $path" + grep -Fq "$OWNER" "$path" || die "unowned file collision: $path" + fi +} + +write_owned_file() { + local path="$1" content="$2" parent tmp + parent="$(dirname "$path")" + ensure_trusted_dir "$parent" + file_is_owned_or_absent "$path" + tmp="$(mktemp "$parent/.rootfiles-exporter-fw.XXXXXX")" + trap 'rm -f -- "$tmp"' EXIT + printf '%s' "$content" > "$tmp" + chown root:root "$tmp" + chmod 0644 "$tmp" + mv -f -- "$tmp" "$path" + trap - EXIT +} + +remove_owned_file() { + local path="$1" + [[ -e "$path" || -L "$path" ]] || return 0 + [[ ! -L "$path" && -f "$path" ]] || die "refusing to remove non-regular managed path: $path" + [[ "$(stat -c '%u:%g' "$path")" == "0:0" ]] || die "refusing to remove non-root-owned file: $path" + grep -Fq "$OWNER" "$path" || die "refusing to remove unowned file: $path" + rm -f -- "$path" +} + +load_config() { + local key value seen_bind=0 seen_allow=0 seen_if=0 owner mode + [[ -f "$CONFIG" && ! -L "$CONFIG" ]] || die "missing regular config file: $CONFIG" + owner="$(stat -c '%u:%g' "$CONFIG")" + [[ "$owner" == "0:0" ]] || die "config must be root:root owned" + mode="$(stat -c '%a' "$CONFIG")" + (( (0$mode & 022) == 0 )) || die "config must not be group/world writable" + while IFS='=' read -r key value || [[ -n "$key$value" ]]; do + case "$key" in + ""|\#*) continue ;; + BIND_IP) + (( seen_bind == 0 )) || die "duplicate BIND_IP" + seen_bind=1 + BIND_IP="$value" + ;; + ALLOW_FROM) + (( seen_allow == 0 )) || die "duplicate ALLOW_FROM" + seen_allow=1 + ALLOW_FROM="$value" + ;; + WEB_INTERFACE) + (( seen_if == 0 )) || die "duplicate WEB_INTERFACE" + seen_if=1 + WEB_INTERFACE="$value" + ;; + *) die "unsupported config key: $key" ;; + esac + done < "$CONFIG" + [[ "$seen_bind" -eq 1 && "$seen_allow" -eq 1 ]] || die "config requires BIND_IP and ALLOW_FROM" + [[ "$BIND_IP" =~ ^[0-9.]+$ ]] || die "BIND_IP must be an IPv4 address" + [[ "$ALLOW_FROM" =~ ^[0-9./]+$ && "$ALLOW_FROM" != *,* ]] || die "ALLOW_FROM must be one IPv4 CIDR" + if [[ -n "$WEB_INTERFACE" && ! "$WEB_INTERFACE" =~ ^br-[0-9a-fA-F]{12}$ ]]; then + die "WEB_INTERFACE must be a Docker bridge name like br-0123456789ab" + fi + command -v python3 >/dev/null || die "python3 is required for strict IPv4/CIDR validation" + python3 -c 'import ipaddress,sys; ipaddress.IPv4Address(sys.argv[1]); n=ipaddress.IPv4Network(sys.argv[2], strict=True); assert n.prefixlen == 32, "exporter source must be one host /32"' "$BIND_IP" "$ALLOW_FROM" || + die "BIND_IP or ALLOW_FROM is not a valid canonical IPv4 value" + command -v iptables >/dev/null || die "iptables is required" + if command -v firewall-cmd >/dev/null && firewall-cmd --state >/dev/null 2>&1; then + die "active firewalld is unsupported by this iptables/UFW template" + fi +} + +ufw_unit_loaded() { + [[ "$(systemctl show -p LoadState --value ufw.service 2>/dev/null || true)" == "loaded" ]] +} + +ufw_active() { + command -v ufw >/dev/null && ufw status 2>/dev/null | grep -qi '^Status: active$' +} + +warn_legacy_publisher() { + command -v docker >/dev/null || return 0 + local name + while IFS= read -r name; do + [[ -n "$name" && "$name" != "rootfiles-dcgm-exporter" ]] && + warn "container '$name' already publishes $DCGM_PORT; it is untouched but gets this source restriction on BIND_IP:$DCGM_PORT" + done < <(docker ps --filter "publish=$DCGM_PORT" --format '{{.Names}}' 2>/dev/null || true) + return 0 +} + +chain_exists() { ipt -S "$1" >/dev/null 2>&1; } +chain_owned() { + local chain="$1" family="$2" line marker_found=0 + while IFS= read -r line; do + [[ "$line" == *"-A $chain "* ]] || continue + [[ "$line" == *"$OWNER:$family:"* ]] || return 1 + [[ "$line" == *"$OWNER:$family:owner"* ]] && marker_found=1 + done < <(ipt -S "$chain" 2>/dev/null) + (( marker_found == 1 )) +} + +check_parent_references() { + local parent="$1" chain="$2" jump_mark="$3" line + while IFS= read -r line; do + [[ "$line" == *"-j $chain"* ]] || continue + [[ "$line" == *"$jump_mark"* ]] || die "unowned jump references $chain from $parent: $line" + done < <(ipt -S "$parent" 2>/dev/null) +} + +check_all_chain_references() { + local parent="$1" chain="$2" jump_mark="$3" line + while IFS= read -r line; do + [[ "$line" == *"-j $chain"* ]] || continue + [[ "$line" == "-A $parent "* && "$line" == *"$jump_mark"* ]] || + die "unowned filter-table reference to $chain: $line" + done < <(iptables-save -t filter) +} + +preflight_chains() { + ipt -S INPUT >/dev/null || die "INPUT chain is unavailable" + ipt -S DOCKER-USER >/dev/null || die "DOCKER-USER is missing; start Docker before setup" + if chain_exists "$INPUT_CHAIN" && ! chain_owned "$INPUT_CHAIN" input; then + die "chain collision: $INPUT_CHAIN exists without this template's owner marker" + fi + if chain_exists "$DOCKER_CHAIN" && ! chain_owned "$DOCKER_CHAIN" docker; then + die "chain collision: $DOCKER_CHAIN exists without this template's owner marker" + fi + check_parent_references INPUT "$INPUT_CHAIN" "$OWNER:jump:input" + check_parent_references DOCKER-USER "$DOCKER_CHAIN" "$OWNER:jump:docker" + check_all_chain_references INPUT "$INPUT_CHAIN" "$OWNER:jump:input" + check_all_chain_references DOCKER-USER "$DOCKER_CHAIN" "$OWNER:jump:docker" + check_guard_config INPUT "$OWNER:guard:input" "-p tcp -d $BIND_IP --dport $NODE_PORT" + check_guard_config DOCKER-USER "$OWNER:guard:docker" "-p tcp -m conntrack --ctdir ORIGINAL --ctorigdst $BIND_IP --ctorigdstport $DCGM_PORT" +} + +preflight_managed_files() { + mode_is_trusted /etc/systemd/system || die "untrusted systemd unit directory" + for dir in /etc/systemd/system/docker.service.d /etc/systemd/system/ufw.service.d /etc/systemd/system/prometheus-node-exporter.service.d /etc/systemd/system/rootfiles-dcgm-exporter.service.d /etc/systemd/system/rootfiles-monitoring-hub.service.d; do + if [[ -e "$dir" || -L "$dir" ]]; then mode_is_trusted "$dir" || die "untrusted systemd drop-in directory: $dir"; fi + done + file_is_owned_or_absent "$SERVICE_FILE" + file_is_owned_or_absent "$DOCKER_DROPIN" + file_is_owned_or_absent "$UFW_DROPIN" + file_is_owned_or_absent "$NODE_DROPIN" + file_is_owned_or_absent "$DCGM_DROPIN" + file_is_owned_or_absent "$HUB_DROPIN" + if ! ufw_unit_loaded && ufw_active; then + die "UFW is active but ufw.service is unavailable; cannot persist INPUT rules across UFW reloads" + fi +} + +ensure_owned_chain() { + local chain="$1" family="$2" + if chain_exists "$chain"; then + chain_owned "$chain" "$family" || die "refusing to modify unowned chain: $chain" + else + ipt -N "$chain" + fi +} + +remove_owned_jumps() { + local parent="$1" chain="$2" marker="$3" + while ipt -C "$parent" -m comment --comment "$marker" -j "$chain" >/dev/null 2>&1; do + ipt -D "$parent" -m comment --comment "$marker" -j "$chain" + done +} + +check_guard_config() { + local parent="$1" marker="$2" exact="$3" line + while IFS= read -r line; do + [[ "$line" == *"$marker"* ]] || continue + ipt -C "$parent" $exact -m comment --comment "$marker" -j DROP >/dev/null 2>&1 || + die "owned fail-closed guard has stale or unexpected parameters in $parent; inspect it before changing config" + done < <(ipt -S "$parent" 2>/dev/null) +} + +ensure_input_guard() { + local marker="$OWNER:guard:input" + check_guard_config INPUT "$marker" "-p tcp -d $BIND_IP --dport $NODE_PORT" + ipt -C INPUT -p tcp -d "$BIND_IP" --dport "$NODE_PORT" -m comment --comment "$marker" -j DROP >/dev/null 2>&1 || + ipt -I INPUT 1 -p tcp -d "$BIND_IP" --dport "$NODE_PORT" -m comment --comment "$marker" -j DROP +} + +ensure_docker_guard() { + local marker="$OWNER:guard:docker" + check_guard_config DOCKER-USER "$marker" "-p tcp -m conntrack --ctdir ORIGINAL --ctorigdst $BIND_IP --ctorigdstport $DCGM_PORT" + ipt -C DOCKER-USER -p tcp -m conntrack --ctdir ORIGINAL --ctorigdst "$BIND_IP" --ctorigdstport "$DCGM_PORT" -m comment --comment "$marker" -j DROP >/dev/null 2>&1 || + ipt -I DOCKER-USER 1 -p tcp -m conntrack --ctdir ORIGINAL --ctorigdst "$BIND_IP" --ctorigdstport "$DCGM_PORT" -m comment --comment "$marker" -j DROP +} + +remove_input_guard() { + while ipt -C INPUT -p tcp -d "$BIND_IP" --dport "$NODE_PORT" -m comment --comment "$OWNER:guard:input" -j DROP >/dev/null 2>&1; do + ipt -D INPUT -p tcp -d "$BIND_IP" --dport "$NODE_PORT" -m comment --comment "$OWNER:guard:input" -j DROP + done +} + +remove_docker_guard() { + while ipt -C DOCKER-USER -p tcp -m conntrack --ctdir ORIGINAL --ctorigdst "$BIND_IP" --ctorigdstport "$DCGM_PORT" -m comment --comment "$OWNER:guard:docker" -j DROP >/dev/null 2>&1; do + ipt -D DOCKER-USER -p tcp -m conntrack --ctdir ORIGINAL --ctorigdst "$BIND_IP" --ctorigdstport "$DCGM_PORT" -m comment --comment "$OWNER:guard:docker" -j DROP + done +} + +apply_host_rules() { + local jump="$OWNER:jump:input" + check_parent_references INPUT "$INPUT_CHAIN" "$jump" + check_all_chain_references INPUT "$INPUT_CHAIN" "$jump" + check_guard_config INPUT "$OWNER:guard:input" "-p tcp -d $BIND_IP --dport $NODE_PORT" + ensure_input_guard + ensure_owned_chain "$INPUT_CHAIN" input + remove_owned_jumps INPUT "$INPUT_CHAIN" "$jump" + ipt -F "$INPUT_CHAIN" + # Leave the owner marker in place even if the process is interrupted. The + # parent DROP guard keeps this endpoint closed until the rebuilt chain is + # attached and complete. + ipt -A "$INPUT_CHAIN" -m comment --comment "$OWNER:input:owner" -j RETURN + ipt -I "$INPUT_CHAIN" 1 -p tcp -d "$BIND_IP" --dport "$NODE_PORT" -m comment --comment "$OWNER:input:node-drop" -j DROP + ipt -I "$INPUT_CHAIN" 1 -p tcp -s "$ALLOW_FROM" -d "$BIND_IP" --dport "$NODE_PORT" -m conntrack --ctstate NEW,ESTABLISHED -m comment --comment "$OWNER:input:web" -j ACCEPT + if [[ -n "$WEB_INTERFACE" ]]; then + ipt -I "$INPUT_CHAIN" 1 -i "$WEB_INTERFACE" -p tcp -d "$BIND_IP" --dport "$NODE_PORT" -m comment --comment "$OWNER:input:web-bridge" -j ACCEPT + fi + ipt -I "$INPUT_CHAIN" 1 -i lo -p tcp -d "$BIND_IP" --dport "$NODE_PORT" -m comment --comment "$OWNER:input:loopback" -j ACCEPT + ipt -I INPUT 1 -m comment --comment "$jump" -j "$INPUT_CHAIN" + if [[ "$KEEP_GUARDS" -eq 0 ]]; then remove_input_guard; fi +} + +apply_docker_rules() { + local jump="$OWNER:jump:docker" + ipt -S DOCKER-USER >/dev/null || die "DOCKER-USER is missing; start Docker before applying DCGM protection" + check_parent_references DOCKER-USER "$DOCKER_CHAIN" "$jump" + check_all_chain_references DOCKER-USER "$DOCKER_CHAIN" "$jump" + check_guard_config DOCKER-USER "$OWNER:guard:docker" "-p tcp -m conntrack --ctdir ORIGINAL --ctorigdst $BIND_IP --ctorigdstport $DCGM_PORT" + ensure_docker_guard + ensure_owned_chain "$DOCKER_CHAIN" docker + remove_owned_jumps DOCKER-USER "$DOCKER_CHAIN" "$jump" + ipt -F "$DOCKER_CHAIN" + ipt -A "$DOCKER_CHAIN" -m comment --comment "$OWNER:docker:owner" -j RETURN + ipt -I "$DOCKER_CHAIN" 1 -p tcp -m conntrack --ctdir ORIGINAL --ctorigdst "$BIND_IP" --ctorigdstport "$DCGM_PORT" -m comment --comment "$OWNER:docker:dcgm-drop" -j DROP + ipt -I "$DOCKER_CHAIN" 1 -p tcp -s "$ALLOW_FROM" -m conntrack --ctstate NEW,ESTABLISHED --ctdir ORIGINAL --ctorigdst "$BIND_IP" --ctorigdstport "$DCGM_PORT" -m comment --comment "$OWNER:docker:web" -j ACCEPT + ipt -I "$DOCKER_CHAIN" 1 -p tcp -m conntrack --ctstate ESTABLISHED,RELATED --ctdir REPLY --ctorigdst "$BIND_IP" --ctorigdstport "$DCGM_PORT" -m comment --comment "$OWNER:docker:reply" -j RETURN + ipt -I DOCKER-USER 1 -m comment --comment "$jump" -j "$DOCKER_CHAIN" + if [[ "$KEEP_GUARDS" -eq 0 ]]; then remove_docker_guard; fi +} + +check_rules() { + ipt -S INPUT | grep -Fq -- "$OWNER:guard:input" && return 1 + ipt -S DOCKER-USER | grep -Fq -- "$OWNER:guard:docker" && return 1 + ipt -C INPUT -m comment --comment "$OWNER:jump:input" -j "$INPUT_CHAIN" || return 1 + ipt -C "$INPUT_CHAIN" -i lo -p tcp -d "$BIND_IP" --dport "$NODE_PORT" -m comment --comment "$OWNER:input:loopback" -j ACCEPT || return 1 + if [[ -n "$WEB_INTERFACE" ]]; then + ipt -C "$INPUT_CHAIN" -i "$WEB_INTERFACE" -p tcp -d "$BIND_IP" --dport "$NODE_PORT" -m comment --comment "$OWNER:input:web-bridge" -j ACCEPT || return 1 + fi + ipt -C "$INPUT_CHAIN" -p tcp -s "$ALLOW_FROM" -d "$BIND_IP" --dport "$NODE_PORT" -m conntrack --ctstate NEW,ESTABLISHED -m comment --comment "$OWNER:input:web" -j ACCEPT || return 1 + ipt -C "$INPUT_CHAIN" -p tcp -d "$BIND_IP" --dport "$NODE_PORT" -m comment --comment "$OWNER:input:node-drop" -j DROP || return 1 + ipt -C DOCKER-USER -m comment --comment "$OWNER:jump:docker" -j "$DOCKER_CHAIN" || return 1 + ipt -C "$DOCKER_CHAIN" -p tcp -s "$ALLOW_FROM" -m conntrack --ctstate NEW,ESTABLISHED --ctdir ORIGINAL --ctorigdst "$BIND_IP" --ctorigdstport "$DCGM_PORT" -m comment --comment "$OWNER:docker:web" -j ACCEPT || return 1 + ipt -C "$DOCKER_CHAIN" -p tcp -m conntrack --ctdir ORIGINAL --ctorigdst "$BIND_IP" --ctorigdstport "$DCGM_PORT" -m comment --comment "$OWNER:docker:dcgm-drop" -j DROP || return 1 + return 0 +} + +remove_owned_chain() { + local parent="$1" chain="$2" family="$3" jump="$4" line + chain_exists "$chain" || return 0 + chain_owned "$chain" "$family" || die "refusing to remove unowned chain: $chain" + check_parent_references "$parent" "$chain" "$jump" + remove_owned_jumps "$parent" "$chain" "$jump" + while IFS= read -r line; do + [[ "$line" == *"-j $chain"* ]] && die "chain $chain still has an external reference: $line" + done < <(iptables-save -t filter) + ipt -F "$chain" + ipt -X "$chain" +} + +preflight_rollback() { + if chain_exists "$INPUT_CHAIN" && ! chain_owned "$INPUT_CHAIN" input; then die "refusing unowned chain $INPUT_CHAIN"; fi + if chain_exists "$DOCKER_CHAIN" && ! chain_owned "$DOCKER_CHAIN" docker; then die "refusing unowned chain $DOCKER_CHAIN"; fi + check_parent_references INPUT "$INPUT_CHAIN" "$OWNER:jump:input" + if ipt -S DOCKER-USER >/dev/null 2>&1; then check_parent_references DOCKER-USER "$DOCKER_CHAIN" "$OWNER:jump:docker"; fi + check_all_chain_references INPUT "$INPUT_CHAIN" "$OWNER:jump:input" + if ipt -S DOCKER-USER >/dev/null 2>&1; then check_all_chain_references DOCKER-USER "$DOCKER_CHAIN" "$OWNER:jump:docker"; fi + check_guard_config INPUT "$OWNER:guard:input" "-p tcp -d $BIND_IP --dport $NODE_PORT" + check_guard_config DOCKER-USER "$OWNER:guard:docker" "-p tcp -m conntrack --ctdir ORIGINAL --ctorigdst $BIND_IP --ctorigdstport $DCGM_PORT" +} + +install_systemd() { + preflight_managed_files + local service_content docker_content ufw_content exporter_content + ensure_trusted_dir /etc/systemd/system/docker.service.d + if ufw_unit_loaded; then ensure_trusted_dir /etc/systemd/system/ufw.service.d; fi + ensure_trusted_dir "$(dirname "$NODE_DROPIN")" + ensure_trusted_dir "$(dirname "$DCGM_DROPIN")" + ensure_trusted_dir "$(dirname "$HUB_DROPIN")" + service_content="# $OWNER +[Unit] +Description=Rootfiles exporter firewall guard +After=network-online.target ufw.service docker.service +Wants=network-online.target +Requires=docker.service + +[Service] +Type=oneshot +RemainAfterExit=yes +ExecStart=$HELPER apply + +[Install] +WantedBy=multi-user.target +" + docker_content="# $OWNER +[Unit] +After=ufw.service + +[Service] +ExecStartPost=$HELPER apply-docker +" + write_owned_file "$SERVICE_FILE" "$service_content" + write_owned_file "$DOCKER_DROPIN" "$docker_content" + exporter_content="# $OWNER +[Unit] +Requires=$SERVICE +After=$SERVICE +" + write_owned_file "$NODE_DROPIN" "$exporter_content" + write_owned_file "$DCGM_DROPIN" "$exporter_content" + write_owned_file "$HUB_DROPIN" "$exporter_content" + if ufw_unit_loaded; then + ufw_content="# $OWNER +[Service] +ExecStartPost=$HELPER apply-host +" + write_owned_file "$UFW_DROPIN" "$ufw_content" + fi + systemctl daemon-reload +} + +legacy_warning() { + local name + command -v docker >/dev/null || return 0 + while IFS= read -r name; do + [[ -n "$name" && "$name" != "rootfiles-dcgm-exporter" ]] && + warn "container '$name' already publishes $DCGM_PORT; it is untouched but gets this source restriction on BIND_IP:$DCGM_PORT" + done < <(docker ps --filter "publish=$DCGM_PORT" --format '{{.Names}}' 2>/dev/null || true) + return 0 +} + +setup_firewall() { + validate_config + preflight_chains + require_helper_installed + preflight_managed_files + legacy_warning + install_systemd + systemctl enable "$SERVICE" >/dev/null + apply_all + note "installed and applied INPUT $BIND_IP:$NODE_PORT and DOCKER-USER original destination $BIND_IP:$DCGM_PORT" +} + +apply_all() { + validate_config + require_helper_installed + preflight_chains + ensure_input_guard + ensure_docker_guard + KEEP_GUARDS=1 + apply_host_rules + apply_docker_rules + KEEP_GUARDS=0 + remove_input_guard + remove_docker_guard + legacy_warning + note "rules reapplied" +} + +check_mode() { + validate_config + if check_rules; then + systemctl is-enabled --quiet "$SERVICE" || die "$SERVICE is not enabled" + grep -Fq "$OWNER" "$DOCKER_DROPIN" || die "Docker restart hook is missing" + grep -Fq "$OWNER" "$NODE_DROPIN" || die "node exporter dependency drop-in is missing" + grep -Fq "$OWNER" "$DCGM_DROPIN" || die "DCGM exporter dependency drop-in is missing" + grep -Fq "$OWNER" "$HUB_DROPIN" || die "monitoring hub dependency drop-in is missing" + if ufw_unit_loaded; then grep -Fq "$OWNER" "$UFW_DROPIN" || die "UFW reload hook is missing"; fi + note "PASS: INPUT limits node exporter and DOCKER-USER limits only the $BIND_IP:$DCGM_PORT published destination" + legacy_warning + else + die "required owner-marked firewall rules are missing or drifted" + fi +} + +rollback_mode() { + require_root + validate_config + preflight_managed_files + preflight_rollback + systemctl disable --now "$SERVICE" >/dev/null 2>&1 || true + remove_owned_file "$DOCKER_DROPIN" + remove_owned_file "$UFW_DROPIN" + remove_owned_file "$NODE_DROPIN" + remove_owned_file "$DCGM_DROPIN" + remove_owned_file "$HUB_DROPIN" + remove_owned_file "$SERVICE_FILE" + systemctl daemon-reload + remove_owned_chain INPUT "$INPUT_CHAIN" input "$OWNER:jump:input" + if ipt -S DOCKER-USER >/dev/null 2>&1; then + remove_owned_chain DOCKER-USER "$DOCKER_CHAIN" docker "$OWNER:jump:docker" + fi + remove_input_guard + if ipt -S DOCKER-USER >/dev/null 2>&1; then remove_docker_guard; fi + note "removed only this template's marked hooks, jumps, and chains; config/helper retained" +} + +main() { + require_root + case "$ACTION" in + plan) + validate_config + note "would allow WEB CIDR $ALLOW_FROM to host INPUT $BIND_IP:$NODE_PORT" + [[ -n "$WEB_INTERFACE" ]] && note "would allow local scrape traffic on interface $WEB_INTERFACE to INPUT $BIND_IP:$NODE_PORT" + [[ -n "$WEB_INTERFACE" ]] || warn "WEB_INTERFACE is unset; same-host Prometheus scraping is not admitted by this template" + note "would restrict only forwarded original destination $BIND_IP:$DCGM_PORT in DOCKER-USER" + note "host-originated and container-local Docker checks are outside the DOCKER-USER rule" + legacy_warning + ;; + setup) + setup_firewall + ;; + apply) + apply_all + ;; + apply-host) + validate_config + apply_host_rules + ;; + apply-docker) + validate_config + apply_docker_rules + ;; + check) + check_mode + ;; + rollback) + rollback_mode + ;; + *) + echo "Usage: $0 {plan|setup|apply|apply-host|apply-docker|check|rollback}" >&2 + exit 2 + ;; + esac +} + +main "$@" From cc49f599ccd17acef4b8491a79936de241971271 Mon Sep 17 00:00:00 2001 From: Young Joon Lee Date: Tue, 20 Oct 2026 19:57:36 +0900 Subject: [PATCH 2/3] fix(monitoring): reset a start-limited exporter before starting it; fix scrape check under pipefail Claude-Session: https://claude.ai/code/session_0177Zk7to4EYohgM59v4Fcj7 --- internal/module/monitoring_guard.go | 13 ++++++++++++- internal/module/monitoring_guard_test.go | 2 +- tests/scenarios/exporter-firewall.sh | 5 ++++- 3 files changed, 17 insertions(+), 3 deletions(-) diff --git a/internal/module/monitoring_guard.go b/internal/module/monitoring_guard.go index a97dd4c..8eebaf8 100644 --- a/internal/module/monitoring_guard.go +++ b/internal/module/monitoring_guard.go @@ -128,8 +128,10 @@ func applyExporterGuard(ctx context.Context, rc *RunContext) (*ApplyResult, erro cfg := rc.Config.Modules.Monitoring for unit, on := range map[string]bool{nodeExporterUnit: cfg.NodeExporter, dcgmExporterUnit: cfg.DCGMExporter} { if on && !serviceActive(ctx, rc, unit) { + // Refused starts may have hit the unit's start limit. + _, _ = rc.Runner.Run(ctx, "systemctl", "reset-failed", unit) if _, err := rc.Runner.Run(ctx, "systemctl", "start", unit); err != nil { - out.Warnings = append(out.Warnings, "starting "+unit+": "+firstLine(err.Error())) + out.Warnings = append(out.Warnings, "starting "+unit+": "+lastLine(err.Error())) } } } @@ -144,3 +146,12 @@ func guardError(err error) string { } return firstLine(msg) } + +// lastLine returns the stderr Runner errors end with, or their first line. +func lastLine(s string) string { + lines := strings.Split(strings.TrimSpace(s), "\n") + if last := strings.TrimSpace(strings.TrimPrefix(lines[len(lines)-1], "stderr:")); len(lines) > 1 && last != "" { + return last + } + return firstLine(s) +} diff --git a/internal/module/monitoring_guard_test.go b/internal/module/monitoring_guard_test.go index 4992d60..9b56fd0 100644 --- a/internal/module/monitoring_guard_test.go +++ b/internal/module/monitoring_guard_test.go @@ -106,7 +106,7 @@ func TestExporterGuardStartsExporterRefusedWithoutRules(t *testing.T) { if _, err := applyExporterGuard(context.Background(), rc); err != nil { t.Fatal(err) } - if !strings.Contains(readCalls(t, calls), "systemctl start "+nodeExporterUnit) { + if !strings.Contains(readCalls(t, calls), "systemctl reset-failed "+nodeExporterUnit+"\nsystemctl start "+nodeExporterUnit) { t.Fatalf("inactive exporter not started after setup: %s", readCalls(t, calls)) } if strings.Contains(readCalls(t, calls), dcgmExporterUnit) { diff --git a/tests/scenarios/exporter-firewall.sh b/tests/scenarios/exporter-firewall.sh index aede824..0e66ab1 100755 --- a/tests/scenarios/exporter-firewall.sh +++ b/tests/scenarios/exporter-firewall.sh @@ -32,7 +32,9 @@ check() { # check DESCRIPTION COMMAND... } refused() { ! "$@" >/dev/null 2>&1; } scrape() { # scrape SOURCE_IP: fetch node exporter metrics from that address - docker run --rm --network efw-test --ip "$1" "$CURL" -fsS --max-time 5 "http://$BIND:9100/metrics" 2>/dev/null | grep -q '^node_exporter_build_info' + local out + out="$(docker run --rm --network efw-test --ip "$1" "$CURL" -fsS --max-time 5 "http://$BIND:9100/metrics" 2>&1)" || { echo " scrape from $1: ${out:0:200}"; return 1; } + grep -q '^node_exporter_build_info' <<<"$out" } rules() { # the owned rules, without counters { sudo iptables -S RFILES_EXPORTER_IN; sudo iptables -S RFILES_DCGM_DU @@ -101,6 +103,7 @@ check "check reports the drift" refused $RF check --config "$CFG" --module monit $RF apply --config "$CFG" --module monitoring --yes check "apply restores the rules" sudo "$HELPER" check check "apply starts the refused exporter" systemctl is-active --quiet prometheus-node-exporter +systemctl is-active --quiet prometheus-node-exporter || sudo systemctl status prometheus-node-exporter --no-pager -l | tail -20 check "allowed source scrapes after restore" scrape "$ALLOWED" echo "--- AC3: Docker stop and start ---" From 5bfed9fba192dc79dc00d37aec82ca0ccd12a74d Mon Sep 17 00:00:00 2001 From: Young Joon Lee Date: Tue, 20 Oct 2026 20:11:19 +0900 Subject: [PATCH 3/3] fix(monitoring): install the exporter guard before exporters, repair unsafe helper files, back up setup units Claude-Session: https://claude.ai/code/session_0177Zk7to4EYohgM59v4Fcj7 --- internal/module/monitoring.go | 19 ++++-- internal/module/monitoring_guard.go | 51 +++++++++++++-- internal/module/monitoring_guard_test.go | 83 +++++++++++++++++++++++- 3 files changed, 140 insertions(+), 13 deletions(-) diff --git a/internal/module/monitoring.go b/internal/module/monitoring.go index 82efe34..7c71ea9 100644 --- a/internal/module/monitoring.go +++ b/internal/module/monitoring.go @@ -53,6 +53,11 @@ func (m *MonitoringModule) Apply(ctx context.Context, rc *RunContext) (*ApplyRes if _, err := checkMonitoringHub(ctx, rc); err != nil { return nil, err } + // The guard goes first, so no exporter starts before its rules exist. + guard, err := applyExporterGuard(ctx, rc) + if err != nil { + return nil, err + } // exporters carries a result next to a blocked-DCGM error; the hub is // still applied before that error fails the module. exporters, exportErr := applyMonitoringExporters(ctx, rc) @@ -63,12 +68,16 @@ func (m *MonitoringModule) Apply(ctx context.Context, rc *RunContext) (*ApplyRes if err != nil { return nil, err } - // After the hub, so a first apply admits the bridge the hub just created. - guard, err := applyExporterGuard(ctx, rc) - if err != nil { - return nil, err + out := mergeApplyResults(mergeApplyResults(guard, exporters), hub) + if rc.Config.Modules.Monitoring.Hub.Enabled { + // Again after the hub, so a first apply admits the bridge it created. + bridge, err := applyExporterGuard(ctx, rc) + if err != nil { + return nil, err + } + out = mergeApplyResults(out, bridge) } - return mergeApplyResults(mergeApplyResults(exporters, hub), guard), exportErr + return out, exportErr } func mergeApplyResults(a, b *ApplyResult) *ApplyResult { diff --git a/internal/module/monitoring_guard.go b/internal/module/monitoring_guard.go index 8eebaf8..1dde695 100644 --- a/internal/module/monitoring_guard.go +++ b/internal/module/monitoring_guard.go @@ -8,6 +8,7 @@ import ( "os" "path/filepath" "strings" + "syscall" ) // The exporter firewall guard admits exporter traffic only from allow_from. @@ -23,8 +24,34 @@ const exporterGuardUnit = "rootfiles-exporter-firewall.service" var ( exporterGuardHelperPath = "/usr/local/sbin/rootfiles-exporter-firewall" exporterGuardConfPath = "/etc/rootfiles/exporter-firewall.conf" + exporterGuardRootUID = uint32(0) + // exporterGuardSetupFiles are the units and drop-ins the helper's setup + // writes. They go into the apply backup, so rollback removes them too. + exporterGuardSetupFiles = []string{ + "/etc/systemd/system/rootfiles-exporter-firewall.service", + "/etc/systemd/system/docker.service.d/90-rootfiles-exporter-firewall.conf", + "/etc/systemd/system/ufw.service.d/90-rootfiles-exporter-firewall.conf", + "/etc/systemd/system/prometheus-node-exporter.service.d/90-rootfiles-exporter-firewall.conf", + "/etc/systemd/system/rootfiles-dcgm-exporter.service.d/90-rootfiles-exporter-firewall.conf", + "/etc/systemd/system/rootfiles-monitoring-hub.service.d/90-rootfiles-exporter-firewall.conf", + } ) +// exporterGuardFileCurrent reports whether path holds content as a regular, +// root-owned file that group and others cannot write: systemd runs the +// helper as root, and the helper trusts its config. +func exporterGuardFileCurrent(rc *RunContext, path string, content []byte) bool { + fi, err := os.Lstat(path) + if err != nil || !fi.Mode().IsRegular() || fi.Mode().Perm()&0o022 != 0 { + return false + } + if st, ok := fi.Sys().(*syscall.Stat_t); !ok || st.Uid != exporterGuardRootUID { + return false + } + cur, err := rc.Runner.ReadFile(path) + return err == nil && bytes.Equal(cur, content) +} + // exporterGuardConf renders the helper config. On a hub host the hub's // Compose bridge is admitted too, so its Prometheus can scrape this host. func exporterGuardConf(ctx context.Context, rc *RunContext) string { @@ -72,10 +99,10 @@ func checkExporterGuard(ctx context.Context, rc *RunContext) []Change { } setup := exporterGuardHelperPath + " setup" var changes []Change - if cur, err := rc.Runner.ReadFile(exporterGuardHelperPath); err != nil || !bytes.Equal(cur, exporterGuardHelper) { + if !exporterGuardFileCurrent(rc, exporterGuardHelperPath, exporterGuardHelper) { changes = append(changes, Change{Description: "Install the exporter firewall helper", Command: "write " + exporterGuardHelperPath + " && " + setup}) } - if cur, err := rc.Runner.ReadFile(exporterGuardConfPath); err != nil || string(cur) != exporterGuardConf(ctx, rc) { + if !exporterGuardFileCurrent(rc, exporterGuardConfPath, []byte(exporterGuardConf(ctx, rc))) { changes = append(changes, Change{Description: "Write the exporter firewall config", Command: "write " + exporterGuardConfPath + " && " + setup}) } if len(changes) == 0 { @@ -95,12 +122,19 @@ func applyExporterGuard(ctx context.Context, rc *RunContext) (*ApplyResult, erro return nil, fmt.Errorf("%s", block) } write := func(path string, content []byte, mode os.FileMode) (bool, error) { - if cur, err := rc.Runner.ReadFile(path); err == nil && bytes.Equal(cur, content) { + if exporterGuardFileCurrent(rc, path, content) { return false, nil } if err := rc.Runner.MkdirAll(filepath.Dir(path), 0o755); err != nil { return false, err } + // A fresh file gets root ownership and mode; writing through an + // existing one would keep its owner, mode or symlink. + if _, err := os.Lstat(path); err == nil { + if err := rc.Runner.Remove(path); err != nil { + return false, err + } + } return true, rc.Runner.WriteFile(path, content, mode) } helperChanged, err := write(exporterGuardHelperPath, exporterGuardHelper, 0o750) @@ -116,6 +150,13 @@ func applyExporterGuard(ctx context.Context, rc *RunContext) (*ApplyResult, erro return out, nil } } + if !rc.Runner.DryRun { + for _, p := range exporterGuardSetupFiles { + if err := rc.Runner.Backup.Preserve(p); err != nil { + return nil, fmt.Errorf("backing up %s before exporter firewall setup: %w", p, err) + } + } + } // setup rebuilds the owned chains behind a DROP guard, so the exporters // stay closed while it runs. if _, err := rc.Runner.Run(ctx, exporterGuardHelperPath, "setup"); err != nil { @@ -123,11 +164,11 @@ func applyExporterGuard(ctx context.Context, rc *RunContext) (*ApplyResult, erro } out.Changed = true out.Messages = append(out.Messages, "exporter firewall guard installed and applied") - // An exporter refused to start while the rules were missing (its + // An enabled exporter refused to start while the rules were missing (its // ExecStartPre verify failed) starts now that they are back. cfg := rc.Config.Modules.Monitoring for unit, on := range map[string]bool{nodeExporterUnit: cfg.NodeExporter, dcgmExporterUnit: cfg.DCGMExporter} { - if on && !serviceActive(ctx, rc, unit) { + if on && serviceEnabled(ctx, rc, unit) && !serviceActive(ctx, rc, unit) { // Refused starts may have hit the unit's start limit. _, _ = rc.Runner.Run(ctx, "systemctl", "reset-failed", unit) if _, err := rc.Runner.Run(ctx, "systemctl", "start", unit); err != nil { diff --git a/internal/module/monitoring_guard_test.go b/internal/module/monitoring_guard_test.go index 9b56fd0..9d22f54 100644 --- a/internal/module/monitoring_guard_test.go +++ b/internal/module/monitoring_guard_test.go @@ -8,6 +8,7 @@ import ( "testing" "github.com/STAIxBWLB/rootfiles/internal/config" + "github.com/STAIxBWLB/rootfiles/internal/exec" ) // fakeGuardHost points the guard at a temp tree and swaps the embedded @@ -18,12 +19,14 @@ func fakeGuardHost(t *testing.T, active bool) (string, string) { t.Helper() dir := t.TempDir() calls := filepath.Join(dir, "calls.log") - oldHelper, oldConf, oldBytes := exporterGuardHelperPath, exporterGuardConfPath, exporterGuardHelper + oldHelper, oldConf, oldBytes, oldUID, oldFiles := exporterGuardHelperPath, exporterGuardConfPath, exporterGuardHelper, exporterGuardRootUID, exporterGuardSetupFiles t.Cleanup(func() { - exporterGuardHelperPath, exporterGuardConfPath, exporterGuardHelper = oldHelper, oldConf, oldBytes + exporterGuardHelperPath, exporterGuardConfPath, exporterGuardHelper, exporterGuardRootUID, exporterGuardSetupFiles = oldHelper, oldConf, oldBytes, oldUID, oldFiles }) exporterGuardHelperPath = filepath.Join(dir, "sbin", "rootfiles-exporter-firewall") exporterGuardConfPath = filepath.Join(dir, "etc", "exporter-firewall.conf") + exporterGuardRootUID = uint32(os.Getuid()) + exporterGuardSetupFiles = []string{filepath.Join(dir, "units", "guard.service"), filepath.Join(dir, "units", "docker.conf")} exporterGuardHelper = []byte("#!/bin/sh\necho \"helper $*\" >> " + calls + "\ncase $1 in\ncheck) test -f " + dir + "/rules-ok || { echo 'ERROR: required owner-marked firewall rules are missing or drifted' >&2; exit 1; } ;;\nsetup) touch " + dir + "/rules-ok ;;\nesac\n") state := "inactive" if active { @@ -32,7 +35,7 @@ func fakeGuardHost(t *testing.T, active bool) (string, string) { fakeMonitoringCommand(t, "iptables", "#!/bin/sh\nexit 0\n") fakeMonitoringCommand(t, "iptables-save", "#!/bin/sh\nexit 0\n") fakeMonitoringCommand(t, "python3", "#!/bin/sh\nexit 0\n") - fakeMonitoringCommand(t, "systemctl", "#!/bin/sh\necho \"systemctl $*\" >> "+calls+"\ncase $1 in is-active) echo "+state+";; esac\n") + fakeMonitoringCommand(t, "systemctl", "#!/bin/sh\necho \"systemctl $*\" >> "+calls+"\ncase $1 in is-active) echo "+state+";; is-enabled) echo enabled;; esac\n") return dir, calls } @@ -259,3 +262,77 @@ func TestDockerDependentUnitsStartWithDocker(t *testing.T) { } } } + +// A helper with the right bytes but unsafe metadata is not current: systemd +// runs it as root. +func TestExporterGuardReplacesUnsafeHelper(t *testing.T) { + dir, _ := fakeGuardHost(t, true) + rc := guardRC(t, false) + ctx := context.Background() + if _, err := applyExporterGuard(ctx, rc); err != nil { + t.Fatal(err) + } + t.Run("group writable", func(t *testing.T) { + if err := os.Chmod(exporterGuardHelperPath, 0o770); err != nil { + t.Fatal(err) + } + if changes := checkExporterGuard(ctx, rc); len(changes) != 1 { + t.Fatalf("want the helper reported, got %+v", changes) + } + if _, err := applyExporterGuard(ctx, rc); err != nil { + t.Fatal(err) + } + if fi, _ := os.Stat(exporterGuardHelperPath); fi.Mode().Perm() != 0o750 { + t.Fatalf("mode not repaired: %v", fi.Mode()) + } + }) + t.Run("symlink", func(t *testing.T) { + target := filepath.Join(dir, "elsewhere") + if err := os.WriteFile(target, exporterGuardHelper, 0o750); err != nil { + t.Fatal(err) + } + if err := os.Remove(exporterGuardHelperPath); err != nil { + t.Fatal(err) + } + if err := os.Symlink(target, exporterGuardHelperPath); err != nil { + t.Fatal(err) + } + if changes := checkExporterGuard(ctx, rc); len(changes) != 1 { + t.Fatalf("want the symlinked helper reported, got %+v", changes) + } + if _, err := applyExporterGuard(ctx, rc); err != nil { + t.Fatal(err) + } + if fi, err := os.Lstat(exporterGuardHelperPath); err != nil || !fi.Mode().IsRegular() { + t.Fatalf("symlink not replaced: %v %v", fi, err) + } + if changes := checkExporterGuard(ctx, rc); len(changes) != 0 { + t.Fatalf("check after repair: %+v", changes) + } + }) +} + +// The units setup writes are in the apply backup, so rollback removes them +// along with the helper and config. +func TestExporterGuardSetupFilesAreBackedUp(t *testing.T) { + _, _ = fakeGuardHost(t, true) + rc := guardRC(t, false) + root := t.TempDir() + rc.Runner.Backup = exec.NewBackup(root, "test") + if _, err := applyExporterGuard(context.Background(), rc); err != nil { + t.Fatal(err) + } + m, err := exec.LoadBackup(root, rc.Runner.Backup.ID) + if err != nil { + t.Fatal(err) + } + created := map[string]bool{} + for _, e := range m.Entries { + created[e.Path] = !e.Existed + } + for _, p := range append([]string{exporterGuardHelperPath, exporterGuardConfPath}, exporterGuardSetupFiles...) { + if !created[p] { + t.Errorf("%s missing from the backup as a created file: %+v", p, m.Entries) + } + } +}