Make i3lock support PAM prompts (better support for PAM messages and passwordless PAM methods like fingerprint-gui)

pstray commented 5 years ago

I'm submitting a…

[ ] Bug
[x] Feature Request
[ ] Other (Please describe in detail)

Current Behavior

Currently you need to submit a password to trigger any PAM method, so just fingerprint won't work. For some reason I actually need to supply both a valid password and fingerprint when fingerprint-gui is enabled.

Expected Behavior

Unlock with just fingerprint, or just password.

Environment

Output of i3lock --version:

i3lock version:  2.10 (2017-11-25)

Airblader commented 5 years ago

This is largely a duplicate of i3/i3#852. The question there still stands: we need some way to know when a fingerprint has been scanned to trigger pam.

pstray commented 5 years ago

Well... I guess you have to trigger PAM to make it read a fingerprint and check it. Not sure how other screensavers (like gnome) does this, but they seem to managet to do it.

Airblader commented 5 years ago

I would expect the workflow to go the other way and the sensor to be always ready and somehow informing the kernel of the read, which pam hooks into. We certainly don't want to do this ourselves, so we'd need some connection through pam.

But however it works, if someone can investigate and report back it would help making a decision here.

webknjaz commented 5 years ago

This is largely a duplicate of #852.

@Airblader that's a typo. Which one did you mean?

camparijet commented 5 years ago

This is largely a duplicate of #852.

@Airblader that's a typo. Which one did you mean?

@webknjaz I believe he meant https://github.com/i3/i3/issues/852

Airblader commented 5 years ago

Yes, correct issue number, wrong repository. Sorry :-)

webknjaz commented 5 years ago

@Airblader it probably makes sense to edit the original comment ;)

webknjaz commented 5 years ago

Oh, and does it have to rely on i3 itself? I'm using just i3lock without i3, for example...

Airblader commented 5 years ago

No, the issue over at i3 is just so old that it predates Github. This is the correct repository.

webknjaz commented 5 years ago

The suggestion is that #852 now points to nowhere but once there's a new issue with this number in this repo it will point there and be totally misleading. If you replace it with i3/i3#852 in this comment https://github.com/i3/i3lock/issues/217#issuecomment-451696732 GitHub will create correct links and backreference from there.

Airblader commented 5 years ago

Yes, I was gonna update the comment, I just didn't wanna do it from mobile. Willdo now, thanks ;-)

andreesteve commented 4 years ago

Reading around seems that one way to do it (at least the way I understood it from reading this proposal in GDM) is to start multiple PAM calls in parallel using different service names (e.g. i3lock, i3lock_1, i3lock_2, etc) where the system admin could configure independent auth flows (password, fingerprint, smartcard, etc) and the application would accept any of these flows to successfully authenticate the user.

evamvid commented 4 years ago

This is the way fprintd seems to expect it to be done as well:

https://github.com/freedesktop/libfprint-fprintd/tree/master/pam

pam_fprintd doesn't support entering either the password or a fingerprint, as pam_thinkfinger does, because it's a gross hack, and could be fixed by having the login managers run 2 separate PAM stacks

evamvid commented 4 years ago

I've started (attempting) to implement multiple PAM stacks here: (https://github.com/evamvid/i3lock/commit/a703e99079a9f2990117996116f93a330bff3cf2). As of right now, it will accept the correct password or you can hit enter and scan your fingerprint (or theoretically any other pan module you like).

