google / google-authenticator-libpam

Apache License 2.0
1.76k stars 281 forks source link

grace_period doesn't honor forward_pass #181

Open hallaj opened 3 years ago

hallaj commented 3 years ago

I was trying to implement grace_period with our existing forward_pass option to no avail. While grace_period itself authenticates without issue with the LASTIP option, I believe the full password gets forwarded without being trimmed (see https://github.com/google/google-authenticator-libpam/blob/master/src/pam_google_authenticator.c#L1888 and https://github.com/google/google-authenticator-libpam/blob/master/src/pam_google_authenticator.c#L1923-L1926) which causes the behaviour of forward_pass to behave differently.

Was wondering if my suspicion is correct and if so, could someone help with the bug fix?

ThomasHabets commented 3 years ago

Hmm… yeah that makes sense.

But how does this work for the user? How do they know if grace_period will allow them in without an OTP?

hallaj commented 3 years ago

The current implementation I have for this is with OpenVPN, with the combination of pam_unix.so along with pam_google_authenticator.so. So, when reconnecting would fail the authentication; this is usually the case when I suspend my laptop or when my machine goes to sleep and tries to reconnect.

boehmerp commented 10 months ago

Reviving years later, I suspect I have the same problem when trying to implement pam_google_authenticator with our Dovecot email server.

In our configuration, users have to supply password+OTP for every IMAP authentication. Depending on email client, this could be every time a user sends an email (SMTP sending of mail, followed by an IMAP mail check). Ideally once the user authenticates with a valid OTP, following authentications during the grace_period are accepted (PAM_SUCCESS) , stripped of the (email client) supplied/cached OTP, and then forward_pass on to pam_unix, allowing user to retrieve email. Once the grace_period expires, PAM_FAIL and user is prompted for new password+OTP.

If anyone has information on how to implement this it would be greatly appreciated.

ThomasHabets commented 10 months ago

Not remembering any of the context, I think the fix here is for a grace period hit to strip off the (now stale) OTP, before forwarding.

What's needed is to:

  1. Break out the code that strips the OTP from the password into its own function.
  2. Call that if grace_limit hits.
  3. Also update the password from there.

One problem is that we don't know how many digits were used. Scratch code is 8 digits.

I'm not sure how to solve that, honestly. And something that doesn't work for scratch codes seems not very fit for purpose. Also it could add a blocker for those who want to change the code to accept 8 digit non-scratch codes.

boehmerp commented 10 months ago

Since the email client is sending the cached password of the last successful authentication (password+otp), I would think all you would have to do is run the forward_pass routine which strips off the OTP anyways, right?

I'm playing around with the code and having moderate success just commenting out the "goto out" on line 1905 pam_google_authenticator.c. and allowing the resulting "rc = PAM_SUCCESS" fall through the rest of the routine.

ThomasHabets commented 10 months ago

If the email client sends Secret12345678, how do you know how much to strip off? If it was a normal code, then the password is Secret12. If it's a scratch code then it's Secret.

The PAM module doesn't know. It erased any scratch code used, and time / counter has progressed, so it doesn't know the old code.

boehmerp commented 10 months ago

I really appreciate your time on this. I guess I'm confused about the scratch/normal code (lack of knowledge on my part). We are just appending the Google Authenticator App's 6 digit code to the user's password when prompted for an IMAP password. We have our Dovecot pam configured as:

auth            required        pam_google_authenticator.so    forward_pass debug grace_period=86400
auth            required        pam_unix.so                    use_first_pass debug

I thought the forward_pass option takes care of stripping off the OTP and passing it to the next module?

If we remove grace_period option, everything works properly with the exception of being repeatedly being prompted for a new password every time the OTP expires.

ThomasHabets commented 10 months ago

Scratch codes, or "emergency codes", are optionally created by the google-authenticator command, and are stored in the ~/.google_authenticator config. And they're 8 digits, because they never expire (though they are single use).

Without grace_period the code is always checked. And since it matched, we know if it's a scratch code or a TOTP/HOTP code.

Well, technically an 8 digit scratch code has a one in a million chance of matching the correct 6 digit code too, so Secret12345678 could sometimes batch both password Secret plus 8 digit scratch code, and password Secret12 plus 6 digit scratch code. Someone using such a scratch code will be very surprised to have their password rejected.

If your environment doesn't use scratch codes, then you can assume that all codes are 6 digits, like they are in the app. And then just strip off 6 digits. But that patch will not be accepted here, upstream.

boehmerp commented 10 months ago

Thanks for the explanation. How does the forward_pass option processes scratch vs normal codes? Or does forward_pass only process the 6 digit codes?

Sorry, misread your last response. I see the appropriate code in ~1955 and I think understand what exactly is going on. During the grace_period (say two hours), but outside of the OTP lifespan (3 mins), there is no way to verify if the OTP is 6 or 8 digits because they will both fail the verification process.

boehmerp commented 10 months ago

A colleague suggested this:

Could we not just add the last successful OTP to the grace_period entry in the secret file:

" LAST0 127.0.0.1 1696436777 123456 " LAST1 192.168.1.1 1696437426 12345678

When grace period is expired, this entry would get deleted anyway, right? If within grace_period, compare stored OTP to provided OTP, if matches, we strip the stored OTP from the password and pass remaining string on to next module.

ThomasHabets commented 10 months ago

Makes sense to me.