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

Don't try to relabel mountpoints which already exist #1452

Closed
tormath1 opened this issue Aug 22, 2022 · 3 comments · Fixed by #1608
Closed

Don't try to relabel mountpoints which already exist #1452

tormath1 opened this issue Aug 22, 2022 · 3 comments · Fixed by #1608
Assignees
Labels

Comments

@tormath1
Copy link
Contributor

Bug

Hi, we're working to get Flatcar fully labelled from SELinux point of view. We're currently hitting an issue when a file created by Ignition needs to be relabeled (/etc/docker/daemon.json).
A Flatcar contributor tried back in the days to enable the sefiles feature for Ignition: flatcar-archive/coreos-overlay#1784 (with selinuxRelabel) but we were hitting some relabeling issue with read-only filesystems (like OEM partition).

[    2.081130] ignition[570]: CRITICAL : Ignition failed: exit status 255: Cmd: "setfiles" "-vF0" "-r" "/sysroot" "/sysroot/usr/lib/selinux/mcs/contexts/files/file_contexts" "-f" "-" Stdout: "" Stderr: "setfiles: Could not set context for /sysroot/usr/share/oem:  Read-only file system\n"

Operating System Version

Flatcar

Ignition Version

2.14.0

Expected Behavior

Skip read-only filesystems.

Actual Behavior

[    2.081130] ignition[570]: CRITICAL : Ignition failed: exit status 255: Cmd: "setfiles" "-vF0" "-r" "/sysroot" "/sysroot/usr/lib/selinux/mcs/contexts/files/file_contexts" "-f" "-" Stdout: "" Stderr: "setfiles: Could not set context for /sysroot/usr/share/oem:  Read-only file system\n"

Additional Information

What do you think about implementing an extra-check util.FileSystemIsReadOnly(path) before calling the RelabelFiles function ?

@jlebon
Copy link
Member

jlebon commented Aug 22, 2022

Ignition should only be trying to relabel files it wrote. Do you know why it's trying to relabel /usr/share/oem?

@tormath1
Copy link
Contributor Author

tormath1 commented Aug 23, 2022

@jlebon thanks for your answer. I checked, the issue comes from this configuration:

storage:
  filesystems:
     - name: oem
       mount:
         device: "/dev/disk/by-label/OEM"
         format: "btrfs"
  files:
    - path: /grub.cfg
      filesystem: oem
      mode: 0644
      contents:
        inline: |
          set linux_append="flatcar.autologin"

I also forgot to share the full log:

[    2.023297] ignition[570]: CRITICAL : mount: op(1): [failed]   relabeling 1 patterns: exit status 255: Cmd: "setfiles" "-vF0" "-r" "/sysroot" "/sysroot/usr/lib/selinux/mcs/contexts/files/file_contexts" "-f" "-" Stdout: "" Stderr: "setfiles: Could not set context for /sysroot/usr/share/oem:  Read-only file system\n"
[    2.081130] ignition[570]: CRITICAL : Ignition failed: exit status 255: Cmd: "setfiles" "-vF0" "-r" "/sysroot" "/sysroot/usr/lib/selinux/mcs/contexts/files/file_contexts" "-f" "-" Stdout: "" Stderr: "setfiles: Could not set context for /sysroot/usr/share/oem:  Read-only file system\n"

It appears in the mount stage - so I'm not sure why it's being mounted as read-only.

@bgilbert
Copy link
Contributor

The return value of FindFirstMissingPathComponent("/usr/share/oem") doesn't distinguish between a) /usr/share/oem already existing and b) /usr/share already existing but /usr/share/oem not existing. As a result, mountFs() tries to relabel the /usr/share/oem mountpoint before mounting on top of it, and can't because /usr is read-only.

As a workaround, the Ignition config can specify that the OEM partition should be mounted somewhere else [1]. That only affects the Ignition run and won't affect the installed system at all. But yes, we should fix this.

[1] ...in Ignition spec 3.x syntax. The config you posted uses Ignition spec 2.x.

@jlebon jlebon changed the title setfiles on read-only filesystem Don't try to relabel mountpoints which already exist Aug 23, 2022
@Adam0Brien Adam0Brien self-assigned this Apr 6, 2023
@Adam0Brien Adam0Brien added spec2x and removed spec2x labels Apr 11, 2023
@Adam0Brien Adam0Brien added the jira for syncing to jira label Apr 17, 2023
Adam0Brien added a commit to Adam0Brien/ignition that referenced this issue Jun 16, 2023
Adam0Brien added a commit to Adam0Brien/ignition that referenced this issue Jun 28, 2023
Now when file systems are being mounted ignition will relabel directories
if they did not exist previously.
Whereas before file systems could be relabelled if they did/didnt exist previously
Fix coreos#1452
Adam0Brien added a commit to Adam0Brien/ignition that referenced this issue Jun 28, 2023
This PR introduces a new change to how ignition mounts filesystems,
Now when file systems are being mounted ignition will relabel directories
if they did not exist previously.
Whereas before file systems could be relabelled if they did/didnt exist previously
Fix coreos#1452
Adam0Brien added a commit to Adam0Brien/ignition that referenced this issue Jun 28, 2023
…ectory even if it previously existed.

This isn't necessary, and fails if the mountpoint is on a read-only filesystem, such as /usr/share/oem on Flatcar.
Relabel the mountpoint only if we create it.
Fixes: coreos#1452
Adam0Brien added a commit to Adam0Brien/ignition that referenced this issue Jun 28, 2023
When mounting filesystems, Ignition was relabeling the mountpoint
directory even if it previously existed.
This isn't necessary, and fails if the mountpoint is on a read-only
filesystem, such as /usr/share/oem on Flatcar.
Relabel the mountpoint only if we create it.
Fixes: coreos#1452
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants