Make client updates download get signed builds if possible

[AdamWill]

This enhances the client bodhi updates download command to find the appropriate GPG key for the release (from a local file if possible, from dist-git if not) and pass --key to koji download-build, so we get signed builds if possible. It's intended to work for Fedora, ELN and EPEL package updates. It doesn't work for Flatpak updates, but that doesn't work without this patch either, so fine.

[AdamWill]

I split the dist-git enhancement out into a separate commit in case someone thought that was going a bit too far, makes it easy to leave it out. But I think it's probably OK. Note we already depend on requests elsewhere in the module, it's not a new dep.

[mattiaverga]

This is a great addition, however I'd like to avoid calling gpg through subprocess and instead use python bindings, if that's not too hard, of course.

I've just played a bit and wrote this:

import gnupg
gpg = gnupg.GPG()
keys = gpg.list_keys(keys='fedora-42-primary')
if not keys:
    gpg.import_keys_file('/etc/pki/rpm-gpg/RPM-GPG-KEY-fedora-42-primary')
    keys = gpg.list_keys(keys='fedora-42-primary')
keys[0].get('keyid')
'C8AC4916105EF944'

This will also import the key in the ~/.gnupg keyring, so after the first time the key will already be available to the same user. Which I think it should be good, but otherwise I suppose we can load the keyring from a random temp dir in gpg = gnupg.GPG() call.

[AdamWill]

I considered that, but the gnupg module itself is just a wrapper for gpg anyway. Here's list_keys, and import_keys_file runs through _handle_io, which does the same. So it seemed pointless to add a new external Python dep to wrap gpg if we could just...call gpg ourselves.

[mattiaverga]

Well, using gnupg module, even as a wrapper, would save us the burden to deal with future cli output pattern changes and/or handling locale. We could just rely on the module output and rest assured any breaking change will be catched by the gnupg module upstream before we are hit.

However, I don't want to force my opinion, so if things are ok for you I'll just merge the change. But I think we should add a requires: or at least a recommends: for gnupg to the bodhi-cli spec file. And a news fragment.

[carlwgeorge]

This isn't going to work for EPEL 10 updates after they are no longer the leading minor version. For example, currently EPEL 10.1 has a version of 10, but EPEL 10.0 has a version of 10.0.

[AdamWill]

ah, thanks, I did check an EPEL update but wasn't aware of that wrinkle. I'll try and deal with it. Just take the integer portion of the version, I guess?

[carlwgeorge]

What I've done in a few other places is split on the dot in the version, which python handles well enough when there is no dot. Something like update['release']['version'].split('.')[0].

[AdamWill]

yes, of course, i meant the concept, I can handle the execution :)

[carlwgeorge]

Is checking the filesystem and dist-git the best approach here? The Koji API has a queryRPMSigs method that will list all the available signatures for a given RPM. I think in most cases there will only be one and we can use that, and if there are multiple we could print those and require the user specify an exact one.

[AdamWill]

oh, neat, wasn't aware of that. hmm. I wonder if we could even implement this in koji download-build, in that case?

[AdamWill]

I'm working on doing this in koji download-build, it seems entirely doable. I'm adding it as a new mode to download-build. If that gets accepted, we can just have bodhi use that mode.

[AdamWill]

Uf. So I implemented a queryRPMSigs-based Koji version of this, and it works, but..."in most cases there will only be one and we can use that" turns out not to be exactly true. There are a few things - like shim - which are not built separately for every Fedora release, and these are signed with the keys for all releases. But more significantly, at branch time - when we're branching F-NN from Rawhide, and Rawhide now maps to F-NN+1 - everything gets signed with both the NN and NN+1 keys. So right now, anything which hasn't been built for Rawhide since F42 branched - which is quite a lot of stuff, anything that's in Rawhide but still has .fc42 extension - is signed with both 42 and 43 keys. e.g. (using my prototype koji implementation):

[adamw@toolbx cli (download-build-autodetect-sig)]$ PYTHONPATH=./ python3 ./koji download-build --autokey Agda-2.6.4.3-53.fc42
All packages signed with these keys: 105ef944 31645531. Use --key to specify

so...now I'm not sure what to do :/ I do kinda like this queryRPMSigs approach, it's elegant in a sense, but it does have this big problem, so maybe practically speaking the Bodhi-side 'get the key from the release number' version is better? I...dunno. Here's the koji patch:

From 384ccfb7f131e92930aae8f04deef89edcdc58dd Mon Sep 17 00:00:00 2001
From: Adam Williamson <[email protected]>
Date: Mon, 3 Mar 2025 16:44:21 -0800
Subject: [PATCH] cli: download-build: add an 'autokey' mode

