[Feature request] Support eth_decrypt - metamask encryption and decryption methods.

[MoMannn]

Since metamask never exposes the private key of a wallet they added another way of encrypting and decrypting data:
https://docs.metamask.io/guide/rpc-api.html#eth-decrypt
https://docs.metamask.io/guide/rpc-api.html#eth-getencryptionpublickey
https://docs.metamask.io/guide/rpc-api.html#encrypting

Since metamask is "special" in this way it is very hard to support encrypting in multiple wallets. So this is a request to add metamasks methods to ethers.js.

Here is how MetaMask can decrypt messages:

https://github.com/MetaMask/eth-sig-util/blob/3d96bb1e1e5d1a1c548095b18e17fdb2802abe5b/src/index.ts#L503-L537

  // string to buffer to UInt8Array
  const recieverPrivateKeyUint8Array = nacl_decodeHex(receiverPrivateKey);
  const recieverEncryptionPrivateKey = nacl.box.keyPair.fromSecretKey(
    recieverPrivateKeyUint8Array
  ).secretKey;

  // assemble decryption parameters
  const nonce = naclUtil.decodeBase64(encryptedData.nonce);
  const ciphertext = naclUtil.decodeBase64(encryptedData.ciphertext);
  const ephemPublicKey = naclUtil.decodeBase64(
    encryptedData.ephemPublicKey
  );

  // decrypt
  const decryptedMessage = nacl.box.open(
    ciphertext,
    nonce,
    ephemPublicKey,
    recieverEncryptionPrivateKey
  );

  // return decrypted msg data
  let output;
  try {
    output = naclUtil.encodeUTF8(decryptedMessage);
  } catch (err) {
    throw new Error('Decryption failed.');
  }

  if (output) {
    return output;
  }
  throw new Error('Decryption failed.');

Source:

https://github.com/MetaMask/eth-sig-util/blob/3d96bb1e1e5d1a1c548095b18e17fdb2802abe5b/src/index.ts#L503-L537
https://github.com/MetaMask/eth-simple-keyring/blob/b23bbaf5cb2ac21a33ebaafc567af6cb1cb57cae/index.js
https://github.com/MetaMask/eth-simple-keyring/blob/b23bbaf5cb2ac21a33ebaafc567af6cb1cb57cae/README.md
https://github.com/MetaMask/eth-simple-keyring/search?q=decryptMessage&type=code
https://github.com/MetaMask/KeyringController/blob/master/package.json#L48
https://github.com/MetaMask/KeyringController/blob/1a0dbbaf45e54e7b80200d4a9c6b21ce74b61b2f/index.js#L424
https://github.com/MetaMask/metamask-extension/blob/38fe75b7d9255b0fb7515ef2f78250a5b9791aa3/app/scripts/lib/encryption-public-key-manager.js#L82

[fulldecent]

💰 I commit a bounty of USD 250 to anybody that can solve this issue. Terms:

• The PR must follow all project contributing guidelines here
• The PR must be accepted here into the main branch by 2021-07-02
• The PR must address the required features in this issue
  • Implement functions compatible with eth-decrypt, eth-getencryptionpublickey, encrypting above.
  • Include a test case (automated or just a video) showing cross compatibility with MetaMask for file encryption.
• Payment made by PayPal, any taxes/fees are your responsibility

[ricmoo]

Please don’t jump on opening a PR or working on this until I have a chance to evaluate whether this is something that makes sense to add to ethers.

I’ve considered adding similar features to ethers before, but the consequences and caveats to making encryption “easy” require people to think carefully what they are doing. The example I usually give of this is that when data is encrypted, the length does not change, so you could imagine encrypting a vote for “yes” vs “no”; even when encrypted, if the encrypted text is 3 bytes long, the input was “yes”. So appropriate padding (or random size) is essential to prevent side-channel attacks.

I’ll research this more. Ideally there is an EIP in progress for something like this, and then the ability can be added more genetically to Signer. :)

[ricmoo]

(Or if you would like, an ancillary package doesn’t require my blessings on any level, if you want to jump the gun ;))

[MoMannn]

@ricmoo This is more request to copy the functionality of metamask as is. Since we would like compatibility to encrypt with ethers and decrypt via metamask and the other way around.

Normally you could use the wallets public key to encrypt and its private key to decrypt but metamask does not allow access to the private key via interface and has completely different way of generating an encryption public key and a metamask only eth_decrypt method to decrypt this. This makes it a pain in the ass for compatibility and since metamask will not change their ways the other solution is to support this here 🤷‍♂️

[ricmoo]

