Skip to content

Commit

Permalink
Share the implementation of hashing for both vars_prompt and password…
Browse files Browse the repository at this point in the history
…_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.
  • Loading branch information
mfuchs authored and abadger committed Aug 27, 2018
1 parent ad993ca commit 7871027
Show file tree
Hide file tree
Showing 10 changed files with 361 additions and 93 deletions.
18 changes: 18 additions & 0 deletions changelogs/fragments/hashing-changes.yaml
@@ -0,0 +1,18 @@
---
bugfixes:
- vars_prompt with encrypt does not require passlib for the algorithms
supported by crypt.
- Additional checks ensure that there is always a result of hashing passwords
in the password_hash filter and vars_prompt, otherwise an error is returned.
Some modules (like user module) interprets None as no password at all,
which can be dangerous if the password given above is passed directly into
those modules.
- Avoids deprecated functionality of passlib with newer library versions.
- password_hash does not hard-code the salt-length, which fixes bcrypt
in connection with passlib as bcrypt requires a salt with length 22.
minor_changes:
- The password_hash filter supports all parameters of passlib.
This allows users to provide a rounds parameter.
(https://github.com/ansible/ansible/issues/15326)
- password_hash is not restricted to the subset provided by crypt.crypt
(https://github.com/ansible/ansible/issues/17266)
19 changes: 17 additions & 2 deletions docs/docsite/rst/porting_guides/porting_guide_2.7.rst
Expand Up @@ -71,6 +71,16 @@ In Ansible 2.7 a new module argument named ``public`` was added to the ``include

There is an important difference in the way that ``include_role`` (dynamic) will expose the role's variables, as opposed to ``import_role`` (static). ``import_role`` is a pre-processor, and the ``defaults`` and ``vars`` are evaluated at playbook parsing, making the variables available to tasks and roles listed at any point in the play. ``include_role`` is a conditional task, and the ``defaults`` and ``vars`` are evaluated at execution time, making the variables available to tasks and roles listed *after* the ``include_role`` task.

vars_prompt with unknown algorithms
-----------------------------------

vars_prompt now throws an error if the hash algorithm specified in encrypt is not supported by
the controller. This increases the safety of vars_prompt as it previously returned None if the
algorithm was unknown. Some modules, notably the user module, treated a password of None as
a request not to set a password. If your playbook starts erroring because of this, change the
hashing algorithm being used with this filter.


Deprecated
==========

Expand All @@ -81,7 +91,7 @@ Expedited Deprecation: Use of ``__file__`` in ``AnsibleModule``

We are deprecating the use of the ``__file__`` variable to refer to the file containing the currently-running code. This common Python technique for finding a filesystem path does not always work (even in vanilla Python). Sometimes a Python module can be imported from a virtual location (like inside of a zip file). When this happens, the ``__file__`` variable will reference a virtual location pointing to inside of the zip file. This can cause problems if, for instance, the code was trying to use ``__file__`` to find the directory containing the python module to write some temporary information.

Before the introduction of AnsiBallZ in Ansible 2.1, using ``__file__`` worked in ``AnsibleModule`` sometimes, but any module that used it would fail when pipelining was turned on (because the module would be piped into the python interpreter's standard input, so ``__file__`` wouldn't contain a file path). AnsiBallZ unintentionally made using ``__file__`` always work, by always creating a temporary file for ``AnsibleModule`` to reside in.
Before the introduction of AnsiBallZ in Ansible 2.1, using ``__file__`` worked in ``AnsibleModule`` sometimes, but any module that used it would fail when pipelining was turned on (because the module would be piped into the python interpreter's standard input, so ``__file__`` wouldn't contain a file path). AnsiBallZ unintentionally made using ``__file__`` work, by always creating a temporary file for ``AnsibleModule`` to reside in.

Ansible 2.8 will no longer create a temporary file for ``AnsibleModule``; instead it will read the file out of a zip file. This change should speed up module execution, but it does mean that starting with Ansible 2.8, referencing ``__file__`` will always fail in ``AnsibleModule``.

Expand Down Expand Up @@ -190,7 +200,12 @@ Noteworthy module changes
Plugins
=======

No notable changes.
* The hash_password filter now throws an error if the hash algorithm specified is not supported by
the controller. This increases the safety of the filter as it previously returned None if the
algorithm was unknown. Some modules, notably the user module, treated a password of None as
a request not to set a password. If your playbook starts erroring because of this, change the
hashing algorithm being used with this filter.


Porting custom scripts
======================
Expand Down
12 changes: 12 additions & 0 deletions docs/docsite/rst/user_guide/playbooks_filters.rst
Expand Up @@ -783,6 +783,18 @@ An idempotent method to generate unique hashes per system is to use a salt that
Hash types available depend on the master system running ansible,
'hash' depends on hashlib password_hash depends on passlib (https://passlib.readthedocs.io/en/stable/lib/passlib.hash.html).

.. versionadded:: 2.7

Some hash types allow providing a rounds parameter::

{{ 'secretpassword'|password_hash('sha256', 'mysecretsalt', rounds=10000) }}

When`Passlib <https://passlib.readthedocs.io/en/stable/>`_ is installed
`password_hash` supports any crypt scheme and parameter supported by 'Passlib'::

{{ 'secretpassword'|password_hash('sha256_crypt', 'mysecretsalt', rounds=5000) }}
{{ 'secretpassword'|password_hash('bcrypt', ident='2b', rounds=14) }}

.. _combine_filter:

Combining hashes/dictionaries
Expand Down
10 changes: 10 additions & 0 deletions docs/docsite/rst/user_guide/playbooks_prompts.rst
Expand Up @@ -90,6 +90,16 @@ However, the only parameters accepted are 'salt' or 'salt_size'. You can use you
'salt', or have one generated automatically using 'salt_size'. If nothing is specified, a salt
of size 8 will be generated.

.. versionadded:: 2.7

When Passlib is not installed the `crypt <https://docs.python.org/2/library/crypt.html>`_ library is used as fallback.
Depending on your platform at most the following crypt schemes are supported:

- *bcrypt* - BCrypt
- *md5_crypt* - MD5 Crypt
- *sha256_crypt* - SHA-256 Crypt
- *sha512_crypt* - SHA-512 Crypt

.. seealso::

:doc:`playbooks`
Expand Down
56 changes: 14 additions & 42 deletions lib/ansible/plugins/filter/core.py
Expand Up @@ -41,19 +41,14 @@

from jinja2.filters import environmentfilter, do_groupby as _do_groupby

try:
import passlib.hash
HAS_PASSLIB = True
except ImportError:
HAS_PASSLIB = False

from ansible.errors import AnsibleFilterError
from ansible.module_utils.six import iteritems, string_types, integer_types
from ansible.errors import AnsibleError, AnsibleFilterError
from ansible.module_utils.six import iteritems, string_types, integer_types, reraise
from ansible.module_utils.six.moves import reduce, shlex_quote
from ansible.module_utils._text import to_bytes, to_text
from ansible.module_utils.common.collections import is_sequence
from ansible.parsing.ajson import AnsibleJSONEncoder
from ansible.parsing.yaml.dumper import AnsibleDumper
from ansible.utils.encrypt import passlib_or_crypt
from ansible.utils.hashing import md5s, checksum_s
from ansible.utils.unicode import unicode_wrap
from ansible.utils.vars import merge_hash
Expand Down Expand Up @@ -252,42 +247,19 @@ def get_hash(data, hashtype='sha1'):
return h.hexdigest()


def get_encrypted_password(password, hashtype='sha512', salt=None):

# TODO: find a way to construct dynamically from system
cryptmethod = {
'md5': '1',
'blowfish': '2a',
'sha256': '5',
'sha512': '6',
def get_encrypted_password(password, hashtype='sha512', salt=None, salt_size=None, rounds=None):
passlib_mapping = {
'md5': 'md5_crypt',
'blowfish': 'bcrypt',
'sha256': 'sha256_crypt',
'sha512': 'sha512_crypt',
}

if hashtype in cryptmethod:
if salt is None:
r = SystemRandom()
if hashtype in ['md5']:
saltsize = 8
else:
saltsize = 16
saltcharset = string.ascii_letters + string.digits + '/.'
salt = ''.join([r.choice(saltcharset) for _ in range(saltsize)])

if not HAS_PASSLIB:
if sys.platform.startswith('darwin'):
raise AnsibleFilterError('|password_hash requires the passlib python module to generate password hashes on macOS/Darwin')
saltstring = "$%s$%s" % (cryptmethod[hashtype], salt)
encrypted = crypt.crypt(password, saltstring)
else:
if hashtype == 'blowfish':
cls = passlib.hash.bcrypt
else:
cls = getattr(passlib.hash, '%s_crypt' % hashtype)

encrypted = cls.encrypt(password, salt=salt)

return encrypted

return None
hashtype = passlib_mapping.get(hashtype, hashtype)
try:
return passlib_or_crypt(password, hashtype, salt=salt, salt_size=salt_size, rounds=rounds)
except AnsibleError as e:
reraise(AnsibleFilterError, AnsibleFilterError(str(e), orig_exc=e), sys.exc_info()[2])


def to_uuid(string):
Expand Down
13 changes: 2 additions & 11 deletions lib/ansible/plugins/lookup/password.py
Expand Up @@ -100,7 +100,7 @@
from ansible.module_utils._text import to_bytes, to_native, to_text
from ansible.parsing.splitter import parse_kv
from ansible.plugins.lookup import LookupBase
from ansible.utils.encrypt import do_encrypt, random_password
from ansible.utils.encrypt import do_encrypt, random_password, random_salt
from ansible.utils.path import makedirs_safe


Expand Down Expand Up @@ -207,15 +207,6 @@ def _gen_candidate_chars(characters):
return chars


def _random_salt():
"""Return a text string suitable for use as a salt for the hash functions we use to encrypt passwords.
"""
# Note passlib salt values must be pure ascii so we can't let the user
# configure this
salt_chars = _gen_candidate_chars(['ascii_letters', 'digits', './'])
return random_password(length=8, chars=salt_chars)


def _parse_content(content):
'''parse our password data format into password and salt
Expand Down Expand Up @@ -329,7 +320,7 @@ def run(self, terms, variables, **kwargs):

if params['encrypt'] and not salt:
changed = True
salt = _random_salt()
salt = random_salt()

if changed and b_path != to_bytes('/dev/null'):
content = _format_content(plaintext_password, salt, encrypt=params['encrypt'])
Expand Down

0 comments on commit 7871027

Please sign in to comment.