Security Context protocol

[ids1024]

The compositor should not expose the wp_security_context_manager_v1 global to clients connected to a security context. Otherwise, I guess this would be up to the compositor to set its own restrictions.

This implementation is currently incomplete.

[codecov-commenter]

Codecov Report

Patch coverage: 13.77% and project coverage change: -0.07% ⚠️

Comparison is base (c569c87) 24.01% compared to head (608f151) 23.94%.
Report is 4 commits behind head on master.

❗ Current head 608f151 differs from pull request most recent head 56d2741. Consider uploading reports for the commit 56d2741 to get more accurate results

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #1106      +/-   ##
==========================================
- Coverage   24.01%   23.94%   -0.07%     
==========================================
  Files         140      142       +2     
  Lines       21591    21756     +165     
==========================================
+ Hits         5184     5209      +25     
- Misses      16407    16547     +140     

Flag	Coverage Δ
wlcs-buffer	20.98% <13.77%> (-0.05%)	⬇️
wlcs-core	20.61% <13.77%> (-0.05%)	⬇️
wlcs-output	8.51% <13.77%> (+0.05%)	⬆️
wlcs-pointer-input	22.65% <13.77%> (-0.06%)	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Changed	Coverage Δ
src/backend/egl/display.rs	0.00% <0.00%> (ø)
src/backend/renderer/element/memory.rs	0.00% <0.00%> (ø)
src/utils/clock.rs	42.55% <0.00%> (+1.31%)	⬆️
src/wayland/security_context/listener_source.rs	0.00% <0.00%> (ø)
src/wayland/security_context/mod.rs	15.78% <15.78%> (ø)
anvil/src/state.rs	38.00% <23.80%> (-0.47%)	⬇️

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[ids1024]

Rebased on top of #1108, so CI can pass. This seems to be working correctly now, testing Anvil with a flatpak containing the wayland-rs list_globals example, I can confirm the security context global appear when run in a flatpak when flatpak is built from master with support for this.

[Drakulix]

Looks good to me in general, though I wonder, if we should force can_view to just return false for any client, that already has a security context?

While I am all for flexibility, I am not sure, for what scenario you could want a faulty implementation.

A filter for more restrictive filtering could still be provided.

[ids1024]

I wonder, if we should force can_view to just return false for any client, that already has a security context?

I think so, but I couldn't think of a good way to do that. The ClientData associated with the client is managed by the compositor, not Smithay, so I don't know if Smithay has a good way to test if Client has a security context... except by having a method to query that implemented by the compositor.

If Smithay doesn't use the security context for anything except excluding clients with a security context from using this protocol, the filter function already handles that so it's a bit redundant to add anything else.

[Drakulix]

I wonder, if we should force can_view to just return false for any client, that already has a security context?

I think so, but I couldn't think of a good way to do that. The ClientData associated with the client is managed by the compositor, not Smithay, so I don't know if Smithay has a good way to test if Client has a security context... except by having a method to query that implemented by the compositor.

We could add a trait requirement on ClientData to the implementation, so that we could query the security context. Though I get how that might be a bit annoying.

If Smithay doesn't use the security context for anything except excluding clients with a security context from using this protocol, the filter function already handles that so it's a bit redundant to add anything else.

Smithay itself wouldn't do anything else, but I think it is a good idea to allow downstream to add additional filters.

[ids1024]

I don't think there's anywhere we could a dd a trait bound for the ClientData since that uses dynamic dispatch, and insert_client is called by the compositor. And yeah, that would be a bit annoying and add some complexity if it were possible.

[Drakulix]

Yeah, I remember creative workarounds to this I wrote in the past. They wouldn't really add any more safety over this approach.

[ids1024]

Yeah, I thought about something like that. Though you couldn't even call client_compositor_state in a can_view function since it is only passed the client and global_data, not the compositor's state.

And if it's only going to be used to test is_some, that's basically just a slightly more complicated filter function.

[Drakulix]

Right. I am gonna merge this as is, once the CI-PR has been sorted out.