diff --git a/man/os-release.xml b/man/os-release.xml index 94617ae243d..adac119c6d0 100644 --- a/man/os-release.xml +++ b/man/os-release.xml @@ -282,10 +282,14 @@ VERSION_ID= - A lower-case string (mostly numeric, no spaces or other characters outside of 0–9, - a–z, ".", "_" and "-") identifying the operating system version, excluding any OS name information - or release code name, and suitable for processing by scripts or usage in generated filenames. This - field is optional. + A string (lower-case recommended, no spaces or other characters outside of 0–9, + a–z, A-Z, ".", "-", "~", "^", "+" and "_") identifying the operating system version, excluding any + OS name information or release code name, and suitable for processing by scripts or usage in + generated filenames. This field is optional. + + It is recommended to follow the UAPI.10 Version + Format Specification for this version string, but this is generally not enforced. Examples: VERSION_ID=17, VERSION_ID=11.04. @@ -294,10 +298,11 @@ VERSION_CODENAME= - A lower-case string (no spaces or other characters outside of 0–9, a–z, ".", "_" - and "-") identifying the operating system release code name, excluding any OS name information or - release version, and suitable for processing by scripts or usage in generated filenames. This field - is optional and may not be implemented on all systems. + A string (lower-case recommended, no spaces or other characters outside of 0–9, + a–z, A-Z, ".", "-", "~", "^", "+", and "_") identifying the operating system release code name, + excluding any OS name information or release version, and suitable for processing by scripts or + usage in generated filenames. This field is optional and may not be implemented on all + systems. Examples: VERSION_CODENAME=buster, VERSION_CODENAME=xenial. @@ -341,10 +346,14 @@ IMAGE_VERSION= - A lower-case string (mostly numeric, no spaces or other characters outside of 0–9, - a–z, ".", "_" and "-") identifying the OS image version. This is supposed to be used together with - IMAGE_ID described above, to discern different versions of the same image. - + A string (lower-case recommended, no spaces or other characters outside of 0–9, + a–z, A-Z, ".", "-", "~", "^", "+" and "_") identifying the OS image version. This is supposed to be + used together with IMAGE_ID described above, to discern different versions of + the same image. + + It is recommended to follow the UAPI.10 Version + Format Specification for this version string, but this is generally not enforced. Examples: IMAGE_VERSION=33, IMAGE_VERSION=47.1rc1. diff --git a/src/analyze/analyze-compare-versions.c b/src/analyze/analyze-compare-versions.c index 5c15fd044d6..a3293d73b72 100644 --- a/src/analyze/analyze-compare-versions.c +++ b/src/analyze/analyze-compare-versions.c @@ -17,9 +17,9 @@ int verb_compare_versions(int argc, char *argv[], uintptr_t _data, void *userdat /* We only output a warning on invalid version strings (instead of failing), since the comparison * functions try to handle invalid strings gracefully and it's still interesting to see what the * comparison result will be. */ - if (!version_is_valid_versionspec(v1)) + if (!version_is_valid(v1, VERSION_ALLOW_UNDERSCORE|VERSION_ALLOW_PLUS|VERSION_ALLOW_EMPTY)) log_warning("Version string 1 contains disallowed characters, they will be treated as separators: %s", v1); - if (!version_is_valid_versionspec(v2)) + if (!version_is_valid(v2, VERSION_ALLOW_UNDERSCORE|VERSION_ALLOW_PLUS|VERSION_ALLOW_EMPTY)) log_warning("Version string 2 contains disallowed characters, they will be treated as separators: %s", v2); if (argc == 3) { diff --git a/src/basic/string-util.c b/src/basic/string-util.c index 047a58ade57..774a72fd695 100644 --- a/src/basic/string-util.c +++ b/src/basic/string-util.c @@ -1501,28 +1501,47 @@ char* find_line_after_internal(const char *haystack, const char *needle) { return NULL; } -bool version_is_valid(const char *s) { - if (isempty(s)) +bool version_is_valid(const char *s, VersionFlags flags) { + + /* Validates a version string superficially. This does not proces the version string in any + * semantical way, it mostly just validates that its charset is reasonable. */ + + if (FLAGS_SET(flags, VERSION_ALLOW_EMPTY) ? !s : isempty(s)) return false; if (!filename_part_is_valid(s)) return false; - /* This is a superset of the characters used by semver. We additionally allow "," and "_". */ - if (!in_charset(s, ALPHANUMERICAL ".,_-+")) - return false; + /* We always allow all characters specified by the UAPI.10 Version Specification, i.e. 0-9, a-z, A-Z, + * ".", "-", "~", "^". + * + * If the relevant flags are set we'll also allow "+" and "_" separators. + * + * Note that with SemVer allows 0-9, a-z, A-Z, "+", "-", ".", hence with VERSION_ALLOW_PLUS we + * implement a superset of it. + * + * If you wonder when to use which flags: when validating foreign versions (e.g. distribution + * versions in /etc/os-release or so) validate liberally, i.e. add + * VERSION_ALLOW_UNDERSCORE|VERSION_ALLOW_PLUS. When validating our own versioned objects (e.g. vpick + * or so) validate more strictly, and in particular refuse characters such as "_" and "+" that may be + * used for separating component names or boot attempt counters. Also: first – if appropriate – split + * the string into individual components. For example, if the string consists of a name and a + * version, separated by some character, only pass the version part to this function. The name part + * may pass verification, but it's cleaner to not rely on that. + * + * For details about UAPI.10 see: + * + * → https://uapi-group.org/specifications/specs/version_format_specification/ */ - return true; -} + char charset[] = ALPHANUMERICAL ".-~^" /* plus room for the two chars below: */ "\0\0"; + size_t l = strlen(charset); -bool version_is_valid_versionspec(const char *s) { - if (!filename_part_is_valid(s)) - return false; + if (FLAGS_SET(flags, VERSION_ALLOW_UNDERSCORE)) + charset[l++] = '_'; + if (FLAGS_SET(flags, VERSION_ALLOW_PLUS)) + charset[l++] = '+'; - if (!in_charset(s, ALPHANUMERICAL "-.~^")) - return false; - - return true; + return in_charset(s, charset); } ssize_t strlevenshtein(const char *x, const char *y) { diff --git a/src/basic/string-util.h b/src/basic/string-util.h index 8f4cc69a54b..d0b614f775a 100644 --- a/src/basic/string-util.h +++ b/src/basic/string-util.h @@ -315,8 +315,13 @@ char* find_line_after_internal(const char *haystack, const char *needle); #define find_line_after(haystack, needle) \ const_generic(haystack, find_line_after_internal(haystack, needle)) -bool version_is_valid(const char *s) _pure_; -bool version_is_valid_versionspec(const char *s) _pure_; +typedef enum VersionFlags { + VERSION_ALLOW_EMPTY = 1 << 0, + VERSION_ALLOW_UNDERSCORE = 1 << 1, /* Allow "_" as separator (recommended separator) */ + VERSION_ALLOW_PLUS = 1 << 2, /* Allow "+" as separator (sometimes used as separator for boot attempt counters) */ +} VersionFlags; + +bool version_is_valid(const char *s, VersionFlags flags) _pure_; ssize_t strlevenshtein(const char *x, const char *y); diff --git a/src/bootctl/bootctl-link.c b/src/bootctl/bootctl-link.c index b4706670857..ed1753af99e 100644 --- a/src/bootctl/bootctl-link.c +++ b/src/bootctl/bootctl-link.c @@ -1492,7 +1492,7 @@ static int vl_link_prepare(sd_varlink *link, LinkParameters *p) { if (p->context.entry_title && !efi_loader_entry_title_valid(p->context.entry_title)) return sd_varlink_error_invalid_parameter_name(link, "entryTitle"); - if (p->context.entry_version && !version_is_valid_versionspec(p->context.entry_version)) + if (p->context.entry_version && !version_is_valid(p->context.entry_version, /* flags= */ 0)) return sd_varlink_error_invalid_parameter_name(link, "entryVersion"); if (p->context.entry_commit != 0 && !entry_commit_valid(p->context.entry_commit)) diff --git a/src/bootctl/bootctl.c b/src/bootctl/bootctl.c index 72d96bc1b0e..238102095ed 100644 --- a/src/bootctl/bootctl.c +++ b/src/bootctl/bootctl.c @@ -695,7 +695,7 @@ static int parse_argv(int argc, char *argv[], char ***ret_args) { break; } - if (!version_is_valid_versionspec(opts.arg)) + if (!version_is_valid(opts.arg, /* flags= */ 0)) return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "Not a valid boot menu entry version: %s", opts.arg); r = free_and_strdup_warn(&arg_entry_version, opts.arg); diff --git a/src/libsystemd/sd-json/json-util.c b/src/libsystemd/sd-json/json-util.c index d69e37a3ce6..6cbb61cb5e3 100644 --- a/src/libsystemd/sd-json/json-util.c +++ b/src/libsystemd/sd-json/json-util.c @@ -449,7 +449,7 @@ int json_dispatch_const_version(const char *name, sd_json_variant *variant, sd_j return json_log(variant, flags, SYNTHETIC_ERRNO(EINVAL), "JSON field '%s' is not a string.", strna(name)); const char *version = sd_json_variant_string(variant); - if (!version_is_valid(version)) + if (!version_is_valid(version, FLAGS_SET(flags, SD_JSON_STRICT) ? 0 : VERSION_ALLOW_UNDERSCORE|VERSION_ALLOW_PLUS)) return json_log(variant, flags, SYNTHETIC_ERRNO(EINVAL), "JSON field '%s' is not a valid version string.", strna(name)); *n = version; diff --git a/src/shared/bootspec.c b/src/shared/bootspec.c index 77ee218996e..17a3b826d88 100644 --- a/src/shared/bootspec.c +++ b/src/shared/bootspec.c @@ -492,9 +492,12 @@ static int boot_entry_load_type1( r = free_and_strdup(&tmp.title, p); else if (streq(field, "sort-key")) r = free_and_strdup(&tmp.sort_key, p); - else if (streq(field, "version")) + else if (streq(field, "version")) { + if (!version_is_valid(p, VERSION_ALLOW_UNDERSCORE|VERSION_ALLOW_PLUS)) + log_syntax(NULL, LOG_WARNING, tmp.path, line, 0, "Version string '%s' is not a valid version, accepting anyway.", p); + r = free_and_strdup(&tmp.version, p); - else if (streq(field, "machine-id")) + } else if (streq(field, "machine-id")) r = free_and_strdup(&tmp.machine_id, p); else if (streq(field, "architecture")) r = free_and_strdup(&tmp.architecture, p); diff --git a/src/shared/vpick.c b/src/shared/vpick.c index c661f92fafc..4f709c9e19a 100644 --- a/src/shared/vpick.c +++ b/src/shared/vpick.c @@ -447,7 +447,7 @@ static int make_choice( *underscore = 0; } - if (!version_is_valid(e)) { + if (!version_is_valid(e, /* flags= */ 0)) { log_debug("Version string '%s' of entry '%s' is invalid, ignoring entry.", e, (*entry)->d_name); continue; } diff --git a/src/sysupdate/sysupdate-pattern.c b/src/sysupdate/sysupdate-pattern.c index 459c8fda29d..294499f4af8 100644 --- a/src/sysupdate/sysupdate-pattern.c +++ b/src/sysupdate/sysupdate-pattern.c @@ -268,7 +268,7 @@ int pattern_match(const char *pattern, const char *s, InstanceMetadata *ret) { switch (e->type) { case PATTERN_VERSION: - if (!version_is_valid(t)) { + if (!version_is_valid(t, VERSION_ALLOW_UNDERSCORE|VERSION_ALLOW_PLUS)) { log_debug("Version string is not valid, refusing: %s", t); goto nope; } diff --git a/src/sysupdate/sysupdate-transfer.c b/src/sysupdate/sysupdate-transfer.c index 30e879574f0..e80dcaf39a0 100644 --- a/src/sysupdate/sysupdate-transfer.c +++ b/src/sysupdate/sysupdate-transfer.c @@ -141,7 +141,7 @@ static int config_parse_protect_version( return 0; } - if (!version_is_valid(resolved)) { + if (!version_is_valid(resolved, VERSION_ALLOW_UNDERSCORE|VERSION_ALLOW_PLUS)) { log_syntax(unit, LOG_WARNING, filename, line, 0, "ProtectVersion= string is not valid, ignoring: %s", resolved); return 0; @@ -180,7 +180,7 @@ static int config_parse_min_version( return 0; } - if (!version_is_valid(rvalue)) { + if (!version_is_valid(resolved, VERSION_ALLOW_UNDERSCORE|VERSION_ALLOW_PLUS)) { log_syntax(unit, LOG_WARNING, filename, line, 0, "MinVersion= string is not valid, ignoring: %s", resolved); return 0; diff --git a/src/sysupdate/sysupdate.c b/src/sysupdate/sysupdate.c index ee74c2c31fb..62f14b645e2 100644 --- a/src/sysupdate/sysupdate.c +++ b/src/sysupdate/sysupdate.c @@ -553,7 +553,7 @@ static int context_discover_update_sets_by_flag(Context *c, UpdateSetFlags flags if (boundary && strverscmp_improved(i->metadata.version, boundary) >= 0) continue; /* Not older than the boundary */ - if (cursor && strverscmp(i->metadata.version, cursor) <= 0) + if (cursor && strverscmp_improved(i->metadata.version, cursor) <= 0) break; /* Not newer than the cursor. The same will be true for all * subsequent instances (due to sorting) so let's skip to the * next transfer. */ diff --git a/src/sysupdate/sysupdated.c b/src/sysupdate/sysupdated.c index d2a8297d820..13309e352e7 100644 --- a/src/sysupdate/sysupdated.c +++ b/src/sysupdate/sysupdated.c @@ -952,7 +952,7 @@ static int target_method_describe(sd_bus_message *msg, void *userdata, sd_bus_er if (r < 0) return r; - if (!version_is_valid(version)) + if (!version_is_valid(version, VERSION_ALLOW_UNDERSCORE|VERSION_ALLOW_PLUS)) return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, "Invalid version"); if ((flags & ~SD_SYSUPDATE_FLAGS_ALL) != 0) @@ -1102,7 +1102,7 @@ static int target_method_acquire(sd_bus_message *msg, void *userdata, sd_bus_err if (isempty(version)) action = "org.freedesktop.sysupdate1.update"; else { - if (!version_is_valid(version)) + if (!version_is_valid(version, VERSION_ALLOW_UNDERSCORE|VERSION_ALLOW_PLUS)) return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, "Invalid version"); action = "org.freedesktop.sysupdate1.update-to-version"; @@ -1190,7 +1190,7 @@ static int target_method_install(sd_bus_message *msg, void *userdata, sd_bus_err if (isempty(version)) action = "org.freedesktop.sysupdate1.update"; else { - if (!version_is_valid(version)) + if (!version_is_valid(version, VERSION_ALLOW_UNDERSCORE|VERSION_ALLOW_PLUS)) return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, "Invalid version"); action = "org.freedesktop.sysupdate1.update-to-version"; diff --git a/src/test/test-string-util.c b/src/test/test-string-util.c index 970421c7abb..343ddcf3cd7 100644 --- a/src/test/test-string-util.c +++ b/src/test/test-string-util.c @@ -1,5 +1,6 @@ /* SPDX-License-Identifier: LGPL-2.1-or-later */ +#include #include #include "alloc-util.h" @@ -8,6 +9,7 @@ #include "string-util.h" #include "strv.h" #include "tests.h" +#include "version.h" TEST(ellipsize_mem_ansi_short) { _cleanup_free_ char *a = ellipsize_mem("X\x1b[m", 4, 1, 50); @@ -1408,13 +1410,67 @@ TEST(strstrafter) { } TEST(version_is_valid) { - assert_se(!version_is_valid(NULL)); - assert_se(!version_is_valid("")); - assert_se(version_is_valid("0")); - assert_se(version_is_valid("5")); - assert_se(version_is_valid("999999")); - assert_se(version_is_valid("999999.5")); - assert_se(version_is_valid("6.2.12-300.fc38.x86_64")); + ASSERT_FALSE(version_is_valid(NULL, /* flags= */ 0)); + ASSERT_FALSE(version_is_valid("", /* flags= */ 0)); + ASSERT_TRUE(version_is_valid("0", /* flags= */ 0)); + ASSERT_TRUE(version_is_valid("5", /* flags= */ 0)); + ASSERT_TRUE(version_is_valid("999999", /* flags= */ 0)); + ASSERT_TRUE(version_is_valid("999999.5", /* flags= */ 0)); + ASSERT_TRUE(version_is_valid("6.2.12-300.fc38.x86_64", VERSION_ALLOW_UNDERSCORE)); + ASSERT_TRUE(version_is_valid("6.2.12-300.fc38.x86_64", VERSION_ALLOW_UNDERSCORE|VERSION_ALLOW_PLUS)); + ASSERT_FALSE(version_is_valid("6.2.12-300.fc38.x86_64", VERSION_ALLOW_PLUS)); + ASSERT_FALSE(version_is_valid("6.2.12-300.fc38.x86_64", /* flags= */ 0)); + + struct utsname u; + ASSERT_OK_ERRNO(uname(&u)); + ASSERT_TRUE(version_is_valid(u.release, VERSION_ALLOW_UNDERSCORE|VERSION_ALLOW_PLUS)); + + ASSERT_TRUE(version_is_valid(GIT_VERSION, VERSION_ALLOW_UNDERSCORE|VERSION_ALLOW_PLUS)); + ASSERT_TRUE(version_is_valid(PROJECT_VERSION_STR, /* flags= */ 0)); + + /* VERSION_ALLOW_EMPTY permits the empty string, but never NULL */ + ASSERT_TRUE(version_is_valid("", VERSION_ALLOW_EMPTY)); + ASSERT_TRUE(version_is_valid("", VERSION_ALLOW_EMPTY|VERSION_ALLOW_UNDERSCORE|VERSION_ALLOW_PLUS)); + ASSERT_FALSE(version_is_valid("", VERSION_ALLOW_UNDERSCORE|VERSION_ALLOW_PLUS)); + ASSERT_FALSE(version_is_valid(NULL, VERSION_ALLOW_EMPTY)); + ASSERT_FALSE(version_is_valid(NULL, VERSION_ALLOW_EMPTY|VERSION_ALLOW_UNDERSCORE|VERSION_ALLOW_PLUS)); + + /* The full UAPI.10 charset, including "~" and "^", is accepted regardless of flags */ + ASSERT_TRUE(version_is_valid("1.2~rc1^5", /* flags= */ 0)); + ASSERT_TRUE(version_is_valid("1.2~rc1^5", VERSION_ALLOW_EMPTY|VERSION_ALLOW_UNDERSCORE|VERSION_ALLOW_PLUS)); + + /* "_" and "+" require their respective flag, the other flags won't do */ + ASSERT_FALSE(version_is_valid("1_2", /* flags= */ 0)); + ASSERT_TRUE(version_is_valid("1_2", VERSION_ALLOW_UNDERSCORE)); + ASSERT_FALSE(version_is_valid("1_2", VERSION_ALLOW_PLUS)); + ASSERT_FALSE(version_is_valid("1_2", VERSION_ALLOW_EMPTY)); + ASSERT_FALSE(version_is_valid("1+2", /* flags= */ 0)); + ASSERT_TRUE(version_is_valid("1+2", VERSION_ALLOW_PLUS)); + ASSERT_FALSE(version_is_valid("1+2", VERSION_ALLOW_UNDERSCORE)); + ASSERT_FALSE(version_is_valid("1+2", VERSION_ALLOW_EMPTY)); + ASSERT_FALSE(version_is_valid("1_2+3", VERSION_ALLOW_UNDERSCORE)); + ASSERT_FALSE(version_is_valid("1_2+3", VERSION_ALLOW_PLUS)); + ASSERT_TRUE(version_is_valid("1_2+3", VERSION_ALLOW_UNDERSCORE|VERSION_ALLOW_PLUS)); + + /* Characters outside the charset are refused, no matter which flags are set */ + ASSERT_FALSE(version_is_valid("1 2", VERSION_ALLOW_EMPTY|VERSION_ALLOW_UNDERSCORE|VERSION_ALLOW_PLUS)); + ASSERT_FALSE(version_is_valid("1/2", VERSION_ALLOW_EMPTY|VERSION_ALLOW_UNDERSCORE|VERSION_ALLOW_PLUS)); + ASSERT_FALSE(version_is_valid("1=2", VERSION_ALLOW_EMPTY|VERSION_ALLOW_UNDERSCORE|VERSION_ALLOW_PLUS)); + ASSERT_FALSE(version_is_valid("1\n2", VERSION_ALLOW_EMPTY|VERSION_ALLOW_UNDERSCORE|VERSION_ALLOW_PLUS)); + ASSERT_FALSE(version_is_valid("©", VERSION_ALLOW_EMPTY|VERSION_ALLOW_UNDERSCORE|VERSION_ALLOW_PLUS)); + + /* "." and ".." pass the charset check and are OK as *part* of a filename, hence accepted */ + ASSERT_TRUE(version_is_valid(".", /* flags= */ 0)); + ASSERT_TRUE(version_is_valid("..", /* flags= */ 0)); + + /* Version strings must fit in a filename, i.e. no longer than NAME_MAX, no matter the flags */ + _cleanup_free_ char *x = strrep("0", NAME_MAX); + ASSERT_NOT_NULL(x); + ASSERT_TRUE(version_is_valid(x, /* flags= */ 0)); + _cleanup_free_ char *y = strrep("0", NAME_MAX+1); + ASSERT_NOT_NULL(y); + ASSERT_FALSE(version_is_valid(y, /* flags= */ 0)); + ASSERT_FALSE(version_is_valid(y, VERSION_ALLOW_EMPTY|VERSION_ALLOW_UNDERSCORE|VERSION_ALLOW_PLUS)); } TEST(strextendn) { diff --git a/src/vpick/vpick-tool.c b/src/vpick/vpick-tool.c index 85f2b5c1cbb..8dd22e0ea28 100644 --- a/src/vpick/vpick-tool.c +++ b/src/vpick/vpick-tool.c @@ -115,7 +115,7 @@ static int parse_argv(int argc, char *argv[], char ***ret_args) { break; OPTION_SHORT('V', "VERSION", "Look for specified version"): - if (!version_is_valid(opts.arg)) + if (!version_is_valid(opts.arg, /* flags= */ 0)) return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "Invalid version string: %s", opts.arg); r = free_and_strdup_warn(&arg_filter_version, opts.arg);