boltgolt / howdy

🛡️ Windows Hello™ style facial authentication for Linux
MIT License
5.81k stars 300 forks source link

By default biometrics modules should only return PAM_IGNORE or errors, but never PAM_SUCCESS #661

Open nicowilliams opened 2 years ago

nicowilliams commented 2 years ago

I looked at the source code using cscope and I find that pam_sm_authenticate() returns PAM_SUCCESS on success. However, biometrics identification is NOT authentication, therefore biometrics modules' pam_sm_authenticate() should only: a) set the PAM_USER item, b) return PAM_IGNORE or errors, but never `PAM_SUCCESS.

If anyone actually wants to be able to use biometrics alone for login (totally legitimate for low-value accounts) then either biometrics modules should have options for returning PAM_SUCCESS for specific users (or even all, for use on low-value systems), or the admin should stack a module that does nothing but return PAM_SUCCESS after required biometrics modules.

This is one of the main uses of PAM_IGNORE: to allow for modules that can only cause authentication/authorization to fail but not succeed. Another use of PAM_IGNORE is to allow for modules with optional/conditional functionality so that when they cannot contribute to authentication, they return PAM_IGNORE, but otherwise may return errors as well as success. The latter is often mistaken as the only legitimate use of PAM_IGNORE.

(Long long ago, at Sun, we made sure all PAMs in Solaris adhered to this. Of course, that was then, and this is now, and Solaris isn't very relevant, but I believe the principle is still applicable today, both in Solaris/Illumos as well as in Linux/LinuxPAM.)

nicowilliams commented 2 years ago

I hope I'm able to convince you. If LinuxPAM says anything like this in their docs, well, I'd point you to it, but I haven't looked recently.

The sort of configuration one wants, then, is something like:

auth  required pam_howdy.so
auth  required pam_unix.so

If pam_howdy only returns PAM_IGNORE or errors, then this is safe, and makes it so you can have pam_howdy provide identification while pam_unix (or whatever you want to substitute) can validate passwords/PINs/etc, thus yielding 2-factor authentication. Identification factors should never be sufficient. Modules that return PAM_IGNORE make PAM ignore their required/whatever configuration.

In particular, the configuration shown in howdy/src/pam/README.md is risky because a biometrics module should never be sufficient unless it never returns PAM_SUCCESS, then making it sufficient instead of optional, required, or something else, is fine and not at all risky.

nicowilliams commented 2 years ago

And if you want to use biometrics alone? Just add a configuration argument that tells it to return PAM_SUCCESS if the user is identified:

auth required pam_howdy.so single_factor_ok

or something like that.

nicowilliams commented 2 years ago

Aside from that, I'd like to echo the praise you're getting on HN. This looks very nice!

boltgolt commented 2 years ago

Thanks for your input but i think you are not completely aware why most people use Howdy in the first place. This program is not meant to be a secure solution and that's clearly stated in the readme. Howdy CAN be configured as a part of a multi-factor authentication flow, but for almost all users it's about convenience.

It is more risky than a password, and that is clearly stated beforehand. Windows Hello works on a similar principle.

nicowilliams commented 2 years ago

I got as much out of reading the READMEs. The point of the PAM_IGNORE thing is to make it hard to accidentally make biometrics the only factor when one meant to make it one of N>1 factors. It should absolutely be possible to use biometrics as a single factor (convenience), and to require a module argument for that doesn't seem (to me anyways!) like a problem. Feel free to close this if you don't agree -- this isn't critical, just a footgun kind of thing.

lhoward commented 2 years ago

Agree with @nicowilliams (I wrote pam_ldap many years ago).

boltgolt commented 2 years ago

I just don't really see the benefit of adding an extra argument to the PAM line. By far most of the installations are on debian-based systems (that install a PAM configuration file automatically) and those users will never see the line at all.

What i think this change would mostly look like is people using older guides to install Howdy not adding a "single_factor_ok" argument and opening issues about Howdy not unlocking something.

I appreciate the input though, maybe a disable_single_factor argument would be an option?

saidsay-so commented 2 years ago

That wouldn't be too hard to implement as an argument, it's just a matter of parsing the argument and changing the return codes.