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

Add fs_associate_cgroupfs interface #186

Closed
wants to merge 1 commit into from

Conversation

rhatdan
Copy link
Contributor

@rhatdan rhatdan commented Feb 6, 2017

No description provided.

@rhatdan
Copy link
Contributor Author

rhatdan commented Feb 6, 2017

@stephensmalley PTAL

@stephensmalley
Copy link
Contributor

Hmmm...I guess this would work, but not clear on the tradeoffs of doing it this way versus the kernel patch. Recommend asking Paul Moore, probably should be discussed on selinux list. Do you envision defining type transitions for different domains on cgroup/cgroup2?

@rhatdan
Copy link
Contributor Author

rhatdan commented Feb 7, 2017

@stephensmalley I thought this was the patch required for the kernel patch. What is the syntax to make this work with the kernel patch?

I think having named file trans might be useful. but for now I expect container runtimes to do something like

setcon("/sys/fs/cgroup/systemd", "system_u:object_r:container_file_t:s0:c1,c2")

@rhatdan
Copy link
Contributor Author

rhatdan commented Feb 7, 2017

BTW I will send this to upstream as soon as we are sure of the syntax.

@stephensmalley
Copy link
Contributor

No, the kernel patch doesn't require any policy change; it just alters the kernel to support setting of labels from userspace while still defaulting to genfscon as the initial label. You only need to change the policy if you want to support transitions on file creations by userspace. If you change the policy to fs_use_trans, you don't need the kernel patch at all; fs_use_trans filesystems already support labeling from userspace.

@stephensmalley
Copy link
Contributor

Also, if you want to support genfscon per-file labeling, your kernel patch isn't sufficient; you would need to add cgroup/cgroup2 to the other whitelist as well. I would like to be able to label the "release_agent" file within each cgroup mount and limit writes to it since it is similar to modprobe or other kernel usermodehelpers.

@rhatdan
Copy link
Contributor Author

rhatdan commented Feb 7, 2017

@runcom PTAL

@runcom
Copy link

runcom commented Feb 7, 2017

ack, so we'll need to allow labeling per-file also in kernel, right?

@stephensmalley
Copy link
Contributor

Yes, unless Dan actually wants type transitions on cgroup. However, I will note that we want to get rid of these filesystem type whitelists and replace them with something more general, see
SELinuxProject/selinux-kernel#2

@runcom
Copy link

runcom commented Feb 7, 2017

@stephensmalley got this patch:

diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index 2789f0a..d982ec8 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -808,6 +808,8 @@ static int selinux_set_mnt_opts(struct super_block *sb,
 
 	if (!strcmp(sb->s_type->name, "debugfs") ||
 	    !strcmp(sb->s_type->name, "sysfs") ||
+	    !strcmp(sb->s_type->name, "cgroup") ||
+	    !strcmp(sb->s_type->name, "cgroup2") ||
 	    !strcmp(sb->s_type->name, "pstore"))
 		sbsec->flags |= SE_SBGENFS;

@rhatdan could you ack on @stephensmalley comment?

@rhatdan
Copy link
Contributor Author

rhatdan commented Feb 7, 2017

I can live without transition rules on cgroups. I think it is far more likely that we just need to handle setting xattrs.

Also fix some cut-and-paste errors on cgroup interfaces
@rhatdan
Copy link
Contributor Author

rhatdan commented Feb 9, 2017

Changed pull request to just add an interface to allow file types to be associated with cgroup_t file systems.
Need this to be able to assign container_file_t to cgroup_t.

@rhatdan rhatdan changed the title Cgroups can not handle labels Add fs_associate_cgroupfs interface Feb 9, 2017
@wrabcak wrabcak closed this Feb 20, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants