fix: handle pqrpm signature verification and RHEL 10 auditd checksums - #604
Conversation
There was a problem hiding this comment.
Hey - I've found 2 issues
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="test_suite/generic/test_generic.py" line_range="608-609" />
<code_context>
+ # RHEL 9.7+ uses pqrpm: PQ signatures are not in the main
+ # rpmdb's SIGPGP/RSAHEADER fields. Verify GPG keys exist in
+ # the pqrpm db and gpgcheck is enforced instead.
+ pqrpm_db = '/usr/lib/pqrpm/lib/sysimage/rpm'
+ if host.file(pqrpm_db).is_directory:
+ gpg_keys = host.check_output(
+ f"rpm --dbpath {pqrpm_db} -qa gpg-pubkey"
+ ).splitlines()
+ assert len(gpg_keys) > 0, \
+ f'No GPG keys found in pqrpm db ({pqrpm_db})'
+
+ gpgcheck = host.run(
+ "grep -s '^gpgcheck' /etc/dnf/dnf.conf /etc/yum.conf"
+ )
+ assert 'gpgcheck=1' in gpgcheck.stdout, \
+ 'gpgcheck=1 must be set when pqrpm is in use'
+ return
+
# Query all installed RPMs and their GPG signature status
</code_context>
<issue_to_address>
**issue (broader_impact):** When the pqrpm database contains at least one GPG key and the checked configuration files contain `gpgcheck=1`, the function returns without checking whether installed packages have valid signatures or whether exactly one key signed them. An unsigned package or package signed with an unexpected additional key therefore passes this test.
**Triggers:** When a host uses the pqrpm database.
**Suggested fix:** Retain equivalent package-signature and key-ID validation for pqrpm packages, or update the test contract and docstring to explicitly define the reduced guarantees.
```suggestion
"""
Checks that packages have a valid GPG signature,
either SIGPGP or RSAHEADER, and that a single GPG key is used.
For RHEL 9.7+ with pqrpm, only checks that the pqrpm database
contains at least one GPG key and that gpgcheck=1 is configured;
package signature and key-ID validation is not performed.
"""
```
</issue_to_address>
### Comment 2
<location path="test_suite/generic/test_generic.py" line_range="623-626" />
<code_context>
+ f'No GPG keys found in pqrpm db ({pqrpm_db})'
+
+ gpgcheck = host.run(
+ "grep -s '^gpgcheck' /etc/dnf/dnf.conf /etc/yum.conf"
+ )
+ assert 'gpgcheck=1' in gpgcheck.stdout, \
+ 'gpgcheck=1 must be set when pqrpm is in use'
+ return
+
</code_context>
<issue_to_address>
**issue (bug_risk):** The check does not enforce `gpgcheck=1` globally: it searches only `/etc/dnf/dnf.conf` and `/etc/yum.conf`, and passes if any matching line contains `gpgcheck=1`. Enabled repository files under `/etc/yum.repos.d/` can still set `gpgcheck=0` without causing this test to fail.
**Triggers:** When an enabled repository overrides the global setting with `gpgcheck=0`.
**Suggested fix:** Inspect every enabled repository configuration and require each effective repository setting to have `gpgcheck=1`, while also handling whitespace and comments in configuration syntax.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Code change explanationtest_pkg_signature_and_gpg_keys (pqrpm handling) On RHEL 9.7+ the rpm version (4.16) cannot handle post-quantum (PQ) cryptography keys natively, so a separate rpmdb exists at Instead of skipping the test entirely, we verify the two preconditions that guarantee packages were signature-verified at install time:
RHEL 10's rpm handles PQ keys natively (no separate db needed), so the original test_auditd (RHEL 10 checksums) Added |
|
Overall looks good. The MR summary line says oci but i think the changes impact all clouds ? |
b873a4b to
38d60a4
Compare
|
saving offline discussions here for posterity PQ keys are just another algorithm to sign the packages and the intent of the test is to ensure all installed packages have some sort of signature on them (older RSA or newer PQ). Our CIV test needs to ensure if a package is declared unsigned , we also check its signed by PQ algorithm and if the keys are present in PQ DB. The problem is that on PQ systems, rpm's query interface can't tell us whether a package is PQ-signed or just unsigned, both show (none) in SIGPGP/RSAHEADER/OPENPGP. There's no rpm tag that exposes PQ signatures on RHEL9.7+ RHEL versions. in our findings
|
…ksums RHEL 9.7+ uses pqrpm (post-quantum crypto) which strips SIGPGP/RSAHEADER fields from the main rpmdb. Instead of skipping, verify GPG keys exist in the pqrpm db and gpgcheck=1 is enforced. Add RHEL 10.0+ auditd.conf checksum. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Update docstring to document reduced guarantees for the pqrpm path. Reword gpgcheck assertion to clarify it's an image policy requirement, not a pqrpm-specific requirement. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
RHEL 10+ handles PQ keys natively in the main rpmdb but SIGPGP/RSAHEADER fields are still empty. Detect this by checking if ALL packages appear unsigned while GPG keys exist in the main rpmdb, and verify gpgcheck=1 instead. Verified on RHEL 10.2, 10.3, and 10.4. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Address review feedback: - Switch from directory-existence to explicit version checks (RHEL 9.7-9.x for pqrpm, RHEL 10+ for native PQ) - Add repo-level gpgcheck=0 override check - Extract _assert_gpgcheck_enforced helper to avoid duplication - Update docstring to explain the three verification paths
25aec99 to
25bace3
Compare
Summary
/usr/lib/pqrpm/lib/sysimage/rpm) and thatgpgcheck=1is enforced globally. RHEL 10 and older versions use the original signature check path unchanged.auditd.conf(4620bfd853bcd869f7a2cb8b1c6d715f) andaudit.rules(795528bd4c7b4131455c15d5d49991bb).