ansible-collections / community.crypto

The community.crypto collection for Ansible.
https://galaxy.ansible.com/ui/repo/published/community/crypto/
Other
98 stars 89 forks source link

Different modes for private/public keys with `openssh_keypair` #632

Open lonix1 opened 1 year ago

lonix1 commented 1 year ago
SUMMARY

The openssh_keypair sets the same mode for both private and public keys.

So mode: '0600' will set that mode for both keys. Typically the private key has 600 and the public has 644 (if not, then ssh won't read it, so I need to add another task to fix the permissions).

ISSUE TYPE
COMPONENT NAME

openssh_keypair

ADDITIONAL INFORMATION

As above.

- community.crypto.openssh_keypair:
  type: ed25519
  path: /home/me/.ssh/id_ed25519_foo
  mode: '0600'
  mode_public: '0644'             # <------------------
MarkusTeufelberger commented 1 year ago

Afair mode is inherited from the general set of parameters for file based modules. It might be better to remove mode completely and replace it with mode_private and mode_public (or similar) because only adding it for one of the two keys also seems not very intuitive especially if that one is left out.

lonix1 commented 1 year ago

Agreed.

The breaking change may annoy some people though. Maybe a compromise:

(And if you want, you could mark mode as deprecated, to be removed in a future version.)

Ajpantuso commented 1 year ago

It's not so simple to remove mode. You would end up affecting all the file parameters.

You can certainly keep mode and then override using mode_private and mode_public, but then again you could do the same thing by setting mode to the more restrictive of the two files and use a follow-up file task to update the public key mode.

I don't have a strong opinion about what the best interface for this is, but given this is cryptographic material we should be as safe as possible at the time of creation.

MarkusTeufelberger commented 1 year ago

The follow-up task workaround would create idempotence issues. However, I'm not so sure about the initial statement here:

The openssh_keypair sets the same mode for both private and public keys.

By default, private keys are set to 600 and public keys to 644, so the mode attribute should not be necessary in the first place unless some other settings than those are intended. I would understand if you need different modes than 600 and 644, but the use case in the initial description would already work with:

- community.crypto.openssh_keypair:
    type: ed25519
    path: /home/me/.ssh/id_ed25519_foo
Ajpantuso commented 1 year ago

Correct, using the follow-up task would trigger a change on every run.

And yes by default not setting mode will result in 0600 for the private key and 0644 for the public key.

I think the only case where you would want to set mode for this module is if you actually want the public key to be more restricted.

Setting the private key less restrictive historically would cause issues with ssh-keygen since OpenSSH requires that private keys be protected. So mode itself has always had limited functionality with this module.

lonix1 commented 1 year ago

Sorry I'm not an insider, so some of this went over my head.

Are you saying that my problem is that I actually used mode in the first place? So if I want the 600/644 convention, I must not specify mode?

lonix1 commented 1 year ago

Yes... just tested it. The trick is NOT to set mode to begin with. :-) Thanks.

It would probably help others if the the docs had a one liner on this.

lonix1 commented 1 year ago

I tried to do a PR to add that to the docs, but for reasons I don't understand, mode doesn't appear in the source for that doc. (I just clicked the "edit" button, forked, and the file that I'm looking at doesn't have "mode".)

UPDATE: Aah, it's a template. Well I'm not a python dev, so can't really help here! :smile:

MarkusTeufelberger commented 1 year ago

This is already in there:

In the case a custom mode, group, owner, or other file attribute is provided it will be applied to both key files.

But yes, it might also help to document explicitly that by default 600/644 permissions will be applied. The documentation for mode and some other parameters comes from Ansible itself (https://github.com/ansible/ansible/blob/devel/lib/ansible/module_utils/basic.py#L253-L261), I'd add the info down at "notes".

Edit: Here: https://github.com/ansible-collections/community.crypto/blob/57a8c7e65230e427da11bfd5ae72bf1319699fa9/plugins/modules/openssh_keypair.py#L142

lonix1 commented 1 year ago

You are right! Sorry, missed that. I always look into the corresponding parameter row in the table.

MarkusTeufelberger commented 1 year ago

Nah, it is kinda unintuitive sometimes. Thanks for helping out and working on a solution! :-)