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.

Sign up for GitHub

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

Jump to bottom

password_hash/get_encrypted_password uses passlib default of rounds=656000 which is 131 times glibc default #15326

Closed
jlec opened this issue Apr 7, 2016 · 1 comment · Fixed by #21215 or #21997
Closed

password_hash/get_encrypted_password uses passlib default of rounds=656000 which is 131 times glibc default #15326

jlec opened this issue Apr 7, 2016 · 1 comment · Fixed by #21215 or #21997
Labels
affects_2.2 This issue/PR affects Ansible v2.2 affects_2.3 This issue/PR affects Ansible v2.3 feature This issue/PR relates to a feature request.

Comments

@jlec
Copy link
Contributor

jlec commented Apr 7, 2016

In the password_hash filter function the underlying passlib call uses the default rounds parameter.

The default for glibc is 5000, the passlib default for sha512 is 656000. This means on a login in a linux account the hash calculation will take significantly longer.

Actually you get basically no rounds parameter when setting it to 5000

$ python -c "from passlib.hash import sha512_crypt; print sha512_crypt.encrypt('foo', rounds=5000)"
$6$0zDM3P3/MbIoF0G0$pVmR8hc0ZNYdfGLnFhVKeCGhd32kDXo6ky83JmUWTZCfVQm2IrhnIbrrg5Vyl9XxE5aWzdmuYpvTA6gyOFc5Z.
$ python -c "from passlib.hash import sha512_crypt; print sha512_crypt.encrypt('foo', rounds=5001)"
$6$rounds=5001$VFyJ36YLb/RLhx0e$CAFCB/W7ebYEHIFsZtlJSdvuzEtsYOAvtOwCF7ahqpWqLj62TJp4LlzZ/FiW1B2U5kSKh4xibiJgZyd2ceVoI1

Could a simple parameter be added to get_encrypted_password to set the value which has a preferable default what glibc does?
@ansibot ansibot added triage affects_2.2 This issue/PR affects Ansible v2.2 labels Sep 7, 2016
@ansibot ansibot added the affects_2.3 This issue/PR affects Ansible v2.3 label Dec 13, 2016
@bcoca bcoca removed the triage label Dec 16, 2016
@axelsegebrecht axelsegebrecht mentioned this issue Jan 31, 2017
Open
@mfuchs mfuchs mentioned this issue Feb 9, 2017
Merged
pgrenaud added a commit to pgrenaud/ansible that referenced this issue Feb 27, 2017
@pgrenaud
Fixes passlib example in FAQ to reduce the number of rounds to 5000
e5ab340 
As stated in issue ansible#15326, the default number for glibc is 5000, where
the default for passlib is 656000.

I actually found out when I spend few hours trying to understand why
ansible was taking almost x3 the time to run a playbook when using a
user with sudo and password (comparared to sudo with NOPASSWD set).
Well, it was because the user was created using ansible and the passlib
example found in the docs' FAQ.

