From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from atuin.qyliss.net (localhost [IPv6:::1]) by atuin.qyliss.net (Postfix) with ESMTP id 831DD1C986; Sat, 29 Nov 2025 09:51:11 +0000 (UTC) Received: by atuin.qyliss.net (Postfix, from userid 993) id B0E931C870; Sat, 29 Nov 2025 09:51:08 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 4.0.1 (2024-03-26) on atuin.qyliss.net X-Spam-Level: X-Spam-Status: No, score=-0.1 required=3.0 tests=DKIM_SIGNED,DKIM_VALID, DKIM_VALID_AU,DMARC_PASS,FREEMAIL_FROM,RCVD_IN_DNSWL_NONE, SPF_HELO_NONE autolearn=unavailable autolearn_force=no version=4.0.1 Received: from mail-yx1-xb134.google.com (mail-yx1-xb134.google.com [IPv6:2607:f8b0:4864:20::b134]) by atuin.qyliss.net (Postfix) with ESMTPS id 709F31C86E for ; Sat, 29 Nov 2025 09:51:06 +0000 (UTC) Received: by mail-yx1-xb134.google.com with SMTP id 956f58d0204a3-63f996d4e1aso2573941d50.0 for ; Sat, 29 Nov 2025 01:51:05 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1764409864; x=1765014664; darn=spectrum-os.org; h=cc:to:in-reply-to:references:message-id:content-transfer-encoding :mime-version:subject:date:from:from:to:cc:subject:date:message-id :reply-to; bh=R7D6Jsa185O+RBw7unhCKPUzSi8ZDYcTIYo+pJJs+gQ=; b=G+7ItM5gAHR0WHjXW8ftQGgnOMaPDTLNDZgNCUly5br3LiPedZo2o7TsZBaQk5Gxsh EkKpvJVHTKihk+Vmf+RzK7AHCgaEDMMAao7U7VB2OXdbOOcBRlD3gdK9GOpgO8/9217K 4/C4rdiGp5zNVQxyOVJyn1Zvv/M+mvXiU8J4dIelWqEo5RrLXRb08Y5gX+oHGnOxk0Yt 31FS+YOclnrmepjdtYfl+cr2VhZ350+4j9qt7kbwWCQeYnNWTpKHsjuuukRUpTH6djLw LxH9UonzSPeoJfis0FuQw3A7pmD0SvKvHoPlLY38x9epbSNVbp3O+/SrWX5ybZkj7ded 9DIw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1764409864; x=1765014664; h=cc:to:in-reply-to:references:message-id:content-transfer-encoding :mime-version:subject:date:from:x-gm-gg:x-gm-message-state:from:to :cc:subject:date:message-id:reply-to; bh=R7D6Jsa185O+RBw7unhCKPUzSi8ZDYcTIYo+pJJs+gQ=; b=a7596mjLRZJztVzsdIxi1DHuay4VIwCsqA/8sTuebneelVOdN8upDs8iWxBfvDIyEH W5utptDaMHvBygvgqLTGbpO2V88KErbNZFz2zoAhvYEOQhnmYW1hb9zaKZirGsP6OB3X YZoAWXBewDnHZu+NaFyHQvF670Z/ACjpuUf+GA1u6F6xoSEVv6SmWwlm76Vcg6Wz6xUc nEcLwPuMxHtXuzOHzEq0StQ09NZoSzCfdcwirXvm6MXhnzEjTRpvMJuf4SWRqMz9PrrD BH/QiDFrgEq2Fted9lbQAmjOGu1vv3Wc+isYNCOZC0MTeVQ2WL2WmwLiBHFvsKFSysXS djcg== X-Gm-Message-State: AOJu0YwKefwnpCnALFhzf8rkj9VvfPdbwFxeZZcHAn2BrqJQ4LW3Qcwo xloUHgfNPFpL6X+X5W10GfYMc1fXnRS3oFJlSNrIuzqg1+TdPNJyyiGylL4m2Q== X-Gm-Gg: ASbGnctrr4xou5oXecs9mHItDPyrKt5b/7EEOKh4QGRwfkGqRJRZUEnKdWf3+pTEb6f 6s2yuJHjyofXaT6iuKFu7pdT9A6SgM1Svcnq+ktSFkRE+TJ52ZNDjDYg0JBhEjVCSFZ1Rd4zg18 GEt9kLv6Je1XPs7GaP+JDJrPJa5TizhWkrpSM0aX6n1Ilqqm1FP7Wjl8n8yfSSuXV/zRIVY0n4J LS6lJAjUGEtPLGde+aJ4SXOTqT7nYWJzV8ft1cn0tilxH9T2GY+DuRFlOBG6oec9Vlr6u69tkK6 oMdXB5g8sgtgyFYokU3IjSeyqH3QjFhVDaXR0bJx7UkV2jyjvK8LYdSmhYkrZv9H1sDpUdUrRhv x3T8OqAi7/VWXHadOY3RD+AjFIigCxCu9gM8mecQxB2k1zlxRMFaGh7Uo9Enz1K0SRp1EF864hj 4M+Zfd7V+6MI1i+oHh8bEFGLBEkjtwhTDIeA2xZMBSZ0aP7oxDg3tJj9V21XDoKXu7PA2Scd3NA 54FS9MrmJASVyy5Jl0KkriiyGr7Ng1xLgg= X-Google-Smtp-Source: AGHT+IHAG2a8w2u74oNy/GZbUB5/wgOIgbWzE1CN9VZ+7pPrzQOjPeBtUPDLVi+fII5anma1xJ1u4A== X-Received: by 2002:a05:690e:8d2:b0:641:f5bc:68de with SMTP id 956f58d0204a3-64302af1f02mr15830947d50.75.1764409864080; Sat, 29 Nov 2025 01:51:04 -0800 (PST) Received: from localhost.localdomain (h96-60-249-169.cncrtn.broadband.dynamic.tds.net. [96.60.249.169]) by smtp.gmail.com with UTF8SMTPSA id 956f58d0204a3-6433c481d55sm2375638d50.21.2025.11.29.01.51.01 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sat, 29 Nov 2025 01:51:02 -0800 (PST) From: Demi Marie Obenour Date: Sat, 29 Nov 2025 04:49:58 -0500 Subject: [PATCH v6 1/8] tools: Add directory checker for updates MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 7bit Message-Id: <20251129-updates-v6-1-9edb87a2e509@gmail.com> References: <20251129-updates-v6-0-9edb87a2e509@gmail.com> In-Reply-To: <20251129-updates-v6-0-9edb87a2e509@gmail.com> To: Spectrum OS Development X-Mailer: b4 0.14.3 X-Developer-Signature: v=1; a=ed25519-sha256; t=1764409797; l=7498; i=demiobenour@gmail.com; s=20250729; h=from:subject:message-id; bh=x/E3Op228TCj9s9gzDf91xHjH45IcAeNQVPtxwM8gRM=; b=InNFd3sh/4CB44IEtDTY0ouX1pxZyspDlu82UeLf4sT9OsACY3SaNl1++JFl3dnCkAiJstHWi jjLjseLUTBED0iw+Cv+yO8QvUOkffyDLRTlGiWKYXDAVhj1AMNs+fv8 X-Developer-Key: i=demiobenour@gmail.com; a=ed25519; pk=X57Q4/YQDj9t4SBeKaDwvXYKB6quZJVx/DE2Ly2out0= Message-ID-Hash: OUWFAUEQGI5ZHKNTZ2KJTMKBNI7IDIHF X-Message-ID-Hash: OUWFAUEQGI5ZHKNTZ2KJTMKBNI7IDIHF X-MailFrom: demiobenour@gmail.com X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; emergency; loop; banned-address; member-moderation; header-match-devel.spectrum-os.org-0; header-match-devel.spectrum-os.org-1; header-match-devel.spectrum-os.org-2; header-match-devel.spectrum-os.org-3; header-match-devel.spectrum-os.org-4; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; digests; suspicious-header CC: Demi Marie Obenour , Alyssa Ross X-Mailman-Version: 3.3.9 Precedence: list List-Id: Patches and low-level development discussion Archived-At: List-Archive: List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: 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 Reviewed-by: Alyssa Ross --- 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. --- 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 +#include +#include +#include +#include +#include + +#include +#include +#include +#include + +#include + +[[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