Skip to content

fixes #12132 -- implement ssh public key fingerprints #12673

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

Merged
merged 1 commit into from
May 6, 2025

Conversation

alex
Copy link
Member

@alex alex commented Mar 24, 2025

No description provided.

@alex
Copy link
Member Author

alex commented Mar 24, 2025

(This was mostly developed by claude-code, but eventually I got bored of baby sitting it and so I just cleaned it up myself)

@alex alex force-pushed the ssh-key-fingerprint branch 2 times, most recently from cd7538a to 0007fb5 Compare March 24, 2025 17:24
@alex
Copy link
Member Author

alex commented Mar 24, 2025

Two choices I made here:

  1. I do not include the algorithm name as a prefix
  2. For hex, I do not separate the characters by :

My theory was that adding those by a caller is easy-ish, but undoing them is annoying. But I'm not wedded to this.

@alex
Copy link
Member Author

alex commented Mar 28, 2025

an alternative here would be to just compute the hash, no serialization at all.

@alex alex requested a review from Copilot April 4, 2025 12:28
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot reviewed 3 out of 5 changed files in this pull request and generated no comments.

Files not reviewed (2)
  • CHANGELOG.rst: Language not supported
  • docs/hazmat/primitives/asymmetric/serialization.rst: Language not supported

@alex alex force-pushed the ssh-key-fingerprint branch 2 times, most recently from 124e370 to 00c94e8 Compare April 18, 2025 00:50
@alex alex force-pushed the ssh-key-fingerprint branch 2 times, most recently from 3496507 to 407cd20 Compare May 6, 2025 01:07
Copy link
Member

@reaperhulk reaperhulk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does the SSH spec define the md5 fingerprint as hex encoded and sha256 as base64 with no padding? If so, can we put that in a comment somewhere? 😄

Assuming that is a documented property of these fingerprints then I think it's fine to do it for the caller rather than presenting back raw digest bytes.

@alex
Copy link
Member Author

alex commented May 6, 2025

So, I'm not aware of a spec for this, I based this on the behavior of ssh-keygen -l with each hash, and a review of the OpenSSH source code. The more I think about this, the more I want to just do raw hash bytes, ala Certificate.fingerprint(). Do you disagree?

@reaperhulk
Copy link
Member

Yeah if it's just a current implementation detail I'd rather just do digest bytes with examples of how to encode it to match current ssh-keygen behavior.

@alex alex force-pushed the ssh-key-fingerprint branch from 22460ac to d1e0cda Compare May 6, 2025 21:56
@reaperhulk reaperhulk enabled auto-merge (squash) May 6, 2025 22:02
@reaperhulk reaperhulk merged commit 5063c16 into pyca:main May 6, 2025
64 checks passed
@alex alex deleted the ssh-key-fingerprint branch May 6, 2025 22:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants