Two unrelated fixes #2053
Two unrelated fixes #2053
Conversation
Let's distuingish the cases where our code takes an active role in selinux management, or just passively reports whatever selinux properties are set. mac_selinux_have() now checks whether selinux is around for the passive stuff, and mac_selinux_use() for the active stuff. The latter checks the former, plus also checks UID == 0, under the assumption that only when we run priviliged selinux management really makes sense. Fixes: systemd#1941
…h "_sd_" This renames __useless_struct_to_allow_trailing_semicolon__ everywhere to _sd_useless_struct_to_allow_trailing_semicolon_, to follow our usual rule of prefixing stuff from public headers that should be considered internal with "_sd_". While we are at it, also to be safe: when the struct is used in the C++ protector macros make sure to use two different names depending on whether it appears in the C++ or C side of things. After all, there might be compilers that don't consider C++ and C structs the same. See systemd#2052 (comment)
|
I do not know your rationale for this decision. Can you please also explain why you think that access control should only apply to uid 0. Can this be made conditional/tunable? |
|
Regardless of whether this is right or wrong. I think you aren't qualified to make this decision. Simply because you do not know all of the requirements. Also try to not just look at this issue from a Fedora/RedHat usecase only. Creating a PR for RFC is a good idea, but please do not merge this before some people with solid vision ACK this. I do not feel comfortable enough to determine whether this indeed makes sense or not in the bigger picture. Although I tend to think that this should just stay in. This stuff can be disabled in policy simply by allowing full access. At least by doing this we leave room open to maneuver. dbus running as ! UID 0 also enforces selinux access control. What makes this any different? Why are you so eager to remove SELinux functionality but when it comes to adding even the slightest bit it is nearly impossible to get anything done |
|
One last note: I think it is not helpful to address two different issues in a single PR. please consider instead creating two PR's , one for each issue. |
|
@doverride well, for starters, unprivileged code has no write access to the audit logs, hence it couldn't even write anything about failed access there. But generally, user code like "emacs" or "vim" do not ask selinux for security decisions, as they are not placed on a trust boundary. "systemd --user" isn't any different there. And, sorry, for smaller fixes we often do combined PRs, I doubt this is going to change, it's just too much work to create a separate branch for each trivial fix. |
|
@poettering again, dbus has the same characteristics (the session-bus also enforces selinux access control and also has the issue of auditing) Consider implementing a solution to the audit issue that is similar to that of the dbus session instance Instead of comparing systemd --user with "vim" or "emacs" compare it to something more appropriate like dbus session instance (or xserver). The dbus session instance (and xserver) both do enforce selinux access control. They both can be run as unpriv users. They both have the audit requirement. Please do not merge this without the ACK of someone that understands SELinux, and access control |
|
@poettering do not hesitate to ask the SELinux upstream community for advice in this: https://www.nsa.gov/research/selinux/list.shtml You might recall how helpful they can be if you just reach out. |
|
@keszybz It works fine: The only issue is that systemd --user is not permitted to log to audit |
|
Short demo: systemd --user selinux access control |
|
I would want to be able to govern system-wide which applications can control which applications with systemd --user, and this functionality allows me to do that. I can say allow emacs to stop gnome-shell but not gpg-agent via systemd --user for example. |
|
My final comment to this PR: Please do not do this. I believe we will regret this later. This functionality enables for integrity in the user sessions. It is not really compelling currently because systemd --user is not widely used. This will change. processes will , Instead of just directly running some application, at some point start using systemctl --user to run some applications, and we will want to be able to control that so that a compromised process cannot go and control random applications with systemctl --user. So this is for the record. I hope i do not have to refer to this post later some time in the future, and say: "i told you so" do not do it |
|
Well, as Dan Walsh appears fine with not doing this I think we should go ahead, and not do it. @doverride selinux does not exist in thin air, if we do selinux checks from unprivileged clients, we also need audit opened up for unprivileged clients, and I find this unlikely. i'd rather not complicate things by using selinux if the selinux-folks don't ask for it. |
|
I will remind you when you revert this later... |
|
Also Dan Walsh does not represent the SELinux community. Heck he doesnt do SELinux at all anymore. Also this is not a redhat only thing... |
@keszybz SELinux was properly initialsed if I'm not wrong (you don't need to be root to be a SELinux object manager), the only issue was, as explained in my original bugreport, that audit was not due to the lack of AUDIT_WRITE capability |
|
I think the point is, I hope I'm not wrong here, is that if we give a process the access to run one of the user unit, it will have access to all the units. Let's say that someone want to allow a process, gnome-session or gnome-settings-daemon (or whatever) to be able to start/stop/restart a service A but not a service B, while another daemon/process should only be allowed to start that other service B. If I'm not wrong this is possible with the current code. If the SELinux check is removed for the user session systemd, this wouldn't be possible anymore. |
|
Again, I am not interested to maintain half-assed implementations of stuff the SELinux maintainers I trust are not interested in. And SELinux access checks in the --user instance are of that kind: they are not thought to the end, since they cannot log audit stuff, and there's no replacement for that, not even a concept how that should be handled. There's nobody with the C skills to maintain this around, and the folks who do care and who do maintain the selinux stuff are not interested in this. And generally: is it even desirable to have systemd --user privileged enough to get access to the SElinux decisions database to make these checks? So, @doverride find me an interested C hacking SELinux maintainer, who thinks that selinux-based access control in systemd is great and wants to maintain it, and we can talk. Otherwise, I don't care at all, and think this needs to go. |
|
You should blame/thank Dan Walsh for the half-assed implementation. Also you were the one merging this stuff in the first place. People were building on this code, and now you are removing it for no good reason. |
Let's merge access_init() and mac_selinux_access_init(), and only call mac_selinux_use() once, inside the merged function, instead of multiple times, including in the caller. See comments on: systemd#2053
|
"systemd --user" concept is broken as we can see/read from this thread from SELinux point of view. And the point is if we have big benefits of this level of isolation? Are we able truly explain the enforcement here? Do we have real examples how it prevents possible security flaws? Do we talk about confined desktop here? How Lennart wrote we have the half-assed implementation here which brings these question and causes lot of troubles. And I believe we can discuss it and get answers with SELinux folks on SELinux list. |
|
In my original bug report, I never implied that the SELinux support for systemd user session was broken. The only thing that I suggested is that the systemd --user processes should preserve the AUDIT_WRITE capability to be able to log SELinux avc denials.
What privileges are we talking about here? The dbus daemon running in the user session without any special privilege is able to be used as a userpace SELinux object manager (same for the xserver). |
It was working fine except that it was trying to log to the audit system which unprivileged processes arent allowed to do.
I think i was able to at least identify some of it, but i probably have overlooked other compelling arguments: Any processes that needs to be able to "control" any system-wide systemd user unit, will be able to "control" all system-wide systemd user units. In practice that means that a process may be able to for example start a application even though it does not have access to execute that application by telling systemd --user do start that applications on its behalf A process may, for example, be able to use this to stop a sensitive process bypassing SELinux access control
With access control the benefits often depend on the requirements. but in general since selinux touts flexibility and one of the big benefits here is that we can contain access of processes to units actions. without this processes are potentially able to bypass selinux by telling systemd --user to do stuff on their behalf
No, and why should we? SELinux does not prevent possible security flaws. SELinux is Linux access control extension
No we are talking about a set of components that without SELinux access control extension potentionally allows processes |
|
Just a quick note. We take care about possible flaws and how SELinux is useful. Of course I talk about "prevent" but I mean "mitigate" in the most cases. |
Let's merge access_init() and mac_selinux_access_init(), and only call mac_selinux_use() once, inside the merged function, instead of multiple times, including in the caller. See comments on: systemd#2053
No description provided.