Salt: SSH modules spam warning about MD5 fingerprints when there aren't any

[joewreschnig]

Description of Issue/Question

After installing Salt 2016.11.4, my states spews

[WARNING ] Public Key hashing currently defaults to "md5". This will change to "sha256" in the Nitrogen release.

several dozen times when applied. This appears to be the result of #40543, which adds the fingerprint_hash_type option and warns if it's not set. I appreciate the effort to both move off of MD5 and help users migrate. However, some callers of _fingerprint in the same file still call directly without a hash, and there's no way for the user to provide one.

The simplest fix may be to have these callers provide literally any hash function, since most seem to be using it to validate keys rather than actually look at the fingerprint. Above that, I'm not sure why _fingerprint, a private function, is taking the hash as an optional argument in the first place. It seems like this would have been caught earlier had the argument been mandatory and caused an error rather than an ignorable warning, thereby forcing callers passing user data to pass None if they wanted the warning.

Versions Report

Salt Version:
           Salt: 2016.11.4

Dependency Versions:
           cffi: Not Installed
       cherrypy: Not Installed
       dateutil: 2.2
      docker-py: Not Installed
          gitdb: 0.5.4
      gitpython: 0.3.2 RC1
          ioflo: Not Installed
         Jinja2: 2.9.4
        libgit2: Not Installed
        libnacl: Not Installed
       M2Crypto: Not Installed
           Mako: Not Installed
   msgpack-pure: Not Installed
 msgpack-python: 0.4.2
   mysql-python: 1.2.3
      pycparser: Not Installed
       pycrypto: 2.6.1
   pycryptodome: Not Installed
         pygit2: Not Installed
         Python: 2.7.9 (default, Jun 29 2016, 13:08:31)
   python-gnupg: Not Installed
         PyYAML: 3.11
          PyZMQ: 14.4.0
           RAET: Not Installed
          smmap: 0.8.2
        timelib: Not Installed
        Tornado: 4.4.2
            ZMQ: 4.0.5

System Versions:
           dist: debian 8.7 
        machine: x86_64
        release: 4.4.27-x86_64-jb1
         system: Linux
        version: debian 8.7 

[gtmanfred]

Thanks for reporting this, we should get it fixed so that it doesn't spam these.

@rallytime do you have any opinions on how this should be solved?

If you had time to submit a PR we would greatly appreciate it, otherwise I am tagging this for the core team to take a look.

Thanks
Daniel

[rallytime]

@joewreschnig What does your sanitized state look like?

However, some callers of _fingerprint in the same file still call directly without a hash, and there's no way for the user to provide one.

I tried to grab all of the places where a hash would need to be provided, but I certainly could have missed some. Can you tell me where you're seeing those cases? You should be able to state the fingerprint_hash_type in the state file. The default needs to stay at md5 for the time being so we don't break people's states in a point release.

[joewreschnig]

It's not a missing configuration option in my state. Rather it's ssh.py:208 and ssh.py:680, which are called by e.g. set_auth_key which is used by the ssh_auth.present state. These don't even deal with user-provided hashes, and so don't allow specifying hash types.

[MattHoy]

We also have the same problem, we're using the ssh_auth.present state to manage user keys, and we have hundreds of these alerts every time we highstate.

[yongzhang]

+1 I have the same issue.

[rallytime]

Oh, I am not sure why I didn't see these updates earlier, but I see what you mean now. I will work on getting this fixed today. Sorry about that everyone!

[rallytime]

@joewreschnig I have added the fingerprint_hash_type option to the state functions in ssh_auth.py, as well as any functions in the ssh.py execution module calling out to the _fingerprint function. When I added that option to ssh_known_hosts.py I completely missed adding it to ssh_auth.py.

Please give #41837 a try and let me know how it goes. You should be able to specify the fingerprint_hash_type option now and have it work. Note that the warning will go away and the default will change from md5 to sha256 in the upcoming 2017.7.0 release.