sysupdate: add simple "freshness" validation to systemd-sysupdate

In order to make "freeze" attacks against the update logic harder let's
add the ability to encode a "Best Before" date into SHA256SUMS directory
listings: if the current time is already beyond that time, we'll ignore
the SHA256SUMS as "stale" and fail the upgrade. Or in other words: the
freeze attack will now result in a client-side error eventually, instead
of success state.

The best before data is encoded in an optional pseudo-file listed in SHA256SUMS:
any file named BEST-BEFORE-YYYY-MM-DD.

docs/ENVIRONMENT.md

@@ -836,3 +836,10 @@ Tools using the Varlink protocol (such as `varlinkctl`) or sd-bus (such as
   overall number of threads used to load modules by `systemd-modules-load`.
   If unset, the default number of threads is equal to the number of online CPUs,
   with a maximum of 16. If set to `0`, multi-threaded loading is disabled.
+
+`systemd-sysupdate`:
+
+* `$SYSTEMD_SYSUPDATE_VERIFY_FRESHNESS` – takes a boolean. If false the
+  'freshness' check via `BEST-BEFORE-YYYY-MM-DD` files in `SHA256SUMS` manifest
+  files is disabled, and updating from outdated manifests will not result in an
+  error.

man/sysupdate.d.xml

@@ -311,6 +311,16 @@
         </tbody>
       </tgroup>
     </table>
+
+    <para>The <filename>SHA256SUMS</filename> manifest files used by <literal>url-file</literal> and
+    <literal>url-tar</literal> resource types follow the usual file format generated by GNU's <citerefentry
+    project='man-pages'><refentrytitle>sha256sum</refentrytitle><manvolnum>1</manvolnum></citerefentry>
+    tool. It is recommended to use <option>--binary</option> mode, even if that has no real effect on Linux
+    systems. The listing should only contain ASCII characters, and only regular file names (i.e. no absolute
+    or relative paths). If the <filename>SHA256SUMS</filename> listing contains a special file
+    <literal>BEST-BEFORE-YYYY-MM-DD</literal> (with the year, month, day filled in), then the file listing
+    will not be considered valid past the specified date, and the transfer will fail in such a case. This may
+    be used to detect "freshness" of the manifest file.</para>
   </refsect1>
 
   <refsect1>

src/sysupdate/sysupdate-resource.c

@@ -12,6 +12,7 @@
 #include "device-util.h"
 #include "devnum-util.h"
 #include "dirent-util.h"
+#include "env-util.h"
 #include "errno-util.h"
 #include "fd-util.h"
 #include "fdisk-util.h"
@@ -21,6 +22,7 @@
 #include "gpt.h"
 #include "hexdecoct.h"
 #include "import-util.h"
+#include "iovec-util.h"
 #include "pidref.h"
 #include "process-util.h"
 #include "sort-util.h"
@@ -351,6 +353,82 @@ static int download_manifest(
         return 0;
 }
 
+static int process_magic_file(
+                const char *fn,
+                const struct iovec *hash) {
+
+        int r;
+
+        assert(fn);
+        assert(iovec_is_set(hash));
+
+        /* Validates "BEST-BEFORE-*" magic files we find in SHA256SUMS manifests. For now we ignore the
+         * contents of such files (which might change one day), and only look at the file name.
+         *
+         * Note that if multiple BEST-BEFORE-* files exist in the same listing we'll honour them all, and
+         * fail whenever *any* of them indicate a date that's already in the past. */
+
+        const char *e = startswith(fn, "BEST-BEFORE-");
+        if (!e)
+                return 0;
+
+        /* SHA256 hash of an empty file */
+        static const uint8_t expected_hash[] = {
+                0xe3, 0xb0, 0xc4, 0x42, 0x98, 0xfc, 0x1c, 0x14, 0x9a, 0xfb, 0xf4, 0xc8, 0x99, 0x6f, 0xb9, 0x24,
+                0x27, 0xae, 0x41, 0xe4, 0x64, 0x9b, 0x93, 0x4c, 0xa4, 0x95, 0x99, 0x1b, 0x78, 0x52, 0xb8, 0x55,
+        };
+
+        /* Even if we ignore if people have non-empty files for this file, let's nonetheless warn about it,
+         * so that people fix it. After all we want to retain liberty to maybe one day place some useful data
+         * inside it */
+        if (iovec_memcmp(&IOVEC_MAKE(expected_hash, sizeof(expected_hash)), hash) != 0)
+                log_warning("Hash of best before marker file '%s' has unexpected value, ignoring.", fn);
+
+        struct tm parsed_tm = {};
+        const char *n = strptime(e, "%Y-%m-%d", &parsed_tm);
+        if (!n || *n != 0) {
+                /* Doesn't parse? Then it's not a best-before date */
+                log_warning("Found best before marker with an invalid date, ignoring: %s", fn);
+                return 0;
+        }
+
+        struct tm copy_tm = parsed_tm;
+        usec_t best_before;
+        r = mktime_or_timegm_usec(&copy_tm, /* utc= */ true, &best_before);
+        if (r < 0)
+                return log_error_errno(r, "Failed to convert best before time: %m");
+        if (copy_tm.tm_mday != parsed_tm.tm_mday ||
+            copy_tm.tm_mon != parsed_tm.tm_mon ||
+            copy_tm.tm_year != parsed_tm.tm_year) {
+                /* date was not normalized? (e.g. "30th of feb") */
+                log_warning("Found best before marker with a non-normalized data, ignoring: %s", fn);
+                return 0;
+        }
+
+        usec_t nw = now(CLOCK_REALTIME);
+        if (best_before < nw) {
+                /* We are past the best before date! Yikes! */
+
+                r = secure_getenv_bool("SYSTEMD_SYSUPDATE_VERIFY_FRESHNESS");
+                if (r < 0 && r != -ENXIO)
+                        log_debug_errno(r, "Failed to parse $SYSTEMD_SYSUPDATE_VERIFY_FRESHNESS, ignoring: %m");
+
+                if (r == 0) {
+                        log_warning("Best before marker indicates out-of-date file list, but told to ignore this, hence ignoring (%s < %s).",
+                                    FORMAT_TIMESTAMP(best_before), FORMAT_TIMESTAMP(nw));
+                        return 1; /* we processed this line, don't use for pattern matching */
+                }
+
+                return log_error_errno(
+                                SYNTHETIC_ERRNO(ESTALE),
+                                "Best before marker indicates out-of-date file list, refusing (%s < %s).",
+                                FORMAT_TIMESTAMP(best_before), FORMAT_TIMESTAMP(nw));
+        }
+
+        log_info("Found best before marker, and it checks out, proceeding.");
+        return 1; /* we processed this line, don't use for pattern matching */
+}
+
 static int resource_load_from_web(
                 Resource *rr,
                 bool verify,
@@ -391,11 +469,10 @@ static int resource_load_from_web(
 
         while (left > 0) {
                 _cleanup_(instance_metadata_destroy) InstanceMetadata extracted_fields = INSTANCE_METADATA_NULL;
+                _cleanup_(iovec_done) struct iovec h = {};
                 _cleanup_free_ char *fn = NULL;
-                _cleanup_free_ void *h = NULL;
                 Instance *instance;
                 const char *e;
-                size_t hlen;
 
                 /* 64 character hash + separator + filename + newline */
                 if (left < 67)
@@ -404,7 +481,7 @@ static int resource_load_from_web(
                 if (p[0] == '\\')
                         return log_error_errno(SYNTHETIC_ERRNO(EOPNOTSUPP), "File names with escapes not supported in manifest at line %zu, refusing.", line_nr);
 
-                r = unhexmem_full(p, 64, /* secure= */ false, &h, &hlen);
+                r = unhexmem_full(p, 64, /* secure= */ false, &h.iov_base, &h.iov_len);
                 if (r < 0)
                         return log_error_errno(r, "Failed to parse digest at manifest line %zu, refusing.", line_nr);
 
@@ -433,28 +510,35 @@ static int resource_load_from_web(
                 if (string_has_cc(fn, NULL))
                         return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "Filename contains control characters at manifest line %zu, refusing.", line_nr);
 
-                r = pattern_match_many(rr->patterns, fn, &extracted_fields);
+                r = process_magic_file(fn, &h);
                 if (r < 0)
-                        return log_error_errno(r, "Failed to match pattern: %m");
-                if (r == PATTERN_MATCH_YES) {
-                        _cleanup_free_ char *path = NULL;
+                        return r;
+                if (r == 0) {
+                        /* If this isn't a magic file, then do the pattern matching */
 
-                        r = import_url_append_component(rr->path, fn, &path);
+                        r = pattern_match_many(rr->patterns, fn, &extracted_fields);
                         if (r < 0)
-                                return log_error_errno(r, "Failed to build instance URL: %m");
+                                return log_error_errno(r, "Failed to match pattern: %m");
+                        if (r == PATTERN_MATCH_YES) {
+                                _cleanup_free_ char *path = NULL;
 
-                        r = resource_add_instance(rr, path, &extracted_fields, &instance);
-                        if (r < 0)
-                                return r;
+                                r = import_url_append_component(rr->path, fn, &path);
+                                if (r < 0)
+                                        return log_error_errno(r, "Failed to build instance URL: %m");
 
-                        assert(hlen == sizeof(instance->metadata.sha256sum));
+                                r = resource_add_instance(rr, path, &extracted_fields, &instance);
+                                if (r < 0)
+                                        return r;
 
-                        if (instance->metadata.sha256sum_set) {
-                                if (memcmp(instance->metadata.sha256sum, h, hlen) != 0)
-                                        return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "SHA256 sum parsed from filename and manifest don't match at line %zu, refusing.", line_nr);
-                        } else {
-                                memcpy(instance->metadata.sha256sum, h, hlen);
-                                instance->metadata.sha256sum_set = true;
+                                assert(h.iov_len == sizeof(instance->metadata.sha256sum));
+
+                                if (instance->metadata.sha256sum_set) {
+                                        if (memcmp(instance->metadata.sha256sum, h.iov_base, h.iov_len) != 0)
+                                                return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "SHA256 sum parsed from filename and manifest don't match at line %zu, refusing.", line_nr);
+                                } else {
+                                        memcpy(instance->metadata.sha256sum, h.iov_base, h.iov_len);
+                                        instance->metadata.sha256sum_set = true;
+                                }
                         }
                 }
 

test/units/TEST-72-SYSUPDATE.sh

@@ -54,7 +54,11 @@ at_exit() {
 trap at_exit EXIT
 
 update_checksums() {
-    (cd "$WORKDIR/source" && sha256sum uki* part* dir-*.tar.gz >SHA256SUMS)
+    (cd "$WORKDIR/source" && rm -f BEST-BEFORE-* && sha256sum uki* part* dir-*.tar.gz >SHA256SUMS)
+}
+
+update_checksums_with_best_before() {
+    (cd "$WORKDIR/source" && rm -f BEST-BEFORE-* && touch "BEST-BEFORE-$1" && sha256sum uki* part* dir-*.tar.gz "BEST-BEFORE-$1" >SHA256SUMS)
 }
 
 new_version() {
@@ -397,6 +401,21 @@ EOF
     verify_version "$blockdev" "$sector_size" v6 1
     verify_version_current "$blockdev" "$sector_size" v7 2
 
+    # Check with a best before in the past
+    update_checksums_with_best_before "$(date -u +'%Y-%m-%d' -d 'last month')"
+    (! "$SYSUPDATE" --verify=no update)
+
+    # Retry but force check off
+    SYSTEMD_SYSUPDATE_VERIFY_FRESHNESS=0 "$SYSUPDATE" --verify=no update
+
+    # Check with best before in the future
+    update_checksums_with_best_before "$(date -u +'%Y-%m-%d' -d 'next month')"
+    "$SYSUPDATE" --verify=no update
+
+    # Check again without a best before
+    update_checksums
+    "$SYSUPDATE" --verify=no update
+
     # Let's make sure that we don't break our backwards-compat for .conf files
     # (what .transfer files were called before v257)
     for i in "$CONFIGDIR/"*.conf; do echo mv "$i" "${i%.conf}.transfer"; done