Skip to content

bugfix: add missing selinux relabeling for /dev paths #6734

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

Merged
merged 1 commit into from
May 15, 2025

Conversation

aerusso
Copy link
Contributor

@aerusso aerusso commented May 3, 2025

Some objects are created in fs_dev but not labeled. This patch ensures that those objects are properly labeled.

As an aside: I am locally running a fully confined SELinux setup using firejail (this is in fact posted from a fully SELinux contained Firefox instance running within a firejail). This involves a custom SELinux module for firejail (and other programs). So, this patch isn't "abstract" in any way---it's actually essential for me.

Some objects are created in fs_dev but not labeled.  This patch ensures
that those objects are properly labeled.

Signed-off-by: Antonio Enrico Russo <[email protected]>
@kmk3 kmk3 added the bugfix This fixes a bug label May 3, 2025
@kmk3 kmk3 changed the title selinux: more labeling in fs_dev bugfix: add missing selinux relabeling for /dev paths May 3, 2025
Copy link
Collaborator

@kmk3 kmk3 left a comment

Choose a reason for hiding this comment

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

Hello, sorry for the delay.

Looks good to me.

I was going to suggest to also relabel paths after bind-mounting:

--- a/src/firejail/fs_dev.c
+++ b/src/firejail/fs_dev.c
@@ -166,6 +166,7 @@ static void deventry_mount(const char *source,
 		errExit("mounting dev file");
 	}
 	fs_logger2("whitelist", target);
+	selinux_relabel_path(target, target);
 }
 
 // For every path in source_pattern, mount it on the dirname of target_pattern.
@@ -304,6 +305,8 @@ static void mount_dev_shm(void) {
 		empty_dev_shm();
 		return;
 	}
+
+	selinux_relabel_path("/dev/shm", "/dev/shm");
 }
 
 static void process_dev_shm(void) {
@@ -369,6 +372,7 @@ void fs_private_dev(void) {
 			if (mount(RUN_DEVLOG_FILE, "/dev/log", NULL, MS_BIND|MS_REC, NULL) < 0)
 				errExit("mounting /dev/log");
 			fs_logger("clone /dev/log");
+			selinux_relabel_path("/dev/log", "/dev/log");
 		}
 		if (mount(RUN_RO_FILE, RUN_DEVLOG_FILE, "none", MS_BIND, "mode=400,gid=0") < 0)
 			errExit("blacklisting " RUN_DEVLOG_FILE);
@@ -431,6 +435,7 @@ void fs_private_dev(void) {
 		errExit("mounting /dev/pts");
 	free(data);
 	fs_logger("clone /dev/pts");
+	selinux_relabel_path("/dev/pts", "/dev/pts");
 
 	// stdin, stdout, stderr
 	create_link("/proc/self/fd", "/dev/fd");

But I'm not sure if selinux_relabel_path would get the source label from the
correct/original path (I'm not sure how bind-mounting affects the labeling if
at all).

Would you mind looking into this?

If it's not obvious, we could also merge the PR as is and ignore this for now.

@rusty-snake

Any thoughts?

@aerusso
Copy link
Contributor Author

aerusso commented May 12, 2025

These appear to already have the correct SELinux label in a firejail I just tested. This also makes sense to me: bind mounts bring along the SELinux label (just tested that by mount --bind / /mnt).

I also think that selinux_relabel_path might affect the outside object, since you're relabeling the bind-mount of the outside object. I haven't tried to test this, though.

@kmk3 kmk3 merged commit 977eac3 into netblue30:master May 15, 2025
16 of 17 checks passed
@github-project-automation github-project-automation bot moved this from Todo to Done in Release 0.9.76 May 15, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bugfix This fixes a bug
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

2 participants