brocade / ansible

55 stars 33 forks source link

brocade_facts: Credential user name is redacted from brocade_security_user_config facts #140

Open mamoep opened 1 year ago

mamoep commented 1 year ago

When collecting switch information with the brocade_facts module, the name of the user that is used for login is replaced by "VALUE_SPECIFIED_IN_NO_LOG_PARAMETER". I am using Ansible Galaxy collection version 1.3.3.

Task:

  - name: gather Brocade facts
    brocade.fos.brocade_facts:
      credential:
        fos_ip_addr: "{{ switch }}"
        fos_user_name: "{{ username }}"
        fos_password: "{{ password }}"
        https: true
      vfid: -1
      gather_subset:
        - brocade_security_user_config

Output

{
    "name": "admin",
    "role": "admin",
    "account_description": "Administrator",
    "account_enabled": true,
    "default_password_configured": false,
    "password_change_enforced": false,
    "account_locked": false,
    "access_start_time": null,
    "access_end_time": null,
    "auth_token_present": false
},
{
    "name": "user",
    "role": "user",
    "account_description": "User",
    "account_enabled": true,
    "default_password_configured": false,
    "password_change_enforced": false,
    "account_locked": false,
    "access_start_time": null,
    "access_end_time": null,
    "auth_token_present": false
},
{
    "name": "VALUE_SPECIFIED_IN_NO_LOG_PARAMETER",
    "role": "admin",
    "account_description": "Ansible User",
    "account_enabled": true,
    "default_password_configured": false,
    "password_change_enforced": false,
    "account_locked": false,
    "access_start_time": null,
    "access_end_time": null,
    "auth_token_present": false
},
prasad-valmeti-broadcom commented 1 year ago

These security logging parameters are masked by Ansible itself during the facts, nothing is done in the modules. I see you mentioned Ansible FOS 1.3.3 as the version. As per my understanding, it is the same behavior in older Ansible FOS releases as well. This behavior can be overridden with the no_log option in yml or ansible.cfg to show sensitive parameters as well in ansible logging. Did you try no_log option?

What is the FOS Switch release? what is the actual name that should be shown instead of VALUE_SPECIFIED_IN_NO_LOG_PARAMETER?

mamoep commented 1 year ago

I tried to set "no_log: false" in the playbook but it didn't make any difference. I don't think that setting can override "secured" variables from the module code.

Tested with FOS 8.2.3d and 9.1.1c

The actual name is the one used infos_user_name: "{{ username }}" variable. So whichever user I choose as login credential.

prasad-valmeti-broadcom commented 11 months ago

Taking our fos_user_name out of the credentials and treating it as different is working fine. Please let us know if it works for you with these changes: --> In the playbook *.yml file(s), remove the fos_user_name from the credential group. --> In the tasks of facts.yml, add fos_user_name as shown below: brocade_facts: credential: "{{credential}}" fos_user_name: "{{fos_user_name}}" --> In the brocade_facts.py, have the following changes argument_spec = dict( credential=dict(required=True, type='dict', no_log=True),

Please let us know if it resolves this issue.

mamoep commented 11 months ago

The proposed fix works.

mamoep commented 11 months ago

I suggest to following change to keep the current structure of the credential object intact, while still be able to mute the password.

    argument_spec = dict(
        credential=dict(
            required=True,
            type='dict',
            options=dict(
                fos_ip_addr=dict(required=True, type='str'),
                fos_user_name=dict(required=True, type='str'),
                fos_password=dict(required=True, type='str', no_log=True),
                https=dict(required=True, type='bool'),
                ssh_hostkeymust=dict(required=False, type='bool')
            )
        ),
prasad-valmeti-broadcom commented 11 months ago

Thanks for the confirmation and suggestion. Sure, we will handle it in the proposed way so that no need to change existing playbooks. This will be handled in the next release.