Right. My concern is adding methods to Provider that only work on MetaMask. It is important to keep things abstract, and come up with standards so things continue to work in the future and for users not using metamask.

I just want a moment to research this and reach out to MM before I get flooded with a bunch of PRs because of a bounty offered on this. I am quite particular with making changes, especially changing/adding nee methods to fundamental classes.

If the standard is sound, or on its way to broader adoption (EIP or other wallets following suite), then I can also add it to the Wallet object and other Signers.

[zemse]

Just to add to this, you can still use provider.send for any specific functionality like the mentioned methods of metamask, until this is looked upon.

await window.ethereum.enable()
const provider = new ethers.providers.Web3Provider(window.ethereum);
const accounts = await provider.listAccounts();
const pubkey = await provider.send('eth_getEncryptionPublicKey', [accounts[0]]);
console.log(pubkey); // zpKOsHVU1YdbTKwZJ4u/YBSsu+q6VxJvTfnU8LLCmCg=

The docs are available here.

[MoMannn]

@ricmoo Ok, I get it :) Will wait for you decision.

@zemse Yeah the problem here is that users don't have metamask installed. Encrypt and decrypt should be supported without a provider needed since there are not actually blockchain operations if you know what I mean...

[ricmoo]

Now I’m confused again. How would encrypt and decrypt work if they don’t have MetaMask installed?

[zemse]

Oh, so this feature request is about encryption and decryption methods like metamask to be implemented for general wallets/signers. If there is a signer e.g. Wallet, that keeps the private key with it, then it would work. But if there is a Signer other than metamask (like a ledger or trezor) which similarly doesn't expose private key, then how can those methods be implemented for them? Pls correct me if I am misunderstanding.

[MoMannn]

@zemse Exactly. No it would not work for ledger or trezor. This are not actually blockchain functionalities its just metamasks way to reveal them as eth_decrypt and eth_getEncryptionPublicKey RPC calls. And the encryption itself is already a standalone lib as you can see here: https://docs.metamask.io/guide/rpc-api.html#encrypting

So if you have a wallet with privateKey it should be possible to generate the same encryption key as in eth_getEncryptionPublicKey call and be able to decrypt as in eth_decrypt.

[zemse]

Ok, so it sounds like these methods can only be implemented for Wallet class since it is supposed to be initialized with a private key, while no changes in general Signer.

You can still extend the Wallet class and add those methods to it, you can even publish the package.

class WalletExtended extends ethers.Wallet {
    // implement these methods
}

Do you want to add any idea as to why this should be included in the ethers.js code base (since the code snippet appears to contain some dependencies), this might help @ricmoo to take a positive decision.

[MoMannn]

Yeah my point of view is that with NFTs and other kinds of blockchain applications encrypting / decrypting or to better say restricting access to only a specific person / owner will become more and more important. Wallets are literally private/public keys and are very useful for this kinds of applications. Like you can encrypt a file with someones public key only because he has a specific token in his wallet. And he and only he will be able to decrypt it with said wallet. The problem arises with compatibility. There is no coherent way to encrypt / decrypt across different wallets. I believe metamasks approach is ok since this can then also get supported by wallets like ledger and trezor. And having this supported in ethers will further approve the options on the market and allow more secure / better applications. So I only see reason too support rather then not.

[ricmoo]

This is one reason I want to open a dialog with MetaMask too. There are actually EIP-191 compatible ways to do this, which would mean Ledger and any weird signer (that manages a key) could perform these operations.

In a nutshell, you would add a new version to EIP-191 with some qualified indentifying field (probably the hash of the origin?), sign that and then hash that to get the private key. Then any encrypted data should have an ephemeral session key randomly chosen and the public key pretended tot be encrypted data, used for the ECDH shared secret. I’m always a fan of the CTR mode of operation, but CBC might make more sense, since we want the size to be fuzzy anyways.

By choosing an unused version we can be relatively assured that data has not been signed before. Also this require RFC-6979 be used, but most popular signing libraries do that already.

This way won’t be backwards compatible with existing keys, but the sooner we move to the generalized solutions, the better. :)

And we can provide some alternate way methods for legacy encryption...

[ricmoo]

(this is something I have thought a lot about, but want it down properly, not just stapled in ;))

[MoMannn]

@ricmoo I am all for opening a dialog and standardising a way :) So if there are any discussions open I would love to contribute. I would also like to ship apps with best UX asap 😂 But yeah I understand.

[paulmillr]

Just my 5 cents: adding x25519-xsalsa20-poly1305 would increase pkg size. That's three abstractions: curve25519, XSalsa20, Poly1305 - neither of which are browser built-ins.

