Do not terminate on authentication failure

[ymatsiuk]

tuigreet behaves differently than greetd expects:

To reproduce enter unknown credentials in:

• agreety (greetd restarts and I see prompt again):

  jun 20 11:00:36 nixps greetd[29738]: pam_unix(greetd:auth): check pass; user unknown
  jun 20 11:00:36 nixps greetd[29738]: pam_unix(greetd:auth): authentication failure; logname= uid=0 euid=0 tty= ruser= rhost=
  jun 20 11:00:36 nixps greetd[29738]: gkr-pam: error looking up user information
  jun 20 11:00:36 nixps greetd[29738]: pam_unix(greetd:auth): check pass; user unknown
  jun 20 11:00:38 nixps greetd[29738]: error: authentication error: pam_authenticate: USER_UNKNOWN
  jun 20 11:00:38 nixps greetd[29315]: client loop failed: i/o error: Broken pipe (os error 32)

• tuigreet (login prompt never renders - requires manual greetd restart):

  jun 20 11:02:25 nixps greetd[30992]: pam_unix(greetd:auth): check pass; user unknown
  jun 20 11:02:25 nixps greetd[30992]: pam_unix(greetd:auth): authentication failure; logname= uid=0 euid=0 tty= ruser= rhost=
  jun 20 11:02:25 nixps greetd[30992]: gkr-pam: error looking up user information
  jun 20 11:02:25 nixps greetd[30992]: pam_unix(greetd:auth): check pass; user unknown
  jun 20 11:02:27 nixps greetd[30992]: error: authentication error: pam_authenticate: USER_UNKNOWN
  jun 20 11:02:27 nixps greetd[30371]: client loop failed: i/o error: Connection reset by peer (os error 104)
  jun 20 11:02:27 nixps greetd[30371]: error: check_children: greeter exited without creating a session

[apognu]

I am having some difficulty reproducing this. I got the same log messages as you, but the greeter is properly restarted by greetd after a failure. Still investigating to see if I can find a way to reproduce.

[ymatsiuk]

I think it might help: because I'm using initial_session in greetd systemd unit I have no Restart=always

[apognu]

Isn't the issue coming from greetd being configured never to restart? As far as I know, agreety allows you to fail authentication 5 times (by default) before exiting (it loops the prompt internally), but when I launch my greetd with Restart=no, it "freezes" after five attemps without restarting (with the same check_children error that you posted above).

tuigreet, on the other hand, only allows one attempt by session, and exits after the first failure.

Can you try to fail authentication five times on agreety and report if you experience the same? If the prompt does not show, then it is not a bug, but rather different opinionated functional behavior between agreety and tuigreet.

[apognu]

This behavior is subject to discussion, of course, but if I'm right, this only moves the issue back towards the maximum retry count, same as agreety, which I am not really fond of (except if we set up infinite retry count).

[ymatsiuk]

Can you try to fail authentication five times on agreety and report if you experience the same?

~This is what I've done before reporting this issue 🙂 The same config works perfectly with agreety. I feel like if I fail login agreety doesn't propagate the error to greetd and the latter keep running. With tuigreet first attempt breaks immediately after failed login attempt. Sorry I should've put more details in the issue description.~

Edit: apologies for creating confusion. I was consistently ignoring word "five" in your question. And now I've got some spare time to play around with config, I see you point. You are completely right! agreety does handle this exactly 5 times.

[ymatsiuk]

I'd love to have an infinite loop there, so it enables me to use initial_session and make sure that greetd doesn't Restart=always, which is pretty significant security concern. Do you think it's possible/reasonable?

[apognu]

I'm not against it, and I cannot see any significant drawbacks with trying and do so right now (apart from implementation). I would also have to remove the action to "exit" the greeter (pressing ESC).

I am wondering why this is the default behavior with agreety then. I was not aware of the initial_session configuration setting, and indeed this can be problematic, since all one has to do is fail five times in order to gain access to your session.

I'll check the behavior of other greeters too, because initial_session seems dangerous indeed when combined with Restart=always. Maybe this could be raised on greetd's mailing list?

[apognu]

gtkgreet seems to be looping on authentication failure, but does provide a button to close the greeter, which when combined with initial_session, would also give access to the session.

Now that I think of it, inital_session and Restart=always is also "vulnerable" to any crash within the greeter.

[apognu]

@ymatsiuk I am currently working on this, could you give a try to one of the artifacts from this build and report of the behavior is what you would expect?

[ymatsiuk]

Thanks! Tested 0c5b66df2d95d860290db08382f572150a515c05 and it works perfectly as I would expect!

[apognu]

For your interest, I posted on greetd's mailing list here to inquire about best practice.

I want to check if there might be some consequences to doing this (I think there is none since gtkgreet seems to be doing just that) and at least to have the maintainer's opinion on the overall issue, which would happen whenver the greeter exits, whatever the reason (crash, coded exit or manual action). I believe users should at least be warned about this behavior.

I'll wait for a response and continue testing my patch. If everything goes well, I'll release a new version with this.

[apognu]

greetd's maintainer has confirmed that looping on authentication failure is not an issue. So I am going to implement this when I have had enough time to test it thoroughly.

Apparently, this was discussed on greetd's side in this issue, but no solution was found or implemented yet. I'll go have a look in case I can find a solution.

[ymatsiuk]

Awesome! Let me know if anything is needed from my side, I'd be glad to help testing 🙂

[apognu]

My initial tests seem to be OK, I merged this into master.

I will continue testing this version for a few days and probably create a release during the weekend.

Thanks!

[apognu]

This was included in 0.5.0 and published to AUR.

Thanks for raising this!

[ymatsiuk]

I've created version bump PR: https://github.com/NixOS/nixpkgs/pull/128517

[apognu]

Oh I did not know tuigreet had been packaged for NixOS. That's great!

Would you consider creating a PR to update the README here with installation instructions?

[ymatsiuk]

https://github.com/apognu/tuigreet/pull/28 🙂