From ce3299f106a7c3084a3b2a8b66be3755fe45c26f Mon Sep 17 00:00:00 2001 From: David Zaslavsky Date: Mon, 1 May 2023 12:16:42 -0700 Subject: [PATCH] Always generate a new key pair if the private key doesn't exist (#598) * Always generate a new key pair if the private key doesn't exist (#597) This commit updates `KeypairBackend._should_generate()` to first check if the original private key named by the `path` argument exists, and return True if it does not. This brings the code in line with the documentation, which says that a new key will always be generated if the key file doesn't already exist. As an alternative to the approach implemented here, I also considered only modifying the condition in the `fail` branch of the if statement, but I thought that would not map as cleanly to the behavior specified in the documentation, so doing it the way I did should make it easier to check that the code is doing the right thing just by looking at it. I also considered doing something to make the logic more similar to `PrivateKeyBackend.needs_regeneration()` (the openssl version of this functionality), because the two are supposed to be acting the same way, but I thought that'd be going beyond the scope of just fixing this bug. If it'd be useful to make both methods work the same way, someone can refactor the code in a future commit. * Test different regenerate values with nonexistent keys This commit changes the test task that generates new keys to use each of the different values for the `regenerate` argument, which will ensure that the module is capable of generating a key when no previous key exists regardless of the value of `regenerate`. Previously, the task would always run with the `partial_idempotence` value, and that obscured a bug (#597) that would occur when it was set to `fail`. The bug was fixed in the previous commit. --- .../fragments/598-openssh_keypair-generate-new-key.yml | 2 ++ plugins/module_utils/openssh/backends/keypair_backend.py | 8 +++++--- .../targets/openssh_keypair/tests/regenerate.yml | 1 + 3 files changed, 8 insertions(+), 3 deletions(-) create mode 100644 changelogs/fragments/598-openssh_keypair-generate-new-key.yml diff --git a/changelogs/fragments/598-openssh_keypair-generate-new-key.yml b/changelogs/fragments/598-openssh_keypair-generate-new-key.yml new file mode 100644 index 00000000..723e8704 --- /dev/null +++ b/changelogs/fragments/598-openssh_keypair-generate-new-key.yml @@ -0,0 +1,2 @@ +bugfixes: + - "openssh_keypair - always generate a new key pair if the private key does not exist. Previously, the module would fail when ``regenerate=fail`` without an existing key, contradicting the documentation (https://github.com/ansible-collections/community.crypto/pull/598)." diff --git a/plugins/module_utils/openssh/backends/keypair_backend.py b/plugins/module_utils/openssh/backends/keypair_backend.py index 8cc39c6f..e3bc3535 100644 --- a/plugins/module_utils/openssh/backends/keypair_backend.py +++ b/plugins/module_utils/openssh/backends/keypair_backend.py @@ -161,8 +161,10 @@ class KeypairBackend(OpensshModule): pass def _should_generate(self): - if self.regenerate == 'never': - return self.original_private_key is None + if self.original_private_key is None: + return True + elif self.regenerate == 'never': + return False elif self.regenerate == 'fail': if not self._private_key_valid(): self.module.fail_json( @@ -170,7 +172,7 @@ class KeypairBackend(OpensshModule): "To force regeneration, call the module with `generate` set to " + "`partial_idempotence`, `full_idempotence` or `always`, or with `force=true`." ) - return self.original_private_key is None + return False elif self.regenerate in ('partial_idempotence', 'full_idempotence'): return not self._private_key_valid() else: diff --git a/tests/integration/targets/openssh_keypair/tests/regenerate.yml b/tests/integration/targets/openssh_keypair/tests/regenerate.yml index 343df4c7..d1009604 100644 --- a/tests/integration/targets/openssh_keypair/tests/regenerate.yml +++ b/tests/integration/targets/openssh_keypair/tests/regenerate.yml @@ -28,6 +28,7 @@ type: rsa size: 1024 backend: "{{ backend }}" + regenerate: "{{ item }}" loop: "{{ regenerate_values }}" - name: "({{ backend }}) Regenerate - setup password protected keys" command: 'ssh-keygen -f {{ remote_tmp_dir }}/regenerate-b-{{ item }} -N {{ passphrase }}'