openssh_keypair handles encrypted key files poorly

[thomwiggers]

SUMMARY

openssh_keypair handles encrypted key files poorly.

ISSUE TYPE

• Bug Report

COMPONENT NAME

openssh_keypair

ANSIBLE VERSION

ansible 2.8.5
  config file = /home/thom/git/dotfiles-ansible/ansible.cfg
  configured module search path = ['/home/thom/.ansible/plugins/modules', '/usr/share/ansible/plugins/modules']
  ansible python module location = /usr/lib/python3.7/site-packages/ansible
  executable location = /usr/bin/ansible
  python version = 3.7.4 (default, Oct  4 2019, 06:57:26) [GCC 9.2.0]

CONFIGURATION

ANSIBLE_NOCOWS(/home/thom/git/dotfiles-ansible/ansible.cfg) = True
DEFAULT_HOST_LIST(/home/thom/git/dotfiles-ansible/ansible.cfg) = ['/home/thom/git/dotfiles-ansible/hosts']
DEFAULT_MANAGED_STR(/home/thom/git/dotfiles-ansible/ansible.cfg) = This file is managed by Ansible.%n
template: {file}
date: %Y-%m-%d %H:%M:%S
user: {uid}
host: {host}

OS / ENVIRONMENT

Arch Linux

OpenSSH_8.1p1, OpenSSL 1.1.1d 10 Sep 2019

STEPS TO REPRODUCE

  - name: Obtain keyfile from this machine                                          
    openssh_keypair:                                                                
      path: "/tmp/sshkey"
      type: ed25519

I'm using ed25519 because it's independent of size; not sure if it also happens with eg. RSA.

0. Generate /tmp/sshkey using ssh-keygen -t ed25519 -f /tmp/sshkey -N password
1. Run the above playbook, observe it marks it as changed.
2. Observe /tmp/sshkey.pub is now empty.
3. Run it again, it's changed again
4. It has now replaced the SSH key and the public key is now correct.

EXPECTED RESULTS

1. Either give me an error about encrypted .ssh files already existing or properly replace it.

[ansibot]

Files identified in the description:

• lib/ansible/modules/crypto/openssh_keypair.py

If these files are inaccurate, please update the component name section of the description or use the !component bot command.

click here for bot help

[ansibot]

cc @MarkusTeufelberger @Shaps @Spredzy @Xyon @felixfontein @lolcube @puiterwijk @resmo
click here for bot help

[felixfontein]

Interestingly, for me both tasks are unchanged on the second run. My OpenSSH version: OpenSSH_8.1p1, OpenSSL 1.1.1d 10 Sep 2019 I guess we should add tests for this to the integration tests, then we'll probably notice for which versions of works and for which it doesn't. :)

[thomwiggers]

hrm, something funky is going on. If I run my example it indeed works as expected. I'm on the same OpenSSH version.

However, if I target my existing ~/.ssh/id_ed25519{,.pub} files, it first blanks out the .ssh/id_ed25519.pub file and somehow breaks the private key file! (ssh-keygen -lf ~/.ssh/id_ed25519 says is not a key file), although diff can't find a difference??? This is all very weird.

[thomwiggers]

...... Wait, I think the difference is that my key is password protected. I think it probably handles that poorly.

[thomwiggers]

Ahh, and is not a valid key file happens if the .pub file is broken, caused by the former.

[thomwiggers]

I've updated my testcase above, I'll also change the title.

[felixfontein]

Here's a pure-ansible reproducer:

---
- hosts: localhost
  tasks:
    - file:
        path: /tmp/sshkey
        state: absent
    - file:
        path: /tmp/sshkey.pub
        state: absent
    - command: ssh-keygen -t ed25519 -f /tmp/sshkey -N password
    - stat:
        path: "{{ item }}"
      loop:
        - /tmp/sshkey
        - /tmp/sshkey.pub
      register: out
    - debug:
        msg: "Key:{{ out.results[0].stat.size }}/{{ out.results[0].stat.mtime }} Pub:{{ out.results[1].stat.size }}/{{ out.results[1].stat.mtime }}"
    - openssh_keypair:                                                                
        path: "/tmp/sshkey"
        type: ed25519
    - stat:
        path: "{{ item }}"
      loop:
        - /tmp/sshkey
        - /tmp/sshkey.pub
      register: out
    - debug:
        msg: "Key:{{ out.results[0].stat.size }}/{{ out.results[0].stat.mtime }} Pub:{{ out.results[1].stat.size }}/{{ out.results[1].stat.mtime }}"
    - openssh_keypair:                                                                
        path: "/tmp/sshkey"
        type: ed25519
    - stat:
        path: "{{ item }}"
      loop:
        - /tmp/sshkey
        - /tmp/sshkey.pub
      register: out
    - debug:
        msg: "Key:{{ out.results[0].stat.size }}/{{ out.results[0].stat.mtime }} Pub:{{ out.results[1].stat.size }}/{{ out.results[1].stat.mtime }}"

In the first openssh_keypair call, https://github.com/ansible/ansible/blob/devel/lib/ansible/modules/crypto/openssh_keypair.py#L307 is executed and returns '' as stdout. Since that's different from the current public key, it will be overwritten. In the second openssh_keypair call, https://github.com/ansible/ansible/blob/devel/lib/ansible/modules/crypto/openssh_keypair.py#L244 will fail since it uses the .pub file. So it will regenerate the key itself.

I guess we need to add a "can we actually read the key itself" test and a "is they key maybe password protected" test.

Anyone wants to (try to) implement that?

[felixfontein]

Already checking return values for the calls above might already improve the situation a lot.

[MaxBab]

Hi @thomwiggers @felixfontein

The fix will check every private key for the password protected state.
If the key is password protected, the task will fail with the message.