Add support for security-context-v1

[nesteroff]

This adds support for the security-context-v1 protocol. It is used by sandbox engines to restrict the features that sandboxed connections can use. For example, it is currently supported by Flatpak.

The implementation is based on the Sway code but adapted to Labwc. The sandbox_engine and sandbox_app_id parameters are added to window rules in order to be able to apply different rules for sandboxed clients.

The easiest way to test it is probably to take a recent version of Flatpak. Use it to run a Wayland application (for example, Firefox) with WAYLAND_DEBUG enabled and check for wp_security_context in the logs.

[Consolatis]

First of all, thanks for the PR.

What is the purpose of adding support for the sandbox manager engine / app_id to the window rules?

I am not that much in favor of hardcoding a list of "privileged" protocols for any sandboxed client, it seems cleaner to allow the user to define the protocols instead based on the sandbox engine name and potentially the sandbox engine app_id.

[nesteroff]

What is the purpose of adding support for the sandbox manager engine / app_id to the window rules?

It can be used to help users visually identify applications running in sandboxes. One use case is when multiple instances of a single application are running under different security policies. For example, a user might have two browsers running: one for untrusted web apps, which is isolated in a sandbox, and another running normally. In this case, the user can configure different window styles with different colors to quickly identify which is which.

I am not that much in favor of hardcoding a list of "privileged" protocols for any sandboxed client, it seems cleaner to allow the user to define the protocols instead based on the sandbox engine name and potentially the sandbox engine app_id.

Sure, it definitely needs to be more flexible. I just thought we could start with a simple implementation, where there is a reasonable set of protocols blocked for sandboxed clients by default. Later on, we could add the ability to whitelist or blacklist other protocols.

I guess it could be something like this:

<sandboxRules>
    <sandboxRule app_id="org.mozilla.firefox" engine="org.flatpak">
        <protocol name="output_power_manager_v1" restrict="no" />
    </sandboxRule>
</sandboxRules>

[Consolatis]

I am not that much in favor of hardcoding a list of "privileged" protocols for any sandboxed client, it seems cleaner to allow the user to define the protocols instead based on the sandbox engine name and potentially the sandbox engine app_id.

Sure, it definitely needs to be more flexible. I just thought we could start with a simple implementation, where there is a reasonable set of protocols blocked for sandboxed clients by default. Later on, we could add the ability to whitelist or blacklist other protocols.

If we make the hardcoded list depend on the sandbox engine being org.flatpak for now I would feel more comfortable with that approach.

I guess it could be something like this:

Just for reference, some earlier work (without the security context protocol but using different wayland sockets instead that can be bind-mounted into filesystem namespaces):

• [wip] Allow blocking wayland protocols and support multiple listening sockets with their own block list #1004

[nesteroff]

If we make the hardcoded list depend on the sandbox engine being org.flatpak for now I would feel more comfortable with that approach.

Well, it will result in more hardcoding in the sources. What if I just remove all hardcoded protocols and leave only security_context_manager_v1 for now?

Just for reference, some earlier work (without the security context protocol but using different wayland sockets instead that can be bind-mounted into filesystem namespaces):

• [wip] Allow blocking wayland protocols and support multiple listening sockets with their own block list #1004

That's interesting. Thanks for sharing. I didn't know you had already thought about it. Are you still going to work on that PR, or would you like me to use it as a reference to add settings to rc.xml?

[Consolatis]

Well, it will result in more hardcoding in the sources. What if I just remove all hardcoded protocols and leave only security_context_manager_v1 for now?

I would be fine with having a minimal PR first that only adds the protocol support and blocks the security context protocol itself.
Although I don't know if flatpak or other sandbox engines have expectations like "that protocol is supported so these X protocols are blocked" and thus this would actually result in less security (by sandbox engines relying on that assumption instead of spawning a nested compositor for example) rather than enhance it.

In the longer term I think we could add some rule framework and then have hardcoded defaults (potentially per sandbox engine) that are applied when there is no user configuration for the given sandbox engine.

That's interesting. Thanks for sharing. I didn't know you had already thought about it. Are you still going to work on that PR, or would you like me to use it as a reference to add settings to rc.xml?

The later versions of that PR are still somewhere on a local branch but I think the security context protocol is preferable to the separate socket approach, combined with WAYLAND_SOCKET=some_inherited_fd that should handle most use-cases.

[nesteroff]

I would be fine with having a minimal PR first that only adds the protocol support and blocks the security context protocol itself. Although I don't know if flatpak or other sandbox engines have expectations like "that protocol is supported so these X protocols are blocked" and thus this would actually result in less security (by sandbox engines relying on that assumption instead of spawning a nested compositor for example) rather than enhance it.

Ok, sounds good. I've updated the PR with a minimal implementation. Hopefully, sandbox engines will not make any assumptions, since there doesn't seem to be any official list of protocols that the compositor must block for clients with a security context attached.

In the longer term I think we could add some rule framework and then have hardcoded defaults (potentially per sandbox engine) that are applied when there is no user configuration for the given sandbox engine.>

Yep, it would be nice to follow the fail-safe defaults principle, where everything that can potentially be considered insecure is blocked by default for sandboxed clients, and users manually allow permissions when needed.

[Consolatis]

This could just be

return wlr_security_context_manager_v1_lookup_client(
	view->server->security_context_manager_v1, client);

[Consolatis]

code style nit: double indent for multi line ifs. Same in the next condition.

[Consolatis]

Nit: we usually use the sandboxEngine format for user facing settings. Same for sandboxAppId.

[nesteroff]

Sorry for the delay. I was traveling last week. The code has been updated and rebased. Please have a look.

[johanmalm]

I haven't analysed this, but suspect view_matches_query() is called a lot, thus making anything within it is therefore an expensive operation.

Unless I'm wrong in that, would it make sense to declare + initialize it to NULL at the top; and then set it to security_context_from_view() iff match && (query->sandbox_engine || query->sandbox_app_id)?

[nesteroff]

Sure, I reduced the scope of the security_context variable to be used only when needed.

[johanmalm]

LGTM
Keen to understand what the next PR might look like - but not a prerequisite to merging.

[nesteroff]

LGTM Keen to understand what the next PR might look like - but not a prerequisite to merging.

Thanks. I think the next step is to add the ability to configure which protocols are available for sandboxed clients, similar to what @Consolatis shared in this thread. It probably shouldn't be too hard to adapt it to the secure context.

[johanmalm]

Thanks. Excellent work.