Skip to content
This repository has been archived by the owner on Jan 24, 2022. It is now read-only.

Refactor fingerprints for keys #484

Merged
merged 2 commits into from
Jan 18, 2019
Merged

Refactor fingerprints for keys #484

merged 2 commits into from
Jan 18, 2019

Conversation

claudijd
Copy link
Contributor

This attempts to refactor the key handling (formerly fingerprints only) to make it more useful per recent feedback on #476. It's not quite done, and I had to break a few things to get it to work, but it does work at least on the bench. Needs a few more tweaks and should be ready for a pre-release to try out.

@claudijd
Copy link
Contributor Author

Here's a preview of the new visibility this will offer to folks..

"keys": {
  "dsa": {
    "raw": "ssh-dss AAAAB3NzaC1kc3MAAACBANGFW2P9xlGU3zWrymJgI/lKo//ZW2WfVtmbsUZJ5uyKArtlQOT2+WRhcg4979aFxgKdcsqAYW3/LS1T2km3jYW/vr4Uzn+dXWODVk5VlUiZ1HFOHf6s6ITcZvjvdbp6ZbpM+DuJT7Bw+h5Fx8Qt8I16oCZYmAPJRtu46o9C2zk1AAAAFQC4gdFGcSbp5Gr0Wd5Ay/jtcldMewAAAIATTgn4sY4Nem/FQE+XJlyUQptPWMem5fwOcWtSXiTKaaN0lkk2p2snz+EJvAGXGq9dTSWHyLJSM2W6ZdQDqWJ1k+cL8CARAqL+UMwF84CR0m3hj+wtVGD/J4G5kW2DBAf4/bqzP4469lT+dF2FRQ2L9JKXrCWcnhMtJUvua8dvnwAAAIB6C4nQfAA7x8oLta6tT+oCk2WQcydNsyugE8vLrHlogoWEicla6cWPk7oXSspbzUcfkjN3Qa6e74PhRkc7JdSdAlFzU3m7LMkXo1MHgkqNX8glxWNVqBSc0YRdbFdTkL0C6gtpklilhvuHQCdbgB3LBAikcRkDp+FCVkUgPC/7Rw==",
    "length": 1024,
    "fingerprints": {
      "md5": "ad:1c:08:a4:40:e3:6f:9c:f5:66:26:5d:4b:33:5d:8c",
      "sha1": "74:91:97:3e:5f:8b:39:d5:32:7c:d4:e0:8b:c8:1b:05:f7:71:0b:49",
      "sha256": "br9IjFspm1vxR3iA35FWE+4VTyz1hYVLIE2t1/CeyWQ="
    }
  },
  "rsa": {
    "raw": "ssh-rsa AAAAB3NzaC1yc2EAAAABIwAAAQEAq2A7hRGmdnm9tUDbO9IDSwBK6TbQa+PXYPCPy6rbTrTtw7PHkccKrpp0yVhp5HdEIcKr6pLlVDBfOLX9QUsyCOV0wzfjIJNlGEYsdlLJizHhbn2mUjvSAHQqZETYP81eFzLQNnPHt4EVVUh7VfDESU84KezmD5QlWpXLmvU31/yMf+Se8xhHTvKSCZIFImWwoG6mbUoWf9nzpIoaSjB+weqqUUmpaaasXVal72J+UX2B+2RPW3RcT0eOzQgqlJL3RKrTJvdsjE3JEAvGq3lGHSZXy28G3skua2SmVi/w4yCE6gbODqnTWlg7+wC604ydGXA8VJiS5ap43JXiUFFAaQ==",
    "length": 2048,
    "fingerprints": {
      "md5": "16:27:ac:a5:76:28:2d:36:63:1b:56:4d:eb:df:a6:48",
      "sha1": "bf:6b:68:25:d2:97:7c:51:1a:47:5b:be:fb:88:aa:d5:4a:92:ac:73",
      "sha256": "nThbg6kXUpJWGl7E1IGOCspRomTxdCARLviKw6E5SY8="
    }
  }
},

@exploide
Copy link

I like the new structure along with all the information it provides to me 👍

But I'm a bit confused about the fingerprint encoding. The SHA-256 hash is the only one in base64. I would go with consistency and print the SHA-256 hash also in hex (or the other two also in base64 instead; not important for me but consistency would be great).

@claudijd
Copy link
Contributor Author

@exploide I'm using sshkey library to do all this work, I'm not exactly sure about the reasoning for the base64 encoding, but it seems to be the default there. I could of course provided multiple formats for the sha256 as usually more information is less harmful and less information.

If you look at their readme, you'll see it in the fingerprint section here:

https://github.com/bensie/sshkey#fingerprints

@exploide
Copy link

Ok, if it is the default there, it's probably reasonable do follow their scheme.

@claudijd
Copy link
Contributor Author

I think I'm just going to release 0.0.39 today with this included, so people can start playing around with it and find any new bugs I introduced. It will likely break some expectations if people are surfing the JSON to get the details, but hopefully they will be more happy about the added clarity.

@exploide
Copy link

Great, I will try it :)

What I expect instead of bugs is that people become confused when they only see an RSA key enumerated but the server actually also offers ECDSA and ED25519. But that's maybe an other issue.

@claudijd
Copy link
Contributor Author

I think that issue has been present for a while, I consider it a feature add rather than a bug.

@claudijd claudijd changed the title [WIP] Refactor fingerprints for keys Refactor fingerprints for keys Jan 18, 2019
@claudijd claudijd merged commit 3f69b91 into master Jan 18, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants