Description of problem: Currently /run/user is labeled type user_tmp_t This can be a problem: - user runtime content has very little relation to user tmp content Processes fall back to ~/.cache if $XDG_RUNTIME_DIR is unset - currently root as well as xdm maintain directories in /run/user as well These are not ordinary login users and we may want to label their runtime content differently than ordinary login users - the current setup may cause problems in MLS configurations where one might want to poly-instantiate /run/user Systemd-logind creates /run/user/UID on behalf of the user Systemd-logind is not aware of the SELinux implications currently The below proof of concept patch will make it so that we can specify in policy what any given directory in /run/user should be labeled. This allows us to say for example: label directory /run/user/1000 - /run/user/60000 type user_run_dir_t, label /run/user/0 type admin_run_t etc All this patch does is it tells systemd-logind to reset the context of /run/user/UID to what is specified in policy if SELinux is used. This allows us to implement a similar MLS solution as we do with /home we can for example in policy create two new keywords: RUN_ROOT and RUN_DIR then associate polyparent with RUN_ROOT (example run_root_t) and associate polymember with RUN_DIR (example user_run_dir_t) then generic content created in user runtime directories can for example be labeled user_run_t. Here is the patch: diff --git a/src/login/logind-user.c b/src/login/logind-user.c index 3aeac13..2978f8c 100644 --- a/src/login/logind-user.c +++ b/src/login/logind-user.c @@ -19,6 +19,10 @@ along with systemd; If not, see <http://www.gnu.org/licenses/>. ***/ +#ifdef HAVE_SELINUX +#include <selinux/selinux.h> +#endif + #include <string.h> #include <unistd.h> #include <errno.h> @@ -324,6 +328,10 @@ static int user_mkdir_runtime_path(User *u) { return r; } + #ifdef HAVE_SELINUX + selinux_lsetfilecon_default(p); + #endif + u->runtime_path = p; return 0; } To make this meaningful we should probably also make changes to the genhomedircon code in semodule. We need genhomedir code to become aware of the RUN_ROOT and RUN_DIR keywords and treat them similarly to RUN_ROOT and RUN_DIR We should not settle for using user_tmp_t for /run/user, would you like it if /home would be labeled user_home_t instead of home_root_t? This proposal affects 3 components: systemd-logind selinux-policy policycoreutils
I already implemented the above in my own environment based on CIL (it works for me) # ls -alZ /run/user/1000 drwx------. joe joe session_u:object_r:user_run_dir_t . drwxr-xr-x. root root system_u:object_r:run_root_t .. drwxr-xr-x. joe joe session_u:object_r:user_run_t systemd basically i applies the patch above to systemd-logind wrote my own genhomedircon script in shell and wrote my own policy in ci language (filecon "" "HOME_ROOT" dir (system_u object_r home_root_t ((s0) mls_systemhigh))) (filecon "" "HOME_ROOT" symlink (system_u object_r home_root_t ((s0) (s0)))) (filecon "HOME_DIR" "/.*" any (system_u object_r user_home_t ((s0) (s0)))) (filecon "HOME_DIR" "" dir (system_u object_r user_home_dir_t ((s0) mls_systemhigh))) (filecon "" "RUN_ROOT" dir (system_u object_r run_root_t ((s0) mls_systemhigh))) (filecon "" "RUN_ROOT" symlink (system_u object_r run_root_t ((s0) (s0)))) (filecon "RUN_DIR" "/.*" any (system_u object_r user_run_t ((s0) (s0)))) (filecon "RUN_DIR" "" dir (system_u object_r user_run_dir_t ((s0) mls_systemhigh))) etc etc
Ok ... going thru it and thinking about it. I see #ls -lZ /run/user/1000/ drwx------. mgrepl mgrepl unconfined_u:object_r:config_home_t:s0 dconf drwx------. mgrepl mgrepl unconfined_u:object_r:user_tmp_t:s0 gnome-shell dr-x------. mgrepl mgrepl system_u:object_r:fusefs_t:s0 gvfs drwx------. mgrepl mgrepl unconfined_u:object_r:user_tmp_t:s0 gvfs-burn drwx------. mgrepl mgrepl unconfined_u:object_r:user_tmp_t:s0 icedteaplugin-mgrepl-1uHFoY drwx------. mgrepl mgrepl unconfined_u:object_r:gkeyringd_tmp_t:s0 keyring-quWMLz drwx------. mgrepl mgrepl unconfined_u:object_r:user_tmp_t:s0 pulse drwxr-xr-x. mgrepl mgrepl system_u:object_r:user_tmp_t:s0 systemd lrwxrwxrwx. root root system_u:object_r:user_tmp_t:s0 X11-display -> /tmp/.X11-unix/ on my virtual machine. I agree user_tmp_t labeling is not the best solution what we have for /run/user directory in some cases.
The most interesting is MLS from my point of view. Also we might want to discuss it on SELinux list.
OK thanks but please look at it as three or at least two different pieces The systemd-logind patch i posted should, as far as i am concerned, be implemented in one way or another regardless of what one may decide to do on the policycoreutils, and selinux-policy front. You see the systemd-logind patch does not hurt either way and it provides flexibility for he who wants to have a say in how these runtime directories are labeled. It won't affect anything without additional changes to policy So, yes, please consider the systemd-logind patch. What you decide for policycoreutils or selinux-policy is less relevant for me since i am not using semodule or selinux-policy I am however using systemd-logind
Miroslav can you enhance Dominicks patch by adding an if selinux_enabled() wrapper and checking the error code, and return it if SELinux is not in enforcing mode.
We should open a separate bug for the mls issue.
Why don't we just use the existing systemd label handling in this case?
I do not know what you are referring to with "existing systemd label handling" All i know is that my patch works I also know that "systemd label handling" sometimes leaves to much to be desired Let me just give you an example: Please do: ls -alZ /sys/fs/cgroup Do you see the labels on the cpu and cpuacct symlinks? They are tmpfs_t where the parent directory is type cgroup_t I would not want to give processes that follow those symlinks access to tmpfs_t type symlinks. I would must rather have those symlink be labeled cgroup_t like they are supposed to. If you are positive that there is a better way to achieve the desired result then by all means implement that. If you have something worked out, then please let me know. I will test the solution to see if the result is what is expected
..Not to mention the hardcoded file contexts on boot that i reported a while ago , that no one seems to know about, but that are definitly (still) there.. just like my (open) bug report about that issue
I mean we have systemd+selinux_label handling in src/shared/label.c. No need to call selinux_lsetfilecon_default(p); and add additional handling. diff --git a/src/login/logind-user.c b/src/login/logind-user.c index 961cbcb..42946ae 100644 --- a/src/login/logind-user.c +++ b/src/login/logind-user.c @@ -337,6 +337,7 @@ static int user_mkdir_runtime_path(User *u) { } } + label_fix(p,false,false); u->runtime_path = p; return 0; @@ -423,7 +424,9 @@ int user_start(User *u) { log_debug("New user %s logged in.", u->name); /* Make XDG_RUNTIME_DIR */ + label_init("/var/run/user"); r = user_mkdir_runtime_path(u); + label_finish(); if (r < 0) return r;
(In reply to Dominick Grift from comment #9) > ..Not to mention the hardcoded file contexts on boot that i reported a while > ago , that no one seems to know about, but that are definitly (still) > there.. just like my (open) bug report about that issue Which one?
https://bugzilla.redhat.com/show_bug.cgi?id=1008601 By the way those sshd dyndomain hardcoded types (sshd_net_t) are also still there
(In reply to Miroslav Grepl from comment #10) > and add additional handling. > > > diff --git a/src/login/logind-user.c b/src/login/logind-user.c > index 961cbcb..42946ae 100644 > --- a/src/login/logind-user.c > +++ b/src/login/logind-user.c > @@ -337,6 +337,7 @@ static int user_mkdir_runtime_path(User *u) { > } > } > > + label_fix(p,false,false); > u->runtime_path = p; > return 0; > > @@ -423,7 +424,9 @@ int user_start(User *u) { > log_debug("New user %s logged in.", u->name); > > /* Make XDG_RUNTIME_DIR */ > + label_init("/var/run/user"); > r = user_mkdir_runtime_path(u); > + label_finish(); > if (r < 0) > return r; I tested this briefly and it seems to work. Can you please apply it to upstream? Thanks!!
(In reply to Dominick Grift from comment #12) > https://bugzilla.redhat.com/show_bug.cgi?id=1008601 > > By the way those sshd dyndomain hardcoded types (sshd_net_t) are also still > there Sure we know about sshd bug. But I was curious about systemd hardcoded types.
here is the url to that issue: https://bugzilla.redhat.com/show_bug.cgi?id=1008601 I may not directly be systemd. I suspect it is somehow mount related, since the entries are mountpoints and it happens then things get mounted on those entries
We never figured out where those were coming from.
So can we do this all with filename_transition rules?
Yes but again we would need to have filename_transition rules for all /run/user/<UID>
I guess it was Dominick's idea why systemd+SELinux is needed for this.
Why wouldn't they all be labeled the same user_run_t?
Ok I believe Dominick wanted to treat it as well as we have /root vs. /home/mgrepl admin_home_t vs. user_home_dir_t
But again do we really need it? I mean to change it from user_tmp_t.
Hmm, but can't you have one rule for /run/user/0, and then another rule for everything else? Then you can label one admin_run_t, and the other one user_run_t...
gdm, root also maintain dirs there. And who know who else in the future. You can't easily specify transition to this for uid 0 to 999 and to that for uid 1000 to 60000 with named type transitions I would prefer the gdm and root dirs to not be labeled the same as real users runtime directories I suppose, we may also be able to rely on the less reliable name file transition solution but i do not see what the big deal is here to be honest. Even if we implement this we can still also use named file transitions I do not understand why we have to use all these rules if we can just tell systemd to reset the labels. Heck , systemd does all kinds of selinux stuff already Anyhow, do what you please. Ill just patch my own systemd
go ahead and add 59999 named file transition rules for /run/user/1000 to /run/user/60000 better yet , dont bother at all and leave it all user_tmp_t
Yes, we can not do that easily at all using the default SELinux behaviour. Basically we set the labeling in useradd for example if a new homedir is created. So yes, SELinux "hacks" are not good in userspace but also we already do it in some cases where we really need it and where we get security benefits. Lets leave it open and I believe we can still discuss it.
logind also sets DAC labels on user runtime dirs, why no also the SELinux labels?
This bug appears to have been reported against 'rawhide' during the Fedora 22 development cycle. Changing version to '22'. More information and reason for this action is here: https://fedoraproject.org/wiki/Fedora_Program_Management/HouseKeeping/Fedora22
Fedora 22 changed to end-of-life (EOL) status on 2016-07-19. Fedora 22 is no longer maintained, which means that it will not receive any further security or bug fix updates. As a result we are closing this bug. If you can reproduce this bug against a currently maintained version of Fedora please feel free to reopen this bug against that version. If you are unable to reopen this bug, please file a new report against the current release. If you experience problems, please add a comment to this bug. Thank you for reporting this bug and we are sorry it could not be fixed.