[PATCH v4 01/13] tools: Add directory checker for updates

[Demi Marie Obenour <demiobenour@gmail.com>]

Spectrum OS's host has no network access.  Updates must be downloaded by
VMs.  The downloads are placed into a bind-mounted directory.  The VM
can write whatever it wants into that directory.  This includes symlinks
that subsequent code might open, which would create a path traversal
vulnerability.  It also includes paths with names containing containing
terminal escape sequences, newlines, or other nastiness.  Furthermore,
the directory should not have any subdirectories either.

Add a simple C program that checks for such ugliness and indicates
(via its exit code) if the VM misbehaved.  systemd-sysupdate can leave
behind temporary files with names starting with '.', so delete them
instead of failing.  Linux can lose cache coherency if there is an I/O
error, so call syncfs() on the directory before checking anything.  For
the same reason, fsync() the directory if any hidden files were deleted.

The directory checker also serves another critical function: it checks
if the VM actually downloaded anything.  Otherwise, network problems
could cause updates to silently do nothing.  Specifically, it checks
that the VM provided a file starting with the prefix "SHA256SUMS.".
These will be the last ones the in-VM updater downloads.  An additional
mode is provided to clean out all such files.  This will be used to
ensure that before the in-VM updater runs, no such files are present.
Hence, if the VM didn't actually download anything, the user will get a
clear error instead of a false success message or a confusing error.

Signed-off-by: Demi Marie Obenour <demiobenour@gmail.com>
Reviewed-by: Alyssa Ross <hi@alyssa.is>
---
Changes since v2:

- Purge leftover temporary files rather than returning an error.

- Split into two modes: one that deletes signature files, and one that
  checks that at least one signature file exists.  This allows checking
  that the VM actually sent something.

Signed-off-by: Demi Marie Obenour <demiobenour@gmail.com>
---
 tools/default.nix         |   1 +
 tools/meson.build         |   4 ++
 tools/updates-dir-check.c | 134 ++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 139 insertions(+)

diff --git a/tools/default.nix b/tools/default.nix
index 7cb7dc5b72b8394f5383c80ccf110fec55c44f21..da82f075fdba4655bd964ba35e819d669deff3f1 100644
--- a/tools/default.nix
+++ b/tools/default.nix
@@ -77,6 +77,7 @@ stdenv.mkDerivation (finalAttrs: {
       ./sd-notify-adapter.c
       ./start-vmm
       ./subprojects
+      ./updates-dir-check.c
     ] ++ lib.optionals driverSupport [
       ./xdp-forwarder
     ]));
diff --git a/tools/meson.build b/tools/meson.build
index bfa290e891fafa2d03eabb221121b5df4d83fb29..666483b3304224fce9110a2788456955a2d71305 100644
--- a/tools/meson.build
+++ b/tools/meson.build
@@ -33,6 +33,10 @@ if get_option('host')
     install: true)
 
   subdir('start-vmm')
+
+  executable('updates-dir-check', 'updates-dir-check.c',
+    c_args : '-D_GNU_SOURCE',
+    install: true)
 endif
 
 if get_option('build')
diff --git a/tools/updates-dir-check.c b/tools/updates-dir-check.c
new file mode 100644
index 0000000000000000000000000000000000000000..83af806bebf36754f8c794b04933bf6021338c38
--- /dev/null
+++ b/tools/updates-dir-check.c
@@ -0,0 +1,134 @@
+// SPDX-License-Identifier: EUPL-1.2+
+// SPDX-FileCopyrightText: 2025 Demi Marie Obenour <demiobenour@gmail.com>
+#include <assert.h>
+#include <errno.h>
+#include <stddef.h>
+#include <stdlib.h>
+#include <string.h>
+
+#include <fcntl.h>
+#include <sys/types.h>
+#include <dirent.h>
+#include <unistd.h>
+
+#include <err.h>
+
+[[noreturn]] static void bad_char(char c, char *msg_component)
+{
+	if (c >= 0x20 && c <= 0x7E)
+		errx(EXIT_FAILURE, "Forbidden %s character in filename: '%c'",
+		     msg_component, c);
+	errx(EXIT_FAILURE,
+	     "Forbidden %s character in filename: byte 0x%hhx",
+	     msg_component, c);
+}
+
+[[noreturn]] static void usage(void)
+{
+	errx(EXIT_FAILURE, "Usage: updates-dir-check [cleanup|check] DIRECTORIES...");
+}
+
+static void checkdir(int fd, bool check_sig)
+{
+	bool found_sig = false;
+	DIR *d = fdopendir(fd);
+	if (d == NULL)
+		err(EXIT_FAILURE, "fdopendir");
+	// If there is an I/O error while there are dirty pages outstanding,
+	// the dirty pages are silently discarded.  This means that the contents
+	// of the filesystem can change behind userspace's back.  Flush all
+	// dirty pages in the filesystem with the directory to prevent this.
+	if (syncfs(fd) != 0)
+		err(EXIT_FAILURE, "syncfs");
+	bool changed = false;
+	for (;;) {
+		errno = 0;
+		struct dirent *entry = readdir(d);
+		if (entry == NULL) {
+			if (errno)
+				err(EXIT_FAILURE, "readdir");
+			break;
+		}
+		const char *ptr = entry->d_name;
+		if (ptr[0] == '.') {
+			if (ptr[1] == '\0')
+				continue;
+			if (ptr[1] == '.' && ptr[2] == '\0')
+				continue;
+			// systemd-sysupdate uses these for temporary files.
+			// It normally cleans them up itself, but if there is an error
+			// it does not always clean them up.  I'm not sure if it is
+			// guaranteed to clean up temporary files from a past run, so
+			// delete them instead of returning an error.
+			if (unlinkat(fd, ptr, 0))
+				err(EXIT_FAILURE, "Failed to unlink temporary file");
+			changed = true;
+			continue;
+		}
+		char c = ptr[0];
+		if (!((c >= 'A' && c <= 'Z') ||
+		      (c >= 'a' && c <= 'z')))
+			bad_char(c, "initial");
+		while ((c = *++ptr)) {
+			if (!((c >= 'A' && c <= 'Z') ||
+			      (c >= 'a' && c <= 'z') ||
+			      (c >= '0' && c <= '9') ||
+			      (c == '_') ||
+			      (c == '-') ||
+			      (c == '.')))
+				bad_char(c, "subsequent");
+		}
+		// Empty filenames are rejected as having a bad initial character,
+		// and POSIX forbids them from being returned anyway.  Therefore,
+		// this cannot be out of bounds.
+		if (ptr[-1] == '.')
+			errx(EXIT_FAILURE, "Filename %s ends with a '.'", entry->d_name);
+		if (entry->d_type == DT_UNKNOWN)
+			errx(EXIT_FAILURE, "Filesystem didn't report type of file %s", entry->d_name);
+		if (entry->d_type != DT_REG)
+			errx(EXIT_FAILURE, "Entry contains non-regular file %s", entry->d_name);
+		if (strncmp(entry->d_name, "SHA256SUMS.", sizeof("SHA256SUMS.") - 1) == 0) {
+			// Found a signature file!
+			if (check_sig)
+				found_sig = true;
+			else {
+				if (unlinkat(fd, entry->d_name, 0))
+					err(EXIT_FAILURE, "Unlinking old signature file");
+				changed = true;
+			}
+		}
+	}
+	// If a change was made, enforcing cache coherency also requires
+	// another fsync() call.  This is again because Linux can discard
+	// changes if there is an I/O error.
+	if (changed && fsync(fd))
+		errx(EXIT_FAILURE, "fsync");
+	if (check_sig && !found_sig) {
+		warnx("sys.appvm-systemd-sysupdate didn't send a signature file.");
+		warnx("There was probably a problem downloading the update.");
+		errx(EXIT_FAILURE, "Check its logs for more information.");
+	}
+	closedir(d);
+}
+
+int main(int argc, char **argv)
+{
+	if (argc != 3)
+		usage();
+
+	bool check_sig;
+	if (strcmp(argv[1], "cleanup") == 0)
+		check_sig = false;
+	else if (strcmp(argv[1], "check") == 0)
+		check_sig = true;
+	else
+		usage();
+
+	for (int i = 2; i < argc; ++i) {
+		int fd = open(argv[i], O_DIRECTORY|O_RDONLY|O_CLOEXEC);
+		if (fd < 0)
+			err(EXIT_FAILURE, "open(%s)", argv[i]);
+		checkdir(fd, check_sig);
+	}
+	return 0;
+}

-- 
2.52.0