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

OpenSSH public key serialization #1744

Closed
reaperhulk opened this issue Mar 11, 2015 · 11 comments · Fixed by #2957
Closed

OpenSSH public key serialization #1744

reaperhulk opened this issue Mar 11, 2015 · 11 comments · Fixed by #2957

Comments

@reaperhulk
Copy link
Member

We should support serialization of public keys to OpenSSH format.

@adiroiban
Copy link
Contributor

This is my code based on Twisted SSH keys https://github.com/chevah/chevah-keycert

it no longer depends on Twisted and on top of twisted it add support for SSH.Com (Tectia) and Putty keys... fixed for parsing invalid keys

If you want I can help integrate the code with python cryptography.

@reaperhulk reaperhulk added this to the Thirteenth Release milestone Jan 2, 2016
@reaperhulk
Copy link
Member Author

This issue boils down to essentially reversing the code here: https://github.com/pyca/cryptography/blob/master/src/cryptography/hazmat/primitives/serialization.py#L35 (plus adding a new serialization.PublicFormat type and supporting it in the public_bytes methods of the public keys).

@reaperhulk
Copy link
Member Author

One challenge here: our public_bytes method takes an encoding and a format, but for OpenSSH only the format is really meaningful (there's no DER/PEM for openssh public keys). Ideas on how to handle this? We could allow/require None for encoding in this case potentially, but meh.

@alex
Copy link
Member

alex commented Mar 15, 2016

Encoding=SSH(), format=SSH()?

On Mon, Mar 14, 2016 at 11:10 PM, Paul Kehrer notifications@github.com
wrote:

One challenge here: our public_bytes method takes an encoding and a
format, but for OpenSSH only the format is really meaningful (there's no
DER/PEM for openssh public keys). Ideas on how to handle this? We could
allow/require None for encoding in this case potentially, but meh.


You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
#1744 (comment)

"I disapprove of what you say, but I will defend to the death your right to
say it." -- Evelyn Beatrice Hall (summarizing Voltaire)
"The people's good is the highest law." -- Cicero
GPG Key fingerprint: D1B3 ADC0 E023 8CA6

@reaperhulk
Copy link
Member Author

We can do that. We'll need to audit every place we currently use the Encoding enum to make sure we're erroring properly when you pass that value, but we'd have to do that with None as well.

@alex
Copy link
Member

alex commented Mar 15, 2016

I think the worst case scenario is an assertionerror

On Mon, Mar 14, 2016 at 11:15 PM, Paul Kehrer notifications@github.com
wrote:

We can do that. We'll need to audit every place we currently use the
Encoding enum to make sure we're erroring properly when you pass that
value, but we'd have to do that with None as well.


You are receiving this because you commented.
Reply to this email directly or view it on GitHub:
#1744 (comment)

"I disapprove of what you say, but I will defend to the death your right to
say it." -- Evelyn Beatrice Hall (summarizing Voltaire)
"The people's good is the highest law." -- Cicero
GPG Key fingerprint: D1B3 ADC0 E023 8CA6

@reaperhulk
Copy link
Member Author

I'm working a bit on this. Will hopefully have time to put together a full PR tonight. Here's roughly what serializing an RSA public key looks like:

            e = self._bn_to_int(cdata.e)
            n = self._bn_to_int(cdata.n)
            e_bytes = utils.int_to_bytes(e)
            if ord(e_bytes[0]) & 0x80 != 0:
                e_bytes = b"\x00" + e_bytes
            n_bytes = utils.int_to_bytes(n)
            if ord(n_bytes[0]) & 0x80 != 0:
                n_bytes = b"\x00" + n_bytes

            payload = base64.b64encode(b"{0}{1}{2}{3}{4}{5}".format(
                struct.pack(">I", 7),
                b"ssh-rsa",
                struct.pack(">I", len(e_bytes)),
                e_bytes,
                struct.pack(">I", len(n_bytes)),
                n_bytes,
            ))
            return b"ssh-rsa {0}".format(payload)

@adiroiban
Copy link
Contributor

Latest Twisted code was ported to use pyca/cryptography

twisted.conch.ssh.keys was also recently ported to py3 and has pretty good test coverage.

Are you interested in re-using that code in pyca/cryptography ?

Once you have decided on an API I can work on implementing that API based on twisted.conch.ssh.keys.

Once that code is in pyca/cryptography I can updated Twisted to use the shard code from pyca/cryptography.

@reaperhulk
Copy link
Member Author

If you'd like to work on this I'm happy to define the API. It's just an expansion of enums for our current public_bytes method. So public_bytes(encoding, format) where right now Encoding is an enum of PEM/DER and PublicFormat is an enum of subjectPublicKeyInfo/PKCS1 and we'd add "OpenSSH" to each of them (since PEM/DER has no meaning for OpenSSH). So a public key would be serialized via:

public_key.public_bytes(Encoding.OpenSSH, PublicFormat.OpenSSH)

I'm not sure if this work will make it into 1.3 (as that's going out this week) but we can definitely land it for 1.4.

@alex
Copy link
Member

alex commented May 30, 2016

@adiroiban Were you interested in contributing this, or shall we go ahead and port the code ourselves?

@adiroiban
Copy link
Contributor

I am interested into SSH key format support, but I was not aware of the scope of this ticket.

I was looking to see if for example we could completely replace twisted.conch.ssh.keys (https://github.com/twisted/twisted/blob/trunk/twisted/conch/ssh/keys.py) with something implemented here.

From the current feedback, it looks like the scope of this ticket is very specific and target only the OpenSSH key encoding on disk, without support the extra comment field or supporting multiple formats.

At this stage, I think that is better for someone with a good understanding of the scope of this ticket to implement it. Later we can discuss if it make sense to have a high level API in cryptography to manage SSH keys.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 16, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Development

Successfully merging a pull request may close this issue.

3 participants