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

Support reading Secp256k1KeyIdentity from pem #622

Open
ncpenke opened this issue Sep 1, 2022 · 5 comments
Open

Support reading Secp256k1KeyIdentity from pem #622

ncpenke opened this issue Sep 1, 2022 · 5 comments
Assignees

Comments

@ncpenke
Copy link
Contributor

ncpenke commented Sep 1, 2022

Is your feature request related to a problem? Please describe.
Currently, it's not possible to read a Secp256k1KeyIdentity in agent-js via pem. This is possible in agent-rs.

Describe the solution you'd like
Support reading pem format in Secp256k1KeyIdentity.

Describe alternatives you've considered
Custom parsing solutions, and using to/fromJSON, which seems incompatible with agent-rs.

Additional context
This work came out of dscvr.one, where we're migrating some existing javascript to rust. A key issue was sharing keys in a format that worked for both agent-js and agent-rs. We've implemented a solution using the https://github.com/starkbank/ecdsa-node package, which supports reading from a key pair generated by openssl. This format is also compatible with agent-rs. If this is within the scope of this package, I can submit a patch.

@krpeacock
Copy link
Contributor

I've debated it for a while - I'm hesitant to support fromPem and fromSeed, out of the concern that it might encourage risky patterns of UX flows where users copy around high-sensitivity secrets and increase the risk of clipboard and phishing attacks.

The benefits of supporting Node.js application developers probably outweigh those concerns though, and I think that you can go ahead with that PR, @ncpenke

@ncpenke
Copy link
Contributor Author

ncpenke commented Sep 11, 2022

Thanks @krpeacock. I ran into an issue: the ecdsa-node package we used is pure javascript, and doesn't yet have typescript bindings.

Three approaches come to mind:

  1. Add a minimal set of typescript bindings to ecdsa-node. But I didn't see any other @types used as dependancies, so wasn't sure if this was something you were intentionally avoiding.
  2. Add DER decode logic for private keys and the necessary PEM parsing code.
  3. Abandon this issue in this package, and add support to import JSON keys to agent-rs.

Our primary use-case in dscvr, which could be useful to the broader community is interoperability between agent-rs and agent-js, so 3 would be sufficient to achieve this as well. Please let me know what you think would be the best. Thank you!

@krpeacock
Copy link
Contributor

krpeacock commented Oct 10, 2022

We aren't deliberately avoiding @types - we just try to keep our dependencies minimal, and most of the packages we've gone with had typescript support.

I think the bigger issue would be new node-specific dependencies.

@ncpenke an easier workflow would probably be to add seed phrase import support to Secp256k1KeyIdentity, and to ensure it matches dfx identity import --seed-file. What do you think?

@ncpenke
Copy link
Contributor Author

ncpenke commented Oct 11, 2022

@krpeacock That sounds great! Only use-case this doesn't handle is if there are key pairs generated by agent-js that are stored in JSON form, that would be useful to use via agent-rs. Do you think it would make sense to support JSON key/pairs in agent-rs?

@krpeacock
Copy link
Contributor

I think that sounds reasonable

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

No branches or pull requests

2 participants