Show MD5 Hash of Fingerprint in Verbose Output

[thecliguy]

When when verifying host keys, PuTTY, plink and psftp use an md5 hash rather than a sha256 hash.

plink 0.74 - Example Output (click to expand):

C:\sandbox>plink.exe -v scanme.nmap.org
Looking up host "scanme.nmap.org" for SSH connection
Connecting to 45.33.32.156 port 22
We claim version: SSH-2.0-PuTTY_Release_0.74
Remote version: SSH-2.0-OpenSSH_6.6.1p1 Ubuntu-2ubuntu2.13
We believe remote version has SSH-2 channel request bug
Using SSH protocol version 2
No GSSAPI security context available
Doing ECDH key exchange with curve Curve25519 and hash SHA-256 (unaccelerated)
Server also has ecdsa-sha2-nistp256/ssh-dss/ssh-rsa host keys, but we don't know any of them
Host key fingerprint is:
ssh-ed25519 255 33:fa:91:0f:e0:e1:7b:1f:6d:05:a2:b0:f1:54:41:56
The server's host key is not cached in the registry. You
have no guarantee that the server is the computer you
think it is.
The server's ssh-ed25519 key fingerprint is:
ssh-ed25519 255 33:fa:91:0f:e0:e1:7b:1f:6d:05:a2:b0:f1:54:41:56
If you trust this host, enter "y" to add the key to
PuTTY's cache and carry on connecting.
If you want to carry on connecting just once, without
adding the key to the cache, enter "n".
If you do not trust this host, press Return to abandon the
connection.
Store key in cache? (y/n) 

Currently ssh-audit only shows fingerprints in the form of a sha256 hash. Do you have any objection to also showing the md5 hash if the verbose (-v/--verbose) parameter has been provided?

I've built a proof-of-concept that I can share.

By the way, the Fingerprint class is already capable of producing an md5 hash, it's just not currently used:

ssh-audit/src/ssh_audit/fingerprint.py

Lines 33 to 37 in 2f1a2a6

	@property
	def md5(self) -> str:
	h = hashlib.md5(self.__fpd).hexdigest()
	r = u':'.join(h[i:i + 2] for i in range(0, len(h), 2))
	return u'MD5:{}'.format(r)

[thecliguy]

I probably should have explained my use case for this a bit better...

When I work on projects that use an SSH component, I often need to analyse and review logs from a live environment.

As part of this analysis, it's useful to be able to check the fingerprint returned by a client tool against an audit of the server that it's connecting to.

I'm not advocating the use of PuTTY client tools (PuTTY, plink, psftp, etc) but it's quite common to find them being used within Windows environments. So if ssh-audit could include the md5 fingerprint hash, this would just help with the analysis process when dealing with PuTTY tools (or any other client tools/libraries that still use md5 hashes).

[jtesta]

Sure, we can include the MD5 hash for verbose output.

[thecliguy]

@jtesta - Although ssh-audit currently only supports two fingerprint hash types I want to avoid hardcoding rules and values...

Therefore I'd like to have a single source of truth for all the fingerprint hash types that ssh-audit supports.

In my proof of concept, I'm using the Fingerprint class itself as the source of truth:

fingerprint_hash_types_all = [elem for elem in dir(Fingerprint) if not elem.startswith('__')]

This works nicely in my proof of concept but my I'm a bit concerned that this would break if in the future someone adds a new member to the Fingerprint class that doesn't represent a hash type...

To avoid this problem, can you think of any way to tag members of the Fingerprint class that represent a hash type so I can easily identify them? Or am I worrying about this unnecessarily?

I'm open to options if you have a better suggestions...

[thecliguy]

@jtesta Please can you review PR #104.

This PR adds the ability to specify hash types that should appear in verbose output. This is achieved by adding the hash type to the fingerprint_hash_types_verbose list.

Below are some example usages where fingerprint_hash_types_verbose = ['md5'].

Verbose output:

ssh-audit scanme.nmap.org -v

# fingerprints
(fin) ssh-ed25519                           -- MD5:33:fa:91:0f:e0:e1:7b:1f:6d:05:a2:b0:f1:54:41:56
(fin) ssh-ed25519                           -- SHA256:FI7R9RbIFL8J0JynWurg3sSFT30ai8Du6MfPJ0wiqvI
(fin) ssh-rsa                               -- MD5:20:3d:2d:44:62:2a:b0:5a:9d:b5:b3:05:14:c2:a6:b2
(fin) ssh-rsa                               -- SHA256:cx1VAtJyRgCA0wQH1FBIrgkTV3Gx6CG7rYAKfq2yF1o

Verbose output as JSON:

NB: In the example below I've placed each fingerprint object on a separate line to make it easier to read.

ssh-audit scanme.nmap.org -v -j

"fingerprints": [
  {"fp": "MD5:33:fa:91:0f:e0:e1:7b:1f:6d:05:a2:b0:f1:54:41:56", "hashtype": "md5", "type": "ssh-ed25519"}, 
  {"fp": "SHA256:FI7R9RbIFL8J0JynWurg3sSFT30ai8Du6MfPJ0wiqvI", "hashtype": "sha256", "type": "ssh-ed25519"}, 
  {"fp": "MD5:20:3d:2d:44:62:2a:b0:5a:9d:b5:b3:05:14:c2:a6:b2", "hashtype": "md5", "type": "ssh-rsa"}, 
  {"fp": "SHA256:cx1VAtJyRgCA0wQH1FBIrgkTV3Gx6CG7rYAKfq2yF1o", "hashtype": "sha256", "type": "ssh-rsa"}
]

Batch output (batch output implicitly enables verbose output):

ssh-audit scanme.nmap.org -b

(fin) ssh-ed25519 -- MD5:33:fa:91:0f:e0:e1:7b:1f:6d:05:a2:b0:f1:54:41:56
(fin) ssh-ed25519 -- SHA256:FI7R9RbIFL8J0JynWurg3sSFT30ai8Du6MfPJ0wiqvI
(fin) ssh-rsa -- MD5:20:3d:2d:44:62:2a:b0:5a:9d:b5:b3:05:14:c2:a6:b2
(fin) ssh-rsa -- SHA256:cx1VAtJyRgCA0wQH1FBIrgkTV3Gx6CG7rYAKfq2yF1o

[thecliguy]

@jtesta For some reason Travis CI didn't seem to add any feedback to the PR.

There was one thing in the PR that I was not entirely happy with... fp_hash_types (passed to output_fingerprints) is a list of hash types, where each item is a dictionary with two properties (name and verbose). I'm not sure if this was an appropriate choice of data structure or whether a class or something else would have been better?

[thecliguy]

@jtesta Sorry to chase, I just wondered if you've had a chance to review the PR?

[jtesta]

@thecliguy Thanks for the ping. I needed a push to prioritize this again.

After looking over your PR, if I'm understanding your goals correctly, it seems like it can be done with a simpler patch. See the md5_hash_output branch that I just pushed.

I opted to leave the output format of the fingerprints section the same, since it keeps the important information outside of comments. Besides this, does my patch do what you were hoping for?

[thecliguy]

Hi @jtesta

Thanks for looking into this, please see my feedback...

Hardcoded Logic Around MD5

I think hardcoding conditional logic around MD5 is a bit ugly...

What I was trying to do is make it easy to control which algorithms appear in the verbose output.

So let's say in the future there's a desire to add support for additional hashing algorithms and you want them to appear in verbose output only, then all you need to do is update fingerprint_hash_types_verbose, EG: fingerprint_hash_types_verbose = ['md5', 'sha1', 'bubblebabble'].

JSON Property Name Changes

There's a breaking change in your code because you've updated some json property names:

• fp --> hash
• type --> hostkey

I think it's important to tread carefully here... When a tool provides a structured data format like json, users expect this to be a safe way to parse output.

If anyone has written code that parses the json output, it may not work once those property names have changed.

So if you do go ahead and change the property names, I'd suggest noting this as a breaking change in the next release.

Text Colour

There's a convention in ssh-audit that red text indicates something undesirable...

I don't think that MD5 fingerprints should appear in red, I think white text would be more appropriate.

[jtesta]

let's say in the future there's a desire to add support for additional hashing algorithms

I'm doubtful that another standard fingerprint format will be coming any time soon. But if it does, I'd be happy to cross that bridge later. In the meantime, I'm a fan of not over-engineering the code.

So if you do go ahead and change the property names, I'd suggest noting this as a breaking change in the next release.

Yep, that was my plan!

There's a convention in ssh-audit that red text indicates something undesirable... I don't think that MD5 fingerprints should appear in red, I think white text would be more appropriate.

Using MD5 fingerprints is indeed undesirable. But perhaps I can make note of that in a comment.

Thanks for the feedback!

[thecliguy]

I didn't argue my point about the text colour very well...

Red text ought to be reserved specifically for undesirable configuration where the user should take action to fix it.

Whilst I agree that using MD5 hashing is not desirable these days it's presence on an ssh-audit report is not highlighting a defect with the target server that needs to be addressed by the user. It's just an additional piece of information that will appear even on a perfectly configured server.

[thecliguy]

I feel quite strongly that we should not dilute the significance of red text.

A target server that doesn't exhibit any issues that need investigation by the user shouldn't contain any output in red text.

[jtesta]

Ok. Please have a look at 0786248. I made it output the MD5 hash in white instead of red.

[thecliguy]

Looks good to me, thanks for you persistence 👍

[jtesta]

Great! Thanks for the help!!