From eb020fc4923b61c5c627685493e5f570c034b947 Mon Sep 17 00:00:00 2001 From: zouge666 <1615379639@qq.com> Date: Sat, 22 Aug 2026 09:05:14 +0200 Subject: [PATCH] import: merge default OpenPGP keyrings When an administrator installed a custom keyring under /etc, signature verification selected it instead of the vendor keyring under /usr. That could make vendor-signed system updates stop verifying after adding local keys. Collect all readable default keyrings, including the current and legacy /etc paths, and pass temporary copies to GnuPG with separate --keyring options. Keep SYSTEMD_OPENPGP_KEYRING as an explicit override, add regression coverage for keyring coexistence, override behavior, and current-path signing, and silence the expected shellcheck false positive in the test cleanup helper. Closes: #40702 --- NEWS | 5 + docs/ENVIRONMENT.md | 5 +- man/importctl.xml | 14 +-- man/sysupdate.d.xml | 5 +- src/import/pull-common.c | 156 ++++++++++++++++++++++---------- test/units/TEST-72-SYSUPDATE.sh | 146 ++++++++++++++++++++++++++++++ 6 files changed, 272 insertions(+), 59 deletions(-) diff --git a/NEWS b/NEWS index 59f7325d31..8460693200 100644 --- a/NEWS +++ b/NEWS @@ -43,6 +43,11 @@ CHANGES WITH 262: have been provided. This clears the way for a new "systemd-sysupdate@.service" unit for varlink activation of sysupdate. + * The default OpenPGP keyring lookup now combines the vendor keyring in + /usr with the local keyring in /etc. Deployments that relied on a + single /etc keyring to exclude vendor trust should set + $SYSTEMD_OPENPGP_KEYRING explicitly. + * TPM-sealed credentials are now pinned to the TPM's SRK. This prevents MITM interposer attacks from stealing the decrypted credentials by ensuring that communication with the TPM is protected by a private key diff --git a/docs/ENVIRONMENT.md b/docs/ENVIRONMENT.md index 71df55896a..da24c169f7 100644 --- a/docs/ENVIRONMENT.md +++ b/docs/ENVIRONMENT.md @@ -516,8 +516,9 @@ is suppressed by default. * `$SYSTEMD_OPENPGP_KEYRING` — takes an absolute path to an OpenPGP keyring file. If set and non-empty, signature verification on download uses this - keyring instead of the default `/etc/systemd/import-pubring.pgp` and - `/usr/lib/systemd/import-pubring.pgp` keyrings. + keyring instead of the default `/etc/systemd/import-pubring.pgp`, + `/etc/systemd/import-pubring.gpg`, and `/usr/lib/systemd/import-pubring.pgp` + keyrings. Useful when running unprivileged in the user context, with custom transfer definitions (e.g. `systemd-sysupdate --definitions=…`), or for testing. Has no effect when signature verification is disabled. diff --git a/man/importctl.xml b/man/importctl.xml index 3c14b83f08..f969e566d5 100644 --- a/man/importctl.xml +++ b/man/importctl.xml @@ -124,9 +124,10 @@ .sha256 suffixed file or the SHA256SUMS file. With , the sha checksum file is first verified with the detached GPG signature of .sha256 or SHA256SUMS. The public key for - this verification step needs to be available in - /usr/lib/systemd/import-pubring.pgp or - /etc/systemd/import-pubring.pgp. + this verification step needs to be available in the system keyring consisting of + /usr/lib/systemd/import-pubring.pgp and the local keyring in + /etc/systemd/import-pubring.pgp (or, if that does not exist, + /etc/systemd/import-pubring.gpg). If is specified the image will be downloaded and stored in a read-only subvolume/directory in the image directory that is named after the specified URL and its @@ -314,9 +315,10 @@ signature. If no, no verification is done. If checksum is specified, the download is checked for integrity after the transfer is complete, but no signatures are verified. If signature is specified, the checksum - is verified and the image's signature is checked against a local keyring of trustable vendors. It is - strongly recommended to set this option to signature if the server and protocol - support this. Defaults to signature. + is verified and the image's signature is checked against a local keyring of trustable vendors, in + combination with the vendor keyring in /usr/lib/systemd/import-pubring.pgp. + It is strongly recommended to set this option to signature if the server and + protocol support this. Defaults to signature. diff --git a/man/sysupdate.d.xml b/man/sysupdate.d.xml index 9c27bf444a..3572322682 100644 --- a/man/sysupdate.d.xml +++ b/man/sysupdate.d.xml @@ -501,8 +501,9 @@ downloaded resources (specifically: validate the GPG signatures for downloaded SHA256SUMS manifest files, via their detached signature files SHA256SUMS.gpg in combination with the system keyring - /usr/lib/systemd/import-pubring.pgp or - /etc/systemd/import-pubring.pgp). + consisting of /usr/lib/systemd/import-pubring.pgp and the local + keyring in /etc/systemd/import-pubring.pgp (or, if that does not + exist, /etc/systemd/import-pubring.gpg). This option is essential to provide integrity guarantees for downloaded resources and thus should be left enabled, outside of test environments. diff --git a/src/import/pull-common.c b/src/import/pull-common.c index 9799ae959d..93de1bef4b 100644 --- a/src/import/pull-common.c +++ b/src/import/pull-common.c @@ -125,6 +125,22 @@ static int hash_url(const char *url, char **ret) { return 0; } +static int keyring_add_if_exists(char ***keyrings, const char *path) { + assert(keyrings); + assert(path); + + if (access(path, F_OK) < 0) { + if (errno != ENOENT) + log_warning_errno(errno, + "Failed to check if keyring '%s' exists, ignoring: %m", + path); + + return 0; + } + + return strv_extend(keyrings, path); +} + int pull_make_path(const char *url, const char *etag, const char *image_root, const char *prefix, const char *suffix, char **ret) { _cleanup_free_ char *escaped_url = NULL, *escaped_etag = NULL; char *path; @@ -410,12 +426,13 @@ static int verify_gpg( _cleanup_(rm_rf_physical_and_freep) char *gpg_home = NULL; char sig_file_path[] = "/tmp/sigXXXXXX"; _cleanup_(pidref_done_sigkill_wait) PidRef pidref = PIDREF_NULL; - _cleanup_free_ char *keyring_copy = NULL; - const char *keyring_override, *keyring_source; + _cleanup_strv_free_ char **cmd = NULL; + _cleanup_strv_free_ char **keyring_sources = NULL, **keyring_copies = NULL; + const char *keyring_override; int r; assert(iovec_is_valid(payload)); - assert(iovec_is_valid(signature)); + assert(iovec_is_set(signature)); /* Support using a custom keyring, see docs/ENVIRONMENT.md. */ keyring_override = empty_to_null(secure_getenv("SYSTEMD_OPENPGP_KEYRING")); @@ -428,7 +445,7 @@ static int verify_gpg( if (r < 0) return log_error_errno(errno, "Failed to create pipe for gpg: %m"); - if (iovec_is_set(signature)) { + { _cleanup_close_ int sig_file = -EBADF; sig_file = mkostemp(sig_file_path, O_RDWR); @@ -448,28 +465,98 @@ static int verify_gpg( goto finish; } - if (keyring_override) - keyring_source = keyring_override; - else if (access(USER_KEYRING_PATH, F_OK) >= 0) - keyring_source = USER_KEYRING_PATH; - else if (access(USER_KEYRING_PATH_LEGACY, F_OK) >= 0) - keyring_source = USER_KEYRING_PATH_LEGACY; - else - keyring_source = VENDOR_KEYRING_PATH; + if (keyring_override) { + r = strv_extend(&keyring_sources, keyring_override); + if (r < 0) { + log_error_errno(r, "Failed to store custom keyring path: %m"); + goto finish; + } + } else { + r = keyring_add_if_exists(&keyring_sources, USER_KEYRING_PATH); + if (r < 0) { + log_error_errno(r, "Failed to collect keyring '%s': %m", USER_KEYRING_PATH); + goto finish; + } + + if (strv_isempty(keyring_sources)) { + r = keyring_add_if_exists(&keyring_sources, USER_KEYRING_PATH_LEGACY); + if (r < 0) { + log_error_errno(r, "Failed to collect keyring '%s': %m", USER_KEYRING_PATH_LEGACY); + goto finish; + } + } + + r = keyring_add_if_exists(&keyring_sources, VENDOR_KEYRING_PATH); + if (r < 0) { + log_error_errno(r, "Failed to collect keyring '%s': %m", VENDOR_KEYRING_PATH); + goto finish; + } + } + + if (strv_isempty(keyring_sources)) { + r = log_error_errno(SYNTHETIC_ERRNO(ENOENT), + "No OpenPGP keyrings found, checked '%s', '%s', and '%s'.", + USER_KEYRING_PATH, USER_KEYRING_PATH_LEGACY, VENDOR_KEYRING_PATH); + goto finish; + } /* For whatever reason gpg --auto-key-import writes the key material embedded in the signature * (the signing subkey) back into the keyring it verifies against instead of just using it * temporarily. Verify against a throwaway copy in the tmp home so we never write to the * original keyring. */ - keyring_copy = path_join(gpg_home, "keyring.gpg"); - if (!keyring_copy) { + STRV_FOREACH(keyring_source, keyring_sources) { + _cleanup_free_ char *copy = NULL; + + if (asprintf(©, "%s/keyring-%zu.gpg", gpg_home, strv_length(keyring_copies)) < 0) { + r = log_oom(); + goto finish; + } + + r = copy_file(*keyring_source, copy, 0, 0600, /* copy_flags= */ 0); + if (r < 0) { + log_error_errno(r, "Failed to copy keyring '%s': %m", *keyring_source); + goto finish; + } + + r = strv_consume(&keyring_copies, TAKE_PTR(copy)); + if (r < 0) { + log_error_errno(r, "Failed to store temporary keyring path: %m"); + goto finish; + } + } + + cmd = strv_new( + "gpg", + "--no-options", + "--no-default-keyring", + "--no-auto-key-locate", + "--no-auto-check-trustdb", + "--batch", + "--trust-model=always", + "--auto-key-import", + "--import-options=merge-only,import-clean"); + if (!cmd) { r = log_oom(); goto finish; } - r = copy_file(keyring_source, keyring_copy, 0, 0600, /* copy_flags= */ 0); + r = strv_extendf(&cmd, "--homedir=%s", gpg_home); if (r < 0) { - log_error_errno(r, "Failed to copy keyring '%s': %m", keyring_source); + log_error_errno(r, "Failed to build gpg command line: %m"); + goto finish; + } + + STRV_FOREACH(keyring_source, keyring_copies) { + r = strv_extendf(&cmd, "--keyring=%s", *keyring_source); + if (r < 0) { + log_error_errno(r, "Failed to build gpg command line: %m"); + goto finish; + } + } + + r = strv_extend_many(&cmd, "--verify", sig_file_path, "-"); + if (r < 0) { + log_error_errno(r, "Failed to build gpg command line: %m"); goto finish; } @@ -480,40 +567,12 @@ static int verify_gpg( FORK_RESET_SIGNALS|FORK_CLOSE_ALL_FDS|FORK_DEATHSIG_SIGTERM|FORK_REARRANGE_STDIO|FORK_LOG|FORK_RLIMIT_NOFILE_SAFE, &pidref); if (r < 0) - return r; + goto finish; if (r == 0) { - const char *cmd[] = { - "gpg", - "--no-options", - "--no-default-keyring", - "--no-auto-key-locate", - "--no-auto-check-trustdb", - "--batch", - "--trust-model=always", - "--auto-key-import", - "--import-options=merge-only,import-clean", - NULL, /* --homedir= */ - NULL, /* --keyring= */ - NULL, /* --verify */ - NULL, /* signature file */ - NULL, /* dash */ - NULL /* trailing NULL */ - }; - size_t k = ELEMENTSOF(cmd) - 6; - /* Child */ - cmd[k++] = strjoina("--homedir=", gpg_home); - cmd[k++] = strjoina("--keyring=", keyring_copy); - cmd[k++] = "--verify"; - if (signature) { - cmd[k++] = sig_file_path; - cmd[k++] = "-"; - cmd[k++] = NULL; - } - - execvp("gpg2", (char * const *) cmd); - execvp("gpg", (char * const *) cmd); + execvp("gpg2", cmd); + execvp("gpg", cmd); log_error_errno(errno, "Failed to execute gpg: %m"); _exit(EXIT_FAILURE); } @@ -543,8 +602,7 @@ static int verify_gpg( } finish: - if (iovec_is_set(signature)) - (void) unlink(sig_file_path); + (void) unlink(sig_file_path); return r; } diff --git a/test/units/TEST-72-SYSUPDATE.sh b/test/units/TEST-72-SYSUPDATE.sh index f35235ce84..5224071ddb 100755 --- a/test/units/TEST-72-SYSUPDATE.sh +++ b/test/units/TEST-72-SYSUPDATE.sh @@ -1281,7 +1281,153 @@ EOF cmp "$sigdir/payload-v2.raw" "$target/payload-v2.raw" } +backup_import_keyring() { + local path="$1" + local backup="$2" + + rm -f "$backup" + if [ -e "$path" ] || [ -L "$path" ]; then + mv "$path" "$backup" + fi +} + +restore_import_keyring() { + local path="$1" + local backup="$2" + + rm -f "$path" + if [ -e "$backup" ] || [ -L "$backup" ]; then + mv "$backup" "$path" + fi +} + +test_signature_keyring_overlay() ( + if ! command -v gpg >/dev/null; then + echo "gpg not available, skipping signature overlay test" + exit 0 + fi + + local gpg_version gpg_rest + gpg_version="$(gpg --version | sed -n '1p' | awk '{print $NF}')" + gpg_rest="${gpg_version#*.}" + if [ "${gpg_version%%.*}" -lt 2 ] || { [ "${gpg_version%%.*}" -eq 2 ] && [ "${gpg_rest%%.*}" -lt 4 ]; }; then + echo "gpg $gpg_version too old (need >= 2.4), skipping signature overlay test" + exit 0 + fi + + local sigdir="$WORKDIR/sigtest-overlay-source" + local defdir="$WORKDIR/sigtest-overlay-defs" + local user_home="$WORKDIR/sigtest-overlay-userhome" + local vendor_home="$WORKDIR/sigtest-overlay-vendorhome" + local other_home="$WORKDIR/sigtest-overlay-otherhome" + local target="$WORKDIR/sigtest-overlay-target" + local user_keyring="$WORKDIR/sigtest-overlay-user.gpg" + local vendor_keyring="$WORKDIR/sigtest-overlay-vendor.gpg" + local other_keyring="$WORKDIR/sigtest-overlay-other.gpg" + local etc_pgp_backup="$WORKDIR/import-pubring.etc.pgp.bak" + local etc_gpg_backup="$WORKDIR/import-pubring.etc.gpg.bak" + local usr_pgp_backup="$WORKDIR/import-pubring.usr.pgp.bak" + # Called via trap on EXIT; shellcheck cannot see the indirect invocation. + # shellcheck disable=SC2329,SC2317 + cleanup_overlay_keyrings() { + set +e + gpgconf --homedir "$user_home" --kill all 2>/dev/null || : + gpgconf --homedir "$vendor_home" --kill all 2>/dev/null || : + gpgconf --homedir "$other_home" --kill all 2>/dev/null || : + restore_import_keyring /etc/systemd/import-pubring.pgp "$etc_pgp_backup" + restore_import_keyring /etc/systemd/import-pubring.gpg "$etc_gpg_backup" + restore_import_keyring /usr/lib/systemd/import-pubring.pgp "$usr_pgp_backup" + } + + mkdir -p "$sigdir" "$defdir" "$user_home" "$vendor_home" "$other_home" "$target" /etc/systemd /usr/lib/systemd + chmod 700 "$user_home" "$vendor_home" "$other_home" + + trap cleanup_overlay_keyrings EXIT + + backup_import_keyring /etc/systemd/import-pubring.pgp "$etc_pgp_backup" + backup_import_keyring /etc/systemd/import-pubring.gpg "$etc_gpg_backup" + backup_import_keyring /usr/lib/systemd/import-pubring.pgp "$usr_pgp_backup" + + GNUPGHOME="$user_home" gpg --batch --pinentry-mode loopback --passphrase '' \ + --quick-gen-key 'Overlay User ' rsa2048 cert,sign never + GNUPGHOME="$user_home" gpg --export --output "$user_keyring" + + GNUPGHOME="$vendor_home" gpg --batch --pinentry-mode loopback --passphrase '' \ + --quick-gen-key 'Overlay Vendor ' rsa2048 cert,sign never + GNUPGHOME="$vendor_home" gpg --export --output "$vendor_keyring" + + GNUPGHOME="$other_home" gpg --batch --pinentry-mode loopback --passphrase '' \ + --quick-gen-key 'Overlay Other ' rsa2048 cert,sign never + GNUPGHOME="$other_home" gpg --export --output "$other_keyring" + + install -Dm0644 "$user_keyring" /etc/systemd/import-pubring.gpg + install -Dm0644 "$vendor_keyring" /usr/lib/systemd/import-pubring.pgp + rm -f /etc/systemd/import-pubring.pgp + + dd if=/dev/urandom of="$sigdir/payload-v1.raw" bs=1024 count=8 status=none + (cd "$sigdir" && sha256sum payload-v1.raw > SHA256SUMS) + GNUPGHOME="$vendor_home" gpg --batch --pinentry-mode loopback --passphrase '' \ + --detach-sign --include-key-block --yes \ + --output "$sigdir/SHA256SUMS.gpg" "$sigdir/SHA256SUMS" + + cat >"$defdir/01-sigtest.transfer" < SHA256SUMS) + GNUPGHOME="$other_home" gpg --batch --pinentry-mode loopback --passphrase '' \ + --detach-sign --include-key-block --yes \ + --output "$sigdir/SHA256SUMS.gpg" "$sigdir/SHA256SUMS" + if "$SYSUPDATE" --definitions="$defdir" update; then + echo "ERROR: accepted an update signed by a key not in the default keyrings" >&2 + exit 1 + fi + if [ -f "$target/payload-v2.raw" ]; then + echo "ERROR: payload-v2 should not have been installed" >&2 + exit 1 + fi + + dd if=/dev/urandom of="$sigdir/payload-v3.raw" bs=1024 count=8 status=none + (cd "$sigdir" && sha256sum payload-v1.raw payload-v2.raw payload-v3.raw > SHA256SUMS) + GNUPGHOME="$vendor_home" gpg --batch --pinentry-mode loopback --passphrase '' \ + --detach-sign --include-key-block --yes \ + --output "$sigdir/SHA256SUMS.gpg" "$sigdir/SHA256SUMS" + if SYSTEMD_OPENPGP_KEYRING="$other_keyring" "$SYSUPDATE" --definitions="$defdir" update; then + echo "ERROR: accepted an update with an override keyring that lacks the signing key" >&2 + exit 1 + fi + if [ -f "$target/payload-v3.raw" ]; then + echo "ERROR: payload-v3 should not have been installed" >&2 + exit 1 + fi + + install -Dm0644 "$user_keyring" /etc/systemd/import-pubring.pgp + rm -f /etc/systemd/import-pubring.gpg + dd if=/dev/urandom of="$sigdir/payload-v4.raw" bs=1024 count=8 status=none + (cd "$sigdir" && sha256sum payload-v1.raw payload-v2.raw payload-v3.raw payload-v4.raw > SHA256SUMS) + GNUPGHOME="$user_home" gpg --batch --pinentry-mode loopback --passphrase '' \ + --detach-sign --include-key-block --yes \ + --output "$sigdir/SHA256SUMS.gpg" "$sigdir/SHA256SUMS" + "$SYSUPDATE" --definitions="$defdir" update + cmp "$sigdir/payload-v4.raw" "$target/payload-v4.raw" +) + test_signature_verification +test_signature_keyring_overlay # Test '**/' as prefix in MatchPattern= for subpaths in SHA256SUMS rm -rf "$CONFIGDIR" "$WORKDIR/blobs" "$WORKDIR/source/sub"