#2547 F34 Change: Signed RPM Contents
Closed: Rejected 3 years ago by ngompa. Opened 3 years ago by bcotton.

We want to add signatures to individual files that are part of shipped RPMs. These signatures will use the Linux IMA (Integrity Measurement Architecture) scheme, which means they can be used to enforce runtime policies to ensure execution of only trusted files.


Let me preface this with: I am unsure whether I understand this proposal correctly. Because to me, this proposal seems like massive overkill for the vast majority of users. And signing every file in every RPM and the size increase that incurs (however large it may be, people on the devil list found between >+2% for rpmdb and ~+100% for RPM files) does not justify the benefits (for maybe a handful of users?) for me. -1 to the proposal as-is.

To also leave constructive feedback here: I can see the benefit of signing individual files in an opt-in fashion, e.g. for doing computation in "trusted environments". Would it be possible to ship the signatures separately (maybe in a separate -signatures RPM, similar to -debuginfo?) so that they don't end up in rpmdb and on all users' systems without a way to "opt out"? Generating and shipping signatures for all RPM contents provided by Fedora by default just doesn't make sense to me.

To also leave constructive feedback here: I can see the benefit of signing individual files in an opt-in fashion, e.g. for doing computation in "trusted environments". Would it be possible to ship the signatures separately (maybe in a separate -signatures RPM, similar to -debuginfo?) so that they don't end up in rpmdb and on all users' systems without a way to "opt out"? Generating and shipping signatures for all RPM contents provided by Fedora by default just doesn't make sense to me.

Please don't leave feedback in the ticket. Respond to the discussion thread on fedora-devel so all of the conversation remains in one place.

Please don't leave feedback in the ticket. Respond to the discussion thread on fedora-devel so all of the conversation remains in one place.

Everything I wrote has already been said on the devel thread, but it has not been responded to by the Change owners (or others). In part I left my -1 vote to block automatic approval until this is resolved (which is why I wrote "-1 to the proposal as-is").

Metadata Update from @sgallagh:
- Issue tagged with: meeting

3 years ago

The change has been updated with actual details, the update was mentioned in the devel thread.

Tagging as fast track, as agreed on the meeting. That means, if this receives +7 and no negative votes, it is immediately approved.

Currently, I'd consider the previous vote voided, given the changes in the proposal. @decathorpe if you'd still like to cast a negative vote, please do so again explicitly.

Metadata Update from @churchyard:
- Issue untagged with: meeting
- Issue tagged with: fast track

3 years ago

+1, and many thanks to @puiterwijk for the added details, particularly the analysis of the real-world storage impact and the explanation of the benefit in terms of trust and remote attestation.

Currently, I'd consider the previous vote voided, given the changes in the proposal.

For the record, I agree. I this must be a completely fresh vote.

Thank you for updating the proposal, it addresses my concerns, so long as we make sure that rpm-plugin-ima is not installed by default.

I change my vote to +1.

After reading the devel thread and Patrick's update of the change proposal, I vote +1

FTR I am not voting as I'd like to give the community some time to review the proposal. I'd like to ask the seventh person here to at least wait a couple days.

Not sure if this is related, but could somebody investigate Backwards-incompatible RPM format change in Fedora 34? to verify that this issue is not caused by "Signed RPM Contents"?

Consider me -1 until it is clarified.

Yup, the file signing for larger packages causes the signature header to grow beyond expectations of the older rpm versions: https://lists.fedoraproject.org/archives/list/devel@lists.fedoraproject.org/message/QKYXSFAVOICXRSW6AXFUDBUZOD5Y5HXK/

Such packages are unreadable to every single rpm version prior to 4.16 out there, including but not limited to released RHEL etc.

@pmatilai Says:

the bad news is that this signing makes packages incompatible with ALL older releases

