Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Known_hosts file loses content when adding new host #1209

Open
pikeas opened this issue Sep 19, 2024 · 0 comments
Open

Known_hosts file loses content when adding new host #1209

pikeas opened this issue Sep 19, 2024 · 0 comments

Comments

@pikeas
Copy link

pikeas commented Sep 19, 2024

Describe the bug

Pyinfra deletes lines from ~/.ssh/known_hosts.

To Reproduce

https://github.com/pyinfra-dev/pyinfra/blob/3.x/pyinfra/connectors/sshuserclient/client.py#L43-L49:

with HOST_KEYS_LOCK:
    host_keys = client.get_host_keys()
    host_keys.add(hostname, key.get_name(), key)
    # The paramiko client saves host keys incorrectly whereas the host keys object does
    # this correctly, so use that with the client filename variable.
    # See: https://github.com/paramiko/paramiko/pull/1989
    host_keys.save(client._host_keys_filename)

This happens because of the way Paramiko parses the file - it throws away comment lines, newlines, and lines it doesn't understand.

https://github.com/paramiko/paramiko/blob/main/paramiko/hostkeys.py#L89-L97:

with open(filename, "r") as f:
    for lineno, line in enumerate(f, 1):
        line = line.strip()
        if (len(line) == 0) or (line[0] == "#"):
            continue
        try:
            entry = HostKeyEntry.from_line(line, lineno)
        except SSHException:
            continue

So, when Pyinfra calls hosts_keys.save(...), this doesn't just add a new entry, it actually overwrites the file with the contents as parsed by Paramiko. This is unexpected - I've been fighting disappearing contents in known_hosts for years and had no idea it was caused by PyInfra.

Expected behavior

Pyinfra should either never modify known_hosts, or only add new hosts.

I understand the intent here, to make running PyInfra as convenient as possible. But IMO this behavior is a foot-gun for infrastructure maintainers and should be changed. It's frustrating to lose formatting, painful to lose comments, and breaks general SSH usage when @cert-authority (not yet support by Paramiko) silently disappears.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant