google / google-authenticator-libpam

Apache License 2.0
1.77k stars 280 forks source link

Security issue using nullok #55

Closed tmolitor-stud-tu closed 7 years ago

tmolitor-stud-tu commented 7 years ago

When using the "nullok" argument, the pam module returns PAM_SUCCESS instead of PAM_IGNORE.

When configuring ssh access with public key and OTP one usually doesn't want to be asked for the password of the user account. Thus this configuration in the pam config is used: auth [success=done new_authtok_reqd=done default=die] pam_google_authenticator.so nullok

But this opens security holes, because all other pam modules are skipped, when not google authenticator config file is found for a user (instead of prompting for the password as expected).

The patch is super simple, just return PAM_IGNORE instead of PAM_SUCCESS which makes the google authenticator pam module chainable with other pam modules again, when using the following config: auth [success=done new_authtok_reqd=done ignore=ignore default=die] pam_google_authenticator.so debug echo_verification_code nullok

This is the line to patch: https://github.com/google/google-authenticator-libpam/blob/master/src/pam_google_authenticator.c#L1681

See also (sorry, it's in german): https://www.debinux.de/2015/01/google-authenticator-als-pam-modul-im-openssh-server/#comment-56222

ThomasHabets commented 7 years ago

I'll check this out when I'm back from vacation. Is there a documented semantics for PAM_IGNORE vs PAM_SUCCESS and it's correct usage, that I can check when back?

tmolitor-stud-tu commented 7 years ago

Well, I found this guide which seems really exhaustive: http://www.rjsystems.nl/en/2100-pam-debian.php Look at chapter 3, especially this part:

The actions can either be unsigned integers, indicating the number of modules in the stack to jump over next, or be one of the following keywords:

ignore − The module's return status will not influence the final result unless there are no other modules in the stack. bad − The module's return status should indicate a module failure. If it is the first failure, this return status will be used for the entire stack. die − Idem, except that the stack also terminates and control is immediately returned to the application. ok − The return code will override a previous success state of the stack, but not a previous failure state. done − Idem, except that the stack also terminates and control is immediately returned to the application. reset − Clear the stack's memory and start again with the next stacked module.

To illustrate how this syntax is used, each of the four simple control directives mentioned above can be replaced with an equivalent complex expression:

required = [ success=ok new_authtok_reqd=ok ignore=ignore default=bad ] requisite = [ success=ok new_authtok_reqd=ok ignore=ignore default=die ] sufficient = [ success=done new_authtok_reqd=done default=ignore ] optional = [ success=ok new_authtok_reqd=ok default=ignore ]

As far as I can tell, when using PAM_SUCCESS pam thinks the module successfully validated the provided credentials and when using required as control keyword (or the long form outlined in the quote above) this means, that other pam modules are also asked, thus pam asks for the otp code AND the password. Everything is ok and secure this way.

But when you configure an ssh server to use the google authenticator you usually use public key auth instead of password auth and you want pam to only ank for the otp code but NOT for the password.

According to various howtos I found, this means you have to list the google authenitcator module first in the pam stack and use this control expression: [success=done new_authtok_reqd=done default=die] which resembles more or less the sufficient control keyword as outlined above (meaning it does NOT ask other pam modules after this one, if the provided otp code is correct). When the google authenticator is configured for the user, everything is ok since access is either allowed or denied by it. But if the authenticator is not configured for the user, the nullok argument does make it return PAM_SUCCESS, which maps to done (this is configured in the cited control expression above). Thus the user is granted access without further checks.

There is essentially no way to use pam to ask for the user's password, if the authenticator is not configured for this user, and to only ask for the otp code if it is configured.

On the other hand, if google authenticator returned PAM_IGNORE in the nullok case, the following control expression could be used to make this possible: [success=done new_authtok_reqd=done ignore=ignore default=die] True PAM_SUCCESS would still be mapped to done, but the returned PAM_IGNORE in the nullok case would be mapped to ignore which means pam is consulting the other modules in the stack as well, essentially ignoring the google authenticator module (which seems to be the desired semantics for the nullok argument anyway).

For sake of completeness here is the openssh configuration used (only relevant parts shown):

# Don't solve authentication internally, but ask pam instead
ChallengeResponseAuthentication yes 

# Combine publickey and pam authentication (only allow access if both succeed)
AuthenticationMethods publickey,keyboard-interactive:pam

# This has to be "yes" (default value)
UsePAM yes

I hope this helps. If something is unclear, just ask :)

ThomasHabets commented 7 years ago

Sorry for the delay. I'm back now.

Thank you very much for the very detailed response, and link.

Yes, I agree.