From ef966fd95b2e33270547b2340d1bd8a9a6897fd0 Mon Sep 17 00:00:00 2001 From: Daniel Hilst <392820+dhilst@users.noreply.github.com> Date: Fri, 21 Aug 2026 19:01:01 -0300 Subject: [PATCH] feat(xcat-dep): arch-array cross-arch finalize; verify every rpm signature Two review follow-ups: - Cross-arch genesis matrix is now driven by a single @GENESIS_ARCHES array (arch -> xCAT tarch) in MockBuildUtils. finalize_xcat_dep discovers, gates, and N-way cross-copies over that list, so adding an arch later (aarch64, riscv64) is one entry there plus wiring its repo root. No behaviour change for the current x86_64/ppc64le pair (tests unchanged + a contract assertion on the array). - PR #62 review #4: the repo completeness gate verified only the repomd.xml signature. It now ALSO verifies every binary rpm's header signature is by the configured signing key -- resolve the key's accepted id set (primary + subkey ids) and check each rpm's RSAHEADER/DSAHEADER pgpsig key id, failing on any unsigned or foreign-signed rpm. Runs per-target in deploy_target and per-cell after --finalize-xcat-dep. A signed repomd over an unsigned rpm otherwise passed the gate yet DNF rejects the package at install. Added pure fixtures. Signed-off-by: Daniel Hilst <392820+dhilst@users.noreply.github.com> --- MockBuildUtils.pm | 117 +++++++++++++++++++++++++++++++--------------- mockbuild-all.pl | 49 ++++++++++++++++++- t/mockbuild-all.t | 27 ++++++++++- 3 files changed, 152 insertions(+), 41 deletions(-) diff --git a/MockBuildUtils.pm b/MockBuildUtils.pm index 19f2ffd..7755450 100644 --- a/MockBuildUtils.pm +++ b/MockBuildUtils.pm @@ -15,7 +15,7 @@ use Digest::MD5 qw(md5_hex); our @EXPORT_OK = qw( sh_quote print_step version_matches required_pkgs have_rpm read_manifest - verify_repo_packages verify_repo_signature + verify_repo_packages verify_repo_signature verify_rpm_signatures rpm_version rpm_release rpm_sigmd5 rpm_is_signed restamp_release_line cross_copy_genesis finalize_xcat_dep bump_dep_release_suffix build_mock_uniqueext @@ -110,6 +110,26 @@ sub verify_repo_signature { return @problems; } +# verify_rpm_signatures: pure decision for the per-rpm signature gate. $rpm_sigs is an arrayref of +# [rpm_basename, observed_keyid|undef] (rpm reports the signing SUBKEY id); $accept is a hashref set +# of acceptable key ids (the signing key's primary + subkey ids, lowercased). Returns one problem per +# rpm that is unsigned or signed by a key not in the set. A signed repomd over unsigned/foreign-signed +# rpms still makes DNF reject the install, so the packages must be checked, not just the metadata. +sub verify_rpm_signatures { + my ($rpm_sigs, $accept) = @_; + my @problems; + for my $rs (@$rpm_sigs) { + my ($name, $kid) = @$rs; + if (!defined($kid) || $kid eq '') { + push @problems, "UNSIGNED rpm $name"; + } elsif (!$accept->{ lc $kid }) { + push @problems, "WRONGKEY rpm $name: signed by $kid, expected one of " + . join('/', sort keys %$accept); + } + } + return @problems; +} + # have_rpm: is there a non-src rpm named -... under $dir? sub have_rpm { my ($dir, $name) = @_; @@ -280,6 +300,17 @@ sub cross_copy_genesis { # a repo whose rpm set changed (or undef). Both injected so this stays free of gpg/createrepo # state and is unit-testable. Requires each arch's own genesis rpm to be present (a pair with no # genesis is a hard error, never a silent no-op) and fails if no repo pair is found at all. +# Architectures whose xCAT-genesis-base is cross-provisioned into every peer repo, so a management +# node can netboot nodes of any arch (issue #7610). Each entry maps the repo/subdir arch name to the +# genesis rpm's xCAT "tarch" (xCAT collapses ppc/ppc64le into tarch ppc64; x86_64 stays x86_64). This +# is the SINGLE SOURCE OF TRUTH for the cross-arch matrix -- to add an arch later (e.g. aarch64, +# riscv64) add an entry here AND wire its repo root into finalize_xcat_dep's %repo (the caller passes +# it). Discovery, the per-arch input gate, and the N-way cross-copy all iterate this list. +our @GENESIS_ARCHES = ( + { arch => 'x86_64', tarch => 'x86_64' }, + { arch => 'ppc64le', tarch => 'ppc64' }, +); + sub finalize_xcat_dep { my ($x86_64_repo, $ppc64le_repo, %opt) = @_; my $sign = $opt{sign}; @@ -287,47 +318,59 @@ sub finalize_xcat_dep { print_step('Finalize xcat-dep: cross-arch genesis-base provisioning (issue #7610)'); print "x86_64-repo: $x86_64_repo\n"; print "ppc64le-repo: $ppc64le_repo\n"; - # Discover the UNION of OS dirs from BOTH arch repos. Anchoring discovery on x86_64 alone let a - # ppc64le-only (an rh that built for ppc but not x86_64) slip through unseen -- finalize - # then never cross-populated that cell's x86_64 genesis and still exited 0 (PR #62 review). Both - # arch peers are required for every discovered below, so the check is now symmetric. + + # Per-arch repo root, keyed by the @GENESIS_ARCHES arch name. A new arch added to that list must + # also get its root wired here (today both roots are the same CD tree); a missing one fails loudly + # below rather than silently skipping. + my %repo = ( x86_64 => $x86_64_repo, ppc64le => $ppc64le_repo ); + + # Discover the UNION of OS dirs across ALL arch repos. Anchoring discovery on one arch let an + # built for only the OTHER arch slip through unseen -- finalize then never cross-populated + # that cell and still exited 0 (PR #62 review). Every discovered must carry every arch below. my %os; - $os{ basename($_) } = 1 for grep { -d "$_/x86_64" } glob("$x86_64_repo/*"); - $os{ basename($_) } = 1 for grep { -d "$_/ppc64le" } glob("$ppc64le_repo/*"); + for my $a (@GENESIS_ARCHES) { + my $root = $repo{ $a->{arch} } + // die "FATAL: [finalize] no repo root configured for arch '$a->{arch}' (wire it in %repo)\n"; + $os{ basename($_) } = 1 for grep { -d "$_/$a->{arch}" } glob("$root/*"); + } + my $pairs = 0; for my $osdir (sort keys %os) { - my $x86dir = "$x86_64_repo/$osdir/x86_64"; - my $ppcdir = "$ppc64le_repo/$osdir/ppc64le"; - # Both arch peers must exist: in the CD both arches build every EL, so a one-arch is an - # incomplete input, not something to skip past (skipping would leave a cell without the - # foreign-arch genesis and still exit 0). Symmetric -- catches an x86_64-only AND a - # ppc64le-only . - die "FATAL: [finalize] $osdir: no x86_64 peer repo at $x86dir\n" - . " (both arches must build every EL before finalize)\n" if !-d $x86dir; - die "FATAL: [finalize] $osdir: no ppc64le peer repo at $ppcdir\n" - . " (both arches must build every EL before finalize)\n" if !-d $ppcdir; - # Require the expected inputs: each arch's build must have produced its OWN genesis rpm - # before finalize cross-populates them. Without this, a pair whose builds produced no - # genesis rpms would make finalize a silent no-op that still exits 0 (the bug this guards). - die "FATAL: [finalize] $osdir: no x86_64 xCAT-genesis-base rpm in $x86dir\n" - if !grep { !/\.src\.rpm$/ } glob("$x86dir/xCAT-genesis-base-x86_64-*.rpm"); - die "FATAL: [finalize] $osdir: no ppc64 xCAT-genesis-base rpm in $ppcdir\n" - if !grep { !/\.src\.rpm$/ } glob("$ppcdir/xCAT-genesis-base-ppc64-*.rpm"); - # xCAT collapses ppc/ppc64/ppc64le into tarch=ppc64, so the ppc genesis rpm is - # named xCAT-genesis-base-ppc64-*. Cross-copy both directions. - my $to_x86 = cross_copy_genesis($ppcdir, $x86dir, 'ppc64', $sign); - my $to_ppc = cross_copy_genesis($x86dir, $ppcdir, 'x86_64', $sign); - # Re-index+sign BOTH repos of the pair every finalize, not only when an rpm was copied this - # run. A crash after a prior run's copy+sign but before its createrepo leaves the genesis rpm - # on disk (so cross_copy_genesis now returns 0) yet ABSENT from repomd.xml -- which no - # signature gate catches. Re-indexing is cheap (these are tiny repos) and idempotent, and it - # heals that partial state; skipped only when no signer/indexer was injected. - if ($reindex) { $reindex->($x86dir); $reindex->($ppcdir); } - printf "[finalize] %s: %d ppc64 genesis -> x86_64, %d x86_64 genesis -> ppc64le\n", - $osdir, $to_x86, $to_ppc; + my %adir = map { $_->{arch} => "$repo{$_->{arch}}/$osdir/$_->{arch}" } @GENESIS_ARCHES; + # Pass 1 -- every arch peer repo dir must exist: a one-arch is an incomplete input, not + # something to skip past (skipping would leave a cell without a foreign-arch genesis and still + # exit 0). Checked before the rpm pass so a missing peer is reported as such. Symmetric across + # all arches (catches an x86_64-only AND a ppc64le-only ). + for my $a (@GENESIS_ARCHES) { + die "FATAL: [finalize] $osdir: no $a->{arch} peer repo at $adir{$a->{arch}}\n" + . " (every arch must build every EL before finalize)\n" if !-d $adir{ $a->{arch} }; + } + # Pass 2 -- every arch must have produced its OWN genesis rpm before finalize cross-populates + # them; otherwise a pair with no genesis rpms would make finalize a silent no-op that still + # exits 0. xCAT collapses ppc/ppc64le into tarch=ppc64, so match on each arch's tarch. + for my $a (@GENESIS_ARCHES) { + die "FATAL: [finalize] $osdir: no $a->{arch} xCAT-genesis-base rpm (tarch $a->{tarch}) in $adir{$a->{arch}}\n" + if !grep { !/\.src\.rpm$/ } glob("$adir{$a->{arch}}/xCAT-genesis-base-$a->{tarch}-*.rpm"); + } + # N-way cross-copy: put each arch's genesis into EVERY other arch's repo dir. + my @summary; + for my $src (@GENESIS_ARCHES) { + for my $dst (@GENESIS_ARCHES) { + next if $src->{arch} eq $dst->{arch}; + my $n = cross_copy_genesis($adir{$src->{arch}}, $adir{$dst->{arch}}, $src->{tarch}, $sign); + push @summary, "$n $src->{tarch} -> $dst->{arch}"; + } + } + # Re-index+sign EVERY arch repo of this each finalize, not only when an rpm was copied + # this run: a crash after a prior run's copy+sign but before its createrepo leaves the genesis + # rpm on disk (so cross_copy_genesis now returns 0) yet ABSENT from repomd.xml -- which no + # signature gate catches. Re-indexing is cheap (tiny repos) and idempotent, and heals that + # partial state; skipped only when no signer/indexer was injected. + if ($reindex) { $reindex->($adir{$_->{arch}}) for @GENESIS_ARCHES; } + print "[finalize] $osdir: " . join(', ', @summary) . "\n"; $pairs++; } - die "FATAL: --finalize-xcat-dep found no /x86_64 + /ppc64le repo pair under\n" + die "FATAL: --finalize-xcat-dep found no repo dir under any arch root\n" . " --x86_64-repo '$x86_64_repo'\n --ppc64le-repo '$ppc64le_repo'\n" if $pairs == 0; print_step('Finalize complete'); } diff --git a/mockbuild-all.pl b/mockbuild-all.pl index 0d3fd52..b091812 100755 --- a/mockbuild-all.pl +++ b/mockbuild-all.pl @@ -14,7 +14,7 @@ use POSIX qw(strftime); use FindBin qw($RealBin); use lib $RealBin; use MockBuildUtils qw(sh_quote print_step version_matches required_pkgs - read_manifest verify_repo_packages verify_repo_signature + read_manifest verify_repo_packages verify_repo_signature verify_rpm_signatures rpm_version rpm_release rpm_sigmd5 restamp_release_line cross_copy_genesis finalize_xcat_dep bump_dep_release_suffix build_mock_uniqueext); @@ -1196,6 +1196,37 @@ sub gpg_key_fingerprint { return $fprs[0]; } +# gpg_key_ids: all acceptable key ids (lowercased) for a signing key NAME -- the primary key id AND +# every subkey id, in both 16-hex (long) and 8-hex (short) forms. rpm header signatures report the +# signing SUBKEY id, so the per-rpm gate accepts any id belonging to the key rather than one exact +# fingerprint. Returns a hashref set (empty if the key can't be listed). +sub gpg_key_ids { + my ($keyname, $home) = @_; + my $h = ($home ne '') ? ' --homedir ' . sh_quote($home) : ''; + my $out = `gpg$h --with-colons --list-keys ${\ sh_quote($keyname)} 2>/dev/null` // ''; + my %ids; + for my $line (split /\n/, $out) { + my @f = split /:/, $line; + next unless ($f[0] // '') =~ /^(?:pub|sub)$/ && defined $f[4] && $f[4] ne ''; + my $id = $f[4]; + $ids{ lc $id } = 1; + $ids{ lc substr($id, -16) } = 1 if length($id) > 16; + $ids{ lc substr($id, -8) } = 1 if length($id) > 8; + } + return \%ids; +} + +# rpm_signer_keyid: the signing key id (lowercased hex) of a built rpm's header signature, or undef +# when the rpm is not signed. Reads the RSA (or DSA) header pgpsig and pulls the "Key ID " field. +sub rpm_signer_keyid { + my ($rpm) = @_; + for my $tag (qw(RSAHEADER DSAHEADER)) { + my $out = `rpm -qp --qf '%{$tag:pgpsig}' ${\ sh_quote($rpm)} 2>/dev/null` // ''; + return lc($1) if $out =~ /Key ID\s+([0-9A-Fa-f]+)/i; + } + return undef; +} + # repomd_observed_signer: run gpg --verify on the detached repomd signature and extract the identity # of the key that actually signed it, as a primary-key fingerprint (the last field of the VALIDSIG # status line). Returns '' when the .asc is absent or verification fails (both read as "unsigned"). @@ -1255,6 +1286,20 @@ sub verify_target_repo { my %exp_sig = ('repomd' => $exp_fpr); my %obs_sig = ('repomd' => repomd_observed_signer($asc, $repomd, $gpg_home)); push @problems, verify_repo_signature(\%exp_sig, \%obs_sig); + + # Per-rpm signature gate: a signed repomd over an unsigned or foreign-signed rpm still + # makes DNF reject that package at install time, so verify EVERY binary rpm -- not just the + # metadata -- is signed by this key (rpm reports the signing subkey id; accept any id of + # the key). Closes the "approves a repo DNF later rejects" gap (PR #62 review #4). + require_command('rpm'); + my $accept = gpg_key_ids($gpg_key_name, $gpg_home); + if (!%$accept) { + push @problems, "SIGKEY: cannot list key ids for '$gpg_key_name' to verify per-rpm signatures"; + } else { + my @rpm_sigs = map { [ basename($_), rpm_signer_keyid($_) ] } + grep { !/\.src\.rpm$/ } glob("$dir/*.rpm"); + push @problems, verify_rpm_signatures(\@rpm_sigs, $accept); + } } } elsif ($sig_required) { # Standalone --verify-repo advertises a signature check; with no keyring we cannot resolve the @@ -1269,7 +1314,7 @@ sub verify_target_repo { die "FATAL: repo INCOMPLETE for $tgt at $dir (" . scalar(@problems) . " problem(s))\n"; } print "[verify-repo] $tgt complete: " . scalar(@names) - . " required packages present + version-pinned in $dir\n"; + . " required packages present + version-pinned, every rpm signed, in $dir\n"; return 1; } diff --git a/t/mockbuild-all.t b/t/mockbuild-all.t index d2201f3..5b038de 100644 --- a/t/mockbuild-all.t +++ b/t/mockbuild-all.t @@ -12,8 +12,8 @@ use File::Path qw(make_path); use File::Basename qw(basename); use MockBuildUtils qw(required_pkgs version_matches rpm_sigmd5 rpm_version rpm_release rpm_is_signed restamp_release_line cross_copy_genesis finalize_xcat_dep read_manifest - verify_repo_packages verify_repo_signature bump_dep_release_suffix - build_mock_uniqueext); + verify_repo_packages verify_repo_signature verify_rpm_signatures + bump_dep_release_suffix build_mock_uniqueext); # Run a printing sub with STDOUT muted so its progress lines do not pollute TAP. sub quiet(&) { @@ -184,6 +184,11 @@ SPEC my $ok4 = eval { quiet { finalize_xcat_dep("$tmp4/x", "$tmp4/p") }; 1 }; ok(!$ok4, 'finalize dies when a ppc64le OS has no x86_64 peer repo (was silently skipped)'); like($@, qr/no x86_64 peer repo/, 'finalize error names the missing x86_64 peer'); + + # @GENESIS_ARCHES is the single source of truth for the cross-arch matrix (add arches there). + my %tarch = map { $_->{arch} => $_->{tarch} } @MockBuildUtils::GENESIS_ARCHES; + is($tarch{x86_64}, 'x86_64', 'GENESIS_ARCHES: x86_64 maps to tarch x86_64'); + is($tarch{ppc64le}, 'ppc64', 'GENESIS_ARCHES: ppc64le maps to xCAT tarch ppc64'); } # ---- restamp_release_line: CD --build-number Release stamping (PR #62 review point 1) ---------- @@ -334,6 +339,24 @@ is(rpm_release(tempdir(CLEANUP => 1), 'nonexistent-pkg'), undef, 'rpm_release is 'verify_repo_signature: mismatch reported as WRONGKEY : signed by , expected '); } +# ---- verify_rpm_signatures: EVERY rpm must be signed by an accepted key (PR #62 review #4) ----- +{ + my %accept = ( '4123c420cb60ad43' => 1, 'cb60ad43' => 1 ); # signing key's long + short id + + my @ok = verify_rpm_signatures( + [ ['a-1.0.rpm', 'cb60ad43'], ['b-2.0.rpm', '4123C420CB60AD43'] ], \%accept); + is_deeply(\@ok, [], 'verify_rpm_signatures: all rpms signed by an accepted key -> no problems (case-insensitive)'); + + my @uns = verify_rpm_signatures([ ['c-3.0.rpm', undef], ['d-4.0.rpm', ''] ], \%accept); + is(scalar(@uns), 2, 'verify_rpm_signatures: undef and empty key id both flagged'); + like($uns[0], qr/^UNSIGNED rpm c-3\.0\.rpm$/, 'verify_rpm_signatures: unsigned rpm reported by name'); + + my @wrong = verify_rpm_signatures([ ['e-5.0.rpm', 'deadbeef'] ], \%accept); + is(scalar(@wrong), 1, 'verify_rpm_signatures: a foreign-signed rpm yields one problem'); + like($wrong[0], qr/^WRONGKEY rpm e-5\.0\.rpm: signed by deadbeef, expected one of\b/, + 'verify_rpm_signatures: wrong key reported as WRONGKEY rpm : signed by , expected one of ...'); +} + # ---- build_mock_uniqueext: distinct per target so concurrent mock roots never collide --------- # (PR #62 review) A long (timestamp) run id must not tail-truncate away the leading EL/arch token: # for the 7-char "ppc64le" arch that dropped the EL digit, so alma+epel-{8,9,10}-ppc64le collapsed to