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

Add PAM parameter to avoid implicit try_first_pass behavior #65

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

tsaarni
Copy link

@tsaarni tsaarni commented Mar 22, 2024

This change introduces a new PAM parameter, clear_first_pass, which prevents the module from using the previously received authentication token from the pam_get_authtok() call during the authentication phase. Currently, the password is stored in context when a password change is required (link).

The purpose of this parameter is to ensure the user re-enters their old password when the LDAP password policy enforces a mandatory password change.

  1. The user is prompted for their password during authentication.
  2. The password policy mandates a password change (password has been reset by administrator).
  3. The user is prompted to re-enter their old password.
  4. The user is prompted to enter a new password.

Currently, step (3) cannot be executed because the old password from step (1) is reused.

This issue can be reproduced with SSHD using KbdInteractiveAuthentication yes.

After applying this PR, you can configure authentication to discard the old password, ensuring it is not available during the password change phase:

auth       sufficient   pam_ldap.so   clear_first_pass
password   sufficient   pam_ldap.so

Although it may seem unusual to prompt the user to enter their password twice in a row, there are valid reasons for this:

  • In my case, we aim to maintain consistent behavior with another PAM module that follows this process. I understand this use case might be uncommon, and I respect if the project does not want to implement it 🙏.
  • For security reasons, some might prefer not to store the user’s password in memory between PAM function calls.

Background

One might wonder why not use the existing try_first_pass parameter to control whether the old password is used. The behavior described by pam_get_authtok() in the Linux PAM implementation does not align with the documentation (quote from man page)

try_first_pass

Before prompting the user for their password, the module first tries the previous stacked module's password in case that satisfies this module as well.

When PAM_OLDAUTHTOK is in the stack pam_get_authtok() should reuse it only if try_first_pass is specified, which should result in a second password prompt by not setting try_first_pass. However, this is not the case as try_first_pass has no effect linux-pam/linux-pam#357 (comment).

The reason is that pam_get_authtok behaves like the try_first_pass is always used. Which, IMO, makes sense. But documentation should be probably improved.

The function pam_get_authtok() seems to have originated from OpenPAM where it adhered to try_first_pass (link). Since its implementation in Linux PAM since 2009, try_first_pass has not been part of the Linux PAM functionality.

Actually, both PAM_AUTHTOK and PAM_OLDAUTHTOK are cleared between PAM function calls for security reasons. This can be seen in the _pam_sanitize() function, which is called when entering and exiting pam_authenticate() and pam_chauthtok(). This likely explains why the password is stored in an internal variable instead of relying on PAM_OLDAUTHTOK to keep it in memory between function calls. This PR could be considered as hardening option to avoid keeping the password in memory in any situation.

The user can add the clear_first_pass parameter to the authentication phase
to prevent pam_authenticate() from retaining the password in memory for use
by pam_chauthtok() during a forced password change.

Signed-off-by: Tero Saarni <tero.saarni@est.tech>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant