Feat(plugins): Add support for arista.avd.secure_hash to hash local_user passwords

[davidhayes9]

Change Summary

Add support for a arista.avd.secure_hash filter to allow a local_user to hash their password.

Related Issue(s)

Fixes #3883

Component(s) name

arista.avd.eos_cli_config_gen

Proposed changes

secure_hash does not accept any parameters other than the users password and returns a SHA512-Crypt password hash

local_users:
  - name: <str>
    sha512_password: "{{ 'clear_password' | secure_hash }}"

There is a dependancy on passlib and the filter been tested with passlib==1.7.4 (latest version)

The filter is named secure_hash as hash is the name of a Python built-in function but happy to hear other naming suggestions.

How to test

Added test coverage in pytest and molecule

Checklist

User Checklist

• N/A

Repository Checklist

• My code has been rebased from devel before I start
• I have read the CONTRIBUTING document.
• My change requires a change to the documentation and documentation have been updated accordingly.
• I have updated molecule CI testing accordingly. (check the box if not applicable)

[github-actions]

Review docs on Read the Docs

To test this pull request:

# Create virtual environment for this testing below the current directory
python -m venv test-avd-pr-4768
# Activate the virtual environment
source test-avd-pr-4768/bin/activate
# Install all requirements including PyAVD
pip install "pyavd[ansible] @ git+https://github.com/davidhayes9/avd.git@hash-filter#subdirectory=python-avd" --force
# Point Ansible collections path to the Python virtual environment
export ANSIBLE_COLLECTIONS_PATH=$VIRTUAL_ENV/ansible_collections
# Install Ansible collection
ansible-galaxy collection install git+https://github.com/davidhayes9/avd.git#/ansible_collections/arista/avd/,hash-filter --force
# Optional: Install AVD examples
cd test-avd-pr-4768
ansible-playbook arista.avd.install_examples

[gmuloc]

Thanks for this PR and the implementation up to the tests! You have done all that was expected and thanks for this.

I have left a couple of comments of things to adjust.

The name in the referenced issue suggested by @ClausHolbechArista was arista.avd.hash not sure if we want to keep this one or use secure_hash

I have a slight concerned regarding not setting the salt as it means on every run of AVD the password will change (which is not going to make molecule happy among others) as it will be re-generated. It means what is produced won't be idempotent anymore (but yes it will work)

I am wondering if we should not add a salt option in the filter and tell people to generate one. And use it in the molecule scenario. Otherwise we need a different way to persist these (maybe one run and then people can take the output and remove the usage of the secure_hash)

Our stance until now also was that it was already supported by just using the builtin password_hash filter: https://docs.ansible.com/ansible/latest/collections/ansible/builtin/password_hash_filter.html doing exactly this (offering the option to use salt)

[davidhayes9]

Thanks for this PR and the implementation up to the tests! You have done all that was expected and thanks for this.

I have left a couple of comments of things to adjust.

Thanks for reviewing and leaving the suggestions, i've pushed new changes and these should be resolve the suggestions.

The name in the referenced issue suggested by @ClausHolbechArista was arista.avd.hash not sure if we want to keep this one or use secure_hash

I did originally start by using arista.avd.hash but it looks like the code format is to use the filter name as the main caller function. When I used hash for this I was getting an error (I think from one of the linters) that a built-in function name was being used. I can try moving it back to hash though and confirming.

I have a slight concerned regarding not setting the salt as it means on every run of AVD the password will change (which is not going to make molecule happy among others) as it will be re-generated. It means what is produced won't be idempotent anymore (but yes it will work)

I am wondering if we should not add a salt option in the filter and tell people to generate one. And use it in the molecule scenario. Otherwise we need a different way to persist these (maybe one run and then people can take the output and remove the usage of the secure_hash)

Our stance until now also was that it was already supported by just using the builtin password_hash filter: https://docs.ansible.com/ansible/latest/collections/ansible/builtin/password_hash_filter.html doing exactly this (offering the option to use salt)

Yes good point about the password being re-generated each time. I can add an optional salt parameter similar to the password_hash

[davidhayes9]

the linter errors when I use hash

ruff....................................................................................................................Failed
- hook id: ruff
- exit code: 1

ansible_collections/arista/avd/plugins/filter/hash.py:18:5: A001 Variable `hash` is shadowing a Python builtin
   |
16 |     from pyavd.j2filters import hash
17 | except ImportError as e:
18 |     hash = RaiseOnUse(
   |     ^^^^ A001
19 |         AnsibleFilterError(
20 |             f"The '{PLUGIN_NAME}' plugin requires the 'pyavd' Python library. Got import error",
   |

python-avd/pyavd/j2filters/hash.py:46:5: A001 Variable `hash` is shadowing a Python builtin
   |
46 | def hash(user_password: str) -> str:
   |     ^^^^ A001
47 |     return _get_password_hash(user_password)
   |

Found 2 errors.

Let me know what the preference is for the filter name.

[ClausHolbechArista]

I don't think we need this function as long as we don't provide any options. You can just rename the main function.

Since we asume it is a user password we can either decide to rename this to user_password_hash or maybe better keep this function with an extra argument to specify the type of password, and then call a _user_password_hash function. (only supported option is the user password for now, but maybe others will come later).

@carlbuchmann @gmuloc comments?

[carlbuchmann]

Sorry, I completely missed this notification. I would keep this function with an extra argument to specify the type of password, and then call a _user_password_hash function. I agree that other options will likely come later.

[davidhayes9]

The extra arg has been done. There is an output_type arg that can take sha512_password for now. More values can be added in the future when there are more use cases. It will also default tosha512_password if the arg is left out. Below are some examples:

local_users:
  # Create a sha512 password hash with a user defined salt value (recommended). The output_type will default to sha512_password.
  - name: cvpadmin
    sha512_password: "{{ 'securepassword' | arista.avd.secure_hash(salt='Yar49ahkzKddRVYS')}}"

    # Create a sha512 password hash with a user defined salt value and specifying the output_type as a sha512_password.
  - name: cvpuser
    sha512_password: "{{ 'newpassword' | arista.avd.secure_hash(salt='Kte5paJ3czRQczbk', output_type='sha512_password')}}"

  # Create a sha512 password hash with a random salt. Note: this will create a new hash each time AVD is run.
  - name: admin
    sha512_password: "{{ 'password123' | arista.avd.secure_hash }}"

[github-actions]

This pull request has conflicts, please resolve those before we can evaluate the pull request.

[alexeygorbunov]

@gmuloc , are you able to install this pyavd from source without adding passlib>=1.7.4 dependency under [build-system]?
I'm facing an issue as passlib is required for filters used in J2 templates that need to be compiled during the build

[gmuloc]

on-hold until we bring some alternative to passlib