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

@@ -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

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
+  keyring instead of the default `/etc/systemd/import-pubring.pgp`,
-  `/usr/lib/systemd/import-pubring.pgp` keyrings.
+  `/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.

man/importctl.xml

@@ -124,9 +124,10 @@
         <filename>.sha256</filename> suffixed file or the <filename>SHA256SUMS</filename> file.  With
         <option>--verify=signature</option>, the sha checksum file is first verified with the detached GPG
         signature of <filename>.sha256</filename> or <filename>SHA256SUMS</filename>.  The public key for
-        this verification step needs to be available in
+        this verification step needs to be available in the system keyring consisting of
-        <filename>/usr/lib/systemd/import-pubring.pgp</filename> or
+        <filename>/usr/lib/systemd/import-pubring.pgp</filename> and the local keyring in
-        <filename>/etc/systemd/import-pubring.pgp</filename>.</para>
+        <filename>/etc/systemd/import-pubring.pgp</filename> (or, if that does not exist,
+        <filename>/etc/systemd/import-pubring.gpg</filename>).</para>
 
         <para>If <option>-keep-download=yes</option> 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 @@
         <literal>signature</literal>.  If <literal>no</literal>, no verification is done. If
         <literal>checksum</literal> is specified, the download is checked for integrity after the transfer is
         complete, but no signatures are verified. If <literal>signature</literal> is specified, the checksum
-        is verified and the image's signature is checked against a local keyring of trustable vendors. It is
+        is verified and the image's signature is checked against a local keyring of trustable vendors, in
-        strongly recommended to set this option to <literal>signature</literal> if the server and protocol
+        combination with the vendor keyring in <filename>/usr/lib/systemd/import-pubring.pgp</filename>.
-        support this. Defaults to <literal>signature</literal>.</para>
+        It is strongly recommended to set this option to <literal>signature</literal> if the server and
+        protocol support this. Defaults to <literal>signature</literal>.</para>
 
         <xi:include href="version-info.xml" xpointer="v256"/></listitem>
       </varlistentry>

man/sysupdate.d.xml

@@ -501,8 +501,9 @@
         downloaded resources (specifically: validate the GPG signatures for downloaded
         <filename>SHA256SUMS</filename> manifest files, via their detached signature files
         <filename>SHA256SUMS.gpg</filename> in combination with the system keyring
-        <filename>/usr/lib/systemd/import-pubring.pgp</filename> or
+        consisting of <filename>/usr/lib/systemd/import-pubring.pgp</filename> and the local
-        <filename>/etc/systemd/import-pubring.pgp</filename>).</para>
+        keyring in <filename>/etc/systemd/import-pubring.pgp</filename> (or, if that does not
+        exist, <filename>/etc/systemd/import-pubring.gpg</filename>).</para>
 
         <para>This option is essential to provide integrity guarantees for downloaded resources and thus
         should be left enabled, outside of test environments.</para>

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;
+        _cleanup_strv_free_ char **cmd = NULL;
-        const char *keyring_override, *keyring_source;
+        _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)
+        if (keyring_override) {
-                keyring_source = keyring_override;
+                r = strv_extend(&keyring_sources, keyring_override);
-        else if (access(USER_KEYRING_PATH, F_OK) >= 0)
+                if (r < 0) {
-                keyring_source = USER_KEYRING_PATH;
+                        log_error_errno(r, "Failed to store custom keyring path: %m");
-        else if (access(USER_KEYRING_PATH_LEGACY, F_OK) >= 0)
+                        goto finish;
-                keyring_source = USER_KEYRING_PATH_LEGACY;
+                }
-        else
+        } else {
-                keyring_source = VENDOR_KEYRING_PATH;
+                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");
+        STRV_FOREACH(keyring_source, keyring_sources) {
-        if (!keyring_copy) {
+                _cleanup_free_ char *copy = NULL;
+
+                if (asprintf(&copy, "%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);
+                execvp("gpg2", cmd);
-                cmd[k++] = strjoina("--keyring=", keyring_copy);
+                execvp("gpg", cmd);
-                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);
                 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;
 }

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 <overlay-user@example.com>' 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 <overlay-vendor@example.com>' 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 <overlay-other@example.com>' 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" <<EOF
+[Source]
+Type=url-file
+Path=file://$sigdir
+MatchPattern=payload-@v.raw
+
+[Target]
+Type=regular-file
+Path=$target
+MatchPattern=payload-@v.raw
+InstancesMax=3
+EOF
+
+    "$SYSUPDATE" --definitions="$defdir" check-new
+    "$SYSUPDATE" --definitions="$defdir" update
+    cmp "$sigdir/payload-v1.raw" "$target/payload-v1.raw"
+
+    dd if=/dev/urandom of="$sigdir/payload-v2.raw" bs=1024 count=8 status=none
+    (cd "$sigdir" && sha256sum payload-v1.raw payload-v2.raw > 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"