amadan
scanner-host: clamd may use /var/tmp under AppArmor, and upgrades keep clamd.conf
Debian's enforced AppArmor profile for clamd allowed temp files under /tmp only, so with TemporaryDirectory /var/tmp every INSTREAM would have errored and the self-test failed. A local include now allows /var/tmp, loaded before clamd first starts. StreamMaxLength is written in M (stream_max_bytes must now be a whole MiB): the package's postinst reads the number as megabytes on every debconf configure, so 26214400 would have become 26214400M. debconf is also frozen off after install, so upgrades leave clamd.conf alone; the clamd drop-in repeats the two ExecStartPre lines of Debian's extend.conf, which the postinst moves aside once debconf is off. The first freshclam gets three tries a minute apart. fail() removes policy-rc.d. The self-test runs with a private /var/tmp and wants the socket, so restarting it by hand reopens the port on a pass. The socket binds with FreeBind. Plan-time tests pin the unit files.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
6 files changed,
+120
−6
infra/modules/scanner-host/files/scanner-selftest.service+7 −0infra/modules/scanner-host/files/scanner-tcp.socket+2 −0infra/modules/scanner-host/main.tf+1 −1infra/modules/scanner-host/scanner-init.sh.tftpl+41 −3infra/modules/scanner-host/tests/scanner_host.tftest.hcl+65 −2infra/modules/scanner-host/variables.tf+4 −0
diff --git a/infra/modules/scanner-host/files/scanner-selftest.service b/infra/modules/scanner-host/files/scanner-selftest.service| index d1bd5285e..b4dc8e677 100644 |
| --- a/infra/modules/scanner-host/files/scanner-selftest.service |
| +++ b/infra/modules/scanner-host/files/scanner-selftest.service |
| @@ -5,12 +5,19 @@ |
| Description=Prove this clamd never answers OK for an over-limit archive |
| BindsTo=clamav-daemon.service |
| After=clamav-daemon.service |
| +# Restarting this unit by hand after a failure reopens the port on a pass. |
| +# Wants= adds no ordering; the socket orders itself after this unit. |
| +Wants=scanner-tcp.socket |
| [Service] |
| Type=oneshot |
| RemainAfterExit=yes |
| ExecStart=/usr/local/sbin/scanner-selftest |
| TimeoutStartSec=1800 |
| +# A killed test leaves its fixtures (about 1.2 GB) behind in /var/tmp. A |
| +# private /var/tmp goes with the unit. clamd never reads the fixture paths: |
| +# every one is streamed. |
| +PrivateTmp=yes |
| [Install] |
| WantedBy=multi-user.target |
diff --git a/infra/modules/scanner-host/files/scanner-tcp.socket b/infra/modules/scanner-host/files/scanner-tcp.socket| index b2233878a..ace642994 100644 |
| --- a/infra/modules/scanner-host/files/scanner-tcp.socket |
| +++ b/infra/modules/scanner-host/files/scanner-tcp.socket |
| @@ -19,6 +19,8 @@ Before=shutdown.target |
| [Socket] |
| ListenStream=@PRIVATE_IP@:3310 |
| +# Binds even if the private address is not up yet at boot. |
| +FreeBind=yes |
| [Install] |
| WantedBy=multi-user.target |
diff --git a/infra/modules/scanner-host/main.tf b/infra/modules/scanner-host/main.tf| index 4ef391bd8..0fa89e1b3 100644 |
| --- a/infra/modules/scanner-host/main.tf |
| +++ b/infra/modules/scanner-host/main.tf |
| @@ -25,7 +25,7 @@ locals { |
| init_script = templatefile("${path.module}/scanner-init.sh.tftpl", { |
| region = var.region |
| - stream_max_bytes = var.stream_max_bytes |
| + stream_max_mib = var.stream_max_bytes / 1048576 |
| scan_limit = local.scan_limit |
| memory_max = var.memory_max |
| memory_high = var.memory_high |
diff --git a/infra/modules/scanner-host/scanner-init.sh.tftpl b/infra/modules/scanner-host/scanner-init.sh.tftpl| index 16f8406b7..4430ffda4 100644 |
| --- a/infra/modules/scanner-host/scanner-init.sh.tftpl |
| +++ b/infra/modules/scanner-host/scanner-init.sh.tftpl |
| @@ -13,7 +13,9 @@ |
| set -euo pipefail |
| say() { echo "scanner-init: $*" | tee /dev/console; } |
| -fail() { say "SCANNER-HOST FAILED: $*"; exit 1; } |
| +# A failure in step 2 could leave policy-rc.d behind, and then nobody fixing |
| +# the box over SSM could start a service. Remove it on every way out. |
| +fail() { rm -f /usr/sbin/policy-rc.d; say "SCANNER-HOST FAILED: $*"; exit 1; } |
| trap 'fail "line $LINENO"' ERR |
| export DEBIAN_FRONTEND=noninteractive |
| @@ -41,13 +43,21 @@ rm -f /usr/sbin/policy-rc.d |
| # ---- 3. config, self-test, gate -------------------------------------------- |
| say "3/6 config and gate" |
| conf=/etc/clamav/clamd.conf |
| +[ -f "$conf" ] || fail "the package wrote no $conf" |
| +# With debconf on (the package default), the postinst regenerates clamd.conf |
| +# on every configure, unattended upgrades included. The file it generated at |
| +# install is the base; from here on it is ours. With debconf off, the postinst |
| +# leaves an existing clamd.conf alone. |
| +echo 'clamav-daemon clamav-daemon/debconf boolean false' | debconf-set-selections |
| # clamd has no include directory: replace each key in place. The ERR trap is |
| # not inherited by functions, so a failure in here reports itself. |
| setkey() { |
| { sed -i "/^$1[[:space:]]/d" "$conf" && echo "$1 $2" >> "$conf"; } \ |
| || fail "could not set $1 in $conf" |
| } |
| -setkey StreamMaxLength ${stream_max_bytes} |
| +# In M, never bytes: if the postinst ever does regenerate this file, it reads |
| +# the number as megabytes and appends M (26214400 would become 25 TiB). |
| +setkey StreamMaxLength ${stream_max_mib}M |
| setkey AlertExceedsMax yes |
| setkey MaxFileSize ${scan_limit} |
| setkey MaxScanSize ${scan_limit} |
| @@ -58,6 +68,22 @@ setkey TemporaryDirectory /var/tmp |
| # clamd itself never listens on TCP. The gated proxy does. |
| sed -i '/^TCPSocket[[:space:]]/d;/^TCPAddr[[:space:]]/d' "$conf" |
| +# Debian's AppArmor profile for clamd is enforced and allows temp files under |
| +# /tmp only, and clamd writes every INSTREAM to a temp file. Without this, |
| +# every scan errors and the self-test fails. The profile includes this local |
| +# file. It is loaded here, before clamd first starts. |
| +if [ -e /etc/apparmor.d/usr.sbin.clamd ]; then |
| + mkdir -p /etc/apparmor.d/local |
| + cat > /etc/apparmor.d/local/usr.sbin.clamd <<'EOF' |
| +# scanner-host: clamd's TemporaryDirectory is /var/tmp. |
| +/var/tmp/ rw, |
| +/var/tmp/** krw, |
| +EOF |
| + if command -v apparmor_parser >/dev/null && [ -d /sys/kernel/security/apparmor ]; then |
| + apparmor_parser -r /etc/apparmor.d/usr.sbin.clamd |
| + fi |
| +fi |
| + |
| echo '${selftest_b64}' | base64 -d > /usr/local/sbin/scanner-selftest |
| chmod 0755 /usr/local/sbin/scanner-selftest |
| echo '${selftest_unit_b64}' | base64 -d > /etc/systemd/system/scanner-selftest.service |
| @@ -87,6 +113,11 @@ Wants=scanner-tcp.socket |
| MemoryMax=${memory_max} |
| MemoryHigh=${memory_high} |
| Restart=on-failure |
| + |
| +# The same two lines as Debian's extend.conf drop-in, which the postinst moves |
| +# aside on the first upgrade now that debconf is off. |
| +ExecStartPre=-/bin/mkdir -p /run/clamav |
| +ExecStartPre=/bin/chown clamav /run/clamav |
| EOF |
| systemctl daemon-reload |
| @@ -94,7 +125,14 @@ systemctl daemon-reload |
| say "4/6 first signatures" |
| # NotifyClamd would complain that no clamd is running yet. It is not. |
| sed '/^NotifyClamd/d' /etc/clamav/freshclam.conf > /run/freshclam-first.conf |
| -freshclam --config-file=/run/freshclam-first.conf |
| +# ClamAV's mirror often rate-limits a new address. Three tries, a minute apart. |
| +tries=1 |
| +until freshclam --config-file=/run/freshclam-first.conf; do |
| + [ $tries -lt 3 ] || fail "first freshclam failed 3 times, 60s apart (rate-limited by ClamAV's mirror?)" |
| + say "freshclam try $tries failed, retrying in 60s" |
| + tries=$((tries + 1)) |
| + sleep 60 |
| +done |
| rm -f /run/freshclam-first.conf |
| for db in main daily bytecode; do |
| ls /var/lib/clamav/$db.c[vl]d >/dev/null 2>&1 || fail "no $db signature database after freshclam" |
diff --git a/infra/modules/scanner-host/tests/scanner_host.tftest.hcl b/infra/modules/scanner-host/tests/scanner_host.tftest.hcl| index 9941565fd..81d9c6a5c 100644 |
| --- a/infra/modules/scanner-host/tests/scanner_host.tftest.hcl |
| +++ b/infra/modules/scanner-host/tests/scanner_host.tftest.hcl |
| @@ -89,8 +89,25 @@ run "the_engine_config" { |
| error_message = "limits must alert" |
| } |
| assert { |
| - condition = strcontains(local.init_script, "setkey StreamMaxLength 26214400") |
| - error_message = "StreamMaxLength is stream_max_bytes" |
| + condition = strcontains(local.init_script, "setkey StreamMaxLength 25M") |
| + error_message = "StreamMaxLength is stream_max_bytes, written in M so a debconf rewrite cannot turn it into 25 TiB" |
| + } |
| + assert { |
| + condition = strcontains(local.init_script, "echo 'clamav-daemon clamav-daemon/debconf boolean false' | debconf-set-selections") |
| + error_message = "package upgrades never regenerate clamd.conf" |
| + } |
| + assert { |
| + condition = alltrue([ |
| + strcontains(local.init_script, "cat > /etc/apparmor.d/local/usr.sbin.clamd <<'EOF'"), |
| + strcontains(local.init_script, "\n/var/tmp/ rw,\n/var/tmp/** krw,\n"), |
| + strcontains(local.init_script, "apparmor_parser -r /etc/apparmor.d/usr.sbin.clamd"), |
| + strcontains(local.init_script, "setkey TemporaryDirectory /var/tmp"), |
| + ]) |
| + error_message = "clamd's AppArmor profile allows its TemporaryDirectory, /var/tmp, or every scan errors" |
| + } |
| + assert { |
| + condition = strcontains(local.init_script, "sed -i '/^TCPSocket[[:space:]]/d;/^TCPAddr[[:space:]]/d' \"$conf\"") |
| + error_message = "any TCPSocket or TCPAddr line the package wrote is deleted" |
| } |
| assert { |
| condition = !strcontains(local.init_script, "setkey TCPSocket") && !strcontains(local.init_script, "setkey TCPAddr") |
| @@ -138,6 +155,52 @@ run "stream_limit_above_the_scan_budget" { |
| expect_failures = [var.stream_max_bytes] |
| } |
| +run "stream_limit_not_a_whole_mib" { |
| + command = plan |
| + variables { |
| + stream_max_bytes = 26214401 |
| + } |
| + expect_failures = [var.stream_max_bytes] |
| +} |
| + |
| +run "the_units" { |
| + command = plan |
| + assert { |
| + condition = alltrue([ |
| + for line in [ |
| + "DefaultDependencies=no", |
| + "Requires=scanner-selftest.service", |
| + "BindsTo=scanner-selftest.service", |
| + "After=scanner-selftest.service", |
| + "ListenStream=@PRIVATE_IP@:3310", |
| + "FreeBind=yes", |
| + ] : strcontains(file("${path.module}/files/scanner-tcp.socket"), "\n${line}\n") |
| + ]) |
| + error_message = "the socket opts out of default dependencies, is bound to the self-test and listens on the private address only" |
| + } |
| + assert { |
| + condition = alltrue([ |
| + for line in [ |
| + "Requires=scanner-selftest.service", |
| + "BindsTo=scanner-selftest.service", |
| + "After=scanner-selftest.service", |
| + ] : strcontains(file("${path.module}/files/scanner-tcp.service"), "\n${line}\n") |
| + ]) |
| + error_message = "the proxy is bound to the self-test, so connections in flight are cut with it" |
| + } |
| + assert { |
| + condition = alltrue([ |
| + for line in [ |
| + "BindsTo=clamav-daemon.service", |
| + "Type=oneshot", |
| + "RemainAfterExit=yes", |
| + "PrivateTmp=yes", |
| + ] : strcontains(file("${path.module}/files/scanner-selftest.service"), "\n${line}\n") |
| + ]) |
| + error_message = "the self-test follows clamd, stays active after a pass, and keeps its fixtures in a private /var/tmp" |
| + } |
| +} |
| + |
| run "another_vpc_needs_a_subnet" { |
| command = plan |
| override_data { |
diff --git a/infra/modules/scanner-host/variables.tf b/infra/modules/scanner-host/variables.tf| index 3b890f635..3624478cf 100644 |
| --- a/infra/modules/scanner-host/variables.tf |
| +++ b/infra/modules/scanner-host/variables.tf |
| @@ -32,6 +32,10 @@ variable "stream_max_bytes" { |
| condition = var.stream_max_bytes >= 8388608 && var.stream_max_bytes <= 104857600 |
| error_message = "stream_max_bytes must be between 8388608 (8 MiB) and 104857600 (100 MiB)." |
| } |
| + validation { |
| + condition = var.stream_max_bytes % 1048576 == 0 |
| + error_message = "stream_max_bytes must be a whole number of MiB (a multiple of 1048576), because clamd.conf holds it in M." |
| + } |
| } |
| variable "memory_max" { |