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

build: Add support for propagating RPM IMA signatures #3610

Merged
merged 3 commits into from
May 6, 2022

Conversation

cgwalters
Copy link
Member

The alignment between ostree and IMA is actually quite good. IMA
signatures are just extended attributes, and ostree has first-class
native support for those.

RPM represents IMA signatures in an ad-hoc way, and then a plugin
pulls bits from the header and does the lsetxattr call.

One thing that ostree is pretty good at is that one can synthesize
data in memory into the final ostree commit, without actually needing
it to hit the physical filesystem at build time. This is crucial
for SELinux, where we want to be able to set security contexts
in the resulting build without actually requiring matching SELinux
policy on the build server.

IMA has similar issues; we don't want to blindly use rpm-plugin-ima
at build time. That would potentially conflict with IMA policy
set up for the host system.

Add a treefile option ima: true that if set, causes the importer
to propagate the RPM IMA signatures into the proper security.ima
extended attribute.

Now, while working on this I made a fundamental realization that
all along, we should have supported committing bare-user checkouts
directly into archive repositories.

This is how it works in coreos-assembler today. It'd make a ton
of sense to add something like ostree commit --from-bare-user that
would honor the ostree.usermeta xattr. (Tempting to do it by default
but the compatibility hazard seems high)

So for now...hack in code in our commit phase to do this.