[paulmillr]

I don't know why everything has to be complicated.

• secp256k1 that we're already using - could have been used for diffie-hellman. No additional code.
• aes-gcm could have been used instead of xsalsa20-poly1305. aes-gcm is browser & nodejs built-in and can be abstracted into two cross-platform methods in 100 loc and 0 deps (just copy-paste this https://github.com/paulmillr/micro-aes-gcm/blob/master/index.js)
• both are good enough for this!

[MoMannn]

Can we open a discussion with metamask regarding this so we can find a solution that works on both ends?
@fulldecent Can you help with this?

[fulldecent]

I understand that there is interest in a new standard for encrypting/decrypting files. That effort is identified as requiring:

• Design a new process
• Pass an EIP
• Implement a different RFC
• Agree with MetaMask
• Broad industry support with Ledger, other hardware wallets

In my experience, it is infeasible that this can be accomplished in 2021 with the slowest part being MetaMask and firmware upgrades. For reference I can say that it has taken several years for MetaMask to adopt ERC-721 and their support was minimal at that. I am a biased source here but some people consider 721 to be a super-important use case.

Please use a new issue to discuss any new possible standards and achieve consensus with other projects.

For this issue I hope we can continue to discuss making Ethers.JS compatible with the existing and well-documented (i.e. open source software) process that MetaMask already supports.

I can help by bringing resources to this effort. I quadruple my original bounty (now USD 1,000) and extend this commitment to 2021-07-31 (previously 2021-07-02).

[fulldecent]

For this issue I hope we can continue to discuss making Ethers.JS compatible with the existing and well-documented (i.e. open source software) process that MetaMask already supports.

I can help by bringing resources to this effort. I octuple my original bounty (now USD 2,000) and extend this commitment to 2021-09-07 (previously 2021-07-02).

[ricmoo]

Is there an EIP for this yet?

[fulldecent]

No. I make EIPs after multiple competing implementations of a thing exist.

[imsys]

Related:
ethereum/EIPs#130
LedgerHQ/app-ethereum#189
ethereum/EIPs#1098 (closed)

ethereum/EIPs#2844 (current)
https://eips.ethereum.org/EIPS/eip-2844
ethereum/EIPs#2845 (discussion about EIP 2844)

[crazyrabbitLTC]

I've been following this but only decided to chime in now. Chasing universal standards and support feels....endless. Maybe for OP it might be best to just require users to have MetaMask and leave it at that. Support for Ledger and Trezor and other hardware wallets feels like it's always "on the edge" of breaking. I'm not sure if wallet connect supports this...

EDIT: I agree with @fulldecent below. It would be nice to have this in ethers.

[fulldecent]

I have participated in and delivered major standards including ERC-721 and IEEE 802.3 ten gigabit ethernet, and I agree standards should not be embarked on lightly.

But this issue is much more modest and limited in scope: it is simply asking one major JavaScript framework, Ethers, to be compatible with another major browser extension, MetaMask.

[amirgamil]

hello! Is there an update on this?

[ricmoo]

@amirgamil Have there been any progress in terms of a standard and adoption?

[ricmoo]

Keep in mind any feature in ethers must a long time, being maintained and kept backwards compatible. It must also be included in every dapp that uses ethers; for this reason, and keeping things lean, an attempt is made to find a balance between features that are used frequently by a large number of users and keeping the library lean.

It is fairly trivial to add additional libraries that work against ethers porcelain API; for example, creating an Cryptor object that accepts a JsonRpcSigner, which can use the signer.provider.send method for JSON-RPC interaction.

Ethers makes an effort to remain flexible and extensible so that application-specific features are easy to add.

I don't believe a core library should add features that are not mature and more widely adopted; this is how we get the eth_signTypedData_v4, where there are _v1, _v2, _v3 and I believe an newer _v5 variant, all of which are slightly incompatible and apps break because they were designed against an older API and the new API is insufficient to map all features from one to the next.

This is the job os an extension library, until all the sharp corners are smoothed over.

[paulmillr]

And I want to reiterate we must not add new algorithms just for a bunch of new features. Cryptography is hard, having different cryptography everywhere is bad.

secp256k1 + aes can do their job. There is no need to bring in Nacl/secretbox. If someone would be implementing a EIP or something, make sure it's not nacl.

[crazyrabbitLTC]

Just checkin as well!

[ricmoo]

Moving this to the "Ideas Discussion". This may make more sense as an extension package, as the additional size requirements seem like they may be substantial.