diff --git a/lib/ansible/modules/system/known_hosts.py b/lib/ansible/modules/system/known_hosts.py index aff17b9e078..57f32e246c5 100644 --- a/lib/ansible/modules/system/known_hosts.py +++ b/lib/ansible/modules/system/known_hosts.py @@ -74,13 +74,16 @@ EXAMPLES = ''' # hash_host = yes|no (default: no) hash the hostname in the known_hosts file # state = absent|present (default: present) +import base64 +import hashlib +import hmac import os import os.path import tempfile import errno import re -from ansible.module_utils._text import to_native +from ansible.module_utils._text import to_bytes, to_native from ansible.module_utils.basic import AnsibleModule @@ -91,23 +94,25 @@ def enforce_state(module, params): host = params["name"].lower() key = params.get("key", None) - port = params.get("port", None) path = params.get("path") hash_host = params.get("hash_host") state = params.get("state") # Find the ssh-keygen binary sshkeygen = module.get_bin_path("ssh-keygen", True) - # Trailing newline in files gets lost, so re-add if necessary - if key and key[-1] != '\n': - key += '\n' - - if key is None and state != "absent": + if not key and state != "absent": module.fail_json(msg="No key specified when adding a host") + if key and hash_host: + key = hash_host_key(host, key) + + # Trailing newline in files gets lost, so re-add if necessary + if key and not key.endswith('\n'): + key += '\n' + sanity_check(module, host, key, sshkeygen) - found, replace_or_add, found_line, key = search_for_host_key(module, host, key, hash_host, path, sshkeygen) + found, replace_or_add, found_line = search_for_host_key(module, host, key, path, sshkeygen) params['diff'] = compute_diff(path, found_line, replace_or_add, state, key) @@ -123,7 +128,7 @@ def enforce_state(module, params): # Now do the work. # Only remove whole host if found and no key provided - if found and key is None and state == "absent": + if found and not key and state == "absent": module.run_command([sshkeygen, '-R', host, '-f', path], check_rc=True) params['changed'] = True @@ -137,24 +142,19 @@ def enforce_state(module, params): else: module.fail_json(msg="Failed to read %s: %s" % (path, str(e))) try: - outf = tempfile.NamedTemporaryFile(mode='w+', dir=os.path.dirname(path)) - if inf is not None: - for line_number, line in enumerate(inf): - if found_line == (line_number + 1) and (replace_or_add or state == 'absent'): - continue # skip this line to replace its key - outf.write(line) - inf.close() - if state == 'present': - outf.write(key) - outf.flush() - module.atomic_move(outf.name, path) + with tempfile.NamedTemporaryFile(mode='w+', dir=os.path.dirname(path), delete=False) as outf: + if inf is not None: + for line_number, line in enumerate(inf): + if found_line == (line_number + 1) and (replace_or_add or state == 'absent'): + continue # skip this line to replace its key + outf.write(line) + inf.close() + if state == 'present': + outf.write(key) except (IOError, OSError) as e: module.fail_json(msg="Failed to write to file %s: %s" % (path, to_native(e))) - - try: - outf.close() - except: - pass + else: + module.atomic_move(outf.name, path) params['changed'] = True @@ -170,7 +170,7 @@ def sanity_check(module, host, key, sshkeygen): sshkeygen is the path to ssh-keygen, found earlier with get_bin_path ''' # If no key supplied, we're doing a removal, and have nothing to check here. - if key is None: + if not key: return # Rather than parsing the key ourselves, get ssh-keygen to do it # (this is essential for hashed keys, but otherwise useful, as the @@ -183,26 +183,22 @@ def sanity_check(module, host, key, sshkeygen): module.fail_json(msg="Comma separated list of names is not supported. " "Please pass a single name to lookup in the known_hosts file.") - try: - outf = tempfile.NamedTemporaryFile(mode='w+') - outf.write(key) - outf.flush() - except IOError as e: - module.fail_json(msg="Failed to write to temporary file %s: %s" % + with tempfile.NamedTemporaryFile(mode='w+') as outf: + try: + outf.write(key) + outf.flush() + except IOError as e: + module.fail_json(msg="Failed to write to temporary file %s: %s" % (outf.name, to_native(e))) - sshkeygen_command = [sshkeygen, '-F', host, '-f', outf.name] - rc, stdout, stderr = module.run_command(sshkeygen_command) - try: - outf.close() - except: - pass + sshkeygen_command = [sshkeygen, '-F', host, '-f', outf.name] + rc, stdout, stderr = module.run_command(sshkeygen_command) if stdout == '': # host not found module.fail_json(msg="Host parameter does not match hashed host field in supplied key") -def search_for_host_key(module, host, key, hash_host, path, sshkeygen): +def search_for_host_key(module, host, key, path, sshkeygen): '''search_for_host_key(module,host,key,path,sshkeygen) -> (found,replace_or_add,found_line) Looks up host and keytype in the known_hosts file path; if it's there, looks to see @@ -214,7 +210,7 @@ def search_for_host_key(module, host, key, hash_host, path, sshkeygen): sshkeygen is the path to ssh-keygen, found earlier with get_bin_path ''' if os.path.exists(path) is False: - return False, False, None, key + return False, False, None sshkeygen_command = [sshkeygen, '-F', host, '-f', path] @@ -222,23 +218,17 @@ def search_for_host_key(module, host, key, hash_host, path, sshkeygen): # 1 if no host is found, whereas previously it returned 0 rc, stdout, stderr = module.run_command(sshkeygen_command, check_rc=False) if stdout == '' and stderr == '' and (rc == 0 or rc == 1): - return False, False, None, key # host not found, no other errors + return False, False, None # host not found, no other errors if rc != 0: # something went wrong module.fail_json(msg="ssh-keygen failed (rc=%d, stdout='%s',stderr='%s')" % (rc, stdout, stderr)) # If user supplied no key, we don't want to try and replace anything with it - if key is None: - return True, False, None, key + if not key: + return True, False, None lines = stdout.split('\n') new_key = normalize_known_hosts_key(key) - sshkeygen_command.insert(1, '-H') - rc, stdout, stderr = module.run_command(sshkeygen_command, check_rc=False) - if rc not in (0, 1) or stderr != '': # something went wrong - module.fail_json(msg="ssh-keygen failed to hash host (rc=%d, stdout='%s',stderr='%s')" % (rc, stdout, stderr)) - hashed_lines = stdout.split('\n') - for lnum, l in enumerate(lines): if l == '': continue @@ -251,19 +241,25 @@ def search_for_host_key(module, host, key, hash_host, path, sshkeygen): module.fail_json(msg="failed to parse output of ssh-keygen for line number: '%s'" % l) else: found_key = normalize_known_hosts_key(l) - if hash_host is True: - if found_key['host'][:3] == '|1|': - new_key['host'] = found_key['host'] - else: - hashed_host = normalize_known_hosts_key(hashed_lines[lnum]) - found_key['host'] = hashed_host['host'] - key = key.replace(host, found_key['host']) + if new_key['host'][:3] == '|1|' and found_key['host'][:3] == '|1|': # do not change host hash if already hashed + new_key['host'] = found_key['host'] if new_key == found_key: # found a match - return True, False, found_line, key # found exactly the same key, don't replace + return True, False, found_line # found exactly the same key, don't replace elif new_key['type'] == found_key['type']: # found a different key for the same key type - return True, True, found_line, key + return True, True, found_line + # No match found, return found and replace, but no line - return True, True, None, key + return True, True, None + + +def hash_host_key(host, key): + hmac_key = os.urandom(20) + hashed_host = hmac.new(hmac_key, to_bytes(host), hashlib.sha1).digest() + parts = key.strip().split() + # @ indicates the optional marker field used for @cert-authority or @revoked + i = 1 if parts[0][0] == '@' else 0 + parts[i] = '|1|%s|%s' % (to_native(base64.b64encode(hmac_key)), to_native(base64.b64encode(hashed_host))) + return ' '.join(parts) def normalize_known_hosts_key(key): @@ -275,7 +271,7 @@ def normalize_known_hosts_key(key): from the end (like the username@host tag usually present in hostkeys, but absent in known_hosts files) ''' - k = key.strip() # trim trailing newline + key = key.strip() # trim trailing newline k = key.split() d = dict() # The optional "marker" field, used for @cert-authority or @revoked diff --git a/test/integration/targets/known_hosts/tasks/main.yml b/test/integration/targets/known_hosts/tasks/main.yml index cac0f5580bc..4ea91c3542d 100644 --- a/test/integration/targets/known_hosts/tasks/main.yml +++ b/test/integration/targets/known_hosts/tasks/main.yml @@ -19,7 +19,7 @@ - name: copy an existing file in place copy: src: existing_known_hosts - dest: "{{ output_dir | expanduser }}/known_hosts" + dest: "{{ output_dir }}/known_hosts" # test addition @@ -29,13 +29,13 @@ name: example.org key: "{{ example_org_rsa_key }}" state: present - path: "{{output_dir|expanduser}}/known_hosts" + path: "{{output_dir}}/known_hosts" register: diff - name: assert that the diff looks as expected (the key was added at the end) assert: that: - - 'diff.changed' + - 'diff is changed' - 'diff.diff.before_header == diff.diff.after_header == output_dir|expanduser + "/known_hosts"' - 'diff.diff.after.splitlines()[:-1] == diff.diff.before.splitlines()' - 'diff.diff.after.splitlines()[-1] == example_org_rsa_key.strip()' @@ -45,17 +45,17 @@ name: example.org key: "{{ example_org_rsa_key }}" state: present - path: "{{output_dir|expanduser}}/known_hosts" + path: "{{output_dir}}/known_hosts" register: result - name: get the file content - shell: cat "{{output_dir|expanduser}}/known_hosts" + command: "cat {{output_dir}}/known_hosts" register: known_hosts - name: assert that the key was added and ordering preserved assert: that: - - 'result.changed' + - 'result is changed' - 'known_hosts.stdout_lines[0].startswith("example.com")' - 'known_hosts.stdout_lines[4].startswith("# example.net")' - 'known_hosts.stdout_lines[-1].strip() == example_org_rsa_key.strip()' @@ -68,13 +68,13 @@ name: example.org key: "{{ example_org_rsa_key }}" state: present - path: "{{output_dir|expanduser}}/known_hosts" + path: "{{output_dir}}/known_hosts" register: check - name: assert that no changes were expected assert: that: - - 'not check.changed' + - 'check is not changed' - 'check.diff.before == check.diff.after' - name: add the same host @@ -82,17 +82,17 @@ name: example.org key: "{{ example_org_rsa_key }}" state: present - path: "{{output_dir|expanduser}}/known_hosts" + path: "{{output_dir}}/known_hosts" register: result - name: get the file content - shell: cat "{{output_dir|expanduser}}/known_hosts" + command: "cat {{output_dir}}/known_hosts" register: known_hosts_v2 - name: assert that no changes happened assert: that: - - 'not result.changed' + - 'result is not changed' - 'result.diff.before == result.diff.after' - 'known_hosts.stdout == known_hosts_v2.stdout' @@ -104,7 +104,7 @@ name: example.org key: "{{ example_org_rsa_key }}" state: absent - path: "{{output_dir|expanduser}}/known_hosts" + path: "{{output_dir}}/known_hosts" register: diff - name: assert that the diff looks as expected (the key was removed) @@ -119,17 +119,17 @@ name: example.org key: "{{ example_org_rsa_key }}" state: absent - path: "{{output_dir|expanduser}}/known_hosts" + path: "{{output_dir}}/known_hosts" register: result - name: get the file content - shell: cat "{{output_dir|expanduser}}/known_hosts" + command: "cat {{output_dir}}/known_hosts" register: known_hosts_v3 - name: assert that the key was removed and ordering preserved assert: that: - - 'result.changed' + - 'result is changed' - '"example.org" not in known_hosts_v3.stdout' - 'known_hosts_v3.stdout_lines[0].startswith("example.com")' - 'known_hosts_v3.stdout_lines[-1].startswith("# example.net")' @@ -142,13 +142,13 @@ name: example.org key: "{{ example_org_rsa_key }}" state: absent - path: "{{output_dir|expanduser}}/known_hosts" + path: "{{output_dir}}/known_hosts" register: check - name: assert that no changes were expected assert: that: - - 'not check.changed' + - 'check is not changed' - 'check.diff.before == check.diff.after' - name: remove the same host @@ -156,27 +156,203 @@ name: example.org key: "{{ example_org_rsa_key }}" state: absent - path: "{{output_dir|expanduser}}/known_hosts" + path: "{{output_dir}}/known_hosts" register: result - name: get the file content - shell: cat "{{output_dir|expanduser}}/known_hosts" + command: "cat {{output_dir}}/known_hosts" register: known_hosts_v4 - name: assert that no changes happened assert: that: - - 'not result.changed' + - 'result is not changed' - 'result.diff.before == result.diff.after' - 'known_hosts_v3.stdout == known_hosts_v4.stdout' +# test addition as hashed_host + +- name: add a new hashed host + known_hosts: + name: example.org + key: "{{ example_org_rsa_key }}" + state: present + path: "{{output_dir}}/known_hosts" + hash_host: yes + register: result + +- name: get the file content + command: "cat {{output_dir}}/known_hosts" + register: known_hosts_v5 + +- name: assert that the key was added and ordering preserved + assert: + that: + - 'result is changed' + - 'known_hosts_v5.stdout_lines[0].startswith("example.com")' + - 'known_hosts_v5.stdout_lines[4].startswith("# example.net")' + - 'known_hosts_v5.stdout_lines[-1].strip().startswith("|1|")' + - 'known_hosts_v5.stdout_lines[-1].strip().endswith(example_org_rsa_key.strip().split()[-1])' + +# test idempotence of hashed addition + +- name: add the same host hashed + known_hosts: + name: example.org + key: "{{ example_org_rsa_key }}" + state: present + path: "{{output_dir}}/known_hosts" + hash_host: yes + register: result + +- name: get the file content + command: "cat {{output_dir}}/known_hosts" + register: known_hosts_v6 + +- name: assert that no changes happened + assert: + that: + - 'result is not changed' + - 'result.diff.before == result.diff.after' + - 'known_hosts_v5.stdout == known_hosts_v6.stdout' + +# test hashed removal + +- name: remove the hashed host + known_hosts: + name: example.org + key: "{{ example_org_rsa_key }}" + state: absent + path: "{{output_dir}}/known_hosts" + register: result + +- name: get the file content + command: "cat {{output_dir}}/known_hosts" + register: known_hosts_v7 + +- name: assert that the key was removed and ordering preserved + assert: + that: + - 'result is changed' + - 'example_org_rsa_key.strip().split()[-1] not in known_hosts_v7.stdout' + - 'known_hosts_v7.stdout_lines[0].startswith("example.com")' + - 'known_hosts_v7.stdout_lines[-1].startswith("# example.net")' + +# test idempotence of removal + +- name: remove the same hashed host + known_hosts: + name: example.org + key: "{{ example_org_rsa_key }}" + state: absent + path: "{{output_dir}}/known_hosts" + register: result + +- name: get the file content + command: "cat {{output_dir}}/known_hosts" + register: known_hosts_v8 + +- name: assert that no changes happened + assert: + that: + - 'result is not changed' + - 'result.diff.before == result.diff.after' + - 'known_hosts_v7.stdout == known_hosts_v8.stdout' + +# test roundtrip plaintext => hashed => plaintext +# The assertions are rather relaxed, because most of this hash been tested previously + +- name: add a new host + known_hosts: + name: example.org + key: "{{ example_org_rsa_key }}" + state: present + path: "{{output_dir}}/known_hosts" + +- name: get the file content + command: "cat {{output_dir}}/known_hosts" + register: known_hosts_v8 + +- name: assert the plaintext host is there + assert: + that: + - 'known_hosts_v8.stdout_lines[-1].strip() == example_org_rsa_key.strip()' + +- name: update the host to hashed mode + known_hosts: + name: example.org + key: "{{ example_org_rsa_key }}" + state: present + path: "{{output_dir}}/known_hosts" + hash_host: true + +- name: get the file content + command: "cat {{output_dir}}/known_hosts" + register: known_hosts_v9 + +- name: assert the hashed host is there + assert: + that: + - 'known_hosts_v9.stdout_lines[-1].strip().startswith("|1|")' + - 'known_hosts_v9.stdout_lines[-1].strip().endswith(example_org_rsa_key.strip().split()[-1])' + +- name: downgrade the host to plaintext mode + known_hosts: + name: example.org + key: "{{ example_org_rsa_key }}" + state: present + path: "{{output_dir}}/known_hosts" + +- name: get the file content + command: "cat {{output_dir}}/known_hosts" + register: known_hosts_v10 + +- name: assert the plaintext host is there + assert: + that: + - 'known_hosts_v10.stdout_lines[5].strip() == example_org_rsa_key.strip()' + +# ... and remove the host again for the next test + +- name: copy an existing file in place + copy: + src: existing_known_hosts + dest: "{{ output_dir }}/known_hosts" + +# Test key changes + +- name: add a hashed host + known_hosts: + name: example.org + key: "{{ example_org_rsa_key }}" + state: present + path: "{{output_dir}}/known_hosts" + hash_host: true + +- name: change the key of a hashed host + known_hosts: + name: example.org + key: "{{ example_org_rsa_key.strip()[:-7] + 'RANDOM=' }}" + state: present + path: "{{output_dir}}/known_hosts" + hash_host: true + +- name: get the file content + command: "cat {{output_dir}}/known_hosts" + register: known_hosts_v11 + +- name: assert the change took place and the key got modified + assert: + that: + - 'known_hosts_v11.stdout_lines[-1].strip().endswith("RANDOM=")' + # test errors - name: Try using a comma separated list of hosts known_hosts: name: example.org,acme.com key: "{{ example_org_rsa_key }}" - path: "{{output_dir|expanduser}}/known_hosts" + path: "{{output_dir}}/known_hosts" ignore_errors: yes register: result @@ -190,7 +366,7 @@ known_hosts: name: example.com key: "{{ example_org_rsa_key }}" - path: "{{output_dir|expanduser}}/known_hosts" + path: "{{output_dir}}/known_hosts" ignore_errors: yes register: result