The improvement here is that previously, with fprintd, I could just hit enter and scan my fingerprint (it didn't matter whether or not a password was entered), but if I entered the password I'd still have to scan my fingerprint for i3 to accept it.

This is literally the first thing I've ever done in c/cpp, so I'm a bit over my head! I'm frankly not even quite sure if this adds any functionality that wasn't already there, because I think there's a possibility my pam might have been misconfigured. Any guidance would be much appreciated!

I set the PAM config to by default ignore the second stack (with pam_deny.so), but I included the config that I'd use for fprintd.

Note that this still requires you to hit enter to trigger the PAM call, but it doesn't require you to have entered a password.

Arinerron commented 4 years ago

Wouldn't it be simple to just fork, execve fprintd-verify, and if the exit code == 0, the fingerprint is verified? Of course, if the user enters the password first, you can just kill the process by pid

PAM is the better solution but seems a lot more complicated.

stapelberg commented 4 years ago

I think it’d be better to not run subprocesses from i3lock. This should be done via PAM.

Arinerron commented 4 years ago

I absolutely agree; however, I am tired of pressing enter everytime first. Is there a way around that?

volker-fr commented 4 years ago

I looked at the code and you need to refactor the logic a bit. The current logic is to only trigger a pam authentication request after you pressed enter.

In a PAM friendlier world you would trigger the general authentication request first, then read what kind of input is expected (fingerprint, password, yubikey) so you can print this out to the screen. After that you can provide the input (or password that has to be read by i3lock).

I implemented in a local POC already the option to read what PAM needs and print this out on the lock screen. But this still requires pressing return.

To not require this return, which leads to a refactoring of the logic of i3lock, will be in my limited opinion the biggest obstacle.

kaworu commented 4 years ago

Hi there,

I took a quick look into this over the weekend. My understanding from this thread and a bit of experimenting so far is:

To allow both the usual password login along with the fingerprint "smoothly" (i.e. not one after the other) we need two PAM stack. Thus, a new PAM service for i3lock is required (e.g. i3lock-fprint).
We need a thread / subprocess to handle the fprintd PAM conversation as pam_authenticate(3) is blocking. In the fprintd authentication PAM conversation we get a PAM_TEXT_INFO message before each fingerprint auth try.

A quick search into lightdm and gdm shows that they both have a worker handling the PAM conversation (see session-child.c and gdm-session-worker.c) with some kind of IPC (respectively pipe(2) and DBus).

I wrote a quick proof of concept patch for i3lock at https://github.com/kAworu/i3lock/tree/fprintd. The indicator blink pink when the fingerprint is ready. Once you failed a defined number of time (can be tweaked in the PAM service but default is 3 on my machine) it will count as an auth failure and the fingerprint will not be available anymore.

Technically it fork(2) a child process into fprint_main handling the i3lock-fprint PAM service. IPC is handled with OpenBSD's imsg(3) (bundled into the compat/ directory). I thought it was a good trade-off as I wished to avoid lightdm's pipe communication approach because it isn't flexible and DBus is, well, DBus.

To test you need to create i3lock-fprint PAM service:

echo "auth sufficient pam_fprintd.so" > /etc/pam.d/i3lock-fprint

Then, build and launch i3lock with the -2 or --fprint-auth flag.

Please note that this is very much a work in progress. There is a ton of thing that need improvement (signal handling, feedback UI) as I am not familiar yet with libev, X, nor PAM.

stapelberg commented 4 years ago

Thank you very much for looking into this and summarizing, this is very helpful.

In my cursory read, it seems like you only ever look at imsg.hdr.type. If that’s correct, I suggest just printing the different values in text onto stdout, line-by-line, e.g. “PAM_SUCCESS\n”. That way, we don’t need to import imsg into our tree.

The overall architecture of spawning a helper process to handle this seems reasonable to me.

kaworu commented 4 years ago

@stapelberg correct, at the moment only msg.hdr.type is used. I expected the IPC to grow in complexity, but hopefully in the end it'll be simple enough to get rid or replace imsg.

volker-fr commented 4 years ago

@kAworu I don't think you need two pam stacks. A single one should be sufficient. You just need to start that one earlier so you can get the message from PAM on what input is expected.

If you trigger PAM right after i3lock starts, the issue is that there can be timeouts in case you don't provide the fingerprint input in a timely manner which will never be the case.

In a POC I modified the conv_callback that provides the PAM message and hand that message over to the unlock indicator to print out what input is expected. You can find the diff here: https://github.com/volker-fr/i3lock/commit/2ce7800c96e14235d3238960ab3257c7e6217dbd

There are be two main changes/open questions required for it.

how to trigger PAM (pressing space, enter, ...)
modify when/how the password is read.

(1) is import due to the input timeout issue described above. Maybe there is a way to skip it but I am not sure.

(2) the current process is to store the password on the first input. This password is stored and used as soon as PAM expects an input. If your first PAM rule is to use fingerprint and it fails, it will jump to the password and hand over that password. This isn't logical (enter password while you can still use fingerprint) and can be confusing (first input is the password? Or can I enter it again? But on the 2nd run the first authentication will be fingerprint again, ...).

Backward compatibility to not confuse existing users will be one challenge here. One user presses enter to use fingerprint, but if it fails the user expects to get a way to input the password. Another user wants to use the password only but doesn't want to enter it twice.

stapelberg commented 4 years ago

how to trigger PAM (pressing space, enter, ...)

The advantage of @kAworu’s approach is that you don’t need to worry about this. The fingerprint reader will be picked up when used, and doesn’t require extra steps. That is what we should strive for.

kaworu commented 4 years ago

The advantage of @kAworu’s approach is that you don’t need to worry about this. The fingerprint reader will be picked up when used, and doesn’t require extra steps. That is what we should strive for.

After digging, I'm not sure we can get away without interaction to trigger PAM. From the libfprint-fprintd README:

Known issues:

pam_fprintd does not support identifying the user itself as that would mean having the fingerprint reader on for all the time the user selection is displayed, and could damage the hardware. It could be fixed by having gdm/login only start the PAM conversation when there is activity

This mean we should avoid having the fingerprint reader always on. The default timeout is 30 seconds. Once fprintd timeout, I think we need to wait some kind of interaction (being mouse event or keyboard) before triggering it again.

pam_fprintd doesn't support entering either the password or a fingerprint, as pam_thinkfinger does, because it's a gross hack, and could be fixed by having the login managers run 2 separate PAM stacks

@volker-fr from my experimentation fprint will block while reading the fingerprint and during that time we are unable to collect and/or submit the password input. The README hint towards two PAM stack, but if there's a way to handle both password and fingerprint with only one PAM stack I am all ears as PAM is new to me.

kaworu commented 4 years ago

I am still slowly making progress on this. My latest diff is at https://github.com/kAworu/i3lock/tree/fprintd-without-imsg.

This patch simply use pipes for IPC as suggested by @stapelberg. After thinking a bit we're not a PAM friendly application (i.e. we don't display conversation messages) so I agree that it is better to avoid the complexity of bringing imsg & al. in tree.

