Skip to content
This repository has been archived by the owner on Jul 21, 2023. It is now read-only.

Proposal: Separate key export functionality #190

Open
Gozala opened this issue Jun 29, 2021 · 3 comments
Open

Proposal: Separate key export functionality #190

Gozala opened this issue Jun 29, 2021 · 3 comments
Labels
exp/expert Having worked on the specific codebase is important P3 Low: Not priority right now

Comments

@Gozala
Copy link

Gozala commented Jun 29, 2021

I've been working on updating this implementation to align it with libp2p/js-libp2p-interfaces#100 and I am noticing that export function on private keys introduces unnecessary dependencies as in export functionality is is not key specific. So I would suggest factoring that functionality out into separate interface e.g.

interface KeyExporter<Format extends string = string> {
   format: Format
   export(key:PrivateKey, password?:Uint8Array): Uint8Array
}

This would make remove indirection and instead of doing e.g.

key.export('my secret', 'pkcs-8')
key.export('my secret', 'libp2p-key')

One would be a able to do following instead:

pkcs8.export(key, 'my secret')
libp2pExporter.export(key, 'my secret')

We did a similar change in js-multiformats for base encoding and hashing where instead of passing identifiers you pass references and it simplified things greatly.

@Gozala
Copy link
Author

Gozala commented Jun 29, 2021

Also if we really want to we could still keep export method on PrivateKey but we could just swap format identifier with KeyExporter as described above so that same example could be rewritten as follows:

key.export('my secret', pkcs8)
key.export('my secret', libp2pKey)

@Gozala
Copy link
Author

Gozala commented Jun 29, 2021

I see now that pkcs8 uses rsa specific node-forge calls

const privateKey = forge.pki.privateKeyFromAsn1(asn1)
const options = {
algorithm: 'aes256',
count: 10000,
saltSize: 128 / 8,
prfAlgorithm: 'sha512'
}
return forge.pki.encryptRsaPrivateKey(privateKey, password, options)

Which may explain why export was made key specific. Yet I still think my argument still stands and we could even get compile time compatibility errors when user attempts to export incompatible key

image

@BigLep BigLep added exp/expert Having worked on the specific codebase is important P3 Low: Not priority right now labels Nov 12, 2021
@BigLep
Copy link

BigLep commented Nov 12, 2021

2021-11-12 discussion: to pull this off, we need an "awesome endeavor" to pull all the crypto stuff apart so can have reduced bundle size.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
exp/expert Having worked on the specific codebase is important P3 Low: Not priority right now
Projects
None yet
Development

No branches or pull requests

2 participants