Note there was previous work here to automatically translate the
user.ima xattr to security.ima (because it was apparently easier
than patching rpm-ostree core?). This code kind of obsoletes that,
but on the other hand one could today be writing user.ima attributes
during a build process for things that are not from RPM, and that's
kind of useful to continue to support. Though, I think we should
handle that in a more principled way (that's a bigger topic).

Note that Fedora and CentOS do not ship native IMA signatures by
default. And in fact, there was a huge mess around the representation
of IMA signatures in rpm - so patches to fix that landed in C9S/RHEL9
but not in at least Fedora 35 I believe.

Which means: actually using this functionality for e.g. RHEL9 IMA
signatures requires using rpm-ostree (and really, librpm) from
a RHEL9 host system - it won't work today to "cross build" as
we do with coreos-assembler.

Also, testing this out with RHEL9 content I needed to do:

remove-from-packages:
  - - systemd-libs
    - /usr/share/licenses/systemd/LICENSE.LGPL2.1

because there are two copies of that file, but they apparently
have separate IMA signatures and only when IMA is enabled, ostree
(correctly) reports them as conflicting.

Closes: #3547

@cgwalters cgwalters force-pushed the rhel9-ima branch 2 times, most recently from 34c5571 to 6c957f0 Compare April 18, 2022 13:31
@cgwalters
Copy link
Member Author

Ah I see, I messed up the file capability handling in this. Hooray for CI correctly failing.

Fixing that up then would conflict with #3606 so I rebased on that.

@cgwalters
Copy link
Member Author

Another big further detail on this: AIUI today, because osbuild is only using the postprocess+commit phases of rpm-ostree, this wouldn't work for them. I hope osbuild can change to actually translate blueprints into manifests, use our lockfile implementation and our package caching. Then this PR would work for them.

As is though I think what they'd have to do is use rpm-plugin-ima in their build process, with all the accompanying downsides mentioned above.

@cgwalters
Copy link
Member Author

Also xref ostreedev/ostree-rs-ext#283

@cgwalters
Copy link
Member Author

cgwalters commented Apr 19, 2022

[2022-04-19T15:38:59.566Z] FAIL: layering-non-root-caps
[2022-04-19T15:38:59.566Z]    layering-non-root-caps: + [[ root != root ]]
[2022-04-19T15:38:59.566Z]    layering-non-root-caps: + check_fcap /etc/nrc-link.conf ''
[2022-04-19T15:38:59.566Z]    layering-non-root-caps: ++ vm_cmd getcap /etc/nrc-link.conf
[2022-04-19T15:38:59.566Z]    layering-non-root-caps: ++ ssh -o User=root -o ControlMaster=auto -o ControlPath=/dev/shm/ssh-vmcheck-1650382693698654778.sock -o ControlPersist=yes -F /tmp/test.6qg10h/ssh-config vmcheck getcap /etc/nrc-link.conf
[2022-04-19T15:38:59.566Z]    layering-non-root-caps: + local fcap=
[2022-04-19T15:38:59.566Z]    layering-non-root-caps: + local fcap=
[2022-04-19T15:38:59.566Z]    layering-non-root-caps: + fcap=
[2022-04-19T15:38:59.566Z]    layering-non-root-caps: + test -z ''
[2022-04-19T15:38:59.566Z]    layering-non-root-caps: + test -n ''
[2022-04-19T15:38:59.566Z]    layering-non-root-caps: + return
[2022-04-19T15:38:59.566Z]    layering-non-root-caps: + echo 'ok correct user/group and fcaps'
[2022-04-19T15:38:59.566Z]    layering-non-root-caps: ok correct user/group and fcaps
[2022-04-19T15:38:59.566Z]    layering-non-root-caps: + vm_cmd ostree fsck
[2022-04-19T15:38:59.566Z]    layering-non-root-caps: + ssh -o User=root -o ControlMaster=auto o ControlPath=/dev/shm/ssh-vmcheck-1650382693698654778.sock -o ControlPersist=yes -F /tmp/test.6qg10h/ssh-config vmcheck ostree fsck
[2022-04-19T15:38:59.566Z]    layering-non-root-caps: Validating refs...
[2022-04-19T15:38:59.566Z]    layering-non-root-caps: Validating refs in collections...
[2022-04-19T15:38:59.566Z]    layering-non-root-caps: Enumerating objects...
[2022-04-19T15:38:59.566Z]    layering-non-root-caps: Verifying content integrity of 3 commit objects...
[2022-04-19T15:38:59.566Z]    layering-non-root-caps: fsck objects (1/22108) 0%
[2022-04-19T15:38:59.566Z]    layering-non-root-caps: error: In commits 74bfe710c530a920f869ec4f88c0e1baf6384967b1009d359807b9e48984ef4d, cbf807bd68248df26504a7fe0c39ab8fbb36fbd038fe08407c4a3ac7725ff317: fsck content object 337e8a1826efd4737bcc47b0d3d3c5b363f44c110bfa234d77bd6b9e08bb82bd: Corrupted file object; checksum expected='337e8a1826efd4737bcc47b0d3d3c5b363f44c110bfa234d77bd6b9e08bb82bd' actual='99bcde967c5601490ea89d22b5076b279d3fad4b2ad74b42f6ffe88f00755478'

Hmm, exciting.

@cgwalters
Copy link
Member Author

Well, this one is clearly nontrivial and could use a bit more baking time, so let's cut 2022.8 after landing #3619
and try this after.

@7flying
Copy link

7flying commented Apr 28, 2022

@cgwalters I'm trying to replicate/test this to get a better understanding on what's the current status on getting IMA operational after these changes (what needs to be done, where can we help), but I'm running into some issues trying to replicate this on RHEL9.

Can you give me more info on how were you testing this? Thanks.

@cgwalters
Copy link
Member Author

cgwalters commented May 2, 2022

I completely hacked up github.com/openshift/os until it built a minimal RHCOS-from-RHEL9, and used coreos-assembler.

That said I think we can pretty easily add testing here that uses Fedora or C9S content but just manually IMA signs an RPM, or perhaps better use an external "fixture" with pre-signed RPMs.

@cgwalters
Copy link
Member Author

cgwalters commented May 5, 2022

OK I'm not reproducing the CI failure locally. I'm pretty sure it's related to the "automatically bridge bare user xattrs" bits, but going to try to break this patch up into separate parts.

Prep for further work.
This is actually what we should have been doing from the start
for all metadata actually.  It is a key to ultimately fully operating
as non-root.

This is specifically prep for having the importer pull IMA signatures
from RPM headers, translate them into the standard `security.ima`
value, but store them inside a `bare-user` repo in the `user.ostreemeta`
xattr.
The alignment between ostree and IMA is actually quite good.  IMA
signatures are just extended attributes, and ostree has first-class
native support for those.

RPM represents IMA signatures in an ad-hoc way, and then a plugin
pulls bits from the header and does the `lsetxattr` call.

One thing that ostree is pretty good at is that one can synthesize
data in memory into the final ostree commit, without actually needing
it to hit the physical filesystem at build time.  This is crucial
for SELinux, where we want to be able to set security contexts
in the resulting build without actually requiring matching SELinux
policy on the *build* server.

IMA has similar issues; we don't want to blindly use `rpm-plugin-ima`
at *build* time.  That would potentially conflict with IMA policy
set up for the host system.

Add a treefile option `ima: true` that if set, causes the importer
to propagate the RPM IMA signatures into the proper `security.ima`
extended attribute.

Now, while working on this I made a fundamental realization that
all along, we should have supported committing `bare-user` checkouts
directly into `archive` repositories.

This is how it works in coreos-assembler today.  It'd make a ton
of sense to add something like `ostree commit --from-bare-user` that
would honor the `ostree.usermeta` xattr.  (Tempting to do it by default
but the compatibility hazard seems high)

So for now...hack in code in our commit phase to do this.

Note there was previous work here to automatically translate the
`user.ima` xattr to `security.ima` (because it was apparently easier
than patching rpm-ostree core?).  This code kind of obsoletes that,
but on the other hand one could today be writing `user.ima` attributes
during a build process for things that are not from RPM, and that's
kind of useful to continue to support.  Though, I think we should
handle that in a more principled way (that's a bigger topic).

Note that Fedora and CentOS do not ship native IMA signatures by
default.  And in fact, there was a *huge* mess around the representation
of IMA signatures in rpm - so patches to fix that landed in C9S/RHEL9
but not in at least Fedora 35 I believe.

Which means: actually using this functionality for e.g. RHEL9 IMA
signatures requires using rpm-ostree (and really, librpm) from
a RHEL9 host system - it won't work today to "cross build" as
we do with coreos-assembler.

Also, testing this out with RHEL9 content I needed to do:
```
remove-from-packages:
  - - systemd-libs
    - /usr/share/licenses/systemd/LICENSE.LGPL2.1
```

because there are two copies of that file, but they apparently
have separate IMA signatures and only when IMA is enabled, ostree
(correctly) reports them as conflicting.

Closes: coreos#3547
@cgwalters
Copy link
Member Author

OK so that was fun.

[2022-04-19T15:38:59.566Z] layering-non-root-caps: error: In commits 74bfe710c530a920f869ec4f88c0e1baf6384967b1009d359807b9e48984ef4d, cbf807bd68248df26504a7fe0c39ab8fbb36fbd038fe08407c4a3ac7725ff317: fsck content object 337e8a1826efd4737bcc47b0d3d3c5b363f44c110bfa234d77bd6b9e08bb82bd: Corrupted file object; checksum expected='337e8a1826efd4737bcc47b0d3d3c5b363f44c110bfa234d77bd6b9e08bb82bd' actual='99bcde967c5601490ea89d22b5076b279d3fad4b2ad74b42f6ffe88f00755478'

Basically what was going on is is that we were ending up with two security.capability xattrs because there was one in the user.ostreemeta and we were picking up the on-disk one. We need to make these two cases exclusive for sanity.

And the reason I wasn't reproducing this is that in order to trigger the bug you need to use the patched rpm-ostree at build time, not (just) client side.

The actual cause of the apparent corruption is that ostree happily serializes duplicate xattrs into the object (which is a bug), but on disk there can be only one. So when we reconstruct the canonical object in memory from the disk state there appears to be drift.

Anyways, for now we continue to use the "label at commit time" bits of ostree, so we skip security.selinux in the bare-user objects. I think longer term though we should try to defer to that actually.

Copy link
Member

@jmarrero jmarrero left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

support passing through RPM IMA signatures into ostree commit
3 participants