saltstack / salt

Software to automate the management and configuration of any infrastructure or application at scale. Get access to the Salt software package repository here:
https://repo.saltproject.io/
Apache License 2.0
14.14k stars 5.47k forks source link

ssh_known_hosts.present port option requires hashed_known_hosts False with older ssh-keyscan #40152

Closed shallot closed 5 years ago

shallot commented 7 years ago

From @hackel on December 10, 2015 22:13

This just started happening after upgrading to 2015.8: I have the following clause in my users pillar:

    ssh_known_hosts:
      slsapp.com:
        port: 1234
        fingerprint: xx:xx:xx
        enc: ssh-dss

This results in:

          ID: users_ssh_known_hosts_username_0
    Function: ssh_known_hosts.present
        Name: slsapp.com
      Result: False
     Comment: argument port can not be used in conjunction with argument hash_known_hosts

hash_known_hosts is not present, so I'm not sure what is going on here. I tried to explicitly set hash_known_hosts to False in case it was the default, but it didn't help.

Copied from original issue: saltstack-formulas/users-formula#103

shallot commented 7 years ago

From @hackel on December 10, 2015 22:22

Adding the deprecated option hash_hostname: False seems to do the trick. This may be a larger issue in the salt ssh_known_hosts module.

shallot commented 7 years ago

From @kbuilds on September 20, 2016 19:50

I am also running into this same issue in salt-minion 2016.3.3

shallot commented 7 years ago

From @dosercz on January 12, 2017 18:53

Same in 2016.11.1 and hack with hash_hostname doesn't work there:(

shallot commented 7 years ago

From @dosercz on January 12, 2017 19:3

But port could be set if hash_known_hosts is set to False

shallot commented 7 years ago

This error seems to have been added in https://github.com/saltstack/salt/commit/c7466e789436adb3c8ede811e18a0d04cfd3366a but there's no explanation for it. Hashed known_hosts files as such do of course have to support arbitrary ports.

I think this is actually documenting an unrelated bug in the underlying ssh-keyscan program used by the recv_known_host() function, cf. https://bugs.debian.org/824010

It should be removed or replaced with a warning telling people they need to upgrade their ssh-keyscan for port to work.

gtmanfred commented 7 years ago

@jahamn since you wrote this, could you provide us information about why the port does not work with hash_known_host (previously hash_hostname)?

It looks like the hash_hostname workaround no longer works because of changes made in this PR https://github.com/saltstack/salt/pull/27726

And that you should be disabling this now?

Thanks, Daniel

shallot commented 7 years ago

@gtmanfred please see the latest message in the thread, posted by me yesterday, it has a specific description of what's wrong with it currently (I traced the original commit more as a historical curiosity)

gtmanfred commented 7 years ago

We would love to have a PR making this change if you wouldn't mind submitting one?

Thanks, Daniel

rsaffi commented 6 years ago

I have just stumbled on the same problem. Did anything happen to this since March 2017?

Versions on my system:

Salt Version:
           Salt: 2017.7.2

Dependency Versions:
           cffi: 1.5.2
       cherrypy: Not Installed
       dateutil: 2.4.2
      docker-py: Not Installed
          gitdb: 0.6.4
      gitpython: 1.0.1
          ioflo: Not Installed
         Jinja2: 2.8
        libgit2: 0.24.0
        libnacl: Not Installed
       M2Crypto: 0.21.1
           Mako: 1.0.3
   msgpack-pure: Not Installed
 msgpack-python: 0.4.6
   mysql-python: Not Installed
      pycparser: 2.14
       pycrypto: 2.6.1
   pycryptodome: Not Installed
         pygit2: 0.24.0
         Python: 2.7.12 (default, Dec  4 2017, 14:50:18)
   python-gnupg: Not Installed
         PyYAML: 3.11
          PyZMQ: 15.2.0
           RAET: Not Installed
          smmap: 0.9.0
        timelib: Not Installed
        Tornado: 4.2.1
            ZMQ: 4.1.4

System Versions:
           dist: Ubuntu 16.04 xenial
         locale: UTF-8
        machine: x86_64
        release: 4.8.0-32-generic
         system: Linux
        version: Ubuntu 16.04 xenial
aabouzaid commented 6 years ago

So the workaround to make it work with older ssh-keyscan is using default ssh port 22 just to make it accept the host as a known_hosts.

xenadmin commented 5 years ago

Is there any update to this? I'm currently building a PoC for our company and stumbled upon this error in the latest version of 2018.3.

gtmanfred commented 5 years ago

@saltstack/team-triage can you take a look at this please?

waynew commented 5 years ago

✋ I'll take a look at this

waynew commented 5 years ago

@xenadmin If you're using the formula, it looks like you should be able to run hash_known_hosts: False to the pillar, and you can provide a non-standard port. I'm looking into this a bit more to make sure I've got my head wrapped around what's going on here.

xenadmin commented 5 years ago

@waynew Thanks for your reply. I tried that already, but sadly this doesn't suffice my issue :-( See below example:

host.domain.com:
  ssh_known_hosts:
    - present
    - port: 2222
    - hash_known_hosts: False
    - user: root
    - fingerprint: abcdefghijklmnopqrstuvwxyz
    - fingerprint_hash_type: SHA256
    - enc: ECDSA

The problem is, that I have two issues with the salt.states.ssh_known_hosts (https://docs.saltstack.com/en/latest/ref/states/all/salt.states.ssh_known_hosts.html) state. One is the port we taplk about in this issue, but additionally the sha265 fingerprint doesn't work. This is handled in #41653 with the proposal #49047 that seemed to work, but was never finished.

waynew commented 5 years ago

@xenadmin Okay, yeah I dug into this and now I see what the problem is. The tl;dr version is that if you're wanting to use a non-standard port, just pass hash_known_hosts: False you don't gain a whole lot from hashing, apparently, anyway.

It's not exactly an error in Salt, so much as Salt is not doing The Wrong Thing(tm) with ssh-keyscan, of which older versions (including my modern mac with touchbar) are basically broken.


For those curious about the underlying problem here - when you hash known hosts, instead of having a known hosts file that looks like this:

[example.com]:55555 ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAABAQDCY7tcbLrsTFPb2je3VFiH9cC9+ac04H0X8BQG7croyqvdUY5zTLmIidXJe6R1zUS7Jqpy/pXwHSB5HWpsMu+ytovPZ/LKl6AiYlcdcpS//QASb7TbcDzHFIlcdCoL5C5TOHXdRKrgIa64akuPMxvXxbgXAHjud+2jK1FhGTBbTkbrWA4xhDukWkswLpCRyHhsNzJd/seP651UDd/3rkrbgFSN9o/4LXZtsEfV3xRfJOaZq5/SW+sDVNlArFgg9EXXOzrKKWkSjS9BnN0hBaK3IyJfUAwppLYHgF0LvcNl4jF38EAU00pkNX5mknGbAFF7OMkcQI9/vkl+jaajv8Q3

Instead you get this:

|1|hgYNM0RCjd3UQc7YWEiQ2VVnS1k=|yktjOenUAajV/+EiT9IVWCKjRFw= ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAABAQDCY7tcbLrsTFPb2je3VFiH9cC9+ac04H0X8BQG7croyqvdUY5zTLmIidXJe6R1zUS7Jqpy/pXwHSB5HWpsMu+ytovPZ/LKl6AiYlcdcpS//QASb7TbcDzHFIlcdCoL5C5TOHXdRKrgIa64akuPMxvXxbgXAHjud+2jK1FhGTBbTkbrWA4xhDukWkswLpCRyHhsNzJd/seP651UDd/3rkrbgFSN9o/4LXZtsEfV3xRfJOaZq5/SW+sDVNlArFgg9EXXOzrKKWkSjS9BnN0hBaK3IyJfUAwppLYHgF0LvcNl4jF38EAU00pkNX5mknGbAFF7OMkcQI9/vkl+jaajv8Q3

Ostensibly this helps with some security-through-obscurity. However, the problem comes when you do ssh-keyscan on a non-standard port. For port 22 it's totally fine, but when you include the port, it should hash [example.com]:55555 with the salt. It doesn't. It just produces example.com. Ooops.

Why is this a problem? Well, if you're trying to check your known_hosts file for a domain with a port, you do that with ssh-keygen -F [example.com]:55555 -f ~/.ssh/known_hosts. If you do this with the result of ssh-keyscan, like so:

ssh-keyscan -p 55555 -H example.com > /tmp/keys.txt
ssh-keygen -F [example.com]:55555 -f /tmp/keys.txt

It won't find anything. ssh-keygen -F example.com -f /tmp/keys.txt will, though 🤦‍♂️

So what does that mean for us? Well, fortunately, hashing the hostnames doesn't really buy a whole ton of security - if someone compromises your machine there are a whole host of other things they will be able to do, the worst that can happen is that someone sees your known_hosts file and knows what sort of hosts you've been connecting to. It's probably fine to just ignore the hashing of hostnames.

However, if you're so inclined, I'd be more than happy to see a PR that adjusts the ssh module and removes these lines, then goes in and changes https://github.com/saltstack/salt/blob/8f8117f7c9824ed37316977272310e79db715b15/salt/modules/ssh.py#L929 so that if hash_known_hosts is passed, instead of providing the -H flag, it dumps the output of ssh-keyscan to a file, then runs ssh-keygen -H -f <file> to get the corrected hashed output. With a couple of tests, that would probably be the best way to fix this.

In the interim, assuming you're using the users formula, if you really do need the hashing functionality I just put together a quick state. I think it should work with multiple users and multiple known-hosts.

include:
  - users

{%- for name, user in pillar.get('users', {}).items()
        if user.absent is not defined or not user.absent %}

{% if 'ssh_known_hosts' in user %}
{%- set current = salt.user.info(name) -%}
{%- set home = user.get('home', current.get('home', '/home/%s' % name)) -%}
hash_{{ name }}_known_hosts:
  cmd.run:
    - name: ssh-keygen -H -f {{ home }}/.ssh/known_hosts
    - runas: {{ name }}
    - onchanges_any:
    {% for _ in user['ssh_known_hosts'] %}
      - ssh_known_hosts: users_ssh_known_hosts_{{ name }}_{{ loop.index0 }}
    {%- endfor %}

cleanup_old_files:
  file.absent:
    - name: {{ home }}/.ssh/known_hosts.old
    - onchanges:
      - cmd: hash_{{ name }}_known_hosts

{%- endif -%}
{%- endfor -%}
xenadmin commented 5 years ago

Hi and thanks for your analysis. I did not understand all of it, but I hope I did understand most of it. But I think this is heading in the wrong direction maybe. To clarify my use-case: I use saltstack for only a few days now. I'm building a PoC for the Zabbix formula, to take central configuration management control of our Zabbix Proxys. Part of that is, that on each Zabbix Proxy there is a file.managed rsync script + crontab entry, that syncs binary files from a central repository once a day. This works with rsync over ssh. My plan is to deploy these future Zabbix Proxys from scratch with saltstack, and that would include to pre-fetch the known_hosts of this central repository, so ssh will connect without further questions. Maybe I'm over-complicating this, an I should just file.managed the ".ssh/known_hosts" instead, but I had the feeling that this state would be the better solution.

waynew commented 5 years ago

@xenadmin I'm assuming that it's connecting over a non-standard (i.e. 22) port?

Are you just using the ssh_known_hosts state, or are you using the users formula?

If you're using the users formula, add hash_known_hosts: False to your pillar.

If you're using the state then just add - hash_known_hosts: False to your state file.

Does that work for you?

waynew commented 5 years ago

Ah, just saw your question about fingerprints - yeah, that part is... confusing. I'm not entirely sure that I understand it yet, either. But I can provide the information to get the fingerprint that's expected.

waynew commented 5 years ago

@xenadmin To get the fingerprint, you should use: salt <minion> ssh.recv_known_host_entries host.domain.com port=2222 fingerprint_hash_type=SHA256 enc=ECDSA. Give that a shot and let me know if you're still unable to add the known host!

waynew commented 5 years ago

(note that the hashed hostname here uses the [probably] broken ssh-keyscan host hashing, so don't use that)

xenadmin commented 5 years ago

@waynew Yes, I'm connecting ssh NOT over port 22, but a non-standard port.

I'm not using a "users formula" afaik. I wrote a simple state myself, which I presented already here: https://github.com/saltstack/salt/issues/40152#issuecomment-455685130

I already added - hash_known_hosts: False to my state, see my example state I posted a few posts above please.

No, it still doesn't work completely, because of the fingerprint. But this circumvents the ports issue. I don't understand why, but it circumvents it.

"ssh.recv_known_host_entries" I take a look at this one... So I experimented with this the last 30 minutes or so. I still don't understand it fully. It kinda works now, but doesn't do what I want it to do. I tried to squeeze this module into a state, as would need more of a module.

ssh.recv_known_host_entries:
  module.run:
    - hostname: FQDN
    - enc: ECDSA
    - port: 2222
    - hash_known_hosts: True

I get a positive execution return, but I'm missing the results I expected:

waynew commented 5 years ago

@xenadmin Does this work for you?

add ssh key:
  ssh_known_hosts.present:
    - user: root
    - name: YOUR_HOSTNAME_HERE
    - fingerprint: OUTPUT_OF_FINGERPRINT_COMMAND
    - enc: ECDSA
    - port: 2222
    - hash_known_hosts: False

hash known hosts:
  cmd.run:
    - name: ssh-keygen -H
    - runas: root
    - onchanges:
      - ssh_known_hosts: add ssh key
  file.absent:
    - name: /root/.ssh/known_hosts.old
    - onchanges:
      - ssh_known_hosts: add ssh key

Where the OUTPUT_OF_FINGERPRINT command is the fingerprint that you get from this:

salt <minion> ssh.recv_known_host_entries YOUR_HOSTNAME_HERE port=2222 fingerprint_hash_type=SHA256 enc=ECDSA

Does that do what you need?

xenadmin commented 5 years ago

@waynew This works really good! Wow! I'm not sure, I understand the second part, yet, but it works good! Thank you! But this doesn't resolve this issue, does it?

waynew commented 5 years ago

@xenadmin Great! And you're right, it doesn't really resolve the issue - the root cause is that ssh-keyscan is broken most everywhere it's available. We can work around it, and I think we should.

What that looks like is:

It's a couple more steps than we currently do, but it's the most reliable way to get the output that SSH expects - my experience has been that most platforms have a broken ssh-keyscan.

waynew commented 5 years ago

I'm giving a workaround a bit of attention today, hopefully should have a PR up by the EOD

waynew commented 5 years ago

I've just created a PR - If you can, I'd appreciate folks testing to ensure that it works for you! (If my fix fails for you - or fixes your problem - please go comment on the PR above!)

waynew commented 5 years ago

I'm going to go ahead and close this - if my fix doesn't work, please let me know and I'll re-open