We start the fingerprint reader when i3lock is launched with -2 or --fprint-auth. Then, one of three things happen.

The auth succeed (either through fingerprint or password) and i3lock exit:

PAM_AUTH_STARTING
CONV: Place your right index finger on the fingerprint reader
PAM_AUTH_SUCCESS

The fingerprint auth fail a configured number of times (default to 3 attempts) and the fingerprint reader is put to sleep. It wakes up on any input:

PAM_AUTH_STARTING
CONV: Place your right index finger on the fingerprint reader
CONV: Failed to match fingerprint
CONV: Place your right index finger on the fingerprint reader
CONV: Failed to match fingerprint
CONV: Place your right index finger on the fingerprint reader
CONV: Failed to match fingerprint
PAM_PERM_DENIED

The fingerprint auth timeout after a configured delay (default to 30 seconds) and the fingerprint reader is put to sleep. It wakes up on any input:
```
PAM_AUTH_STARTING
CONV: Place your right index finger on the fingerprint reader
CONV: Verification timed out
PAM_PERM_DENIED
```

Notice that outside of the PAM conversation there is no way to tell the difference between a timeout and an auth failure as in both cases we get PAM_AUTH_STARTING then PAM_PERM_DENIED. In other words, it is hard to distinguish between case 2 and 3. Looking at the PAM conversation is hacky because it is designed to be presented as-is to the user rather than parsed (I think for example that it support i18n). Consequently, I am leaning towards ignoring fprint's PAM_PERM_DENIED as an authentication failure when -f or --show-failed-attempts is given.

