Skip to content

Rework SELinux labeling more#397

Merged
cgwalters merged 1 commit intobootc-dev:mainfrom
cgwalters:more-labeling
Mar 18, 2024
Merged

Rework SELinux labeling more#397
cgwalters merged 1 commit intobootc-dev:mainfrom
cgwalters:more-labeling

Conversation

@cgwalters
Copy link
Copy Markdown
Collaborator

First, in the install code, acquire a proper policy object.

Add helpers for writing files/directories that take a policy object and operate solely using fd-relative operations and don't fork off helper processes.

This is a notable cleanup because we don't need to juggle absolute file paths and fds, which avoids a lot of confusion. Our usage of a wrapper for the cap-std-ext atomic write API for generating files ensures that if the file is present, it will always have the correct label without any race conditions.

Change the one place we now call chcon as a helper process to be an explicit recursive selinux relabeling. In the future we should switch to using a direct API instead of forking off /usr/bin/chcon - then everything would be fd-relative.

@openshift-ci
Copy link
Copy Markdown

openshift-ci Bot commented Mar 17, 2024

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

@github-actions github-actions Bot added the area/install Issues related to `bootc install` label Mar 17, 2024
@cgwalters cgwalters force-pushed the more-labeling branch 2 times, most recently from 943f601 to d4f4311 Compare March 17, 2024 22:05
@cgwalters cgwalters marked this pull request as ready for review March 17, 2024 22:12
@cgwalters cgwalters force-pushed the more-labeling branch 2 times, most recently from 560e6b4 to 0cbe32e Compare March 18, 2024 00:08
@cgwalters
Copy link
Copy Markdown
Collaborator Author

Ah OK right,

ERROR Failed to look up context for "./boot.1.1": No data available (os error 61)

We have more forcible labeling to do here. Conceptually this one should be ostree side probably.

@cgwalters
Copy link
Copy Markdown
Collaborator Author

Ohh yeah, looking at the warning output there's quite a bit of work in ostree to handle this. Updated ostreedev/ostree#2804 (comment)

Maybe what we need here for all of that is just a single pass "relabel everything that isn't a deployment with usr_t".

Although

No SELinux label found for: "./deploy/default/deploy/f512f95348cbaa175f3361dea0f09ccaadd089909dacc3ffb0c0b7e602f4f2dc.0/etc/tmpfiles.d/bootc-root-ssh.conf"

That one should have worked with this. But not sure about blocking on that.

Copy link
Copy Markdown
Contributor

@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

Copy link
Copy Markdown
Collaborator

@jeckersb jeckersb left a comment

Choose a reason for hiding this comment

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

Nice improvement! 👍

Comment thread lib/src/install.rs Outdated
@cgwalters cgwalters force-pushed the more-labeling branch 3 times, most recently from 2362ace to 9ba1f47 Compare March 18, 2024 20:58
@cgwalters
Copy link
Copy Markdown
Collaborator Author

OK yeah, I'm going to have to set up a dev/test environment for this "install from host with selinux=0" case, but...I think this latest push should get us a lot closer to working in that path.

@cgwalters cgwalters force-pushed the more-labeling branch 2 times, most recently from 0f914c1 to 0a5f152 Compare March 18, 2024 22:29
First, in the install code, acquire a proper policy object.

Add helpers for writing files/directories that take a policy
object and operate *solely* using fd-relative operations and
don't fork off helper processes.

This is a notable cleanup because we don't need to juggle
absolute file paths *and* fds, which avoids a lot of confusion.
Our usage of a wrapper for the cap-std-ext atomic write API
for generating files ensures that if the file is present,
it will always have the correct label without any race conditions.

Change the one place we now call `chcon` as a helper process
to be an explicit recursive selinux relabeling.  In the
future we should switch to using a direct API instead of
forking off `/usr/bin/chcon` - then everything would be fd-relative.

Signed-off-by: Colin Walters <walters@verbum.org>
@cgwalters
Copy link
Copy Markdown
Collaborator Author

OK cool! Now we're just down to

No SELinux label found for: "./deploy/default/deploy/6f41b63072258824399f24f39c499e5ce22dd8e5d6942917b2b21b11f4ab5463.0/.ostree.cfs"

Which should be pretty tractable to fix in ostree. (In practice it's not really relevant I think because policy isn't loaded until post-switchroot)

@cgwalters cgwalters enabled auto-merge March 18, 2024 23:31
@cgwalters cgwalters merged commit d039f26 into bootc-dev:main Mar 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/install Issues related to `bootc install`

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants