pam_unix: should not treat CRYPT_SALT_METHOD_LEGACY hash as expired

[msharov]

libxcrypt 4.4.22 has deprecated multiple hashes, including SHA-256, resulting in login failures for many people. The failures are uninformative:

PAM failed: Authentication token is no longer valid; new one required
pam_unix(login:account): expired password for user (root enforced)
pam_chauthtok: Authentication token manipulation error

The messages do not indicate what the real problem is and the only way I was able to figure it out was by reading the news board on my distribution's home page (Arch). The token can not be made valid if pam configuration explicitly sets the hash, as the documentation previously recommended. Until the configuration changes, logins will continue to fail. If the root partition is mounted read-only, the system will be completely unbootable.

A more informative error message is needed to tell people to change the hash setting (or, in my case, downgrade libxcrypt and blacklist its upgrades until they get over their God complex) instead of helplessly banging their heads on the keyboard.

[t8m]

I have to say that I was disappointed by the libxcrypt change as well. IMO this was not well managed there. In the end it would be much better if this was handled somehow in coordination with Linux PAM project.

We could drop the crypt_checksalt() call completely in pam_unix check_shadow_expiry(). It was probably not a right thing there.

[ldv-alt]

On Thu, Jun 10, 2021 at 07:02:58AM -0700, Tomáš Mráz wrote: I have to say that I was disappointed by the libxcrypt change as well. IMO this was not well managed there. In the end it would be much better if this was handled somehow in coordination with Linux PAM project.

The libxcrypt change itself was OK, but ...

We could drop the crypt_checksalt() call completely in pam_unix check_shadow_expiry(). It was probably not a right thing there.

... pam_unix check_shadow_expiry() is not correct in treating CRYPT_SALT_METHOD_LEGACY hash as expired.

[zackw]

libxcrypt author here.

The intended meaning of CRYPT_SALT_LEGACY is "passwd(1) should not use this hashing method." It is not supposed to mean "force a password change on next login for any user with an existing stored hash using this method." At most, login(1) should silently re-hash the user's existing password using a stronger method upon successful authentication, and if it can't update the password database (e.g. because the root filesystem is read-only) then it should just leave things alone.

[ldv-alt]

I suppose reverting commit 4da9feb would fix this issue.

[zackw]

@ldv-alt For a quick fix, yeah. Long term I think it would be a good idea to make CRYPT_SALT_METHOD_LEGACY trigger a silent re-hash of the existing password with a stronger method, but I have no idea how to do that and also, given what happened here, I don't think that should be added without a way to turn it off (at runtime, not by recompiling PAM).

This should not be done for CRYPT_SALT_TOO_CHEAP because right now there's no way to know what cost parameters would qualify as "not too cheap".

Note also that checking for CRYPT_SALT_METHOD_DISABLED and CRYPT_SALT_INVALID is pointless here; hashes in those categories will be rejected by crypt().

[msharov]

@zackw "I don't think that should be added without a way to turn it off"

I would expect that the only people who want to turn it off are those who already have configured a different hash via the options, such as:

password required       pam_unix.so sha256 shadow rounds=131072

Simply honoring these settings, if present, will be sufficient. I think that anyone who does not have them can be presumed to not care what the password hash is.

[t8m]

We decided to revert this completely. So CRYPT_SALT_METHOD_LEGACY and CRYPT_SALT_TOO_CHEAP won't cause the expiration of a password.

[paulmenzel]

Could you cut a 1.5.2 release with the revert included? That way distribution can easily pick it up?