ext-session-lock-v1: clarify to fix race

Clients such as swaylock 1 or waylock 2 provide options to fork and detach from the controlling terminal when the session is locked. The point of these options is avoid a race on suspending the system. A command to suspend the system (e.g. zzz) may safely be chained with e.g. waylock as so:

waylock -fork-on-lock && zzz

However, there is no guarantee that the compositor has actually blanked all outputs before sending the locked event. Therefore there is still a race as new "locked" frames may not have been presented on all outputs before the system is suspended.

On my Linux system at least, the current framebuffer seems to be preserved on suspend and restored on resume, leading to an "unlocked" frame potentially being displayed when the system is resumed. Blanking all outputs before suspend eliminates this vulnerability.

Currently clients could theoretically implement such -fork-on-lock options a bit better if the compositor supports the presentation-time protocol, however no clients I've seen currently do this and it seems wise to make clients to do the right thing by default in this security sensitive context. The presentation-time protocol is also not sufficient in all cases, for example if the compositor has turned off power of an output but still exposes it to clients. In this case the client would wait forever to get a presentation feedback that will never come.

Unfortunately, the protocol currently states that the locked event will be sent immediately on creation of the ext_session_lock_v1 object rather than after all normal content is hidden.

Several different approaches have been considered for how to fix this in the protocol specification.

One possibility would be to add a new event sent when all normal content is hidden. This is however opt-in for clients and therefore less likely to be properly implemented by all clients in practice.

Another alternative is to bump the version of the ext_session_lock_v1 interface and state that the semantics of when the compositor will send the locked event. However, this still requires clients to opt-in by binding version 2 of the interface. The compositor could technically deny the attempts of any version 1 clients to lock the session, but this would likely be a bad breaking change for users of version 1 clients. While session lock clients should inform the user in some way that their attempt to lock the session was denied (e.g. by exiting non-zero) it does not seem to be the case that such exit codes are widely checked.

The option to fix the protocol that is all around the most secure is changing the semantics of the locked event without bumping the version of the interface. This is technically a breaking change, but the failure mode is that a client relying on the locked event being sent immediately hangs or crashes and the session stays locked.

I also have been unable to find any session lock client in the wild that relies on the locked event being sent immediately.

The river wayland compositor 3 in fact already implements the fix for this race condition since the 0.2.0 release and has not received any bug reports about broken session lock clients yet.

Therefore, I think that making this technically breaking change to the protocol is our all around best option in this situation. Prioritizing security over compatibility seems like the right trade-off to make for a security critical protocol.

Signed-off-by: Isaac Freund mail@isaacfreund.com