Reducing the numbers of rounds to 5000 will ensure a better experience
with ansible for newcomers when using sudo with a password.
@pgrenaud pgrenaud mentioned this issue Feb 27, 2017
Merged
bcoca pushed a commit that referenced this issue Feb 27, 2017
@pgrenaud @bcoca
Fix Passlib example in FAQ (#21997)
d6b40c9 
* Fixes passlib example in FAQ to reduce the number of rounds to 5000

As stated in issue #15326, the default number for glibc is 5000, where
the default for passlib is 656000.

I actually found out when I spend few hours trying to understand why
ansible was taking almost x3 the time to run a playbook when using a
user with sudo and password (comparared to sudo with NOPASSWD set).
Well, it was because the user was created using ansible and the passlib
example found in the docs' FAQ.

Reducing the numbers of rounds to 5000 will ensure a better experience
with ansible for newcomers when using sudo with a password.

* Fixes passlib example in FAQ to reflect the API changes in passlib 1.7

Method encrypt() was deprecated in 1.7 and renamed to hash(), which
happened almost a year ago.

https://passlib.readthedocs.io/en/stable/lib/passlib.ifc.html#passlib.ifc.PasswordHash.encrypt
@pgrenaud pgrenaud mentioned this issue Feb 27, 2017
Closed
@ansibot ansibot added feature This issue/PR relates to a feature request. and removed feature_idea labels Mar 2, 2018
@BarbzYHOOL
Copy link

This issue is still there in 2.5

mfuchs added a commit to mfuchs/ansible that referenced this issue Aug 24, 2018
@mfuchs
Share the implementation of hashing for both vars_prompt and password…
5945826 
…_hash.

* vars_prompt with encrypt does not require passlib for the algorithms
  supported by crypt.
* Additional checks ensure that there is always a result.
  This works around issues in the crypt.crypt python function that returns
  None for algorithms it does not know.
  Some modules (like user module) interprets None as no password at all,
  which is misleading.
* The password_hash filter supports all parameters of passlib.
  This allows users to provide a rounds parameter, fixing ansible#15326.
* password_hash is not restricted to the subset provided by crypt.crypt,
  fixing one half of ansible#17266.
* Updated documentation fixes other half of ansible#17266.
* password_hash does not hard-code the salt-length, which fixes bcrypt
  in connection with passlib.
  bcrypt requires a salt with length 22, which fixes ansible#25347
* Salts are only generated by ansible when using crypt.crypt.
  Otherwise passlib generates them.
* Avoids deprecated functionality of passlib with newer library versions.
* When no rounds are specified for sha256/sha256_crypt and sha512/sha512_crypt
  always uses the default values used by crypt, i.e. 5000 rounds.
  Before when installed passlibs' defaults were used.
  passlib changes its defaults with newer library versions, leading to non
  idempotent behavior.

  NOTE: This will lead to the recalculation of existing hashes generated
        with passlib and without a rounds parameter.
        Yet henceforth the hashes will remain the same.
        No matter the installed passlib version.
        Making these hashes idempotent.

Fixes ansible#15326
Fixes ansible#17266
Fixes ansible#25347 except bcrypt still uses 2a, instead of the suggested 2b.
abadger pushed a commit that referenced this issue Aug 27, 2018
@mfuchs @abadger
Share the implementation of hashing for both vars_prompt and password…
7871027 
…_hash (#21215)

* Share the implementation of hashing for both vars_prompt and password_hash.
* vars_prompt with encrypt does not require passlib for the algorithms
  supported by crypt.
* Additional checks ensure that there is always a result.
  This works around issues in the crypt.crypt python function that returns
  None for algorithms it does not know.
  Some modules (like user module) interprets None as no password at all,
  which is misleading.
* The password_hash filter supports all parameters of passlib.
  This allows users to provide a rounds parameter, fixing #15326.
* password_hash is not restricted to the subset provided by crypt.crypt,
  fixing one half of #17266.
* Updated documentation fixes other half of #17266.
* password_hash does not hard-code the salt-length, which fixes bcrypt
  in connection with passlib.
  bcrypt requires a salt with length 22, which fixes #25347
* Salts are only generated by ansible when using crypt.crypt.
  Otherwise passlib generates them.
* Avoids deprecated functionality of passlib with newer library versions.
* When no rounds are specified for sha256/sha256_crypt and sha512/sha512_crypt
  always uses the default values used by crypt, i.e. 5000 rounds.
  Before when installed passlibs' defaults were used.
  passlib changes its defaults with newer library versions, leading to non
  idempotent behavior.

  NOTE: This will lead to the recalculation of existing hashes generated
        with passlib and without a rounds parameter.
        Yet henceforth the hashes will remain the same.
        No matter the installed passlib version.
        Making these hashes idempotent.

Fixes #15326
Fixes #17266
Fixes #25347 except bcrypt still uses 2a, instead of the suggested 2b.

* random_salt is solely handled by encrypt.py.
  There is no _random_salt function there anymore.
  Also the test moved to test_encrypt.py.
* Uses pytest.skip when passlib is not available, instead of a silent return.
* More checks are executed when passlib is not available.

* Moves tests that require passlib into their own test-function.

* Uses the six library to reraise the exception.

* Fixes integration test.

When no rounds are provided the defaults of crypt are used.
In that case the rounds are not part of the resulting MCF output.
@ansible ansible locked and limited conversation to collaborators Apr 25, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
affects_2.2 This issue/PR affects Ansible v2.2 affects_2.3 This issue/PR affects Ansible v2.3 feature This issue/PR relates to a feature request.
Projects
None yet
Milestone
No milestone
4 participants
@jlec @bcoca @ansibot @BarbzYHOOL