So I am indeed -1. This was never mentioned in the proposal :(

Metadata Update from @churchyard:
- Issue untagged with: fast track
- Issue tagged with: meeting

3 years ago

@pmatilai thank you for investigating.

This new info definitely changes my vote to -1.

Metadata Update from @decathorpe:
- Issue untagged with: meeting

3 years ago

And it should also be disabled in Fedora infra ASAP.

Metadata Update from @decathorpe:
- Issue tagged with: meeting

3 years ago

As I said in my earlier my earlier message regarding this feature on fedora devel [1], before any mass deployment, the file signatures should be made to use a saner encoding to limit the overhead a bit. Using plain hex encoding for the binary data is nothing but terrible waste of precious space.

[1] https://lists.fedoraproject.org/archives/list/devel@lists.fedoraproject.org/message/H4RZ2EM4ZXZN5VTLFB4VLB57VXVMGERT/

Edit: link added

My vote changes to -1 as well.

I think this needs a documented trust model, to determine if we need Fedora policy and infrastructure changes which block packagers from create file conflicts. A revocation facility also appears to be missing.

@pmatilai So as I understand it from reading that patch... the problem is that the total set of signatures may not exceed 64kiB of space in the RPM header in order to be compatible with older versions of RPM.

The signature hash should be of a fixed size as currently written (right?), so we could at least identify the maximum number of files that can be present in a single signed RPM and require that we separate RPMs into smaller subpackages if they would exceed this number. Also there may be ways to reduce the space consumed by the signatures (traditional compression?).

Do I have that right?

require that we separate RPMs into smaller subpackages if they would exceed this number

That sounds really incontinent.

BTW Let's have technical discussion on the list instead please.

Splitting to sub-packages is not at all a feasible nor sensible thing to do for this, the patch to bump the allowed signature header size would simply have to be backported to any relevant distros.
That would be necessary no matter how the data is encoded in the header, 64kb is just way too tiny.

@pmatilai So as I understand it from reading that patch... the problem is that the total set of signatures may not exceed 64kiB of space in the RPM header in order to be compatible with older versions of RPM.

The problem is that when the file signatures were moved over from the header into the sigheader, the patch said it would allow a 64MB header. Due to a typo, it was 64KB.
People have hit this just by using a large enough GPG key if I understood the PR (https://github.com/rpm-software-management/rpm/pull/1252) correctly.

That PR fixed it from the bug 64KB to the documented 64MB max size, and it's a one-line patch that cherry-picks neatly on top of e.g. the Fedora 32 and RHEL8 RPM builds.

The signature hash should be of a fixed size as currently written (right?), so we could at least identify the maximum number of files that can be present in a single signed RPM and require that we separate RPMs into smaller subpackages if they would exceed this number. Also there may be ways to reduce the space consumed by the signatures (traditional compression?).

The space consumed can be reduced quite a bit by using base64 encoding instead of hex encoding, but limiting the number of files is in my honest opinion not a good workaround.
I think distributions should just backport the one-line patch that fixes the documented behaviour.

Do I have that right?

OK, I was trying to come up with a stopgap solution while that patch worked its way into being backported to older RPM releases. I don't think it's wise for us to ship this change in Fedora before that patch is applied to a released package in at least F32+ and RHEL 8.

I guess I will reluctantly vote -1 for this Change in F34.

I agree. Whichever way this issue is solved in the future, it is definitely way too late for F34 now. The proposal was submitted late, and the mass rebuild was scheduled to have started two days ago, and I'd rather not rush anything in at the last possible second.

I don't see the issue with the el8 support as any different to when the zstd compression support landed in Fedora. When the Fedora release went out with that it initially wasn't supported in el8 and that support was back ported.

It's very different. It's one thing for RPM to say it can't install it. It's quite another to say it can't even read it. That means that people who mirror Fedora content on RHEL based systems would not be able to do so. Even with the Zstd change, that part always worked.

It's very different. It's one thing for RPM to say it can't install it. It's quite another to say it can't even read it. That means that people who mirror Fedora content on RHEL based systems would not be able to do so. Even with the Zstd change, that part always worked.

If people just mirror the contents of the Fedora created repos from mirror manager it will work as it has in the past because it's just syncing and serving the pre created repo files, not generating them with createrepo, if people regenerate with createrepo there will currently be a problem as mirror manager would discard this mirror anyway (even now) as the meta data wouldn't exactly match the expected data and it would be considered stale data and hence not be served.

With @pbrobinson 's latest comments, I'm withdrawing my -1 and going to 0 for now.

Due to the mass-rebuild being delayed for other reasons, we have a few days to figure out if this is really going to be an issue (and also to get a fix for F32+ proposed and pushed stable).

If indeed a standard mirror of the repos will continue to work, I'm less concerned about the RHEL 8 situation.

If indeed a standard mirror of the repos will continue to work, I'm less concerned about the RHEL 8 situation.

The mirror system is a rsync the files and serve them out over http/https/ftp (likely other protocols), I'm sure there's a lot of non Fedora/RHEL Linux mirrors, AWS runs a mirror on something, I don't know the exact details but I suspect that a mirror could in fact run on MS/traditional unix platform, the redirect/closest logic runs in Fedora infra and the mirrors are purely a platform to retrieve files from.

For reference the bug is fixed in this commit upstream:
https://github.com/rpm-software-management/rpm/pull/1252

Fixing this commit:
https://github.com/rpm-software-management/rpm/commit/f558e886050c4e98f6cdde391df679a411b3f62c

The fix is very straight forward, we're working to get it into el8 and it doesn't just affect IMA.

I don't believe it would be in el7, I don't have means to quickly test unfortunaltely, but then there's other restrictions of el7 for general use for later rpms as I don't believe it supports zstd.

I don't see the issue with the el8 support as any different to when the zstd compression support landed in Fedora.

Like @ngompa said, the immediate effect here is far worse. The zstd episode was bad enough, people on the EL side of things would be rather upset if that were to happen again.

The fix is very straight forward, we're working to get it into el8 and it doesn't just affect IMA.

IMA is the only thing in released rpm versions that can cause the signature header to grow beyond 64k. So it is "just IMA" really. Whether it makes any difference is a whole other question, just to set the record straight.

If indeed a standard mirror of the repos will continue to work, I'm less concerned about the RHEL 8 situation.

Sorry, when I mean mirror, I mean mirror with something like Pulp (part of Red Hat Satellite), which parses packages and regenerates the metadata. This is extremely common in enterprise use-cases for mirroring (heck I'm doing it at work). That's why, to me, this is extremely serious.

We mark this as fasttrack, the need for quick discussion on fedora-devel is highlighted. I asked some questions, other people too, and it's 4 days later with no answer. Where did all the haste go?

We mark this as fasttrack, the need for quick discussion on fedora-devel is highlighted. I asked some questions, other people too, and it's 4 days later with no answer. Where did all the haste go?

I'm catching up, for me a combination of a LOT of meetings and a weekend.

Fasttracking such a controversial decision that needs a lot of clarification (just see how some FESCo members changed their vote more than once) and that is far from having community consensus on the mailing list does not sound like a good idea to me. The feature is clearly late, but that means it makes more sense to push it to F35 or to drop it entirely than to rush it into F34 at all costs.

And why the heck was this apparently enabled in Rawhide without the Change being approved? That is unacceptable, especially considering the breakage it caused. (RPMs in F34 must be compatible with the RPM in F32 because we support upgrades from Fn to Fn+2.) IMHO, this behavior must not be rewarded, so please just reject this Change.

Fasttracking such a controversial decision that needs a lot of clarification (just see how some FESCo members changed their vote more than once) and that is far from having community consensus on the mailing list does not sound like a good idea to me. The feature is clearly late, but that means it makes more sense to push it to F35 or to drop it entirely than to rush it into F34 at all costs.

I totally agree.

And why the heck was this apparently enabled in Rawhide without the Change being approved?

See https://lists.fedoraproject.org/archives/list/devel@lists.fedoraproject.org/message/OMUF2MGUTSHYWUDCTG52RKVS2UK5XVLH/

The enable-before-approval was not approved (nor known) by FESCo so your criticism (while understandable) is misplaced here.

That is unacceptable, especially considering the breakage it caused. ... IMHO, this behavior must not be rewarded, so please just reject this Change.

I agree that this is not acceptable. I believe people who enabled this now see that as well. But prior errors in judgement are not a good reason to outright reject something. I believe others have risen very valid points in the devel discussion against this proposal, but let's not reject it as a "punishment for unacceptable behavior" -- that would not ba an acceptable conduct.

Fasttracking such a controversial decision that needs a lot of clarification (just see how some FESCo members changed their vote more than once) and that is far from having community consensus on the mailing list does not sound like a good idea to me. The feature is clearly late, but that means it makes more sense to push it to F35 or to drop it entirely than to rush it into F34 at all costs.

We were not trying to fast track anything, all our testing locally in F-33 and rawhide showed us it was ready. We missed a one line bug in older releases.

And why the heck was this apparently enabled in Rawhide without the Change being approved? That is unacceptable, especially considering the breakage it caused. (RPMs in F34 must be compatible with the RPM in F32 because we support upgrades from Fn to Fn+2.) IMHO, this behavior must not be rewarded, so please just reject this Change.

Ultimately we wanted wider testing to ensure it was ready and we couldn't do that in staging as outlined by Kevin. I feel that enabling as we did was the right thing to do, if we hadn't FESCo would have voted for it, and with the voting after we provided the requested more details in the change, right before the mass rebuild began and it would have been enabled right before the mass rebuild began and the RHEL-8 bug wouldn't have been discovered until after the mass rebuild has completed.

I feel that enabling as we did was the right thing to do

Just because it worked out this time (barely) does not make this OK in general. Next time, please ask FESCo or, even better, announce such infrastructure changes on the mailing lists.

Enabling this silently was certainly not a good thing to do. And yes, this was proposed as a fast track (arguably by FESCo members, not the change owners).

Just because it worked out this time (barely) does not make this OK in general. Next time, please ask FESCo or, even better, announce such infrastructure changes on the mailing lists.

Believe me that was not intentional, it was a completely missed oversight by people that are trying to do a lot.

I feel the assumption we're guilty of bad intent rather than something rather than it being an oversight (I had honestly thought I had sent a mail out to devel) is unfair

I don't think we understand each other. The negative response you see was for saying it was a good thing.

I think we all agree here, but we talk past each other: Testing it early was a good idea, but it should have been communicated. Enabling it without communication was wrong, but it was not done with bad intentions. What's done is done and it cannot be undone, let's all please move on.

This Change was rejected in today's FESCo meeting.

AGREED: This change is rejected for F34, please resubmit again for the F35 cycle, ideally amended wrt the received feedback (+7, 0, 0)

Announced: https://lists.fedoraproject.org/archives/list/devel@lists.fedoraproject.org/message/CJKN5LTRBDQE53THF77UNWQ2DVBNFNVZ/

Metadata Update from @ngompa:
- Issue untagged with: meeting
- Issue close_status updated to: Rejected
- Issue status updated to: Closed (was: Open)

3 years ago

Log in to comment on this ticket.

Metadata