Other than that, here is what I think is also required before it could be made a PR:

The current unlock indicator is a quick hack: a pink circle is displayed when fprint is ready and hidden when it isn't. I am thinking of a green fingerprint icon in the bottom right corner (as I believe most fingerprint reader are located on the right side) when fprint is ready, blink red on fprint auth failure, and hidden when fprint is not "listening".
Both the i3lock fprint option (-2 or --fprint-auth) and PAM service (i3lock-fprint) may need to be renamed to something more meaningful and both need to be documented for sure.
Ensure that any mouse movement do wake up fprint (I think it should).

Any testing, feedback, and comment welcome!

Arinerron commented 4 years ago

@kAworu Thanks so much! I wish I was a C developer and could help with development, but unfortunately I'm not. I help with testing if possible.

volker-fr commented 4 years ago

Great work. I actually would suggest printing the PAM message to the screen. Even if you have a fingerprint reader, how do you know which finger to put on? How do you know if you have to put your finger on it again after it timed out or failed at the first try?

See https://github.com/volker-fr/i3lock/commit/2ce7800c96e14235d3238960ab3257c7e6217dbd for a way on how you could hand over the pam msg to the screen.

f0s3 commented 3 years ago

Any update on this issue?

kaworu commented 3 years ago

@f0s3 I haven't found time to make progress on this sadly.

The items outlined here are still TODO.

Additionally, I'm having trouble with fprintd after suspend. My patch will repeatedly trigger authentication failure and unable to login. This could be a driver bug but we need to check whether we can disable fprintd when it behave like this (I think it is possible because sudo / login seems to do so).

Animeshz commented 3 years ago

Any updates on this?

Also does i3lock really not support it, or something else is there on the issue? I see somebody using it without any problem https://www.reddit.com/r/i3wm/comments/gzk4hw/i3lock_fingerprint_only just like in any display manager, adding a line to the i3lock pam config placed at /etc

Animeshz commented 3 years ago

I just saw a reddit post posted about a month ago, they also seem to confirm they are able to use it with PAM methods https://www.reddit.com/r/i3wm/comments/no2u72/unlocking_i3lock_with_your_fingerprint I'm not able to understand if this issue is an active issue, or is it just resolved already?

kiancross commented 3 years ago

I just saw a reddit post posted about a month ago, they also seem to confirm they are able to use it with PAM methods https://www.reddit.com/r/i3wm/comments/no2u72/unlocking_i3lock_with_your_fingerprint I'm not able to understand if this issue is an active issue, or is it just resolved already?

It does seem to work, it's just not very user friendly (at least for me). I have to submit a random password to get the fingerprint sensor to listen and there is no visual feedback for when the fingerprint sensor is listening, if the scan fails etc.

suisseWalter commented 3 years ago

It does seem to work, it's just not very user friendly (at least for me). I have to submit a random password to get the fingerprint sensor to listen and there is no visual feedback for when the fingerprint sensor is listening, if the scan fails etc.

for me this is the exact problem. my setup is slightly different, using howdy instead of fingerprint, but I would like to be able to unlock my screen while in tablet mode with the help of howdy. I can have it as a fallback, so entering a faulty password works to unlock it. but I would like to be able to trigger the unlock call in another way.

what might be a solution for me is: have some other key then enter that can trigger a unlock. this way I could for example have the "power on" button triggering the unlock.

stapelberg commented 2 years ago

