-
Notifications
You must be signed in to change notification settings - Fork 315
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
export to ssh #323
export to ssh #323
Conversation
IIRC, openssh supports only 4 or 5 curves (P-256, P-384, P-521, Ed25519, Ed448), I see no reason not to support all of them... |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
test coverage missing
Not sure about Ed448 |
I am happy to add some tests if you think the code is looking ok-ish. |
Yeah, there is no support for Ed448 yet. |
yes, the code looks ok-ish, but python 2.6 compat and test coverage is mandatory, I won't merge the code otherwise. |
Added unit tests. CI for Py 2.x was working fine. |
you will need to use methods from https://github.com/tlsfuzzer/python-ecdsa/blob/master/src/ecdsa/_compat.py for py2 compat |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
CI failures are relevant
Switched to compat int_to_bytes. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
CI failures on Python 2 still.
Also, please note the code checks run: https://github.com/tlsfuzzer/python-ecdsa/actions/runs/6374029084/job/17310264458?pr=323
Switch to using |
Use |
More |
Fix typo. |
Any blocker for merging? :) |
yes, my time to do a final review :) thanks for the PR! |
Initial support for exporting keys in the SSH format as requested in #111.
For now it only supports Ed25519 curves but it shouldn't be hard to add additional ones.