Support ed25519 and secp256k1 in the import method

[satazor]

Hello!

I was trying to publish a IPNS record using a ed25519 key after seeing that support for ed25519 and secp256k1 keys was added here. I know that peer-id has to be changed to account for different key types, but I still decided to give it a try.

After trying, I concluded that js-ipfs calls crypto.keys.import and fails because the import method assumes only RSA keys.

I can make a PR that adds support for ed25519 and secp256k1 keys by first extracting the private key info stored in the pem and react accordingly.

[satazor]

//cc @vasco-santos @dignifiedquire @AlbertoElias

[AlbertoElias]

That would be amazing if you could get that in. I didn't know that import would be necessary for IPNS, and as it wasn't supported for ed25519 and secp256k1 beforehand, I didn't worry about it in my PRs

[satazor]

After checking node-forge, which we are using to import pkcs8 PEM files, I concluded it doesn't yet support ed25519 nor secp256k1 : digitalbazaar/forge#667

To do this properly, we would need to support PKCS8 either by implementing in node-forge or in a library. I'm more inclined in doing this in a library, which I actually already started doing. The reason is that I also need this for a identity project I'm working on (IDM) and I will share some more details soon.

[satazor]

The library is almost finished, here's a glimpse of it: https://github.com/ipfs-shipyard/js-crypto-key-composer/tree/initial-impl

It would be integrated like so:

• UsedecomposePrivateKey in the import method to understand the key type and then call the key type specific import method.
• Use composePrivateKey in the export method of each key type in order to to create PKCS1/PKCS8keys. This method supports rsa and ed25519 keys but it's easy to add more key types, such as ec (which includes secp256k1).

[dignifiedquire]

@satazor I think this is good step forward. Any chance you could drop the dependency on node-forge though? Depending on it as little as possible seems to make the most sense for the long term.

[satazor]

Here're the things we are depending on:

import pem from 'node-forge/lib/pem';

import { createBuffer } from 'node-forge/lib/util';
import sha1 from 'node-forge/lib/sha1';
import sha256 from 'node-forge/lib/sha256';
import sha512 from 'node-forge/lib/sha512';
import md5 from 'node-forge/lib/md5';
import pbkdf2 from 'node-forge/lib/pbkdf2';
import aes from 'node-forge/lib/aes';
import des from 'node-forge/lib/des';
import rc2 from 'node-forge/lib/rc2';

import random from 'node-forge/lib/random';

As you can see, we are depending on concrete files with functionality that we use 100%, except for util. We are not importing stuff like node-forge/lib/pki, node-forge/lib/pbe and node-forge/lib/rsa, which are monolithic files that contain functionality we don't use in libp2p-crypto. This is great because it will actually reduce the file size compared to what we have now.

To completely remove the dependency, we could either find equivalent libraries or copy them over, removing and refactoring whatever necessary. Note that this is something we can do now or down the road, as this package is part of ipfs-shipyard and may be moved to libp2p as well, so anyone can easily contribute. Let me know what you think.

[dignifiedquire]

Seems like there is at least some duplication there, we already have randomness, and aes defined in a way that doesn't require node-forge and is good in the browser and node in this repo.

For hashing we have everything outside of node-forge as well: the sha family is available from webcrypto, asmcrypto.js and node.js and md5 is available as a standalone version and available natively in node.js

This way we should only need des, pem , pbkdf2 and rc2 from node-forge.

[DougAnderson444]

Hi @AlbertoElias , what ever happened to PR#16 libp2p/js-libp2p-crypto-secp256k1#16 addressing this issue? Did it get merged into libp2p-crypto? Looks like everything stopped last April... but was nearly ready?

[jacobheun]

Getting support for PEM exporting/import is a bit painful at the moment due to lack of support in forge. Node.js crypto support has improved but needs v12+ support for ed25519 keys (it doesn't support seeding keys). Webcrypto doesn't support ed25519 yet, so we still need a lib for browsers.

To get around these issues in the short term, so we can get support sooner rather than later, the plan is to add support for export/import with the libp2p key format (serialized protobuf). We can still encrypt these with a password if provided to keep consistency there. The plan is to eventually support PEM and a variety of other formats, but that will require a significant effort to ensure everything is done properly (node support/browser support/bundle size constraints).

We can add support to each of the key types and still have RSA default to PEM to avoid breaking things. Later, we could include a breaking change to have RSA also default to libp2p key encodings.

I'm planning on moving forward with this approach as we want to support exporting/importing libp2p key encoding's anyway. If you have concerns/issues let me know. I'm hoping to land a PR this week.

[jacobheun]

I'm going to close this. We now support exporting/importing all 3 keys. PEM is only supported for RSA atm, but I will make sure we have an issue to track adding support to the other keys in the future.

This will be bubbled up to js-ipfs shortly.

[lidel]

PEM interop story continues in #244