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

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

Open
pstray opened this issue Jan 5, 2019 · 35 comments

Comments

@pstray
Copy link

pstray commented Jan 5, 2019

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
Copy link
Member

Airblader commented Jan 5, 2019

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
Copy link
Author

pstray commented Jan 6, 2019

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
Copy link
Member

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
Copy link

webknjaz commented Mar 7, 2019

This is largely a duplicate of #852.

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

@camparijet
Copy link

This is largely a duplicate of #852.

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

@webknjaz I believe he meant i3/i3#852

@Airblader
Copy link
Member

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

@webknjaz
Copy link

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

@webknjaz
Copy link

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

@Airblader
Copy link
Member

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

@webknjaz
Copy link

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 #217 (comment) GitHub will create correct links and backreference from there.

@Airblader
Copy link
Member

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

@andreesteve
Copy link

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
Copy link

evamvid commented Dec 29, 2019

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
Copy link

evamvid commented Dec 29, 2019

I've started (attempting) to implement multiple PAM stacks here: (evamvid@a703e99). 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
Copy link

Arinerron commented Apr 24, 2020

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
Copy link
Member

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

@Arinerron
Copy link

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

@volker-fr
Copy link

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
Copy link

kaworu commented May 4, 2020

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:

  1. 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).

  2. 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
Copy link
Member

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
Copy link

kaworu commented May 5, 2020

@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
Copy link

volker-fr commented May 5, 2020

@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: volker-fr@2ce7800

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

  1. how to trigger PAM (pressing space, enter, ...)
  2. 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
Copy link
Member

  • 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
Copy link

kaworu commented May 7, 2020

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
Copy link

kaworu commented May 27, 2020

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.

  1. 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
  1. 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
  1. 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
Copy link

@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
Copy link

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 volker-fr@2ce7800 for a way on how you could hand over the pam msg to the screen.

@f0s3
Copy link

f0s3 commented Jan 24, 2021

Any update on this issue?

@kaworu
Copy link

kaworu commented Jan 25, 2021

@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
Copy link

Animeshz commented Jul 8, 2021

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
Copy link

Animeshz commented Jul 8, 2021

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
Copy link

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
Copy link

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.

sh1r4s3 pushed a commit to sh1r4s3/i3lock-gif that referenced this issue Jun 26, 2022
* Clarify brace behaviour (i3#217)

* Add comment

Co-authored-by: Raymond Li <hi@raymond.li>
@stapelberg
Copy link
Member

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.

@stapelberg stapelberg changed the title Support for passwordless PAM methods like fingerprint-gui Make i3lock support PAM prompts (better support for PAM messages and passwordless PAM methods like fingerprint-gui) Sep 11, 2022
@stapelberg stapelberg pinned this issue Sep 11, 2022
@mid-kid
Copy link

mid-kid commented Jun 7, 2023

I want to point out this fork of swaylock: swaywm/swaylock@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: 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.

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

No branches or pull requests