Share the implementation of hashing for both vars_prompt and password_hash

[mfuchs]

ISSUE TYPE

• Feature Pull Request
• Bugfix Pull Request
• Docs Pull Request

COMPONENT NAME

vars_prompt
filter

ANSIBLE VERSION

2.2.1

SUMMARY

Shares the implementation of hashing secrets 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 password_hash/get_encrypted_password uses passlib default of rounds=656000 which is 131 times glibc default #15326.
• password_hash is not restricted to the subset provided by crypt.crypt,
  fixing one half of properly support passlib module in password_hash #17266.
• Updated documentation fixes other half of properly support passlib module in password_hash #17266.
• password_hash does not hard-code the salt-length, which fixes bcrypt
  in connection with passlib.
  bcrypt requires a salt with length 22.
• Salts are only generated by ansible when using crypt.crypt.
  Otherwise passlib generates them.
• Avoids deprecated functionality of passlib with newer library versions.

Fixes #15326
Fixes #17266

# after
{{ 'secret' | password_hash('bcrypt', rounds=14) }}

[mattclay]

CI failure due to unit test errors. Here's one of the errors:

2017-02-09 23:40:23 =================================== FAILURES ===================================
2017-02-09 23:40:23 _________________________ TestEncrypt.test_do_encrypt __________________________
2017-02-09 23:40:23 
2017-02-09 23:40:23 self = <units.utils.test_encrypt.TestEncrypt testMethod=test_do_encrypt>
2017-02-09 23:40:23 
2017-02-09 23:40:23     def test_do_encrypt(self):
2017-02-09 23:40:23         self.assertTrue(encrypt.PASSLIB_AVAILABLE)
2017-02-09 23:40:23     
2017-02-09 23:40:23 >       with self.assertRaises(AnsibleError):
2017-02-09 23:40:23 E       TypeError: failUnlessRaises() takes at least 3 arguments (2 given)
2017-02-09 23:40:23 
2017-02-09 23:40:23 test/units/utils/test_encrypt.py:76: TypeError

[ansibot]

cc @dharmabumstead
click here for bot help

[mattclay]

CI failure in unit tests due to:

2017-02-10 21:37:00 E           MissingBackendError: bcrypt: no backends available -- recommend you install one (e.g. 'pip install bcrypt')

The full error is quite long, so I'll just link to the logs here: https://app.shippable.com/runs/589e2dee031d631000bae9d4/2/console

[mfuchs]

Thanks mattclay for your input! :)

ready_for_review

[abadger]

Catch and reraise is a better pattern than passing the exception in. Python 2 makes this a pain, though. Ned Batchelder has a good blog post about how to do that right: https://nedbatchelder.com/blog/200711/rethrowing_exceptions_in_python.html

passlib changes number of rounds while crypt has a static value. I'm not sure of al the ramifications for that. It would mean that pre and post change, lookup('password_hash'[...]) will return different strings. If rounds is calculated dynamically at runtime, it could mean that when I use a playbook that has lookup('password_hash'[...]) on my low end laptop, I generate a different string than when someone else runs it on their high end workstation. If that's true it leads to non-repeatability of playbook runs.

[mfuchs]

As discussed with abadger I will make sure that password_hash returns the same value for both 2.2 and when this commit is merged.
No matter if passlib is installed or not.
This will also be part of the unit test.

Furthermore I will look into allowing a rounds parameter for crypt, for example crypt.crypt('secret', '$6$rounds=656000$abc').

Maybe I will also update the vars_prompt documentation to mention that it makes sense to specify a rounds parameter to ensure replayability.

[dharmabumstead]

Doc updates are OK. Thanks @mfuchs!

[mfuchs]

Some tests fail.
I checked and at least test/integration/targets/vars_prompt/vars_prompt-5.yml fails.
The reason is that it expects re.compile('"password": "\\$6\\$rounds=') in the password-output.
Yet since no rounds are provided the default of crypt is used leading to a output like

"password": "$6$jESIyad4F08hP3Ta$hrnwEbDk9uSg1IURdckNLBy9xoeO1y1peRDBHsdmwzlWv1u9Cs0qasiy7THQ88dnzC0f4PaFX4ySUCcyT5emn0"

This change was implemented to be more consistent overall, thus having consistency between vars_prompt and password_hash (no matter if passlib is installed or not) and not depending on the always changing defaults of passlib.

[abadger]

Makes sense to me. Go ahead and change the test.

[mfuchs]

Since the test is rather new I rebased on current devel + adapted the integration test.

[abadger]

Merged to devel for the 2.7.0 release. Thanks @mfuchs

[BarbzYHOOL]

awesome