lae / ansible-role-proxmox

IaC for Proxmox VE clusters.
MIT License
468 stars 139 forks source link

Feature request: Support for controlling PVE Authentication Server password hashes #207

Closed junousi closed 1 year ago

junousi commented 1 year ago

Setting the initial password via Ansible might not be desireable in all situations.

In some situations, it might be preferable to provide the contents of the SHA-256 crypt directly.

This should probably work so that current password parameter is completely ignored if the hash is provided. Unsure if this would mean big or small changes for create_user(). For any of this to work, /etc/pve/priv/shadow.cfg would probably need to be taken under control of this role.

junousi commented 1 year ago

Oh, and I could maybe write a PR for this one, but prefer first to check if the general idea makes sense.

lae commented 1 year ago

Ideally most operations are done via pvesh (PVE's API). In this case it might not be possible, but I would need to double check.

If I recall correctly, the issue with modifying files in /etc/pve/ is that it is technically a FUSE filesystem (created by pmxcfs) with very limited functionality and thus using Ansible on them can be a bit tricky (i.e. some modules don't work properly).

But it does seem we do it for datacenter.cfg, albeit in this case we know we'd control the entire config file. That might not be true for the user directory on a PVE installation.

junousi commented 1 year ago

Ideally most operations are done via pvesh (PVE's API).

Hear hear!

If I recall correctly, the issue with modifying files in /etc/pve/ is that it is technically a FUSE filesystem (created by pmxcfs) with very limited functionality and thus using Ansible on them can be a bit tricky (i.e. some modules don't work properly).

Ah yes, I think I've encountered this with template module already.

But it does seem we do it for datacenter.cfg, albeit in this case we know we'd control the entire config file. That might not be true for the user directory on a PVE installation.

Yes, and I think even there @trickert76 is spot on that if it's available in the API, one should use the API. So rather than take things further into a direction where one uses more under-the-hood tricks, I daresay maybe even that datacenter.cfg part should be rewritten to be API driven, if it can be.

OTOH, I wouldn't want to store user dictionaries in too many places, either. I now pushed some test cases and will ponder a bit, would I then just consume this kind of data, and enforce it, outside the role.

junousi commented 1 year ago

At this time I can report that running a post_tasks with the mechanism borrowed from datacenter.cfg e.g.

+    - name: Enforce password hashes
+      copy:
+        dest: "/etc/pve/priv/shadow.cfg"
+        owner: "root"
+        group: "www-data"
+        mode: "0600"
+        content: |
+          {% for k,v in pve_sha256_hashes.items() %}
+          {{ k }}:{{ v }}:
+          {% endfor %}

...is an adequate workaround.

From issue creator perspective this issue could be closed. But not closing straight away, if it's worth considering to add this as builtin.

lae commented 1 year ago

correct me if i misunderstand

i think this will break existing deployments. end users have definitely created users manually on their pve clusters, so when they update to a release with this task it'll wipe everything out.

we need to have a way to have it only manage the users specified in the role variable. if there's a definition for a user, check and compare that specific user and update it as necessary (using state: absent in the user definition to decide whether to delete the user), otherwise don't touch them.

junousi commented 1 year ago

But not closing straight away, if it's worth considering to add this as builtin.

What I wrote here should have been s/this/something like this, but more robust/.

So yes, you are absolutely right. Mentioned workaround is unilateral, not intended as-is for a PR, just happens to work for the greenfield/labcluster cases at hand.

I'm starting to feel that really, the proper place to address this would be upstream SHA support in the access/users API.

lae commented 1 year ago

I think you should be able to open a feature request on their issue tracker (my memory is hazy but I think it was public somewhere?) or post on the pve-user mailing list. Developers have been pretty responsive in my experience.

trickert76 commented 1 year ago

The main reason for this ticket is to have a PVE integrated user management. I wondered myself, why I don‘t had the problem before and maybe I just want to give a hint: I‘m using the PAM users. This users are managed via Ansible too, they cannot login via SSH but have a physical user account on the host. Together with the Proxmox groups and roles a useful ACL management is already present via pvesh. I don‘t know if the PVE integrated access management is something, that will be developed in the future. I would expect, that they wanna switch more to LDAP, OAuth or something like that, which is more an enterprise feature. And use PAM for community solutions. But - you should ask them as proposed. From my perspective - use PAM. Am 24.10.2022 um 13:14 schrieb Musee Ullah @.***>: I think you should be able to open a feature request on their issue tracker (my memory is hazy but I think it was public somewhere?) or post on the pve-user mailing list. Developers have been pretty responsive in my experience.

—Reply to this email directly, view it on GitHub, or unsubscribe.You are receiving this because you were mentioned.Message ID: @.***>

lae commented 1 year ago

some maybe necessary clarifications and my thoughts

junousi commented 1 year ago

Right, indeed for PAM there are numerous options for controlling the node local shadow file. As a minor annoyance compared to PVE, one of course has to ensure the shadow files are kept in sync on all nodes.

The main thing I'm trying to avoid here is providing any of the passwords (initial or non-initial) to Ansible in cleartext. Thus preferring any option that allows managing the hashes direct. I'll reconsider PAM. LDAP is not an option at this time.

@trickert76 @lae Thanks again for the comments! I will look into the feature request part and then reopen/revisit this topic if needed.

lae commented 1 year ago

Thus preferring any option that allows managing the hashes direct. I'll reconsider PAM.

I think your current workaround is fine, actually. Again I'd recommend asking the devs to implement a change to allow setting/validating hashed passwords (or if they have another idea that lets us automate configuring users in an idempotent fashion). Then we could implement supporting that in the proxmox_user module.

junousi commented 1 year ago

Again I'd recommend asking the devs to implement a change to allow setting/validating hashed passwords

Ack, just started the registration process on their bugzilla.

junousi commented 1 year ago

For posterity: https://bugzilla.proxmox.com/show_bug.cgi?id=4312