-
Notifications
You must be signed in to change notification settings - Fork 157
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
overlay/boot-mount-generator: Mount /boot{,efi} read-only,nodev,nosuid #659
Conversation
Heh I forgot about #356 and just saw it - incorporated that too. |
This needs to resolve |
One thing this does break is that I think it would be nice if sysadmins could just |
Also after this let's revisit #407 |
I was debugging a failure to write the statefile which turned out to be me having coreos/fedora-coreos-config#659 locally. Anyways let's add some error context to aid future debugging.
I was debugging a failure to write the statefile which turned out to be me having coreos/fedora-coreos-config#659 locally. Anyways let's add some error context to aid future debugging.
I was debugging a failure to write the statefile which turned out to be me having coreos/fedora-coreos-config#659 locally. Anyways let's add some error context to aid future debugging.
coreos/fedora-coreos-config#659 attempts to mount `/boot` read-only. Currently, the firstboot network dir in `/boot` is cleaned up by a tmpfiles.d conf file. This may not be possible once `/boot` is read-only, so we do the clean up here.
coreos#659 attempts to mount `/boot` read-only. `15-coreos-firstboot-network.conf`'s job should be handled by `ignition-firstboot-complete.service` which remounts writable (privately/temporarily) `/boot` from the real root.
We would like to mount `/boot` read-only in the real root, so add a new unit in 15coreos-network to temporarily mount /boot rw and clean up firstboot networking configuration files late in the initramfs. Remove the current 15-coreos-firstboot-network.conf since it would not work once `/boot` is mounted ro. xref coreos#659
We would like to mount `/boot` read-only in the real root, so add a new unit in 15coreos-network to temporarily mount /boot rw and clean up firstboot networking configuration files late in the initramfs. Remove the current 15-coreos-firstboot-network.conf since it would not work once `/boot` is mounted ro. xref coreos#659
We would like to mount `/boot` read-only in the real root, so add a new unit in 15coreos-network to temporarily mount /boot rw and clean up firstboot networking configuration files late in the initramfs. Remove the current 15-coreos-firstboot-network.conf since it would not work once `/boot` is mounted ro. xref coreos#659
We would like to mount `/boot` read-only in the real root, so add a new unit in 15coreos-network to temporarily mount /boot rw and clean up firstboot networking configuration files late in the initramfs. Remove the current 15-coreos-firstboot-network.conf since it would not work once `/boot` is mounted ro. xref coreos#659
We would like to mount `/boot` read-only in the real root, so remove the current 15-coreos-firstboot-network.conf since it would not work once `/boot` is mounted ro. Drop a stamp file instead so that `coreos-boot-edit.service` would notice and perform the clean up later in the initramfs. xref coreos#659
We would like to mount `/boot` read-only in the real root, so remove the current 15-coreos-firstboot-network.conf since it would not work once `/boot` is mounted ro. Drop a stamp file instead so that `coreos-boot-edit.service` would notice and perform the clean up later in the initramfs. xref #659
@cgwalters With the updates to how |
We would like to mount `/boot` read-only in the real root, so remove the current 15-coreos-firstboot-network.conf since it would not work once `/boot` is mounted ro. Drop a stamp file instead so that `coreos-boot-edit.service` would notice and perform the clean up later in the initramfs. xref coreos#659
09e5e48
to
7b576a3
Compare
This is remounting |
Ahh OK, is this related to openshift/os#480 (comment) ? I.e. are we backtracking for the time being and going back to mounting |
@jlebon Actually that was my bad, the latest release of bootupd should be able to handle a not-mounted EFI and a read-only
I think https://github.com/coreos/bootupd/releases/tag/v0.2.5 should mean we don't need to backtrack. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Commit title still references efi
and also andd
-> add
in the commit message.
LGTM otherwise!
boot_mount_options=ro,nodev,nosuid | ||
mk_mount /boot boot "${boot_mount_options}" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Minor/optional: meh, I'd just inline these. It made sense before because we were re-using it for the EFI mount as well.
ostree has had support for leaving `/boot` mounted read-only for a long time: ostreedev/ostree#1767 (And then later extended to `/sysroot`) Particularly for CoreOS, only a few things should be touching `/boot`, and we control all of them. Those projects should create a new mount namespace and remount these partitions writable just while they need it. The main thing we're accomplishing here is making the system more resilient against accidental damage from a sysadmin root shell as well as configuration management tools like Puppet/Ansible. None of those should be directly manipulating files on these partitions, they should go through the API of one of our projects (e.g. `rpm-ostree kargs`, `bootupctl`) etc. While we're here, also add `nodev,nosuid` because some OS hardening scanners like to see this. IMO it's of minimal value, but hey, might as well.
The recent change in coreos/fedora-coreos-config#659 broke some of our tests that do `mount -o remount,rw /sysroot` but leave `/boot` read-only. We had code for having `/boot` read-only before `/sysroot` but in practice we had a file descriptor for `/sysroot` that we opened before the remount that would happen later on. Clean things up here so that in the library, we also remount `/boot` at the same time we remount `/sysroot` if either are readonly. Second, adapt the client side code to check for either being readonly to enable the mount namespace. Now honestly we could almost certainly just set this unconditionally - the original client code here is just being excessively conservative. But I'd like to make that cleanup independently of this fix.
The recent change in coreos/fedora-coreos-config#659 broke some of our tests that do `mount -o remount,rw /sysroot` but leave `/boot` read-only. We had code for having `/boot` read-only before `/sysroot` but in practice we had a file descriptor for `/sysroot` that we opened before the remount that would happen later on. Clean things up here so that in the library, we also remount `/boot` at the same time we remount `/sysroot` if either are readonly. Delete the legacy code for remounting `/boot` rw if we're not in a mount namespace. I am fairly confident most users are either using the `ostree` CLI, or they're using the mount namespace.
The recent change in coreos/fedora-coreos-config#659 broke some of our tests that do `mount -o remount,rw /sysroot` but leave `/boot` read-only. We had code for having `/boot` read-only before `/sysroot` but in practice we had a file descriptor for `/sysroot` that we opened before the remount that would happen later on. Clean things up here so that in the library, we also remount `/boot` at the same time we remount `/sysroot` if either are readonly. Delete the legacy code for remounting `/boot` rw if we're not in a mount namespace. I am fairly confident most users are either using the `ostree` CLI, or they're using the mount namespace.
The recent change in coreos/fedora-coreos-config#659 broke some of our tests that do `mount -o remount,rw /sysroot` but leave `/boot` read-only. We had code for having `/boot` read-only before `/sysroot` but in practice we had a file descriptor for `/sysroot` that we opened before the remount that would happen later on. Clean things up here so that in the library, we also remount `/boot` at the same time we remount `/sysroot` if either are readonly. Delete the legacy code for remounting `/boot` rw if we're not in a mount namespace. I am fairly confident most users are either using the `ostree` CLI, or they're using the mount namespace.
ostree has had support for leaving
/boot
mounted read-onlyfor a long time: ostreedev/ostree#1767
(And then later extended to
/sysroot
)Particularly for CoreOS, only a few things should be touching
/boot
, and we control all of them. Those projects shouldcreate a new mount namespace and remount these partitions
writable just while they need it.
The main thing we're accomplishing here is making the system
more resilient against accidental damage from a sysadmin
root shell as well as configuration management tools like
Puppet/Ansible. None of those should be directly manipulating
files on these partitions, they should go through the API
of one of our projects (e.g.
rpm-ostree kargs
,bootupctl
) etc.While we're here, also andd
nodev,nosuid
because someOS hardening scanners like to see this. IMO it's of minimal
value, but hey, might as well.