Running `fixfiles` results in errors from grep: ``` $ sudo fixfiles -B onboot grep: Invalid back reference grep: Invalid back reference grep: Invalid back reference grep: Invalid back reference System will relabel on next boot ``` I tracked it down to lines 48 and 50 in /usr/sbin/fixfiles and found that the command `grep " $i " /proc/self/mounts` produces these errors when the variable FS contains luks-encrypted filesystems. Reproducible: Always Steps to Reproduce: To reproduce, one can simulate the error: ``` $ FS=("/run/credentials/systemd-cryptsetup@luks\134xsomeothercontent.service") $ for i in $FS; do grep " $i " /proc/self/mounts | echo $i; done /run/credentials/systemd-cryptsetup@luks\134xsomeothercontent.service grep: Invalid back reference ``` Actual Results: An error message appears when fixfiles is run with a luks-encrypted filesystem. Expected Results: There should be no error from grep when running fixfiles. Changing `grep " $i " /proc/self/mounts` to `grep -F " $i " /proc/self/mounts` seems to fix the error, but I do not know if using `-F` is the correct behavior. Since `fixfiles` is an optional post-upgrade action documented at https://docs.fedoraproject.org/en-US/quick-docs/upgrading-fedora-offline/#sect-relabel-files-with-the-latest-selinux-policy , and since filesystem encryption is somewhat common, this should be listed in the CommonBugs.
Is there any negative effect, in addition to the error printouts? I'm asking because of your CommonBugs nomination.
(In reply to Kamil Páral from comment #1) > Is there any negative effect, in addition to the error printouts? I'm asking > because of your CommonBugs nomination. I'm not really sure I understand what that part of the script is doing, so I can't say for certain what the negative impact is. Certainly, the error is preventing that part of the script from doing whatever it was supposed to be doing, but I don't know what that is. What I know, though, is that the fixfiles command still creates the /.autorelabel file, so it still gets relabeled on the next boot. Other than that, I'm not sure what they intended behavior is for that broken part of the script.
OK. My assumption is that for just creating the /.autorelabel file, the error is harmless, because the file gets created anyway. But it might impact other use cases. Until we have concrete examples of some negative impact, I'm withdrawing the CommonBugs nomination, it wouldn't qualify.
The proposed `grep -F` seems to be the correct solution for this issue.
FEDORA-2024-df8a6c3ddf (policycoreutils-3.7-5.fc41) has been submitted as an update to Fedora 41. https://bodhi.fedoraproject.org/updates/FEDORA-2024-df8a6c3ddf
(In reply to Kamil Páral from comment #3) > OK. My assumption is that for just creating the /.autorelabel file, the > error is harmless, because the file gets created anyway. But it might impact > other use cases. Until we have concrete examples of some negative impact, > I'm withdrawing the CommonBugs nomination, it wouldn't qualify. Is it required for the bug to cause serious harm to qualify as common? To me, I think common means likely to be encountered. This is likely to be encountered because encrypted drives are not rare, and this command is in the upgrade instructions. If a common bug doesn't cause serious harm, the description of it could just explain that it's safe to ignore. Users are still going to encounter it, and they don't know if it's serious or not unless they read the description. If it's not listed, they might think that they've encountered something unusual and they could freak out like I did, and track it down like I did. If it was listed as a common bug that said "don't worry", that would be better, I think. However, I still don't know what this line is supposed to be doing, so I still don't even know if it's serious or not.
FEDORA-2024-df8a6c3ddf has been pushed to the Fedora 41 testing repository. Soon you'll be able to install the update with the following command: `sudo dnf upgrade --enablerepo=updates-testing --refresh --advisory=FEDORA-2024-df8a6c3ddf` You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2024-df8a6c3ddf See also https://fedoraproject.org/wiki/QA:Updates_Testing for more information on how to test updates.
(In reply to Christopher Tubbs from comment #6) > Is it required for the bug to cause serious harm to qualify as common? I don't want to stray much from the issue here, but I'll reply. Yes, it's required. Otherwise we would need to document a half of the whole bugzilla, which would lose the point. See https://discussion.fedoraproject.org/t/about-the-common-issues-category/74711: "a serious issue that seems to affect a large number of Fedora users" If you want further discussion on this topic, post is here and tag me: https://discussion.fedoraproject.org/c/help/3
FEDORA-2024-df8a6c3ddf (policycoreutils-3.7-5.fc41) has been pushed to the Fedora 41 stable repository. If problem still persists, please make note of it in this bug report.
I spent a little more time investigating the full impact of this bug. It seems like the error I saw would prevent any files from being relabeled if they were on a mount point with a backslash escape character contained in them. For the luks case, it seems those are mounted as ro, so they would be skipped anyway, so for that particular case, the error message is harmless. However, the underlying bug is not harmless, and it may manifest without any error message at all. What is actually happening is that the fixfiles script actually tries to read each mount point as a grep PATTERN. So, any valid grep PATTERN expression contained in a mount point's name could match on other mount points in /proc/self/mounts. This could lead to all kinds of bad behavior, such as incorrectly reporting that a rw filesystem will be skipped as ro, even though it's not, from attempts to write to a ro filesystem. This could lead to a lot of confusion about the secure state of the system, and it's possible somebody could take advantage of this bug by creating a mount point that triggers it. One serious consequence of this bug is that a mount point could contain a pattern that fails to match on itself, leading to circumvention of the the secure labeling of the contents of that filesystem, because the filesystem will never be added to the set of filesystems for relabeling, and will not be relabeled. While the error message I saw was due to the PATTERN being invalid, the more serious consequences of this bug occur when the mount point contains a valid PATTERN that leads to these problems. I'm not a security researcher, so I don't intend to spend time constructing a proof-of-concept exploit for this, but I think any reasonable cursory analysis of the fixfiles source code would lead to the same conclusion. One reassurance is that mount point creation typically requires admin access, so it's not likely a non-admin will be able to add mount points that exploit this. Given the context of secure filesystem labeling being critical to the security of the system, I now believe that this is a much bigger problem than merely an error message that is safe to ignore, and would reassert my request to have it listed in the CommonBugs, due to its severity, and the possibility of impacting system security. It is much more likely to occur in the normal benign case like what I found with luks, but the severity of the issue goes beyond that.
While the current patch seems to fix the main cause of the error, I think a much better fix would be to rewrite the script to iterate through `/proc/self/mounts` a single time, and process each line separately, rather than trying to capture the set of mounts once, and then going back through multiple times, trying to process lines using grep to find the line to process. Avoiding grep entirely here would be the most reliable fix.
I do agree that `fixfiles` as an old shell script has its issues. OTOH `fixfiles -B onboot` is supposed to speed up labeling so that it would not touch files with modification time older than current day. The intention of the linked documentation seems to be to use it to relabel only upgraded files. But the wording is not the best as the correct command for "To relabel SELinux on the system, run the following command and then reboot:" would be `fixfiles onboot` or better `fixfiles -F onboot` <- maybe it's even a typo in the documentation s/-B/-F/
The flags documented in the upgrade docs aren't relevant to this bug. The bug is serious enough that it helps facilitate a labeling bypass under *any* circumstance where `/.autorelabel` exists. It doesn't matter whether it was created using a timestamp with `-B` or not. That is because the bug is not primarily in the logic that processes the `-B` timestamp. The most serious part of the bug is primarily in the logic that processes whether a filesystem is mounted `rw`. A mount point named PATTERN can be crafted so that it fails to match on itself, and is therefore skipped instead of being included in the FILESYSTEMSRW list. A trivial demo of such a pattern: `echo '\s' | grep '\s'`. The luks examples I found were examples of an invalid pattern that caused those mount points to be skipped... it turns out they would have been skipped anyway, because they are mounted `ro`. But, one could just as easily use a mount name that is a valid PATTERN, that does not match, so it causes relabeling to be skipped on that mount point without any error message at all.
Could you please demonstrate the issue you've just described so that I better understand it?
(In reply to Petr Lautrbach from comment #14) > Could you please demonstrate the issue you've just described so that I > better understand it? # create a blank 1G disk image, format it, and mount it at a well-crafted PATTERN location to avoid relabeling dd if=/dev/zero of=/tmp/file.img bs=64M count=16 sudo mkfs.ext4 -F /tmp/file.img sudo mkdir '/mnt/image[.]' sudo mount -o loop /tmp/file.img '/mnt/image[.]' # simulate fixfiles behavior to get all labeled mounts FS="`cat /proc/self/mounts | sort | uniq | awk '{print $2}'`" for i in $FS; do grep " $i " /proc/self/mounts | awk '{print $4}' | grep -E --silent '(^|,)seclabel(,|$)' && echo $i; done # notice that the mount is undetected by fixfiles, so it will never be relabeled
It works for me using policycoreutils-3.7-5.fc42.x86_64, but the reproducer is tricky: [root@localhost ~]# tail -n 1 /etc/fstab /dev/vdb1 /mnt/image[.] ext4 defaults 0 0 [root@localhost ~]# ls -Z /mnt/image\[.\]/file system_u:object_r:unlabeled_t:s0 file [root@localhost ~]# fixfiles onboot System will relabel on next boot [root@localhost ~]# reboot [root@localhost ~]# Read from remote host 192.168.122.249: Connection reset by peer Connection to 192.168.122.249 closed. client_loop: send disconnect: Broken pipe $ ssh root.122.249 root.122.249's password: Last login: Thu Nov 21 17:43:18 2024 from 192.168.122.1 [root@localhost ~]# ls -Z /mnt/image\[.\]/file system_u:object_r:unlabeled_t:s0 file It really looks like /mnt/image[.]/file was not relabeled. The problem is that there's no filecontext mappings for files inside /mnt/image[.]: [root@localhost ~]# matchpathcon /mnt/image\[.\]/file /mnt/image[.]/file <<none>> So even restorecon does not relabel it: [root@localhost ~]# restorecon -R -F -v /mnt/image\[.\]/ [root@localhost ~]# ls -Z /mnt/image\[.\]/file system_u:object_r:unlabeled_t:s0 '/mnt/image[.]/file' But if the mount point is changed from /mnt/ to /usr and there's a file context mapping for such path: [root@localhost ~]# matchpathcon /usr/image\[.\]/file /usr/image[.]/file system_u:object_r:usr_t:s0 then it works as expected: [root@localhost ~]# sed -i 's|/mnt/image|/usr/image|' /etc/fstab [root@localhost ~]# tail -n 1 /etc/fstab /dev/vdb1 /usr/image[.] ext4 defaults 0 0 [root@localhost ~]# mount /usr/image\[.\]/ [root@localhost ~]# ls -Z /usr/image\[.\]/file system_u:object_r:unlabeled_t:s0 file [root@localhost ~]# fixfiles onboot System will relabel on next boot [root@localhost ~]# reboot Read from remote host 192.168.122.249: Connection reset by peer Connection to 192.168.122.249 closed. client_loop: send disconnect: Broken pipe $ ssh root.122.249 root.122.249's password: Last login: Thu Nov 21 17:44:37 2024 from 192.168.122.1 [root@localhost ~]# ls -Z /usr/image\[.\]/file system_u:object_r:usr_t:s0 file The file `file` was correctly relabeled during reboot.
(In reply to Petr Lautrbach from comment #16) > It works for me using policycoreutils-3.7-5.fc42.x86_64, but the reproducer > is tricky: Right, policycoreutils-3.7-5 contains the fix. If you were to retry your test with the older version (or just revert the patch locally by removing `-F` from the grep commands, just to test), you'd see the bypass behavior I described. It was decided in the earlier conversation to be excluded from the CommonBugs because it wasn't serious. My intent with the demo was to show that the bug was serious. However, since it is fixed, perhaps it no longer matters if it is serious or not, and should instead be excluded from CommonBugs because it is fixed and won't be encountered anymore.
(In reply to Christopher Tubbs from comment #17) > However, since it is fixed, perhaps it no longer > matters if it is serious or not, and should instead be excluded from > CommonBugs because it is fixed and won't be encountered anymore. Correct.
FEDORA-2025-e94bfc5f12 (policycoreutils-3.7-6.fc40) has been submitted as an update to Fedora 40. https://bodhi.fedoraproject.org/updates/FEDORA-2025-e94bfc5f12
FEDORA-2025-e94bfc5f12 has been pushed to the Fedora 40 testing repository. Soon you'll be able to install the update with the following command: `sudo dnf upgrade --enablerepo=updates-testing --refresh --advisory=FEDORA-2025-e94bfc5f12` You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2025-e94bfc5f12 See also https://fedoraproject.org/wiki/QA:Updates_Testing for more information on how to test updates.
FEDORA-2025-e94bfc5f12 (policycoreutils-3.7-6.fc40) has been pushed to the Fedora 40 stable repository. If problem still persists, please make note of it in this bug report.