From f8342a5487a635342c921ead02d6cd0e6fa4c540 Mon Sep 17 00:00:00 2001 From: Adam Williamson Date: Fri, 28 Feb 2025 14:47:13 -0800 Subject: [PATCH] client `updates download`: get signed packages when possible This tries to get signed packages when possible, by calling gpg on the key file in /etc/pki/rpm-gpg to figure out the key ID, and passing it to Koji. If we can't work it out - because the key file isn't there on this system, or gpg isn't there, or whatever - we just fall through to getting unsigned packages. There's also a `--no-gpg` to disable this behaviour. Signed-off-by: Adam Williamson --- bodhi-client/bodhi/client/cli.py | 34 ++++++ bodhi-client/tests/test_cli.py | 182 +++++++++++++++++++++++++++++-- 2 files changed, 206 insertions(+), 10 deletions(-) diff --git a/bodhi-client/bodhi/client/cli.py b/bodhi-client/bodhi/client/cli.py index 94a8417324..c3006fc2b7 100644 --- a/bodhi-client/bodhi/client/cli.py +++ b/bodhi-client/bodhi/client/cli.py @@ -773,6 +773,7 @@ def comment(update: str, text: str, karma: int, url: str, id_provider: str, clie help=('Include debuginfo packages')) @click.option('--updateid', help='Download update(s) by ID(s) (comma-separated list)') @click.option('--builds', help='Download update(s) by build NVR(s) (comma-separated list)') +@click.option('--gpg/--no-gpg', help='Download GPG-signed packages', default=True) @url_option @add_options(openid_options) @debug_option @@ -797,10 +798,12 @@ def download(url: str, id_provider: str, client_id: str, **kwargs): ) requested_arch = kwargs['arch'] debuginfo = kwargs['debuginfo'] + gpg = kwargs['gpg'] del kwargs['staging'] del kwargs['arch'] del kwargs['debuginfo'] + del kwargs['gpg'] # At this point we need to have reduced the kwargs dict to only our # query options (updateid or builds) if not any(kwargs.values()): @@ -832,8 +835,39 @@ def download(url: str, id_provider: str, client_id: str, **kwargs): for update in resp.updates: click.echo(f"Downloading packages from {update['alias']}") + keyid = '' + if gpg: + # try to figure out the key ID we need to get signed packages + relnum = update['release']['version'] + if update['release']['id_prefix'] == 'FEDORA-EPEL': + keyname = f'RPM-GPG-KEY-EPEL-{relnum}' + else: + keyname = f'RPM-GPG-KEY-fedora-{relnum}-primary' + keypath = f'/etc/pki/rpm-gpg/{keyname}' + if os.path.exists(keypath): + try: + ret = subprocess.run( + ('gpg', '--list-packets', keypath), + capture_output=True, + text=True + ) + except FileNotFoundError: + click.echo('WARNING: could not run gpg') + ret = None + if ret and not ret.returncode: + for line in ret.stdout.splitlines(): + if 'keyid: ' in line: + keyid = line.split("keyid: ")[-1][-8:].lower() + elif ret: + click.echo('WARNING: gpg failed') + else: + click.echo(f'WARNING: key file {keypath} does not exist') + if not keyid: + click.echo('WARNING: could not find GPG key, packages will be unsigned') for build in update['builds']: args = ['koji', 'download-build'] + if keyid: + args.append(f'--key={keyid}') if debuginfo: args.append('--debuginfo') # subprocess is icky, but koji module doesn't diff --git a/bodhi-client/tests/test_cli.py b/bodhi-client/tests/test_cli.py index a0dc8d0591..c0c8bd09b4 100644 --- a/bodhi-client/tests/test_cli.py +++ b/bodhi-client/tests/test_cli.py @@ -73,6 +73,9 @@ 'nvr': 'nodejs-pants-0.3.0-2.fc25', 'signed': True }) +EXAMPLE_QUERY_MUNCH_EPEL = copy.deepcopy(client_test_data.EXAMPLE_QUERY_MUNCH) +EXAMPLE_QUERY_MUNCH_EPEL.updates[0]['release']['id_prefix'] = 'FEDORA-EPEL' +EXAMPLE_QUERY_MUNCH_EPEL.updates[0]['release']['version'] = '7' @pytest.fixture @@ -135,7 +138,13 @@ def test_url_flag(self, mocked_client_class, mocker): result = runner.invoke( cli.download, - ['--builds', 'nodejs-grunt-wrap-0.3.0-2.fc25', '--url', 'http://localhost:6543']) + [ + '--no-gpg', + '--builds', + 'nodejs-grunt-wrap-0.3.0-2.fc25', + '--url', 'http://localhost:6543' + ] + ) assert result.exit_code == 0 assert result.output == 'Downloading packages from FEDORA-2017-c95b33872d\n' @@ -156,7 +165,7 @@ def test_arch_flag(self, mocked_client_class, mocker): result = runner.invoke( cli.download, - ['--builds', 'nodejs-grunt-wrap-0.3.0-2.fc25', '--arch', 'x86_64']) + ['--no-gpg', '--builds', 'nodejs-grunt-wrap-0.3.0-2.fc25', '--arch', 'x86_64']) assert result.exit_code == 0 assert result.output == 'Downloading packages from FEDORA-2017-c95b33872d\n' @@ -174,7 +183,7 @@ def test_arch_all_flag(self, mocked_client_class, mocker): result = runner.invoke( cli.download, - ['--builds', 'nodejs-grunt-wrap-0.3.0-2.fc25', '--arch', 'all']) + ['--no-gpg', '--builds', 'nodejs-grunt-wrap-0.3.0-2.fc25', '--arch', 'all']) assert result.exit_code == 0 assert result.output == 'Downloading packages from FEDORA-2017-c95b33872d\n' @@ -191,7 +200,15 @@ def test_debuginfo_flag(self, mocked_client_class, mocker): result = runner.invoke( cli.download, - ['--builds', 'nodejs-grunt-wrap-0.3.0-2.fc25', '--arch', 'all', '--debuginfo']) + [ + '--no-gpg', + '--builds', + 'nodejs-grunt-wrap-0.3.0-2.fc25', + '--arch', + 'all', + '--debuginfo' + ] + ) assert result.exit_code == 0 assert result.output == 'Downloading packages from FEDORA-2017-c95b33872d\n' @@ -208,8 +225,14 @@ def test_multiple_builds(self, mocked_client_class, mocker): result = runner.invoke( cli.download, - ['--builds', 'nodejs-pants-0.3.0-2.fc25,nodejs-grunt-wrap-0.3.0-2.fc25', - '--arch', 'all']) + [ + '--no-gpg', + '--builds', + 'nodejs-pants-0.3.0-2.fc25,nodejs-grunt-wrap-0.3.0-2.fc25', + '--arch', + 'all' + ] + ) assert result.exit_code == 0 assert result.output == 'Downloading packages from FEDORA-2017-c95b33872d\n' @@ -238,7 +261,7 @@ def test_no_builds_warning(self, mocked_client_class, mocker): mocked_client_class.send_request.return_value = no_builds_response result = runner.invoke( cli.download, - ['--builds', 'nodejs-pants-0.3.0-2.fc25,nodejs-grunt-wrap-0.3.0-2.fc25']) + ['--no-gpg', '--builds', 'nodejs-pants-0.3.0-2.fc25,nodejs-grunt-wrap-0.3.0-2.fc25']) assert result.exit_code == 0 assert result.output == 'WARNING: No builds found!\n' @@ -255,7 +278,7 @@ def test_some_builds_warning(self, mocked_client_class, mocker): result = runner.invoke( cli.download, - ['--builds', 'nodejs-pants-0.3.0-2.fc25,nodejs-grunt-wrap-0.3.0-2.fc25']) + ['--no-gpg', '--builds', 'nodejs-pants-0.3.0-2.fc25,nodejs-grunt-wrap-0.3.0-2.fc25']) assert result.exit_code == 0 assert result.output == ('WARNING: Some builds not found!\nDownloading packages ' @@ -275,7 +298,7 @@ def test_failed_warning(self, mocked_client_class, mocker): result = runner.invoke( cli.download, - ['--builds', 'nodejs-grunt-wrap-0.3.0-2.fc25']) + ['--no-gpg', '--builds', 'nodejs-grunt-wrap-0.3.0-2.fc25']) assert result.exit_code == 0 assert result.output == ('Downloading packages from FEDORA-2017-c95b33872d\n' @@ -294,7 +317,7 @@ def test_updateid(self, mocked_client_class, mocker): result = runner.invoke( cli.download, - ['--updateid', 'FEDORA-2017-c95b33872d', '--url', 'http://localhost:6543']) + ['--no-gpg', '--updateid', 'FEDORA-2017-c95b33872d', '--url', 'http://localhost:6543']) assert result.exit_code == 0 assert result.output == 'Downloading packages from FEDORA-2017-c95b33872d\n' @@ -305,6 +328,145 @@ def test_updateid(self, mocked_client_class, mocker): 'nodejs-grunt-wrap-0.3.0-2.fc25']) +class TestDownloadGPG: + """ + Test the signature handling features of download(). + """ + def setup_method(self, method): + """We always use these patchers.""" + self.ep = mock.patch('bodhi.client.cli.os.path.exists', return_value=True) + self.exists = self.ep.start() + self.cp = mock.patch('bodhi.client.cli.subprocess.call', return_value=0) + self.call = self.cp.start() + self.rp = mock.patch('bodhi.client.cli.subprocess.run') + self.run = self.rp.start() + self.run.return_value.returncode = 0 + self.run.return_value.stdout = """ +# off=0 ctb=99 tag=6 hlen=3 plen=525 +:public key packet: + version 4, algo 1, created 1459446579, expires 0 + pkey[0]: [4096 bits] + pkey[1]: [17 bits] + keyid: 4089D8F2FDB19C98 +# off=528 ctb=b4 tag=13 hlen=2 plen=60 +""" + + def teardown_method(self, method): + """Stop the patchers.""" + self.ep.stop() + self.cp.stop() + self.rp.stop() + + def test_fedora_good(self, mocked_client_class): + """Success path for Fedora update.""" + mocked_client_class.send_request.return_value = client_test_data.EXAMPLE_QUERY_MUNCH + runner = testing.CliRunner() + + result = runner.invoke( + cli.download, + ['--updateid', 'FEDORA-2017-c95b33872d', '--url', 'http://localhost:6543']) + + assert result.exit_code == 0 + assert result.output == 'Downloading packages from FEDORA-2017-c95b33872d\n' + mocked_client_class.send_request.assert_called_once_with( + 'updates/', verb='GET', params={'updateid': 'FEDORA-2017-c95b33872d'}) + self.run.assert_called_once_with( + ('gpg', '--list-packets', '/etc/pki/rpm-gpg/RPM-GPG-KEY-fedora-25-primary'), + capture_output=True, + text=True + ) + self.call.assert_called_once_with([ + 'koji', 'download-build', '--key=fdb19c98', '--arch=noarch', + '--arch={}'.format(platform.machine()), 'nodejs-grunt-wrap-0.3.0-2.fc25']) + + def test_epel_good(self, mocked_client_class): + """Success path for EPEL update.""" + # epel case + mocked_client_class.send_request.return_value = EXAMPLE_QUERY_MUNCH_EPEL + runner = testing.CliRunner() + result = runner.invoke( + cli.download, + ['--updateid', 'FEDORA-2017-c95b33872d', '--url', 'http://localhost:6543']) + + assert result.exit_code == 0 + assert result.output == 'Downloading packages from FEDORA-2017-c95b33872d\n' + self.run.assert_called_once_with( + ('gpg', '--list-packets', '/etc/pki/rpm-gpg/RPM-GPG-KEY-EPEL-7'), + capture_output=True, + text=True + ) + self.call.assert_called_once_with([ + 'koji', 'download-build', '--key=fdb19c98', '--arch=noarch', + '--arch={}'.format(platform.machine()), 'nodejs-grunt-wrap-0.3.0-2.fc25']) + + def test_gpg_fails(self, mocked_client_class): + """Handle gpg command failing.""" + mocked_client_class.send_request.return_value = client_test_data.EXAMPLE_QUERY_MUNCH + runner = testing.CliRunner() + self.run.return_value.returncode = 1 + result = runner.invoke( + cli.download, + ['--updateid', 'FEDORA-2017-c95b33872d', '--url', 'http://localhost:6543']) + assert result.exit_code == 0 + assert result.output == '''Downloading packages from FEDORA-2017-c95b33872d +WARNING: gpg failed +WARNING: could not find GPG key, packages will be unsigned +''' + self.call.assert_called_once_with([ + 'koji', 'download-build', '--arch=noarch', + '--arch={}'.format(platform.machine()), 'nodejs-grunt-wrap-0.3.0-2.fc25']) + + def test_gpg_noexist(self, mocked_client_class): + """Handle gpg command not existing.""" + mocked_client_class.send_request.return_value = client_test_data.EXAMPLE_QUERY_MUNCH + runner = testing.CliRunner() + self.run.side_effect = FileNotFoundError('gpg') + result = runner.invoke( + cli.download, + ['--updateid', 'FEDORA-2017-c95b33872d', '--url', 'http://localhost:6543']) + assert result.exit_code == 0 + assert result.output == '''Downloading packages from FEDORA-2017-c95b33872d +WARNING: could not run gpg +WARNING: could not find GPG key, packages will be unsigned +''' + self.call.assert_called_once_with([ + 'koji', 'download-build', '--arch=noarch', + '--arch={}'.format(platform.machine()), 'nodejs-grunt-wrap-0.3.0-2.fc25']) + + def test_key_noexist(self, mocked_client_class): + """Handle key file not existing.""" + mocked_client_class.send_request.return_value = client_test_data.EXAMPLE_QUERY_MUNCH + runner = testing.CliRunner() + self.exists.return_value = False + result = runner.invoke( + cli.download, + ['--updateid', 'FEDORA-2017-c95b33872d', '--url', 'http://localhost:6543']) + assert result.exit_code == 0 + assert result.output == '''Downloading packages from FEDORA-2017-c95b33872d +WARNING: key file /etc/pki/rpm-gpg/RPM-GPG-KEY-fedora-25-primary does not exist +WARNING: could not find GPG key, packages will be unsigned +''' + self.call.assert_called_once_with([ + 'koji', 'download-build', '--arch=noarch', + '--arch={}'.format(platform.machine()), 'nodejs-grunt-wrap-0.3.0-2.fc25']) + + def test_no_key_id(self, mocked_client_class): + """Handle key file existing but not showing a key ID.""" + mocked_client_class.send_request.return_value = client_test_data.EXAMPLE_QUERY_MUNCH + runner = testing.CliRunner() + self.run.return_value.stdout = "A fish!" + result = runner.invoke( + cli.download, + ['--updateid', 'FEDORA-2017-c95b33872d', '--url', 'http://localhost:6543']) + assert result.exit_code == 0 + assert result.output == '''Downloading packages from FEDORA-2017-c95b33872d +WARNING: could not find GPG key, packages will be unsigned +''' + self.call.assert_called_once_with([ + 'koji', 'download-build', '--arch=noarch', + '--arch={}'.format(platform.machine()), 'nodejs-grunt-wrap-0.3.0-2.fc25']) + + class TestComposeInfo: """ This class tests the info_compose() function.