diff --git a/changelogs/fragments/646-openssh-rc.yml b/changelogs/fragments/646-openssh-rc.yml new file mode 100644 index 000000000..5ca84e391 --- /dev/null +++ b/changelogs/fragments/646-openssh-rc.yml @@ -0,0 +1,5 @@ +bugfixes: + - "openssh_cert, openssh_keypair - the modules ignored return codes of ``ssh`` and ``ssh-keygen`` in some cases (https://github.com/ansible-collections/community.crypto/issues/645, https://github.com/ansible-collections/community.crypto/pull/646)." + - "openssh_keypair - fix comment updating for OpenSSH before 6.5 (https://github.com/ansible-collections/community.crypto/pull/646)." +minor_changes: + - "openssh_keypair - fail when comment cannot be updated (https://github.com/ansible-collections/community.crypto/pull/646)." diff --git a/plugins/module_utils/openssh/backends/common.py b/plugins/module_utils/openssh/backends/common.py index 6e274a6de..46ee1c913 100644 --- a/plugins/module_utils/openssh/backends/common.py +++ b/plugins/module_utils/openssh/backends/common.py @@ -127,7 +127,7 @@ def _get_ssh_version(self): ssh_bin = self.module.get_bin_path('ssh') if not ssh_bin: return "" - return parse_openssh_version(self.module.run_command([ssh_bin, '-V', '-q'])[2].strip()) + return parse_openssh_version(self.module.run_command([ssh_bin, '-V', '-q'], check_rc=True)[2].strip()) @_restore_all_on_failure def _safe_secure_move(self, sources_and_destinations): @@ -208,14 +208,18 @@ def get_matching_public_key(self, private_key_path, **kwargs): def get_private_key(self, private_key_path, **kwargs): return self._run_command([self._bin_path, '-l', '-f', private_key_path], **kwargs) - def update_comment(self, private_key_path, comment, **kwargs): + def update_comment(self, private_key_path, comment, force_new_format=True, **kwargs): if os.path.exists(private_key_path) and not os.access(private_key_path, os.W_OK): try: os.chmod(private_key_path, stat.S_IWUSR + stat.S_IRUSR) except (IOError, OSError) as e: raise e("The private key at %s is not writeable preventing a comment update" % private_key_path) - return self._run_command([self._bin_path, '-q', '-o', '-c', '-C', comment, '-f', private_key_path], **kwargs) + command = [self._bin_path, '-q'] + if force_new_format: + command.append('-o') + command.extend(['-c', '-C', comment, '-f', private_key_path]) + return self._run_command(command, **kwargs) class PrivateKey(object): diff --git a/plugins/module_utils/openssh/backends/keypair_backend.py b/plugins/module_utils/openssh/backends/keypair_backend.py index e3bc3535b..5f54903ef 100644 --- a/plugins/module_utils/openssh/backends/keypair_backend.py +++ b/plugins/module_utils/openssh/backends/keypair_backend.py @@ -323,23 +323,27 @@ def __init__(self, module): self.ssh_keygen = KeygenCommand(self.module) def _generate_keypair(self, private_key_path): - self.ssh_keygen.generate_keypair(private_key_path, self.size, self.type, self.comment) + self.ssh_keygen.generate_keypair(private_key_path, self.size, self.type, self.comment, check_rc=True) def _get_private_key(self): - private_key_content = self.ssh_keygen.get_private_key(self.private_key_path)[1] + rc, private_key_content, err = self.ssh_keygen.get_private_key(self.private_key_path, check_rc=False) + if rc != 0: + raise ValueError(err) return PrivateKey.from_string(private_key_content) def _get_public_key(self): - public_key_content = self.ssh_keygen.get_matching_public_key(self.private_key_path)[1] + public_key_content = self.ssh_keygen.get_matching_public_key(self.private_key_path, check_rc=True)[1] return PublicKey.from_string(public_key_content) def _private_key_readable(self): - rc, stdout, stderr = self.ssh_keygen.get_matching_public_key(self.private_key_path) + rc, stdout, stderr = self.ssh_keygen.get_matching_public_key(self.private_key_path, check_rc=False) return not (rc == 255 or any_in(stderr, 'is not a public key file', 'incorrect passphrase', 'load failed')) def _update_comment(self): try: - self.ssh_keygen.update_comment(self.private_key_path, self.comment) + ssh_version = self._get_ssh_version() or "7.8" + force_new_format = LooseVersion('6.5') <= LooseVersion(ssh_version) < LooseVersion('7.8') + self.ssh_keygen.update_comment(self.private_key_path, self.comment, force_new_format=force_new_format, check_rc=True) except (IOError, OSError) as e: self.module.fail_json(msg=to_native(e)) diff --git a/plugins/modules/openssh_cert.py b/plugins/modules/openssh_cert.py index 7a0194258..b968b14c2 100644 --- a/plugins/modules/openssh_cert.py +++ b/plugins/modules/openssh_cert.py @@ -497,7 +497,10 @@ def _result(self): if self.state != 'present': return {} - certificate_info = self.ssh_keygen.get_certificate_info(self.path)[1] + certificate_info = self.ssh_keygen.get_certificate_info( + self.path, + check_rc=self.state == 'present' and not self.module.check_mode, + )[1] return { 'type': self.type, diff --git a/tests/integration/targets/openssh_keypair/tests/options.yml b/tests/integration/targets/openssh_keypair/tests/options.yml index fdabd7614..0d324939c 100644 --- a/tests/integration/targets/openssh_keypair/tests/options.yml +++ b/tests/integration/targets/openssh_keypair/tests/options.yml @@ -100,8 +100,10 @@ comment: "test_modified@comment" backend: "{{ backend }}" register: modified_comment_output + ignore_errors: true - name: "({{ backend }}) Assert comment preserved public key - comment" + when: modified_comment_output is succeeded assert: that: - comment_output.public_key == modified_comment_output.public_key @@ -111,9 +113,17 @@ assert: that: - modified_comment_output.comment == 'test_modified@comment' + - modified_comment_output is succeeded # Support for updating comments for key types other than rsa1 was added in OpenSSH 7.2 when: not (backend == 'opensshbin' and openssh_version is version('7.2', '<')) +- name: "({{ backend }}) Assert comment not changed - comment" + assert: + that: + - modified_comment_output is failed + # Support for updating comments for key types other than rsa1 was added in OpenSSH 7.2 + when: backend == 'opensshbin' and openssh_version is version('7.2', '<') + - name: "({{ backend }}) Remove key - comment" openssh_keypair: path: "{{ remote_tmp_dir }}/comment" diff --git a/tests/integration/targets/openssh_keypair/tests/regenerate.yml b/tests/integration/targets/openssh_keypair/tests/regenerate.yml index d10096044..f9e2f43b3 100644 --- a/tests/integration/targets/openssh_keypair/tests/regenerate.yml +++ b/tests/integration/targets/openssh_keypair/tests/regenerate.yml @@ -329,22 +329,25 @@ that: - result is changed -- name: "({{ backend }}) Regenerate - adjust comment" - openssh_keypair: - path: '{{ remote_tmp_dir }}/regenerate-a-{{ item }}' - type: dsa - size: 1024 - comment: test comment - regenerate: '{{ item }}' - backend: "{{ backend }}" - loop: "{{ regenerate_values }}" - register: result -- assert: - that: - - result is changed - # for all values but 'always', the key should not be regenerated. - # verify this by comparing fingerprints: - - result.results[0].fingerprint == result.results[1].fingerprint - - result.results[0].fingerprint == result.results[2].fingerprint - - result.results[0].fingerprint == result.results[3].fingerprint - - result.results[0].fingerprint != result.results[4].fingerprint +# Support for updating comments for key types other than rsa1 was added in OpenSSH 7.2 +- when: not (backend == 'opensshbin' and openssh_version is version('7.2', '<')) + block: + - name: "({{ backend }}) Regenerate - adjust comment" + openssh_keypair: + path: '{{ remote_tmp_dir }}/regenerate-a-{{ item }}' + type: dsa + size: 1024 + comment: test comment + regenerate: '{{ item }}' + backend: "{{ backend }}" + loop: "{{ regenerate_values }}" + register: result + - assert: + that: + - result is changed + # for all values but 'always', the key should not be regenerated. + # verify this by comparing fingerprints: + - result.results[0].fingerprint == result.results[1].fingerprint + - result.results[0].fingerprint == result.results[2].fingerprint + - result.results[0].fingerprint == result.results[3].fingerprint + - result.results[0].fingerprint != result.results[4].fingerprint