Skip to content
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

All finished events are resulting in an exit(2), when the protocol states otherwise #346

Open
mattkae opened this issue Feb 22, 2024 · 6 comments

Comments

@mattkae
Copy link

mattkae commented Feb 22, 2024

I noticed that whenever a finished event is sent to ext_session_lock_v1, swaylock exits with 2. According to the spec (source: https://wayland.app/protocols/ext-session-lock-v1#ext_session_lock_v1:event:finished):

If the locked event is sent on creation of this object the finished event may still be sent at some later time in this object's lifetime. This is compositor policy.

Upon receiving this event, the client should make either the destroy request or the unlock_and_destroy request, depending on whether or not the locked event was received on this object.

In this case, I would expect swaylock to send an unlock_and_destroy request and exit gracefully.

Use case: This could happen because the compositor's policy accepts unlocking via logind (or some other 3rd party mechanism) as a valid unlock. In this case, we want the compositor to inform the client that they can exit without any issues.

Let me know if I'm misunderstanding something here. Cheers!

@emersion
Copy link
Member

I'm not sure this is a good thing to do. I'm worried about unlocking in other situations that when the user has correctly typed their password. If the compositor decides to unlock, they shouldn't care that the client sends the unlock_and_destroy request.

@kennylevinsen
Copy link
Member

I suppose this issue is primarily about the exit code. To sort that part out, we could exit(0) if we have previously received confirmation of lock, and exit(2) otherwise, so scripts that called swaylock can see that it was not a failure.

(Sending destroy before existing is redundant, the compositor will clean up.)

@mattkae
Copy link
Author

mattkae commented Feb 26, 2024

An exit(0) would solve my particular problem. However, I see why an exit(2) was being used, because this finished event is sent both when the compositor is done with the lock and when the compositor denies the lock altogether. I would expect an exit(0) in the former and an exit(2) in the latter, as the latter means the lockscreen didn't work at all.

@emersion
Copy link
Member

I'm a little uneasy exiting with a zero code when the user hasn't explicitly unlocked the screen by typing the correct password.

@mattkae
Copy link
Author

mattkae commented Feb 26, 2024

Yeah I understand that. The fact that the finished event is coupled with a theoretical denied event isn't helping matters either. The spec says "This might also be sent because the compositor implements some alternative, secure way to authenticate and unlock the session" but how could we know from the finished event whether or not that was sent in response to a failure.

If that's the case, I'd be okay leaving it for the time being. We might eventually require an updated spec if we're serious about the idea that compositor may implement another avenue for authentication (e.g. logind) 😉

@mattkae
Copy link
Author

mattkae commented Feb 26, 2024

Although... Perhaps I am wrong! There are two scenarios I see:

  1. The finished event is received before the locked event, meaning that the lockscreen never started.
  2. The finished event is received after the locked event, meaning that the compositor decided that the lockscreen is no longer needed

The first case would be a valid exit(2), while the second would be a valid exit(0)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

3 participants