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

identity: Implement protobuf encoding for RSA, ECDSA and secp256k1 keypairs #3630

Open
thomaseizinger opened this issue Mar 18, 2023 · 4 comments

Comments

@thomaseizinger
Copy link
Contributor

thomaseizinger commented Mar 18, 2023

Description

The spec is here: https://github.com/libp2p/specs/blob/master/peer-ids/peer-ids.md#key-types

Related: #3350 (comment).

Motivation

Being feature-complete and compliant with libp2p specs.

Are you planning to do it yourself in a pull request?

No.

@thomaseizinger thomaseizinger added difficulty:easy help wanted getting-started Issues that can be tackled if you don't know the internals of libp2p very well labels Mar 18, 2023
@drHuangMHT
Copy link
Contributor

ring doesn't seem to provide a way to "serialize" RSA keys. Should we implement it ourselves?

@thomaseizinger
Copy link
Contributor Author

ring doesn't seem to provide a way to "serialize" RSA keys. Should we implement it ourselves?

That is a bit unfortunate. There is only one way to construct the keypair, using from_pkcs8. We can store the original bytes next to the decoded key and use that when required. I don't see another solution unfortunately.

@thomaseizinger
Copy link
Contributor Author

thomaseizinger commented May 5, 2023

ring doesn't seem to provide a way to "serialize" RSA keys. Should we implement it ourselves?

That is a bit unfortunate. There is only one way to construct the keypair, using from_pkcs8. We can store the original bytes next to the decoded key and use that when required. I don't see another solution unfortunately.

We've discovered in #3681 that our spec requires PKCS#1 and ring only supports decoding from PKCS#8.

If we were to use the rsa crate, this issue would go away.

@mxinden What is your opinion on switching to the rsa crate to finish this issue? One of the practical implications is that the function for encoding to protobuf cannot be infallible because we need to return an error if somebody attempts to serialize an RSA keypair.

@mxinden
Copy link
Member

mxinden commented May 8, 2023

For others to follow along, past discussion: #1133

I don't have an opinion on this issue. Overall I would deem anything RSA related low priority. That said, I see the advantages that the gained consistency would bring us (see comment by @thomaseizinger above).

Maybe @dignifiedquire since you are one of the main authors and since you know libp2p well, do you have an opinion?

@thomaseizinger thomaseizinger removed help wanted difficulty:easy getting-started Issues that can be tackled if you don't know the internals of libp2p very well labels Oct 5, 2023
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.

3 participants