Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use SHA256 Hash for Key Fingerprints #214

Closed
srueg opened this issue Nov 9, 2020 · 3 comments · Fixed by #215
Closed

Use SHA256 Hash for Key Fingerprints #214

srueg opened this issue Nov 9, 2020 · 3 comments · Fixed by #215

Comments

@srueg
Copy link
Contributor

srueg commented Nov 9, 2020

How would you feel about using the SHA256 hash instead of the current MD5 to calculate public key fingerprints?
https://github.com/jpillora/chisel/blob/v1.7.2/share/ccrypto/keys.go#L35

To remain backward compatible we could add a sha256: prefix to new fingerprints and evaluate old fingerprints (without the prefix) still as md5 hashes (at least until a new major version would allow the breaking change).

References regarding MD5 deprecation:

@jpillora
Copy link
Owner

jpillora commented Nov 9, 2020 via email

srueg added a commit to srueg/chisel that referenced this issue Nov 9, 2020
Closes jpillora#214

Signed-off-by: Simon Rüegg <simon@rueggs.ch>
@srueg
Copy link
Contributor Author

srueg commented Nov 9, 2020

I started implementing a PR for this but realized that this would break the feature of providing only a partial fingerprint in the clients --fingerprint flag: a partial fingerprint will most probably not be a valid base64 string, so it will fall back to MD5 which won't match either.
How would you like to proceed?

  1. Only provide auto-fallback for fully specified fingerprints and deprecate partial fingerprints?
  2. Go with the sha256: prefix?

@srueg
Copy link
Contributor Author

srueg commented Nov 10, 2020

I went with option 1 in PR #215

lmvlmv added a commit to lmvlmv/chisel that referenced this issue Jan 5, 2021
* Use SHA256 hashes for key fingerprints

Closes jpillora#214

Signed-off-by: Simon Rüegg <simon@rueggs.ch>

* Update client to fall-back to MD5 fingerprints

Signed-off-by: Simon Rüegg <simon@rueggs.ch>

* help goes to stdout and exits with 0, remove incorrect versioning from help

* document udp, log client connection failures, expose more settings via env-vars

* go dropped support for mips64p32, ppc, s390

* chisel client uses CHISEL_KEY to generate fingerprint for validation

* Update README.md

* Fewer archs

* Restrict allowed URLs, Allow localhost variants for tests (#2)

* ALlow localhost variants for tests

* Remove unnecessary comment

* Unnecessary +

* regex tweak

Co-authored-by: Leon Verrall <lverrall@slb.com>

* Extend config object in test

Co-authored-by: Simon Rüegg <simon@rueggs.ch>
Co-authored-by: Jaime Pillora <dev@jpillora.com>
Co-authored-by: Leon Verrall <lverrall@slb.com>
lmvlmv added a commit to lmvlmv/chisel that referenced this issue May 18, 2021
* Use SHA256 hashes for key fingerprints

Closes jpillora#214

Signed-off-by: Simon Rüegg <simon@rueggs.ch>

* Update client to fall-back to MD5 fingerprints

Signed-off-by: Simon Rüegg <simon@rueggs.ch>

* help goes to stdout and exits with 0, remove incorrect versioning from help

* document udp, log client connection failures, expose more settings via env-vars

* go dropped support for mips64p32, ppc, s390

* chisel client uses CHISEL_KEY to generate fingerprint for validation

* Update README.md

* Fewer archs

* Restrict allowed URLs, Allow localhost variants for tests (#2)

* ALlow localhost variants for tests

* Remove unnecessary comment

* Unnecessary +

* regex tweak

Co-authored-by: Leon Verrall <lverrall@slb.com>

* Extend config object in test

Co-authored-by: Simon Rüegg <simon@rueggs.ch>
Co-authored-by: Jaime Pillora <dev@jpillora.com>
Co-authored-by: Leon Verrall <lverrall@slb.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants