[PATCH] libpam should return different codes for nonexistant/broken files

[ThomasHabets]

Original issue 18 created by david.search on 2010-11-05T00:07:34.000Z:

Attempts to login with the pam module enabled will fail if a secret file is not setup. This patch causes google_authenticator() to return PAM_IGNORE and not ask for a code if the user has not setup a secret file or if there was an error reading the file.

It also changes the return code for failed authentication to PAM_AUTH_ERR instead of PAM_SESSION_ERR as I think that is more appropriate, but that shouldn't matter too much.

The existing functionality (require a code and fail if one doesn't exists) can be activated like so:

require a token code

auth [success=ok default=bad] pam_google_authenticator.so

[ThomasHabets]

Comment #1 originally posted by markus@google.com on 2011-03-09T20:26:02.000Z:

<empty>

[ThomasHabets]

Comment #2 originally posted by ry4anb on 2011-03-23T04:59:55.000Z:

This no longer applied cleanly -- just a change in one of the context lines, so here's an update against revision 68230188bdc7

[ThomasHabets]

Comment #3 originally posted by shaun@olebox.com on 2011-06-07T17:15:38.000Z:

It appears when this patch is applied on my gentoo machine, the authentication is segfaulting. When I remove the patch, it works correctly again.

I've installed the patch using:

patch < ../ignore_nofile.patch

where the ignore_nofile.patch file is located just outside the libpam directory. While this patch is in affect, the kernel outputs the following during authentication:

kernel: [65548.125770] sshd[21806]: segfault at 0 ip 00007fab4cc43352 sp 00007fff7812ece0 error 4 in pam_google_authenticator.so[7fab4cc42000+5000]

I believe this error has to do with line 1112 of pam_google_authenticator.c (after patched) where the buf var is set when the file doesn't exist (in the if block). Perhaps it's returning null and not evaluating/equating that to false in the if block? I'm happy to provide any further examples or information as needed.

[ThomasHabets]

Comment #4 originally posted by david.search on 2011-06-07T22:28:23.000Z:

Here is an updated patch. The one above doesn't move the rate_limit() into the if statement after the check for the file.

I'm not currently using this so I can't test it right away but I'm 99% sure that this will fix it. It should apply cleanly.

[ThomasHabets]

Comment #5 originally posted by shaun@olebox.com on 2011-06-08T14:32:01.000Z:

David, I can confirm this patch as working correctly. Thanks for the update!

[ThomasHabets]

Comment #6 originally posted by roman.mueller on 2012-07-10T19:22:06.000Z:

The previous patch didn't apply cleanly for me against the current version.
Here is an updated version (pretty much only line numbers changed).

[ThomasHabets]

Comment #7 originally posted by rskaliotis on 2012-09-03T19:42:17.000Z:

Note there is a "nullok" option available in the current version:

http://code.google.com/p/google-authenticator/source/detail?r=8f2cb7b6713fbfbbdb195252a9e1d6fab3dcf5b4

[ThomasHabets]

Comment #8 originally posted by shellsterdude on 2014-08-17T08:40:56.000Z:

Unfortunately, nullok does not seem to work when you have the following scheme (for combining the password and token into one step, which is necessary for sftp clients):
auth requisite pam_google_authenticator.so forward_pass
auth required pam_unix.so use_first_pass

A partial work-around is as follows (where user group B must not use two-factor and everyone else must):
auth [success=2 default=ignore] pam_succeed_if.so user notingroup B
auth requisite pam_google_authenticator.so forward_pass
auth required pam_unix.so use_first_pass

auth [success=1 default=ignore] pam_succeed_if.so user ingroup B
auth required pam_unix.so