-
Notifications
You must be signed in to change notification settings - Fork 1.2k
sway: implement selinux based filtering for globals #8906
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
base: master
Are you sure you want to change the base?
Conversation
eace4c3 to
86268a1
Compare
|
Is there any precedent for this specific use of SELinux, either for Wayland interfaces specifically or similar constructs? One concern is that we natively support FreeBSD, and we generally write code targeting both platform. This does not rule out ifdefs, but core functionality should be portable, and so if we find arbitrary global filtering rules to be a notable feature we would need a mechanism that also works on FreeBSD. Likewise, we also support non-SELinux setups, and those should not be without core functionality. |
Yes. Xorg has XACE, which is actually what I modelled the basic design off of. That uses SELinux to control access to all parts of the X server. SELinux reference policy supports it upstream, and so does the Xorg server.
I do understand. We have
This only adds another filtering mechanism on SELinux setups. Other setups can rely on things like If anything, I'd argue that |
|
Update:
Regardless of FreeBSD's lack of support for this specific form of global filtering, I still think this would be useful to have. I'd personally argue that the static bool filter_global(const struct wl_client *client,
const struct wl_global *global, void *data) {
#if WLR_HAS_XWAYLAND
struct wlr_xwayland *xwayland = server.xwayland.wlr_xwayland;
if (xwayland && global == xwayland->shell_v1->global) {
return xwayland->server != NULL && client == xwayland->server->client;
}
#endif
if (!check_access_selinux(global, client)) {
return false;
}
// Restrict usage of privileged protocols to unsandboxed clients
// TODO: add a way for users to configure an allow-list
const struct wlr_security_context_v1_state *security_context =
wlr_security_context_manager_v1_lookup_client(
server.security_context_manager_v1, (struct wl_client *)client);
if (is_privileged(global)) {
return security_context == NULL;
}
return true;
}Right now, we can consider |
1d842ee to
e66c312
Compare
Note that using security-contect and creating new sockets is not complex: Enforcement can be done with file ACLs, e.g. by making the original socket inaccessible and either setting the WAYLAND_DISPLAY env var or moving the new socket into the original's spot after setup. Note that my point for portability was not to discover N different implementation paths - and as there is no precedent for Wayland, having to invent the all these mechanisms - but rather consider if we could have just one, platform-agnostic path. You could, for example, propose an alternate protocol that allows a client to present a listener, accept new connections and only then forward both the connection and a policy decision to the compositor. However, even if one went with this SELinux-based mechanism, it is something that would require community alignment across display servers for the exact appraoch and interface/protocol/resource naming. It would be a mess if every Wayland client packaged on an SELinux distribution would have to bundle different SELinux policies for every display server it targeted. |
e66c312 to
b6d2817
Compare
This patch exposes privileged globals to SELinux so access to priv globals can be mediated by SELinux. Signed-off-by: Rahul Sandhu <[email protected]>
b6d2817 to
ca7cd98
Compare
It has however the unfortunate need to patch things like launchers. And it gets quite messy in policy as you also need to consider socket "cross contamination" (i.e. you can't simply use the same label for security context sockets as nothing would prevent accessing another socket), and mcs categories have a limit in the number of them that exist, and would also mean that this would not work on policy without mcs support (which there is plenty of).
I did actually previously consider and draft this, however it does have an unfortunate disadvantage of there being a window in which before a client could bind to such protocol, during which nothing is checked, leaving a race condition window for which any client could bind to the access checking protocol.
I do agree with this. I'm also the author of the work upstreaming Wayland policy to SELinux reference policy. I'll outline my plan: |
Not really, just do what needs to be done before running the launcher.
It's not much of a race as the startup process is controlled, but there would be ways to solve this if needed.
The patch as it is written right now is not using wayland interface or protocol names for the SELinux "permissions", but the names of sway struct fields. |
It's not possible to create a socket with a specific context (the important) in advance for something without knowing what is at least the binary path to said socket; I need a specific socket type for each app, e.g.
"It's not much of a race as the startup process is controlled" - would you mind elaborating on that? i.e. the exec's sway runs or? In theory could a malicious client not time the window between sway launching and the first exec and bind to said global, or is there something I'm missing?
Would you have any suggestions? Every way I've thought of I've also thought of a workaround for. The problem imo is that Sway needs to know that whatever client binding to a privileged global is trusted, but the reasoning for this patch at present is because Sway doesn't have a way to identify clients as "secure" or "trusted"; there is a way to create lower privileged clients (with
I don't mind changing it to use the specific protocol names, that'd work fine (although a bit messy as they're longer), my earlier point was moreof this patch establishes the precedent. |
If malicious clients are running before you launch sway, which means before your user session is initiated and therefore before you started any clients, then you have already been compromised with persistent code execution. As this has many other implications that make it unlikely that such system remains safe to use, it might not be relevant to worry about in this context. If you have not already been compromised with persistent code execution, clients will only start when sway exec's them/you tell them to start, with permissions/sandboxing serving to limit the capabilities of processes you chose to run. The timing here is flexible and entirely controllable. |
|
@kennylevinsen Had a discussion with @navi-desu and here is an idea that arose from that discussion:
This solves a number of problems:
Thoughts? If this sounds good overall, I'll start work on a Wayland protocol (in the Thanks |
I understand that the issue on this was previously turned down because you understandably don't want to have to maintain SELinux code, but I think it may be worth a second look:
selinux_check_access (),getcon_raw (),security_getenforce (), andselinux_set_callback (). These are also all stable APIs that have not changed in years; for example, the last change toselinux_check_access ()was 11 years ago at the time of writing, and even calls made previously would still succeed due a typedef. Upstream (libselinux) also promises ABI stability.If it's of any reassurance, outside of some personal emergency, I'm available to respond to any bugs fairly quickly. I'm happy to be pinged on any issues created that even involve builds with the SELinux build option if desired. I've run this on my own personal systems for over 6 months now.
I do understand if you still wish to turn it down, however this would be great to have upstream; it would allow a compelling case to work on improving the Wayland security model in reference policy as well.
Best Regards,
Rahul
Sidenote:
get_selinux_protocol ()is a bit ugly, but I'm not sure if there's a nicer way to write this;->nameand trim doesn't seem to work asstruct wl_globalis opaque :/ - suggestions appreciated.