Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ansible prints credentials to console #289

Open
lexxxel opened this issue Jan 16, 2025 · 9 comments
Open

ansible prints credentials to console #289

lexxxel opened this issue Jan 16, 2025 · 9 comments

Comments

@lexxxel
Copy link
Collaborator

lexxxel commented Jan 16, 2025

Ansible warns about possible leaked passwords, e.g. here:

TASK [lae.proxmox : Configure Proxmox Storage] *************************************************************************************************************************************************************************************************************************************************************************************************************
changed: [myserver] => (item={'name': 'pbs', 'type': 'pbs',  <<<redacted>>>  'password': '  'testpassword', <<<redacted>>>
[WARNING]: Module did not set no_log for password
@rabelmervin
Copy link

Hallo @lexxxel ,
I came across this issue and found it quite interesting. I Would like to contribute ? However, I'm new to ansible any guidance would be appreciated :)

@lae
Copy link
Owner

lae commented Jan 17, 2025

@rabelmervin I would recommend familiarizing yourself with Ansible outside of this role first. The documentation is at https://docs.ansible.com/ansible/latest/getting_started/index.html.

This issue is however more on the module development side of Ansible, for which there's a brief intro at https://docs.ansible.com/ansible/latest/dev_guide/developing_modules_general.html. I believe this would just need some modifications in library/proxmox_storage.py for sensitive fields.

@rabelmervin
Copy link

Sure @lae I'll take a look at ansible

@edv-pi
Copy link
Contributor

edv-pi commented Jan 22, 2025

did you set pve_no_log to true? because that should avoid this warning and also avoids leaking password in log

@lexxxel
Copy link
Collaborator Author

lexxxel commented Jan 23, 2025

did you set pve_no_log to true? because that should avoid this warning and also avoids leaking password in log

yes, pve_no_log: true is set for all 3 clusters I configure.

PS: I run my playbook with --diff (as the only argument). I'm unsure if there is a difference between --diff and -v that is relevant here, but I thought I should mention it.

@lae
Copy link
Owner

lae commented Jan 23, 2025

@lexxxel are you using v1.9.1?

I just got around to double checking but Dimitri fixed this in cf67089/#282 a few months ago, I believe.

@lexxxel
Copy link
Collaborator Author

lexxxel commented Jan 23, 2025

@lae I can't say: - lae.proxmox, (unknown version) ... but I can imagine that I'm on the commit the created my pull request for 1.9.1 (459001c), which would before the fix.

I will retest with a clean install 🙇

@lexxxel
Copy link
Collaborator Author

lexxxel commented Jan 23, 2025

Tested it with 1.9.1 and no, the issue still exists for me:
Image
Edit: I believe that I did everything right, but there might be a skill gap on my side. Meaning, If you believe it's fixed, I'm unsure if the issue still exists.

@lae
Copy link
Owner

lae commented Jan 24, 2025

I'm attempting to reproduce this atm but after fixing my vagrant-libvirt issues it turns out I only have like half a GB free on my system and thus can't bring up a test cluster with the project's Vagrantfile...so I'll probably try some other time when I can deal with that situation.

But if anyone else wants to test if this is still an issue I think the following diff (apply with patch -p1 < this.diff) should expose the problem:

diff --git a/tests/vagrant/group_vars/all b/tests/vagrant/group_vars/all
index 7312d8e..969eb90 100644
--- a/tests/vagrant/group_vars/all
+++ b/tests/vagrant/group_vars/all
@@ -80,6 +80,16 @@ pve_storages:
   - name: no-content-dir
     type: dir
     path: /tmp/fakedir2
+  - name: fake-pbs
+    type: pbs
+    content: [ "backup" ]
+    server: 127.0.0.1
+    username: user@pbs
+    password: PBSPassword1
+    datastore: main
+    fingerprint: f2:fb:85:76:d2:2a:c4:96:5c:6e:d8:71:37:36:06:17:09:55:f7:04:e3:74:bb:aa:9e:26:85:92:63:c8:b9:23
+    encryption_key: autogen
+    namespace: Top/blah
 pve_zfs_create_volumes:
   - testpool/zfs2
 pve_ceph_osds:
@@ -91,3 +101,4 @@ ntp_servers:
   - clock.sjc.he.net
   - clock.fmt.he.net
   - clock.nyc.he.net
+pve_no_log: true

I say I believe it's fixed because the lines that configure the module in library/proxmox_storage.py do have no_log=True for password, so if it's not fixed then I'm not quite sure what else would need to be changed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants