agent-js icon indicating copy to clipboard operation
agent-js copied to clipboard

Support reading Secp256k1KeyIdentity from pem

Open ncpenke opened this issue 3 years ago • 5 comments

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.

ncpenke avatar Sep 01 '22 07:09 ncpenke

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

krpeacock avatar Sep 06 '22 19:09 krpeacock

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!

ncpenke avatar Sep 11 '22 23:09 ncpenke

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?

krpeacock avatar Oct 10 '22 21:10 krpeacock

@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?

ncpenke avatar Oct 11 '22 19:10 ncpenke

I think that sounds reasonable

krpeacock avatar Oct 12 '22 16:10 krpeacock