This adds a --autokey arg to download-build. This figures out if
there is exactly one key with which all packages in the build are
signed (according to queryRPMSigs), and if so, attempts to
download the signed copies. If there is no common signing key,
it prints a warning and downloads unsigned copies. If downloading
signed copies fails - which it can, for old enough builds - it
prints a warning and downloads unsigned copies. If there is *more
than one* key with which all the builds are signed, it errors
out and tells you to use `--key` to specify just one of them.

This will be useful on its own, I guess, but is also intended to
be used by default by `bodhi updates download` so people just
downloading the builds from an update get signed copies, where
possible.

Signed-off-by: Adam Williamson <[email protected]>
---
 cli/koji_cli/commands.py | 33 +++++++++++++++++++++++++++++++--
 1 file changed, 31 insertions(+), 2 deletions(-)

diff --git a/cli/koji_cli/commands.py b/cli/koji_cli/commands.py
index 29b6e0a6..f071f7cf 100644
--- a/cli/koji_cli/commands.py
+++ b/cli/koji_cli/commands.py
@@ -19,6 +19,7 @@ from datetime import datetime
 from dateutil.tz import tzutc
 from optparse import SUPPRESS_HELP, OptionParser
 
+from requests.exceptions import HTTPError
 import six
 import six.moves.xmlrpc_client
 from six.moves import filter, map, range, zip
@@ -6830,6 +6831,8 @@ def anon_handle_download_build(options, session, args):
     parser.add_option("--task-id", action="store_true", help="Interperet id as a task id")
     parser.add_option("--rpm", action="store_true", help="Download the given rpm")
     parser.add_option("--key", help="Download rpms signed with the given key")
+    parser.add_option("--autokey", action="store_true",
+                      help="Download signed rpms, determining the key via API query, if possible")
     parser.add_option("--topurl", metavar="URL", default=options.topurl,
                       help="URL under which Koji files are accessible")
     parser.add_option("--noprogress", action="store_true", help="Do not display progress meter")
@@ -6912,7 +6915,9 @@ def anon_handle_download_build(options, session, args):
                 continue
             rpms.append(rpm)
 
+    downkey = None
     if suboptions.key:
+        downkey = suboptions.key
         with session.multicall() as m:
             results = [m.queryRPMSigs(rpm_id=r['id'], sigkey=suboptions.key) for r in rpms]
         rpm_keys = [x.result for x in results]
@@ -6921,6 +6926,22 @@ def anon_handle_download_build(options, session, args):
                 nvra = "%(nvr)s-%(arch)s.rpm" % rpm
                 warn("No such sigkey %s for rpm %s" % (suboptions.key, nvra))
                 rpms.remove(rpm)
+    elif suboptions.autokey:
+        cands = []
+        with session.multicall() as m:
+            results = [m.queryRPMSigs(rpm_id=r['id']) for r in rpms]
+        results = [res.result for res in results]
+        if all(results):
+            kls = [[entry.get("sigkey") for entry in result] for result in results]
+            cands = [key for key in kls[0] if key and all(key in kl for kl in kls)]
+        if cands:
+            if len(cands) == 1:
+                downkey = cands[0]
+            else:
+                error("All packages signed with these keys: %s. Use --key to specify" %
+                      " ".join(cands))
+        else:
+            warn("No common signing key found for all packages. Will download unsigned packages")
 
     size = len(rpms) + len(archives)
     number = 0
@@ -6928,8 +6949,16 @@ def anon_handle_download_build(options, session, args):
     # run the download
     for rpm in rpms:
         number += 1
-        download_rpm(info, rpm, suboptions.topurl, sigkey=suboptions.key, quiet=suboptions.quiet,
-                     noprogress=suboptions.noprogress, num=number, size=size)
+        try:
+            download_rpm(info, rpm, suboptions.topurl, sigkey=downkey, quiet=suboptions.quiet,
+                         noprogress=suboptions.noprogress, num=number, size=size)
+        except HTTPError as err:
+            if err.response.status_code == 404 and suboptions.autokey and downkey:
+                warn("Signed copy not present, will download unsigned copy")
+                download_rpm(info, rpm, suboptions.topurl, sigkey=None, quiet=suboptions.quiet,
+                             noprogress=suboptions.noprogress, num=number, size=size)
+            else:
+                raise
     for archive in archives:
         number += 1
         download_archive(info, archive, suboptions.topurl, quiet=suboptions.quiet,
-- 
2.48.1

[AdamWill]

hum, the branching case is kinda interesting anyway, even in the Bodhi case. Say you're downloading an update that was for Fedora 42, right now. You're probably downloading it to use on F42. But you might be downloading it to use on Rawhide...so if we automatically download the 42-signed version, that'd be 'wrong'. Still, it's no worse than what we currently do, really (getting an unsigned copy). mmmf.