From cddb56bdf39f179e8e397c926eb37db6c625b090 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Petr=20P=C3=ADsa=C5=99?= Date: Wed, 15 Nov 2023 15:41:18 +0100 Subject: [PATCH] Fix a crash and a revocation misreport in DNS key validation --- ...ation-Fix-parsing-an-armored-PGP-key.patch | 71 ++++++++++++++ ...tion-Fix-handling-keys-without-an-e-.patch | 93 +++++++++++++++++++ ...ation-Fix-caching-negative-responses.patch | 45 +++++++++ dnf.spec | 8 +- 4 files changed, 216 insertions(+), 1 deletion(-) create mode 100644 0001-DNS-key-verification-Fix-parsing-an-armored-PGP-key.patch create mode 100644 0002-DNS-key-verification-Fix-handling-keys-without-an-e-.patch create mode 100644 0003-DNS-key-verification-Fix-caching-negative-responses.patch diff --git a/0001-DNS-key-verification-Fix-parsing-an-armored-PGP-key.patch b/0001-DNS-key-verification-Fix-parsing-an-armored-PGP-key.patch new file mode 100644 index 0000000..f713ce4 --- /dev/null +++ b/0001-DNS-key-verification-Fix-parsing-an-armored-PGP-key.patch @@ -0,0 +1,71 @@ +From 49feb2253d2376d154a5ff5bfb58ec65c2072b63 Mon Sep 17 00:00:00 2001 +From: =?UTF-8?q?Petr=20P=C3=ADsa=C5=99?= +Date: Tue, 14 Nov 2023 12:30:06 +0100 +Subject: [PATCH 1/3] DNS key verification: Fix parsing an armored PGP key +MIME-Version: 1.0 +Content-Type: text/plain; charset=UTF-8 +Content-Transfer-Encoding: 8bit + +A PGP armor message can contain any amount of headers. Up to Fedora 38 +there was one: + + -----BEGIN PGP PUBLIC KEY BLOCK----- + Version: rpm-4.18.0-beta1 + + mQINBGIC2cYBEADJye1aE0AR17qwj6wsHWlCQlcihmqkL8s4gbOk1IevBbH4iXJx + [...] + =CHKS + -----END PGP PUBLIC KEY BLOCK----- + +Since Fedora 39 there is none: + + -----BEGIN PGP PUBLIC KEY BLOCK----- + + mQINBGLykg8BEADURjKtgQpQNoluifXia+U3FuqGCTQ1w7iTqx1UvNhLX6tb9Qjy + l/vjl1iXxucrd2JBnrT/21BdtaABhu2hPy7bpcGEkG8MDinAMZBzcyzHcS/JiGHZ + [...] + =CHKS + -----END PGP PUBLIC KEY BLOCK----- + +RpmImportedKeys._query_db_for_gpg_keys() assumed exactly one header. +As a result if gpgkey_dns_verification configuration option was true, +DNF reported that Fedora 39 keys was revoked because the key +misextratracted from RPM database did not match a key in DNS: + + # dnf-3 upgrade + DNSSEC extension: Testing already imported keys for their validity. + DNSSEC extension: GPG Key fedora-39-primary@fedoraproject.org has been revoked and should be removed immediately + +This patch implements skipping all armor headers. + +https://bugzilla.redhat.com/show_bug.cgi?id=2249380 +Signed-off-by: Petr Písař +--- + dnf/dnssec.py | 11 ++++++++++- + 1 file changed, 10 insertions(+), 1 deletion(-) + +diff --git a/dnf/dnssec.py b/dnf/dnssec.py +index a559853d..ba5b80c7 100644 +--- a/dnf/dnssec.py ++++ b/dnf/dnssec.py +@@ -275,7 +275,16 @@ class RpmImportedKeys: + packager = dnf.rpm.getheader(pkg, 'packager') + email = re.search('<(.*@.*)>', packager).group(1) + description = dnf.rpm.getheader(pkg, 'description') +- key_lines = description.split('\n')[3:-3] ++ # Extract Radix-64-encoded PGP key. Without armor headers and ++ # a checksum. ++ key_lines = [] ++ in_headers = True ++ for line in description.split('\n')[0:-3]: ++ if in_headers: ++ if re.match(r'\A\s*\Z', line, re.NOFLAG): ++ in_headers = False ++ else: ++ key_lines.append(line) + key_str = ''.join(key_lines) + return_list += [KeyInfo(email, key_str.encode('ascii'))] + +-- +2.42.0 + diff --git a/0002-DNS-key-verification-Fix-handling-keys-without-an-e-.patch b/0002-DNS-key-verification-Fix-handling-keys-without-an-e-.patch new file mode 100644 index 0000000..a776839 --- /dev/null +++ b/0002-DNS-key-verification-Fix-handling-keys-without-an-e-.patch @@ -0,0 +1,93 @@ +From 53a5a9397140e35337cf17f0de0792a11f0b9a5b Mon Sep 17 00:00:00 2001 +From: =?UTF-8?q?Petr=20P=C3=ADsa=C5=99?= +Date: Tue, 14 Nov 2023 13:48:03 +0100 +Subject: [PATCH 2/3] DNS key verification: Fix handling keys without an e-mail + address +MIME-Version: 1.0 +Content-Type: text/plain; charset=UTF-8 +Content-Transfer-Encoding: 8bit + +If an PGP key is stored in an RPM database without a "packager" RPM +header, or without an e-mail address there, DNS verification crashed +on converting the undefined address into a DNS domain. That was the +case of Fedora 13 key: + + # dnf-3 upgrade + Traceback (most recent call last): + File "/usr/bin/dnf-3", line 62, in + main.user_main(sys.argv[1:], exit_code=True) + File "/usr/lib/python3.12/site-packages/dnf/cli/main.py", line 201, in user_main + errcode = main(args) + ^^^^^^^^^^ + File "/usr/lib/python3.12/site-packages/dnf/cli/main.py", line 67, in main + return _main(base, args, cli_class, option_parser_class) + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + File "/usr/lib/python3.12/site-packages/dnf/cli/main.py", line 106, in _main + return cli_run(cli, base) + ^^^^^^^^^^^^^^^^^^ + File "/usr/lib/python3.12/site-packages/dnf/cli/main.py", line 122, in cli_run + cli.run() + File "/usr/lib/python3.12/site-packages/dnf/cli/cli.py", line 1040, in run + self._process_demands() + File "/usr/lib/python3.12/site-packages/dnf/cli/cli.py", line 741, in _process_demands + self.base.fill_sack( + File "/usr/lib/python3.12/site-packages/dnf/base.py", line 403, in fill_sack + dnf.dnssec.RpmImportedKeys.check_imported_keys_validity() + File "/usr/lib/python3.12/site-packages/dnf/dnssec.py", line 286, in check_imported_keys_validity + keys = RpmImportedKeys._query_db_for_gpg_keys() + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + File "/usr/lib/python3.12/site-packages/dnf/dnssec.py", line 276, in _query_db_for_gpg_keys + email = re.search('<(.*@.*)>', packager).group(1) + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + File "/usr/lib64/python3.12/re/__init__.py", line 177, in search + return _compile(pattern, flags).search(string) + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + TypeError: expected string or bytes-like object, got 'NoneType' + +This patch defends the crash at two places: In +_query_db_for_gpg_keys() because here we know a NEVRA of the key +and can produce a meaningful message. And in _cache_miss() because +we can get there independenly and called email2location() would also +crash. + +Signed-off-by: Petr Písař +--- + dnf/dnssec.py | 15 ++++++++++++++- + 1 file changed, 14 insertions(+), 1 deletion(-) + +diff --git a/dnf/dnssec.py b/dnf/dnssec.py +index ba5b80c7..fcce0c6d 100644 +--- a/dnf/dnssec.py ++++ b/dnf/dnssec.py +@@ -185,6 +185,10 @@ class DNSSECKeyVerification: + if ctx.add_ta_file("/var/lib/unbound/root.key") != 0: + logger.debug("Unbound context: Failed to add trust anchor file") + ++ if input_key.email is None: ++ logger.debug("A key has no associated e-mail address") ++ return Validity.ERROR ++ + status, result = ctx.resolve(email2location(input_key.email), + RR_TYPE_OPENPGPKEY, unbound.RR_CLASS_IN) + if status != 0: +@@ -273,7 +277,16 @@ class RpmImportedKeys: + return_list = [] + for pkg in packages: + packager = dnf.rpm.getheader(pkg, 'packager') +- email = re.search('<(.*@.*)>', packager).group(1) ++ if packager is None: ++ email = None ++ else: ++ email = re.search('<(.*@.*)>', packager).group(1) ++ if email is None: ++ logger.debug(any_msg(_( ++ "Exempting key package {} from a validation " ++ "because it's not bound to any e-mail address").format( ++ dnf.rpm.getheader(pkg, 'nevra')))) ++ continue + description = dnf.rpm.getheader(pkg, 'description') + # Extract Radix-64-encoded PGP key. Without armor headers and + # a checksum. +-- +2.42.0 + diff --git a/0003-DNS-key-verification-Fix-caching-negative-responses.patch b/0003-DNS-key-verification-Fix-caching-negative-responses.patch new file mode 100644 index 0000000..860df9f --- /dev/null +++ b/0003-DNS-key-verification-Fix-caching-negative-responses.patch @@ -0,0 +1,45 @@ +From 5d9555324d12b5aebc1012aa29d6202da92a5b52 Mon Sep 17 00:00:00 2001 +From: =?UTF-8?q?Petr=20P=C3=ADsa=C5=99?= +Date: Tue, 14 Nov 2023 18:00:58 +0100 +Subject: [PATCH 3/3] DNS key verification: Fix caching negative responses +MIME-Version: 1.0 +Content-Type: text/plain; charset=UTF-8 +Content-Transfer-Encoding: 8bit + +If a user had installed multiple keys for the same e-mail address in +an RPM database, and no records for the address existed in DNS, DNF +validated the first key correctly, but reported that the other key is +revoked: + + # rpm -q gpg-pubkey --qf '%{packager} %{nevra}\n' |grep nokey + nokey1 gpg-pubkey-7460757e-6553a6ab + nokey2 gpg-pubkey-c8d04ba8-6553a6b1 + # dnf-3 upgrade + DNSSEC extension: Testing already imported keys for their validity. + DNSSEC extension: GPG Key nokey@fedoraproject.org has been revoked and should be removed immediately + +The cause was a wrong test for a cached negative reponse. This patch +fixes it. + +https://bugzilla.redhat.com/show_bug.cgi?id=2249380 +Signed-off-by: Petr Písař +--- + dnf/dnssec.py | 2 +- + 1 file changed, 1 insertion(+), 1 deletion(-) + +diff --git a/dnf/dnssec.py b/dnf/dnssec.py +index fcce0c6d..354a15d4 100644 +--- a/dnf/dnssec.py ++++ b/dnf/dnssec.py +@@ -150,7 +150,7 @@ class DNSSECKeyVerification: + if key_union == input_key_string: + logger.debug("Cache hit, valid key") + return Validity.VALID +- elif key_union is NoKey: ++ elif isinstance(key_union, NoKey): + logger.debug("Cache hit, proven non-existence") + return Validity.PROVEN_NONEXISTENCE + else: +-- +2.42.0 + diff --git a/dnf.spec b/dnf.spec index 418ba44..0f6b551 100644 --- a/dnf.spec +++ b/dnf.spec @@ -68,12 +68,15 @@ It supports RPMs, modules and comps groups & environments. Name: dnf Version: 4.18.1 -Release: 1%{?dist} +Release: 2%{?dist} Summary: %{pkg_summary} # For a breakdown of the licensing, see PACKAGE-LICENSING License: GPL-2.0-or-later AND GPL-1.0-only URL: https://github.com/rpm-software-management/dnf Source0: %{url}/archive/%{version}/%{name}-%{version}.tar.gz +Patch0: 0001-DNS-key-verification-Fix-parsing-an-armored-PGP-key.patch +Patch1: 0002-DNS-key-verification-Fix-handling-keys-without-an-e-.patch +Patch2: 0003-DNS-key-verification-Fix-caching-negative-responses.patch BuildArch: noarch BuildRequires: cmake BuildRequires: gettext @@ -383,6 +386,9 @@ popd %{python3_sitelib}/%{name}/automatic/ %changelog +* Tue Nov 14 2023 Petr Pisar - 4.18.1-2 +- Fix a crash and a revocation misreport in DNS key validation (RhBug:2249380) + * Tue Nov 07 2023 Jan Kolarik - 4.18.1-1 - Update to 4.18.1 - Do not translate repoquery time format strings (RhBug:2245773)