server.c: block privileged protocols for sandboxed clients

[Consolatis]

In the longer term we want this to be user-configurable (and also depend on the sandbox engine, app_id and instance).

But dropping privileged protocols for sandboxed clients in the meantime seems like a sensible thing to do and matches user expectations.

Related: #2392

---

I think we should add this before 0.8.2.

When we implement a user config for this we can also move the new functions into its own file, they feel a bit hacked onto server.c with this PR.

Can be tested with https://gitlab.freedesktop.org/wlroots/wlr-clients/-/merge_requests/6

[tokyo4j]

I'm a bit confused with allow_for_sandbox() in this PR. Do you mean we will add user-configurable whitelist for protocols allowed for sandboxed apps, like <allowedProtocols>minimal|minimalEnhanced|minimalEnhancedPlus<allowedProtocols>?

[Consolatis]

Do you mean we will add user-configurable whitelist for protocols allowed for sandboxed apps, like <allowedProtocols>minimal|minimalEnhanced|minimalEnhancedPlus<allowedProtocols>?

No, that was more for being able to comprehend the accept list in code, I could throw them all into one array instead if preferred.

In the longer term I think we should allow users to define the interface names themselves which then get allowed or blocked based on security-context engine, app_id and instance + have a default list in case its not configured by the user.

When we get to implement user defined block lists I think we still need some kind of profiles though to keep the user configs maintainable. Maybe roughly something along the lines of #1004 (comment) just with new selectors rather than socket names, based on the security-context protocol.

[tokyo4j]

OK. I read through #1004 and I support basic ideas suggested there.

But I still wonder why allow_for_sandbox() is added in this PR as it just looks like an inverted version of protocol_is_privileged().

[Consolatis]

But I still wonder why allow_for_sandbox() is added in this PR as it just looks like an inverted version of protocol_is_privileged().

Yep, it is. For sandboxes we really don't want to accidentally expose a privileged protocol, thus allowing everything on an accept list and rejecting everything else. protocol_is_privileged() is basically useless (and only there to influence WLR_ERROR vs WLR_DEBUG logging). The idea was actually to not log in the WLR_ERROR case but instead outright crash via assert(protocol_is_privileged(iface)). That should make sure that our lists are matching the protocols implemented in the compositor. However, I didn't want to take that risk in a cool-down period before a release, thus the log rather than the assert().

I am open to improve the PR though if there are specific things that should be changed.

[johanmalm]

LGTM. Not tested.

[tokyo4j]

LGTM. I've checked all the globals we're exposing are listed.