Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix possible package corruption on --delsign/resign/addsign #3479

Merged
merged 3 commits into from
Dec 3, 2024

Conversation

pmatilai
Copy link
Member

Details in commit messages, the short story is that we could corrupt packages on signing operations due to miscalculations when unexpected stuff happened. That unexpected stuff in this case was misplaced IMA signatures but it's not specific to those.

Also add explicit tests for --delsign/--delfilesign behavior wrt IMA signatures.

Fixes: #3469

RH signing server apparently does IMA signing after the normal signing
has already been done, and in doing so places the IMA signatures outside
the immutable region. This causes us to do all manner of wrong things,
corrupting the package on --delsign and whatnot.

rpmsign of course wont create such a signature by itself, so we need a
pre-built "crafted" package for the purpose with a specially built
rpmsign library: move the includeFileSignatures() call in rpmSign()
in sign/rpmgensig.cc right after the headerReload() call, and filesign
the vanilla tests/data/RPMS/hello-2.0-1.x86_64.rpm package with the
--fskpath=/data/keys/privkey.pem like in the ima test above this.
@pmatilai pmatilai requested a review from a team as a code owner November 29, 2024 11:09
@pmatilai pmatilai requested review from ffesti and removed request for a team November 29, 2024 11:09
Make sure we don't overrun the original signature header when
adjusting reserved size. Fixes a brainfart introduced in commit
be950ea: the count reservation
size is relative to the size of the new header, obviously.

Another crucial difference is that when considering whether we can
transplant the new signature header in the originals place we need
to consider the real on-disk signature, not the size of its
immutable region. The immutable region can be much much smaller than
the physical header if eg the IMA signatures are misplaced outside it,
making our calculations way off.

Fixes: rpm-software-management#3469
Normally IMA signatures should only be deleted with an explicit
rpmsign --delfilesign, but in case the are misplaced outside the
immutable region they get thrown out by rpmsign. This is expected
and desired behavior, it's simply the wrong place to place to put
them and not something we want to encourage in any way.
@ffesti ffesti merged commit 6232bd3 into rpm-software-management:master Dec 3, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

rpmsign --delsign / --addsign regression can corrupt packages in rpm >= 4.18.1
2 participants