From 27e414200f55fc17dc824e7a46e3ffd9d48acd7e Mon Sep 17 00:00:00 2001 From: Maxim Babushkin Date: Sun, 11 Aug 2019 01:57:35 +0300 Subject: [PATCH] openssh_keypair - Add public key and key comment validation (#57993) - Split the key validation to separate private and public. - In case public key does not exist, recreate it. - Validate comment of the key. - In case comment changed, update the private and public keys. --- ...-public-key-and-key-comment-validation.yml | 2 + lib/ansible/modules/crypto/openssh_keypair.py | 73 +++++++++++++++++-- .../targets/openssh_keypair/tasks/main.yml | 12 +++ .../openssh_keypair/tests/validate.yml | 8 +- 4 files changed, 88 insertions(+), 7 deletions(-) create mode 100644 changelogs/fragments/57993-openssh_keypair-add-public-key-and-key-comment-validation.yml diff --git a/changelogs/fragments/57993-openssh_keypair-add-public-key-and-key-comment-validation.yml b/changelogs/fragments/57993-openssh_keypair-add-public-key-and-key-comment-validation.yml new file mode 100644 index 00000000000..9e52f39553a --- /dev/null +++ b/changelogs/fragments/57993-openssh_keypair-add-public-key-and-key-comment-validation.yml @@ -0,0 +1,2 @@ +bugfixes: + - openssh_keypair - add public key and key comment validation on change diff --git a/lib/ansible/modules/crypto/openssh_keypair.py b/lib/ansible/modules/crypto/openssh_keypair.py index 61c8513eb13..152c087f504 100644 --- a/lib/ansible/modules/crypto/openssh_keypair.py +++ b/lib/ansible/modules/crypto/openssh_keypair.py @@ -165,7 +165,7 @@ class Keypair(object): def generate(self, module): # generate a keypair - if not self.isValid(module, perms_required=False) or self.force: + if not self.isPrivateKeyValid(module, perms_required=False) or self.force: args = [ module.get_bin_path('ssh-keygen', True), '-q', @@ -196,11 +196,36 @@ class Keypair(object): self.remove() module.fail_json(msg="%s" % to_native(e)) + elif not self.isPublicKeyValid(module): + pubkey = module.run_command([module.get_bin_path('ssh-keygen', True), '-yf', self.path]) + pubkey = pubkey[1].strip('\n') + try: + self.changed = True + with open(self.path + ".pub", "w") as pubkey_f: + pubkey_f.write(pubkey + '\n') + os.chmod(self.path + ".pub", stat.S_IWUSR + stat.S_IRUSR + stat.S_IRGRP + stat.S_IROTH) + except IOError: + module.fail_json( + msg='The public key is missing or does not match the private key. ' + 'Unable to regenerate the public key.') + self.public_key = pubkey + + if self.comment: + try: + if os.path.exists(self.path) and not os.access(self.path, os.W_OK): + os.chmod(self.path, stat.S_IWUSR + stat.S_IRUSR) + args = [module.get_bin_path('ssh-keygen', True), + '-q', '-o', '-c', '-C', self.comment, '-f', self.path] + module.run_command(args) + except IOError: + module.fail_json( + msg='Unable to update the comment for the public key.') + file_args = module.load_file_common_arguments(module.params) if module.set_fs_attributes_if_different(file_args, False): self.changed = True - def isValid(self, module, perms_required=True): + def isPrivateKeyValid(self, module, perms_required=True): # check if the key is correct def _check_state(): @@ -215,8 +240,6 @@ class Keypair(object): return False fingerprint = proc[1].split() - pubkey = module.run_command([module.get_bin_path('ssh-keygen', True), '-yf', self.path]) - pubkey = pubkey[1].strip('\n') keysize = int(fingerprint[0]) keytype = fingerprint[-1][1:-1].lower() else: @@ -233,13 +256,51 @@ class Keypair(object): return self.size == keysize self.fingerprint = fingerprint - self.public_key = pubkey if not perms_required: return _check_state() and _check_type() and _check_size() return _check_state() and _check_perms(module) and _check_type() and _check_size() + def isPublicKeyValid(self, module): + + def _get_pubkey_content(): + if os.path.exists(self.path + ".pub"): + with open(self.path + ".pub", "r") as pubkey_f: + present_pubkey = pubkey_f.read().strip(' \n') + return present_pubkey + else: + return False + + def _parse_pubkey(): + pubkey_content = _get_pubkey_content() + if pubkey_content: + parts = pubkey_content.split(' ', 2) + return parts[0], parts[1], '' if len(parts) <= 2 else parts[2] + return False + + def _pubkey_valid(pubkey): + if pubkey_parts: + current_pubkey = ' '.join([pubkey_parts[0], pubkey_parts[1]]) + return current_pubkey == pubkey + return False + + def _comment_valid(): + if pubkey_parts: + return pubkey_parts[2] == self.comment + return False + + pubkey = module.run_command([module.get_bin_path('ssh-keygen', True), '-yf', self.path]) + pubkey = pubkey[1].strip('\n') + pubkey_parts = _parse_pubkey() + if _pubkey_valid(pubkey): + self.public_key = pubkey + + if not self.comment: + return _pubkey_valid(pubkey) + + return _pubkey_valid(pubkey) and _comment_valid() + def dump(self): # return result as a dict @@ -309,7 +370,7 @@ def main(): if module.check_mode: result = keypair.dump() - result['changed'] = module.params['force'] or not keypair.isValid(module) + result['changed'] = module.params['force'] or not keypair.isPrivateKeyValid(module) or not keypair.isPublicKeyValid(module) module.exit_json(**result) try: diff --git a/test/integration/targets/openssh_keypair/tasks/main.yml b/test/integration/targets/openssh_keypair/tasks/main.yml index 25116c63a94..0ff369787d3 100644 --- a/test/integration/targets/openssh_keypair/tasks/main.yml +++ b/test/integration/targets/openssh_keypair/tasks/main.yml @@ -67,4 +67,16 @@ force: yes register: output_read_only +- name: Generate privatekey7 - standard with comment + openssh_keypair: + path: '{{ output_dir }}/privatekey7' + comment: 'test@privatekey7' + register: privatekey7_result + +- name: Modify privatekey7 comment + openssh_keypair: + path: '{{ output_dir }}/privatekey7' + comment: 'test_modified@privatekey7' + register: privatekey7_modified_result + - import_tasks: ../tests/validate.yml diff --git a/test/integration/targets/openssh_keypair/tests/validate.yml b/test/integration/targets/openssh_keypair/tests/validate.yml index 266665a4548..93899e80171 100644 --- a/test/integration/targets/openssh_keypair/tests/validate.yml +++ b/test/integration/targets/openssh_keypair/tests/validate.yml @@ -90,4 +90,10 @@ - name: Verify that read-only key will be regenerated assert: that: - - output_read_only is changed \ No newline at end of file + - output_read_only is changed + + +- name: Validate privatekey7 (assert - Public key remains the same after comment change) + assert: + that: + - privatekey7_result.public_key == privatekey7_modified_result.public_key