Fixed hash_host option in known_hosts module. Fixes #44284

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)
 
+    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 key[-1] != '\n':
+    if key and not key.endswith('\n'):
         key += '\n'
 
-    if key is None and state != "absent":
-        module.fail_json(msg="No key specified when adding a host")
-
     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

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