I have read through this issue again, and also the other related issues (#210, #286, #38).

A common theme is that i3lock’s current architecture is not well-suited to PAM’s model of calling back into the application with request/response messages. Currently, i3lock just answers all requests with the password buffer and hopes for the best.

I think I have an idea for how to change i3lock’s design to better match PAM: we could change i3lock to work in a prompt-oriented fashion, where the main event loop returns (using ev_break) after each prompt.

In terms of code changes, the following changes would be required (at the very least):

--- i/i3lock.c
+++ w/i3lock.c
@@ -224,7 +224,7 @@ static void finish_input(void) {
     password[input_position] = '\0';
     unlock_state = STATE_KEY_PRESSED;
     redraw_screen();
-    input_done();
+    ev_break(main_loop, EVBREAK_ONE);
 }

 /*
@@ -269,6 +269,8 @@ static void discard_passwd_cb(EV_P_ ev_timer *w, int revents) {
     STOP_TIMER(discard_passwd_timeout);
 }

+static bool successfully_authenticated = false;
+
 static void input_done(void) {
     STOP_TIMER(clear_auth_wrong_timeout);
     auth_state = STATE_AUTH_VERIFY;
@@ -292,6 +294,7 @@ static void input_done(void) {
     if (pam_authenticate(pam_handle, 0) == PAM_SUCCESS) {
         DEBUG("successfully authenticated\n");
         clear_password_memory();
+        successfully_authenticated = true;

         /* PAM credentials should be refreshed, this will for example update any kerberos tickets.
          * Related to credentials pam_end() needs to be called to cleanup any temporary
@@ -1267,7 +1276,11 @@ int main(int argc, char *argv[]) {
      * received up until now. ev will only pick up new events (when the X11
      * file descriptor becomes readable). */
     ev_invoke(main_loop, xcb_check, 0);
-    ev_loop(main_loop, 0);
+    while (!successfully_authenticated) {
+      ev_run(main_loop, 0);
+      DEBUG("break called, input done\n");
+      input_done();
+    }

 #ifndef __OpenBSD__
     if (pam_cleanup) {

What this design makes possible is that we now can run a nested main loop from within the PAM conv_callback:

--- i/i3lock.c
+++ w/i3lock.c
@@ -806,6 +808,10 @@ static int conv_callback(int num_msg, const struct pam_message **msg,
             msg[c]->msg_style != PAM_PROMPT_ECHO_ON)
             continue;

+        DEBUG("--- nested i3lock main loop ----\n");
+        ev_run(main_loop, 0);
+        DEBUG("--- nested i3lock main loop RETURNED ----\n");
+
         /* return code is currently not used but should be set to zero */
         replies[c].resp_retcode = 0;
         if ((replies[c].resp = strdup(password)) == NULL) {

This means we can change the rendering code to display a prompt, and ask the user different questions as PAM asks for: We would set the prompt, run the main loop with ev_run, and when ev_run returns, the password buffer contains the response to that prompt.

I’m thinking in terms of rendering, the current prompt (if any) should go above the unlock indicator. Any PAM informational messages or PAM error messages can be displayed below the unlock indicator (think like smartphone notifications).

From a UX perspective, i3lock displays the lock screen and waits for you to answer its prompt, either with a password (matching the current usage), or with an empty password buffer just pressing enter. If PAM is configured to directly prompt for a password, i3lock takes the password that the user entered and unlocks (current behavior) or, for more complex PAM configurations, i3lock asks for a challenge, fingerprint, password, etc. until PAM returns success or failure.

If anyone wants to give this approach a shot, please post here and send a PR :)

In case you have more questions about the design, or notice something that would break or wouldn’t work, feel free to ask!

PS: For testing, I found it helpful to install the https://sources.debian.org/src/mariadb-10.6/1:10.6.9-1/plugin/auth_pam/testing/pam_mariadb_mtr.c/ PAM test module.

mid-kid commented 1 year ago

I want to point out this fork of swaylock: https://github.com/swaywm/swaylock/compare/master...SL-RU:swaylock-fprintd:fprintd

It adds fingerprint scanning support by calling the fprintd dbus service directly. It activates the fingerprint scanner while the lockscreen is running, to unlock as soon as the fingerprint is scanned. This allows you to still enter a password and authenticate with the password without using the fingerprint. I quite like this solution.

Then there's this fork of i3lock: https://github.com/i3/i3lock/compare/main...YiPrograms:i3lock-powerbtn-fingerprint:main This one just uses the power button to load a different pam config and start the authentication. Not a fan of this one, but it's a solution, I guess.

i3 / i3lock