Use SHA256 to compare SSH public key fingerprint

[combor]

Description

MD5 is deprecated and cf info endpoint returns now SHA256 fingerprint.
Use SHA256 to compare fingerprints instead MD5

Motivation and Context

This fixes ssh to an app instance functionality.

How Has This Been Tested?

Manually

Types of changes

• Bug fix (non-breaking change which fixes an issue)
• Docs update
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to not work as expected)

Checklist:

• My code follows the code style of this project.
• My change requires a change to the documentation.
• I have updated the documentation accordingly.
• I have followed the guidelines in CONTRIBUTING.md, including the required formatting of the commit message

[cfdreddbot]

❌ Hey combor!

All pull request submitters and commit authors must have a Contributor License Agreement (CLA). Click here for details on the CLA process.

The following github user @combor is not covered by a CLA.

After the CLA process is complete, this pull request will need to be closed & reopened. DreddBot will then validate the CLA(s).

[codecov]

Codecov Report

Merging #3249 into v2-master will decrease coverage by 0.01%.
The diff coverage is n/a.

@@              Coverage Diff              @@
##           v2-master    #3249      +/-   ##
=============================================
- Coverage       70.9%   70.89%   -0.02%     
=============================================
  Files            642      642              
  Lines          28204    28204              
  Branches        6412     6412              
=============================================
- Hits           19999    19995       -4     
- Misses          8205     8209       +4

[nwmac]

The fingerprint check needs to cope with both MD5 and SHA256 - this change only supports SHA256, which will be break any deployments that use MD5.

SHS256 Fingerprints are prefixed with 'SHA256', so we should check which type the fingerprint is and use that when checking.

[combor]

@nwmac SHA256 are not prefixed when returned from api /v2/info
If you check my change it does this:

if fmt.Sprintf("SHA256:%s", fingerprint) == ssh.FingerprintSHA256(key) {

It needs to add the prefix for comparison with ssh.FingerprintSHA256 output.

But I agree in general with the point of keeping this backwards compatible. I'll prepare a change.

[combor]

@nwmac please check now. It checks both sums now serially and returns on first hit.

[nwmac]

@combor As I understand it, the fingerprint as returned by cf's /v2/info API will include the SHA256: prefix if that algorithm is being used, otherwise it is an MD5.

So, I think the code should be:

if strings.HasPrefix("SHA256")  && fingerprint == ssh.FingerprintSHA256(key) {
	return nil
}

if fingerprint == ssh.FingerprintLegacyMD5(key) {
	return nil
}

[combor]

@nwmac well, I don't thing /v2/info returns fingerprint with a prefix, that's why I'm using this to include it:

fmt.Sprintf("SHA256:%s", fingerprint)

and later compare with

 ssh.FingerprintSHA256(key)

as this one includes SHA256 prefix.

Just to summarise and be clear:

• variable fingerprint is without prefix
• ssh.FingerprintSHA256(key) creates a string with the SHA256 prefix

[nwmac]

@combor Sorry, you're right it doesn't include SHA256 - I'd found that while looking into this, but its not standard.

I've done a bit more investigation - I think we should adopt the same approach as the CLI team did - using the key length to determine the algorithm - see here: https://github.com/cloudfoundry/cli/pull/1072/files

[combor]

Cool, the cli code looks good. I'll include it into PR.

[combor]

Hi @nwmac, I just updated this PR. Please have a look and check if that's what you meant.

[nwmac]

LGTM