stages/files: relabel /var/home and /var/roothome #632
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Can one of the admins verify this patch? |
The behaviour of how `restorecon` handles symlinks changed between RHCOS and FCOS. More specifically, `restorecon` will follow symlinks that are part of a given path, but not if the target path is a symlink itself. On OSTree-based systems, `/home` and `/root` are just symlinks, so the newer `restorecon` wasn't actually relabeling anything under there. Add the real paths to the list of dirs to relabel and add `-i` so that it's not a fatal error on non-OSTree-based systems. Closes: coreos/fedora-coreos-config#2
jlebon
force-pushed
the
pr/fcos-relabel
branch
from
500a4ef to
45b7518
Compare
cgwalters
reviewed
Sep 14, 2018
| @@ -41,6 +41,9 @@ func (s *stage) createPasswd(config types.Config) error { | |||
| "/etc/.pwd.lock", | |||
| "/home", | |||
| "/root", | |||
| // for OSTree-based systems (newer restorecon doesn't follow symlinks) | |||
| "/var/home", | |||
| "/var/roothome", | |||
There was a problem hiding this comment.
Maybe also add /var/usrlocal and /var/opt? But this LGTM too.
See also rhinstaller/anaconda@43a8455
There was a problem hiding this comment.
Those should be fine, no? Any files/dirs that users add in there through their Ignition configs will be marked for relabeling as Ignition creates them.
There was a problem hiding this comment.
I'm thinking though of things that drop files in /usr/local and /opt.
There was a problem hiding this comment.
I mentioned this in the commit message:
More specifically,
restoreconwill follow symlinks that are
part of a given path, but not if the target path is a symlink itself.
So e.g. any files dropped under /usr/local and /opt will be relabeled properly. (I also just double checked this on FCOS to be triple sure).
There was a problem hiding this comment.
Ah sorry, missed that. LGTM then!
ajeddeloh
approved these changes
Sep 14, 2018
|
ok to test |
|
good to merge |
jlebon
added a commit
to jlebon/ignition
that referenced
this pull request
Dec 19, 2019
On OSTree systems, those are just symlinks in the deployment root. If they're not labeled correctly already, it signals an issue with the disk creation process itself (and might also signal that the next time a deployment root is created, it'll also be mislabeled). Anyway, even on non-OSTree systems, it seems reasonable to expect that `/home` and `/root` at least already exist and don't need to be created (and thus don't need to be relabeld). It's possible that [fixing `getxattr` without a policy loaded](coreos/fedora-coreos-config#245 (comment)) would also fix this, since `setfiles` would see that the symlinks were already correctly labeled. In effect, this is completing what coreos#632 started. Closes: coreos/fedora-coreos-tracker#339
jlebon
added a commit
to jlebon/ignition
that referenced
this pull request
Dec 20, 2019
On OSTree systems, those are just symlinks in the deployment root. If they're not labeled correctly already, it signals an issue with the disk creation process itself (and might also signal that the next time a deployment root is created, it'll also be mislabeled). Anyway, even on non-OSTree systems, it seems reasonable to expect that `/home` and `/root` at least already exist and don't need to be created (and thus don't need to be relabeld). It's possible that [fixing `getxattr` without a policy loaded](coreos/fedora-coreos-config#245 (comment)) would also fix this, since `setfiles` would see that the symlinks were already correctly labeled. In effect, this is completing what coreos#632 started. Closes: coreos/fedora-coreos-tracker#339
jlebon
added a commit
to jlebon/ignition
that referenced
this pull request
Jun 4, 2020
We shouldn't need to run `setfiles` with `-i`, which causes `setfiles` to ignore files that do not exist. All the files which we pass to `setfiles` should exist, and it should be a hard error if `setfiles` fails to find and relabel the file we wrote. This dates from coreos#632, where we added `/var/home` and `/var/roothome` for OSTree-based systems. We actually don't need to special-case OSTree systems at all anymore. The `/var/home` and `/var/roothome` directories themselves are now handled by `ignition-ostree-populate-var.service`. All we need to care of here is to relabel the homedir files we created for each user. This should be cross-platform already.
jlebon
added a commit
to jlebon/ignition
that referenced
this pull request
Jun 4, 2020
We shouldn't need to run `setfiles` with `-i`, which causes `setfiles` to ignore files that do not exist. All the files which we pass to `setfiles` should exist, and it should be a hard error if `setfiles` fails to find and relabel the file we wrote. This dates from coreos#632, where we added `/var/home` and `/var/roothome` for OSTree-based systems. We actually don't need to special-case OSTree systems at all anymore. The `/var/home` and `/var/roothome` directories themselves are now handled by `ignition-ostree-populate-var.service`. All we need to care of here is to relabel the homedir files we created for each user. This should be cross-platform already.
jlebon
added a commit
to jlebon/ignition
that referenced
this pull request
Jun 4, 2020
We shouldn't need to run `setfiles` with `-i`, which causes `setfiles` to ignore files that do not exist. All the files which we pass to `setfiles` should exist, and it should be a hard error if `setfiles` fails to find and relabel the file we wrote. This dates from coreos#632, where we added `/var/home` and `/var/roothome` for OSTree-based systems. We actually don't need to special-case OSTree systems at all anymore. The `/var/home` and `/var/roothome` directories themselves are now handled by `ignition-ostree-populate-var.service`. All we need to take care of here is to relabel the homedir files we created for each user. This should be cross-platform already.
jlebon
added a commit
to jlebon/ignition
that referenced
this pull request
Jun 4, 2020
We shouldn't need to run `setfiles` with `-i`, which causes `setfiles` to ignore files that do not exist. All the files which we pass to `setfiles` should exist, and it should be a hard error if `setfiles` fails to find and relabel the file we wrote. This dates from coreos#632, where we added `/var/home` and `/var/roothome` for OSTree-based systems. We actually don't need to special-case OSTree systems at all anymore. The `/var/home` and `/var/roothome` directories themselves are now handled by `ignition-ostree-populate-var.service`. All we need to take care of here is to relabel the homedir files we created for each user. This is distro-agnostic.
jlebon
added a commit
to jlebon/ignition
that referenced
this pull request
Jun 19, 2020
We shouldn't need to run `setfiles` with `-i`, which causes `setfiles` to ignore files that do not exist. All the files which we pass to `setfiles` should exist, and it should be a hard error if `setfiles` fails to find and relabel the file we wrote. This dates from coreos#632, where we added `/var/home` and `/var/roothome` for OSTree-based systems. We actually don't need to special-case OSTree systems at all anymore. The `/var/home` and `/var/roothome` directories themselves are now handled by `ignition-ostree-populate-var.service`. All we need to take care of here is to relabel the homedir files we created or modified for each user. Because `setfiles` by default doesn't follow the final symlink, we also add a check here to relabel the target if the homedir is a link. (Ideally, we'd change the home directory of `root `to be `/var/roothome` like we do in rpm-ostree based systems for regular users: coreos/rpm-ostree#1726, but it's probably not worth the ripples that would cause.)
jlebon
added a commit
to jlebon/ignition
that referenced
this pull request
Jun 22, 2020
We shouldn't need to run `setfiles` with `-i`, which causes `setfiles` to ignore files that do not exist. All the files which we pass to `setfiles` should exist, and it should be a hard error if `setfiles` fails to find and relabel the file we wrote. This dates from coreos#632, where we added `/var/home` and `/var/roothome` for OSTree-based systems. We actually don't need to special-case OSTree systems at all anymore. The `/var/home` and `/var/roothome` directories themselves are now handled by `ignition-ostree-populate-var.service`. All we need to take care of here is to relabel the homedir files we created or modified for each user. Because `setfiles` by default doesn't follow the final symlink, we also add a check here to relabel the target if the homedir is a link. (Ideally, we'd change the home directory of `root `to be `/var/roothome` like we do in rpm-ostree based systems for regular users: coreos/rpm-ostree#1726, but it's probably not worth the ripples that would cause.)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The behaviour of how
restoreconhandles symlinks changed between RHCOSand FCOS. More specifically,
restoreconwill follow symlinks that arepart of a given path, but not if the target path is a symlink itself.
On OSTree-based systems,
/homeand/rootare just symlinks, so thenewer
restoreconwasn't actually relabeling anything under there.Add the real paths to the list of dirs to relabel and add
-iso thatit's not a fatal error on non-OSTree-based systems.
Closes: coreos/fedora-coreos-config#2