Display SHA256 fingerprint for SSH keys

@winniehell Wanna fix it? :D

@hazelyang I have updated the mockups above. Can you take a look?

@DouweM I have started working on this and it looks like fingerprints are stored in the database. Do you know why? Can't they always be created from the key on the fly?

@winniehell I slightly modified the design based on your mockup.

@hazelyang Looks great, thank you!

fingerprints are stored in the database. Do you know why? Can't they always be created from the key on the fly?

I know why now: because the uniqueness constraint would otherwise be a hassle.

@DouweM I don't think storing the SHA256 hash would work because we can not be sure we have OpenSSH >= 6.8. We could of course every time a hash is queried check if OpenSSH 6.8 is now available. Any thoughts on this?

mentioned in merge request gitlab-foss!5651 (closed)

I don't think storing the SHA256 hash would work because we can not be sure we have OpenSSH >= 6.8.

Problem solved: gitlab-foss!5651 (closed)

Note that in EE this fingerprint is also used to find the authorized key when a user pushes into GitLab over SSH: http://gitlab.com/gitlab-org/gitlab-ee/blob/master/lib/api/internal.rb#L75

@DouweM Thank you for pointing me to the right place! How can I make sure that EE does not break? Should I open a merge request with the EE-only changes which will then be merged at the same time?

I'm closing this because it won't be fixed in the near future. See https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/5651#note_13729880

Status changed to closed

Status changed to reopened

Reopening for second attempt. See https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/5651#note_13795345

Should be implemented after gitlab-foss#21391 (closed)

Assignee removed

mentioned in merge request gitlab-foss!9350 (closed)

This goes a little deeper than display - we actually look up keys by MD5 fingerprint as well.

@briann I don't think this is a glaring security hole, but I think we should aim to migrate the lookup and display to SHA256 fingerprints in the near future. WDYT?

https://gitlab.com/gitlab-org/gitlab-ee/merge_requests/2423 adds fingerprinting code that can be used for all this.

@nick.thomas Yes, I think we should. We can create a migration that computes the SHA256 hash for those keys that don't have it.

This goes a little deeper than display - we actually look up keys by MD5 fingerprint as well.

I was aware of that (see also https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/5651#note_13830578) but wanted to keep the lookup for a second iteration.

We can create a migration that computes the SHA256 hash for those keys that don't have it.

Yep, after gitlab-foss#21391 (closed) is solved, we can. However @jacobvosmaer-gitlab had a strong opinion against it: https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/5651#note_13570314 Maybe the situation is different now because we have background migrations?

@winh I think we can use a background migration to create the SHA256 and not implement authentication based on it until a later release.

We're switching to net-ssh to calculate fingerprints: https://gitlab.com/gitlab-org/gitlab-ce/issues/21391. It has an open PR adding SHA256 fingerprinting support, so I've reached out to the community there: https://github.com/net-ssh/net-ssh/pull/517#issuecomment-320082441

Hopefully this will end up in a net-ssh release soon and we can build on that to complete this issue.

mentioned in issue gitlab-foss#37899

Per https://gitlab.com/gitlab-org/gitlab-ce/issues/37899 , use of MD5 hashes is becoming a problem with some compliance regimes. I think we should consider scheduling this earlier rather than later, especially given the data migration it will involve.

/cc @mydigitalself