ScaleComputing / HyperCoreAnsibleCollection

Official Ansible collection for Scale Computing SC//HyperCore (HC3) v1 API
GNU General Public License v3.0
12 stars 8 forks source link

:lady_beetle: Bug: masked password appears in unrelated output #270

Closed shoriminimoe closed 11 months ago

shoriminimoe commented 12 months ago

Describe the bug

If the password of the user running a playbook appears in an alert email address, VM name, etc. the portion of output that contains the password is masked.

To Reproduce Steps to reproduce the behavior:

  1. Create an alert email or VM whose name contains a user's password
  2. Use that user to read the created email/VM using the appropriate ansible module.
  3. The user's password will be masked in the output email/VM name

Expected behavior

The collection should not be masking passwords in unrelated content.

Screenshots

Here is an example playbook. The screenshot below has example output

---
- name: test masked password output
  hosts: all
  connection: local
  gather_facts: false
  environment:
    SC_HOST: "https://{{ inventory_hostname }}"
    SC_USERNAME: "{{ sc_username | default('someuser') }}"
    SC_PASSWORD: "{{ sc_password | default('password') }}"

  tasks:
    - name: "create alert email: mypasswordmanager@example.com"
      scale_computing.hypercore.email_alert:
        email: mypasswordmanager@example.com
        state: present
      register: alert_email

    - name: "Inspect email output"
      debug:
        msg: "Reported email address: '{{ alert_email.record.email }}'"

    - name: "create VM: the-password-manager-vm"
      scale_computing.hypercore.vm:
        state: present
        vm_name: the-password-manager-vm
        description: The name of this VM contains the test user's password
        tags:
          - mypasswordmanager
        vcpu: 2
        memory: "{{ '512 MB' | human_to_bytes }}"
        disks:
          - type: virtio_disk
            disk_slot: 0
            size: "{{ '100 GB' | human_to_bytes }}"
          - type: ide_cdrom
            disk_slot: 0
        nics:
          - type: virtio
      register: vm_created

    - name: "Inspect VM output"
      debug:
        msg:
          - "New VM name: {{ vm_created.record[0].vm_name }}"
          - "New VM tags: {{ vm_created.record[0].tags }}"
          - "New VM description: {{ vm_created.record[0].description }}"

image

System Info (please complete the following information):

Additional context

This is concerning for a few reasons:

justinc1 commented 12 months ago

Yes, this is all true.

However the problem occurs only when passwords are weak - short and/or common words. If you password is complex or something like "at-least-a-string-unlikely-to-be-present-in-some-other-text", the problem will not happen. So this problem does happen in testing environments, but should never in production environments. It is more nasty annoyance than real problem.

The demo task name: "Delete created email" is a really nasty one, because after you get used to your password being replaced by *** in terminal output, you (or at least I) was really surprised when I figure out that the password value is being replaced also in values in registered variables. Note this is done be the core ansible. The only thing collection can do is to mark variables as being a password or not (the nolog flag).

Also again the problem should never happen in production environments.

@shoriminimoe I thing we should only document this behaviour in documentation. Likely also include an example (it is already above :)), to make whole story easy to understood.

justinc1 commented 11 months ago

Documentation fixed in https://github.com/ScaleComputing/HyperCoreAnsibleCollection/pull/272

shoriminimoe commented 11 months ago

I hesitate to make general statements about when this type of issue can occur without analysis. I agree that it is more likely to occur with weak passwords. I also agree that it should never happen in production environments, but it can, and we are not doing anything to prevent it.

I recognize the challenge to resolve this given the way ansible handles no_log parameters and output. I am confident there is a way to solve this. Although documenting the issue is a good thing, I don't believe it solves the problem.

@ddemlow does the documentation change resolve this issue for you?

justinc1 commented 11 months ago

Regarding

it should never happen in production environments, but it can

If password is not a common string (like it should be) - could this happen then? I would say no. So with production grade password, production environment will not have a problem.