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

pam_unix: Better approach to add crypt_default method, if supported. #84

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

Conversation

besser82
Copy link
Contributor

libxcrypt since v4.4.0 supports a default method for its gensalt function on most system configurations.
As the default method is to be considered the strongest available hash method, it should be preferred over all other hash methods supported by pam.

  • modules/pam_unix/pam_unix.8.xml: Documentation for crypt_default.
  • modules/pam_unix/passverify.c: Add crypt_default method.
  • modules/pam_unix/support.c (_set_ctrl): Issue an error to syslog, if
    the crypt_default method is not supported by the system's libcrypt
  • modules/pam_unix/support.h: Likewise.

@t8m This is the new version of #78.

libxcrypt since v4.4.0 supports a default method for its gensalt
function on most system configurations.  As the default method is
to be considered the strongest available hash method, it should be
preferred over all other hash methods supported by pam.

* modules/pam_unix/pam_unix.8.xml: Documentation for crypt_default.
* modules/pam_unix/passverify.c: Add crypt_default method.
* modules/pam_unix/support.c (_set_ctrl): Issue an error to syslog, if
the crypt_default method is not supported by the system's libcrypt
* modules/pam_unix/support.h: Likewise.
@t8m
Copy link
Member

t8m commented Nov 27, 2018

Thinking about this I believe this is a wrong approach. We already provide a kind-of default method which is obtained from /etc/login.defs. Why not handle the default method from libcrypt in a similar way.

I would not add a new option at all, but just call the crypt_preferred_method() if available and use its value. You would still need to indicate somehow (via ctrl flag) that no algo option was used with the pam module because the value from login.defs should be overriden by the libcrypt preferred method.

The preference order should be:

  1. explicit module option
  2. libcrypt preferred method
  3. login.defs
  4. some fallback built-in default - probably SHA512 as that is still reasonably secure and supported well.

There are also other possibilities how to handle the 4. - make it build-time option - using a define that can be passed into build should be sufficient.

@t8m
Copy link
Member

t8m commented Mar 4, 2020

@besser82 If we could do something with this one before the 1.4.0 release it would be nice. But I'd like the release to be completed during March.

@besser82
Copy link
Contributor Author

besser82 commented Mar 4, 2020

@t8m working on it. =)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants