diff --git a/man/sysupdate.d.xml b/man/sysupdate.d.xml index ea1c297c08..29daeda81a 100644 --- a/man/sysupdate.d.xml +++ b/man/sysupdate.d.xml @@ -316,11 +316,13 @@ url-tar resource types follow the usual file format generated by GNU's sha256sum1 tool. It is recommended to use 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 SHA256SUMS listing contains a special file - BEST-BEFORE-YYYY-MM-DD (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. + systems. The listing should only contain ASCII characters. Entries are usually plain file names, but may + also be relative subpaths (e.g., foo_1/bar.efi). Absolute paths, non-normalized paths + containing ./, ../, or // components, and + entries containing a % character are rejected. If the SHA256SUMS + listing contains a special file BEST-BEFORE-YYYY-MM-DD (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. @@ -446,6 +448,14 @@ whose name begins with foobar_, followed by a version ID and suffixed by .raw.xz. + A source MatchPattern= may additionally be prefixed with + **/. It can not contain **/ anywhere else nor further + subdirectories in the rest of the pattern. This prefix matches any number of nested subdirectories or + none, meaning only the basenames get matched against the rest of the pattern. The **/ + prefix is not a wildcard like those in the table above because it captures nothing and can't be used in + a target MatchPattern=. It's also not supported for the source types + directory and subvolume. + Do not confuse the @ pattern matching wildcard prefix with the % specifier expansion prefix. The former encapsulate a variable part of a match pattern string, the latter are simple shortcuts that are expanded while the drop-in files are @@ -629,10 +639,22 @@ wildcard, so that a version identifier may be extracted from the filename. All other wildcards are optional. - If the source type is regular-file or directory, the - pattern may contain slash characters. In this case, it will match the file or directory in - corresponding subdirectory. For example MatchPattern=foo_@v/bar.efi will match - bar.efi in directory foo_1. + The pattern may contain slash characters. In this case, it will match the file or directory in + the corresponding subdirectory. For example MatchPattern=foo_@v/bar.efi will match + bar.efi in directory foo_1. For url-file + and url-tar sources, the corresponding SHA256SUMS manifest + may then list entries as subpaths such as foo_1/bar.efi; such entries are fetched + from the matching subpath of the source URL. Manifest entries that are absolute, non-normalized + (containing ./, ../, or // components), or + that contain a % character are rejected. + + A **/ prefix on a source MatchPattern= matches any number + of subdirectories and will as a result only match the basename against the rest of the pattern. For + url-file and url-tar sources, SHA256SUMS + entries may then list the file under any subpath and the subpath is still used to construct the actual + download URL. The pattern after **/ must not contain further slashes. + The prefix is not accepted on target patterns or for the source Type=directory and + Type=subvolume. diff --git a/src/sysupdate/sysupdate-pattern.c b/src/sysupdate/sysupdate-pattern.c index c859bd111c..ddc9adb881 100644 --- a/src/sysupdate/sysupdate-pattern.c +++ b/src/sysupdate/sysupdate-pattern.c @@ -5,6 +5,7 @@ #include "list.h" #include "log.h" #include "parse-util.h" +#include "path-util.h" #include "string-util.h" #include "strv.h" #include "sysupdate-instance.h" @@ -457,7 +458,16 @@ int pattern_match_many(char **patterns, const char *s, InstanceMetadata *ret) { int r; STRV_FOREACH(p, patterns) { - r = pattern_match(*p, s, &found); + const char *pat = *p, *input_path; + + /* A glob directory prefix on a pattern means to match the rest of the pattern against the + * last path component only (the basename, "find this file anywhere under the source tree") */ + if (pattern_skip_glob_directory_prefix(&pat)) + input_path = last_path_component(s); + else + input_path = s; + + r = pattern_match(pat, input_path, &found); if (r < 0) return r; if (r != PATTERN_MATCH_NO) { @@ -476,6 +486,18 @@ int pattern_match_many(char **patterns, const char *s, InstanceMetadata *ret) { return PATTERN_MATCH_NO; } +bool pattern_skip_glob_directory_prefix(const char **pattern) { + assert(pattern); + assert(*pattern); + + const char *e = startswith(*pattern, "**/"); + if (!e) + return false; + + *pattern = e; + return true; +} + int pattern_valid(const char *pattern) { int r; diff --git a/src/sysupdate/sysupdate-pattern.h b/src/sysupdate/sysupdate-pattern.h index fb59f0e174..8bc9f09e86 100644 --- a/src/sysupdate/sysupdate-pattern.h +++ b/src/sysupdate/sysupdate-pattern.h @@ -13,3 +13,4 @@ int pattern_match(const char *pattern, const char *s, InstanceMetadata *ret); int pattern_match_many(char **patterns, const char *s, InstanceMetadata *ret); int pattern_valid(const char *pattern); int pattern_format(const char *pattern, const InstanceMetadata *fields, char **ret); +bool pattern_skip_glob_directory_prefix(const char **pattern); diff --git a/src/sysupdate/sysupdate-resource.c b/src/sysupdate/sysupdate-resource.c index 4f46e27869..ab98f649eb 100644 --- a/src/sysupdate/sysupdate-resource.c +++ b/src/sysupdate/sysupdate-resource.c @@ -23,6 +23,7 @@ #include "hexdecoct.h" #include "import-util.h" #include "iovec-util.h" +#include "path-util.h" #include "pidref.h" #include "process-util.h" #include "sort-util.h" @@ -48,6 +49,18 @@ void resource_destroy(Resource *rr) { free(rr->instances); } +bool resource_has_glob_directory_pattern(Resource *rr) { + assert(rr); + + STRV_FOREACH(p, rr->patterns) { + const char *pat = *p; + + if (pattern_skip_glob_directory_prefix(&pat)) + return true; + } + return false; +} + static int resource_add_instance( Resource *rr, const char *path, @@ -126,7 +139,7 @@ static int resource_load_from_directory_recursive( continue; } - if (fstatat(dirfd(d), de->d_name, &st, AT_NO_AUTOMOUNT) < 0) { + if (fstatat(dirfd(d), de->d_name, &st, AT_NO_AUTOMOUNT|AT_SYMLINK_NOFOLLOW) < 0) { if (errno == ENOENT) /* Gone by now? */ continue; @@ -157,23 +170,40 @@ static int resource_load_from_directory_recursive( if (!rel_joined_for_matching) return log_oom(); - r = pattern_match_many(rr->patterns, rel_joined_for_matching, &extracted_fields); - if (r < 0) - return log_error_errno(r, "Failed to match pattern: %m"); - if (r == PATTERN_MATCH_RETRY) { + /* If any pattern has the glob directory prefix descend into subdirectories when looking for + * regular files */ + bool descend = S_ISDIR(st.st_mode) && S_ISREG(m) && resource_has_glob_directory_pattern(rr); + + if (!descend) { + r = pattern_match_many(rr->patterns, rel_joined_for_matching, &extracted_fields); + if (r < 0) + return log_error_errno(r, "Failed to match pattern: %m"); + if (r == PATTERN_MATCH_NO) + continue; + /* Here we descend when the match pattern itself uses subdirectories */ + if (r == PATTERN_MATCH_RETRY) { + if (!S_ISDIR(st.st_mode)) /* The pattern continues with '/' but this is no directory */ + continue; + descend = true; + } + } + + if (descend) { _cleanup_closedir_ DIR *subdir = NULL; - subdir = xopendirat(dirfd(d), rel_joined, 0); - if (!subdir) - continue; + subdir = xopendirat(dirfd(d), de->d_name, O_NOFOLLOW); + if (!subdir) { + if (errno == ENOENT) /* Gone by now? */ + continue; + + return log_error_errno(errno, "Failed to open directory %s/%s: %m", rr->path, rel_joined); + } r = resource_load_from_directory_recursive(rr, subdir, rel_joined, rel_joined_for_matching, m, is_partial, is_pending); if (r < 0) return r; - if (r == 0) - continue; - } else if (r == PATTERN_MATCH_NO) continue; + } if (de->d_type == DT_DIR && m != S_IFDIR) continue; @@ -557,17 +587,26 @@ static int resource_load_from_web( if (!fn) return log_oom(); - if (!filename_is_valid(fn)) + /* Accept either a plain filename or a relative subpath like "subdir/file" but reject + * absolute paths and any paths containing "." or ".." Reject "%" too as it is added to + * the URL verbatim and curl would percent-decode it, which could lead to ".." after the + * normalization check. */ + if (path_is_absolute(fn) || !path_is_normalized(fn) || strchr(fn, '%')) return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "Invalid filename specified at manifest line %zu, refusing.", line_nr); if (string_has_cc(fn, NULL)) return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "Filename contains control characters at manifest line %zu, refusing.", line_nr); + /* Magic files can't be in subdirectories */ + if (strchr(fn, '/') && startswith(last_path_component(fn), "BEST-BEFORE-")) + log_warning("Ignoring best-before marker '%s' in subdirectory at manifest line %zu.", fn, line_nr); + r = process_magic_file(fn, &h); if (r < 0) return r; if (r == 0) { - /* If this isn't a magic file, then do the pattern matching */ - + /* Pattern matching against the full manifest entry. A pattern with the glob + * directory prefix matches against the basename only which pattern_match_many + * handles. */ r = pattern_match_many(rr->patterns, fn, &extracted_fields); if (r < 0) return log_error_errno(r, "Failed to match pattern: %m"); @@ -670,6 +709,32 @@ int resource_load_instances(Resource *rr, bool verify, Hashmap **web_cache) { return r; typesafe_qsort(rr->instances, rr->n_instances, instance_cmp); + + /* There are various cases where the same version can be provided by different files either due to + * multiple match patterns defined in a naming scheme transition or due to a subdirectory containing + * the same file again when the glob directory prefix is used, or due to a wildcard like @s or @t + * being used and the files offer different values there while reporting the same version. Since + * this could lead to unexpected behavior we report this here to the user. */ + for (size_t i = 1; i < rr->n_instances; i++) { + Instance *a = rr->instances[i - 1], *b = rr->instances[i]; + + if (strverscmp_improved(a->metadata.version, b->metadata.version) != 0 || streq(a->path, b->path)) + continue; + + _cleanup_free_ char *suffix = NULL; + + if (!streq(a->metadata.version, b->metadata.version)) { + suffix = strjoin("/'", b->metadata.version, "'"); + if (!suffix) + return log_oom(); + } + + log_info("Multiple instances of version '%s'%s found at '%s' and '%s'.", + a->metadata.version, + strempty(suffix), + a->path, b->path); + } + return 0; } diff --git a/src/sysupdate/sysupdate-resource.h b/src/sysupdate/sysupdate-resource.h index 4e3fd54110..1c6d4cb0f7 100644 --- a/src/sysupdate/sysupdate-resource.h +++ b/src/sysupdate/sysupdate-resource.h @@ -89,6 +89,8 @@ typedef struct Resource { void resource_destroy(Resource *rr); +bool resource_has_glob_directory_pattern(Resource *rr); + int resource_load_instances(Resource *rr, bool verify, Hashmap **web_cache); Instance* resource_find_instance(Resource *rr, const char *version); diff --git a/src/sysupdate/sysupdate-transfer.c b/src/sysupdate/sysupdate-transfer.c index 2a29448836..30e879574f 100644 --- a/src/sysupdate/sysupdate-transfer.c +++ b/src/sysupdate/sysupdate-transfer.c @@ -25,6 +25,7 @@ #include "notify-recv.h" #include "parse-helpers.h" #include "parse-util.h" +#include "path-util.h" #include "percent-util.h" #include "pidref.h" #include "process-util.h" @@ -48,6 +49,11 @@ /* Default value for InstancesMax= for fs object targets */ #define DEFAULT_FILE_INSTANCES_MAX 3 +enum { + MATCH_PATTERN_SOURCE, + MATCH_PATTERN_TARGET, +}; + Transfer* transfer_free(Transfer *t) { if (!t) return NULL; @@ -290,6 +296,7 @@ static int config_parse_resource_pattern( char ***patterns = ASSERT_PTR(data); Transfer *t = ASSERT_PTR(userdata); + bool is_source = ltype == MATCH_PATTERN_SOURCE; int r; assert(rvalue); @@ -301,6 +308,7 @@ static int config_parse_resource_pattern( for (;;) { _cleanup_free_ char *word = NULL, *resolved = NULL; + const char *body; r = extract_first_word(&rvalue, &word, NULL, EXTRACT_CUNESCAPE|EXTRACT_UNESCAPE_RELAX); if (r < 0) { @@ -318,8 +326,23 @@ static int config_parse_resource_pattern( return 0; } - if (!pattern_valid(resolved)) - return log_syntax(unit, LOG_ERR, filename, line, SYNTHETIC_ERRNO(EINVAL), + /* The glob directory prefix on a source MatchPattern= means "match the rest against the + * basename" thus the remainder must be a valid filename (no slashes). + * Target patterns can not use it. */ + body = resolved; + if (pattern_skip_glob_directory_prefix(&body)) { + if (!is_source) + return log_syntax(unit, LOG_ERR, filename, line, SYNTHETIC_ERRNO(EINVAL), + "'**/' prefix is only supported in a source MatchPattern=, refusing: %s", resolved); + if (!filename_is_valid(body)) + return log_syntax(unit, LOG_ERR, filename, line, SYNTHETIC_ERRNO(EINVAL), + "The pattern after a '**/' prefix must be a valid filename, refusing: %s", resolved); + } + + /* The glob directory prefix is not allowed in the remainder and pattern_valid will catch this. */ + r = pattern_valid(body); + if (r <= 0) + return log_syntax(unit, LOG_ERR, filename, line, r < 0 ? r : SYNTHETIC_ERRNO(EINVAL), "MatchPattern= string is not valid, refusing: %s", resolved); r = strv_consume(patterns, TAKE_PTR(resolved)); @@ -488,33 +511,33 @@ int transfer_read_definition(Transfer *t, const char *path, const char **dirs, H assert(t); ConfigTableItem table[] = { - { "Transfer", "MinVersion", config_parse_min_version, 0, &t->min_version }, - { "Transfer", "ProtectVersion", config_parse_protect_version, 0, &t->protected_versions }, - { "Transfer", "Verify", config_parse_bool, 0, &t->verify }, - { "Transfer", "ChangeLog", config_parse_transfer_url_specifiers_many, 0, &t->changelog }, - { "Transfer", "AppStream", config_parse_transfer_url_specifiers_many, 0, &t->appstream }, - { "Transfer", "Features", config_parse_strv, 0, &t->features }, - { "Transfer", "RequisiteFeatures", config_parse_strv, 0, &t->requisite_features }, - { "Source", "Type", config_parse_resource_type, 0, &t->source.type }, - { "Source", "Path", config_parse_resource_path, 0, &t->source }, - { "Source", "PathRelativeTo", config_parse_resource_path_relto, 0, &t->source.path_relative_to }, - { "Source", "MatchPattern", config_parse_resource_pattern, 0, &t->source.patterns }, - { "Target", "Type", config_parse_resource_type, 0, &t->target.type }, - { "Target", "Path", config_parse_resource_path, 0, &t->target }, - { "Target", "PathRelativeTo", config_parse_resource_path_relto, 0, &t->target.path_relative_to }, - { "Target", "MatchPattern", config_parse_resource_pattern, 0, &t->target.patterns }, - { "Target", "MatchPartitionType", config_parse_resource_ptype, 0, &t->target }, - { "Target", "PartitionUUID", config_parse_partition_uuid, 0, t }, - { "Target", "PartitionFlags", config_parse_partition_flags, 0, t }, - { "Target", "PartitionNoAuto", config_parse_tristate, 0, &t->no_auto }, - { "Target", "PartitionGrowFileSystem", config_parse_tristate, 0, &t->growfs }, - { "Target", "ReadOnly", config_parse_tristate, 0, &t->read_only }, - { "Target", "Mode", config_parse_mode, 0, &t->mode }, - { "Target", "TriesLeft", config_parse_uint64, 0, &t->tries_left }, - { "Target", "TriesDone", config_parse_uint64, 0, &t->tries_done }, - { "Target", "InstancesMax", config_parse_instances_max, 0, &t->instances_max }, - { "Target", "RemoveTemporary", config_parse_bool, 0, &t->remove_temporary }, - { "Target", "CurrentSymlink", config_parse_current_symlink, 0, &t->current_symlink }, + { "Transfer", "MinVersion", config_parse_min_version, 0, &t->min_version }, + { "Transfer", "ProtectVersion", config_parse_protect_version, 0, &t->protected_versions }, + { "Transfer", "Verify", config_parse_bool, 0, &t->verify }, + { "Transfer", "ChangeLog", config_parse_transfer_url_specifiers_many, 0, &t->changelog }, + { "Transfer", "AppStream", config_parse_transfer_url_specifiers_many, 0, &t->appstream }, + { "Transfer", "Features", config_parse_strv, 0, &t->features }, + { "Transfer", "RequisiteFeatures", config_parse_strv, 0, &t->requisite_features }, + { "Source", "Type", config_parse_resource_type, 0, &t->source.type }, + { "Source", "Path", config_parse_resource_path, 0, &t->source }, + { "Source", "PathRelativeTo", config_parse_resource_path_relto, 0, &t->source.path_relative_to }, + { "Source", "MatchPattern", config_parse_resource_pattern, MATCH_PATTERN_SOURCE, &t->source.patterns }, + { "Target", "Type", config_parse_resource_type, 0, &t->target.type }, + { "Target", "Path", config_parse_resource_path, 0, &t->target }, + { "Target", "PathRelativeTo", config_parse_resource_path_relto, 0, &t->target.path_relative_to }, + { "Target", "MatchPattern", config_parse_resource_pattern, MATCH_PATTERN_TARGET, &t->target.patterns }, + { "Target", "MatchPartitionType", config_parse_resource_ptype, 0, &t->target }, + { "Target", "PartitionUUID", config_parse_partition_uuid, 0, t }, + { "Target", "PartitionFlags", config_parse_partition_flags, 0, t }, + { "Target", "PartitionNoAuto", config_parse_tristate, 0, &t->no_auto }, + { "Target", "PartitionGrowFileSystem", config_parse_tristate, 0, &t->growfs }, + { "Target", "ReadOnly", config_parse_tristate, 0, &t->read_only }, + { "Target", "Mode", config_parse_mode, 0, &t->mode }, + { "Target", "TriesLeft", config_parse_uint64, 0, &t->tries_left }, + { "Target", "TriesDone", config_parse_uint64, 0, &t->tries_done }, + { "Target", "InstancesMax", config_parse_instances_max, 0, &t->instances_max }, + { "Target", "RemoveTemporary", config_parse_bool, 0, &t->remove_temporary }, + { "Target", "CurrentSymlink", config_parse_current_symlink, 0, &t->current_symlink }, {} }; @@ -624,6 +647,11 @@ int transfer_read_definition(Transfer *t, const char *path, const char **dirs, H return log_syntax(NULL, LOG_ERR, path, 1, SYNTHETIC_ERRNO(EINVAL), "Source specification lacks MatchPattern=."); + if (IN_SET(t->source.type, RESOURCE_DIRECTORY, RESOURCE_SUBVOLUME) && + resource_has_glob_directory_pattern(&t->source)) + return log_syntax(NULL, LOG_ERR, path, 1, SYNTHETIC_ERRNO(EINVAL), + "MatchPattern= with '**/' prefix is not supported for source Type=directory and Type=subvolume, refusing."); + if (!t->target.path && !t->target.path_auto) return log_syntax(NULL, LOG_ERR, path, 1, SYNTHETIC_ERRNO(EINVAL), "Target specification lacks Path= field."); @@ -639,6 +667,19 @@ int transfer_read_definition(Transfer *t, const char *path, const char **dirs, H t->target.patterns = strv_copy(t->source.patterns); if (!t->target.patterns) return log_oom(); + + /* Strip any glob directory prefix when inheriting from source because it's only used for finding and + * not to replicate the same directory layout at the target, so we don't support it there. */ + STRV_FOREACH(p, t->target.patterns) { + const char *body = *p; + + if (pattern_skip_glob_directory_prefix(&body)) + memmove(*p, body, strlen(body) + 1); + } + + /* Stripping the glob directory prefix can turn distinct source patterns into duplicates, + * so re-uniq for parity with the source side. */ + strv_uniq(t->target.patterns); } if (t->current_symlink && !RESOURCE_IS_FILESYSTEM(t->target.type) && !path_is_absolute(t->current_symlink)) diff --git a/test/units/TEST-72-SYSUPDATE.sh b/test/units/TEST-72-SYSUPDATE.sh index 88d7143906..5c812420d0 100755 --- a/test/units/TEST-72-SYSUPDATE.sh +++ b/test/units/TEST-72-SYSUPDATE.sh @@ -1192,4 +1192,138 @@ EOF test_signature_verification +# Test '**/' as prefix in MatchPattern= for subpaths in SHA256SUMS +rm -rf "$CONFIGDIR" "$WORKDIR/blobs" "$WORKDIR/source/sub" +mkdir -p "$CONFIGDIR" "$WORKDIR/blobs" "$WORKDIR/source/sub" + +printf '1234567' >"$WORKDIR/source/sub/blob-v10.bin" +printf 'abcdefg' >"$WORKDIR/source/sub/blob-v11.bin" +(cd "$WORKDIR/source" && rm -f BEST-BEFORE-* && sha256sum sub/blob-*.bin >SHA256SUMS) + +# A regular-file source where '**/' descends into sub/ to match against the basename +cat >"$CONFIGDIR/01-basename-dir.transfer" <"$CONFIGDIR/01-basename-url.transfer" <"$CONFIGDIR/01-explicit-url.transfer" <"$CONFIGDIR/01-basename-url.transfer" <&2 + exit 1 +fi +grep "Invalid filename" "$WORKDIR/traversal.log" >/dev/null +mv "$WORKDIR/source/SHA256SUMS.bak" "$WORKDIR/source/SHA256SUMS" +rm "$CONFIGDIR/01-basename-url.transfer" + +# Rejection test for a manifest entry with a percent-encoded ".." which curl would +# decode when pulling via file://, escaping the source dir past path_is_normalized() +rm -rf "$WORKDIR/blobs" +mkdir -p "$WORKDIR/blobs" +cp "$WORKDIR/source/SHA256SUMS" "$WORKDIR/source/SHA256SUMS.bak" +sed -i 's,sub/,sub/nested/%2e%2e/,g' "$WORKDIR/source/SHA256SUMS" +cat >"$CONFIGDIR/01-basename-url.transfer" <&2 + exit 1 +fi +grep "Invalid filename" "$WORKDIR/percent.log" >/dev/null +mv "$WORKDIR/source/SHA256SUMS.bak" "$WORKDIR/source/SHA256SUMS" +rm "$CONFIGDIR/01-basename-url.transfer" + +# A MatchPattern= with two subdirectory levels must descend beyond the first level +rm -rf "$CONFIGDIR" "$WORKDIR/blobs" "$WORKDIR/source/depth-v3" +mkdir -p "$CONFIGDIR" "$WORKDIR/blobs" "$WORKDIR/source/depth-v3/contents" +printf 'version-three' >"$WORKDIR/source/depth-v3/contents/image.raw" +cat >"$CONFIGDIR/01-depth.transfer" <