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

Add support for encryption #1190

Closed
chafey opened this issue Mar 8, 2017 · 99 comments
Closed

Add support for encryption #1190

chafey opened this issue Mar 8, 2017 · 99 comments
Labels
needs-design Needs design support. type-discussion
Milestone

Comments

@chafey
Copy link

chafey commented Mar 8, 2017

I have a use case where I want to send an encrypted message to an account managed by metamask. I can already get the public key for a metamask account from a digital signature, so I can encrypt data with that. What I now need is a way to either access the private key directly (yikes) - or better yet - pass the encrypted data to metamask to be decrypted using the private key - perhaps with a confirmation dialog. I could of course solve this by creating my own key pair and managing it myself, but I would much prefer to piggyback on metamask infrastructure. Ideally this encryption api would be made a standard JS API so other account management systems (e.g. MIST/geth) could implement it too.

@miohtama
Copy link

miohtama commented Mar 8, 2017

You might be aware of this already, but Go Ethereum includes Whisper protocol that provide plausible deniability messaging directly. The JavaScript API is available. However Whisper protocol infrastructure is not readily available as Ethereum itself yet.

@danfinlay
Copy link
Contributor

I also want this, I think we need to refine the API for it a bit first? We'd probably need both encrypt & recover, and we might want to add a prefix to any encrypted text, to prevent chosen-ciphertext attacks, the way personal_sign improves eth_sign.

I opened an EIP for this back in July!:
ethereum/EIPs#130

@2-am-zzz 2-am-zzz added this to the Public Release milestone Mar 13, 2017
@danfinlay
Copy link
Contributor

I think we should keep these api-layer discussions in public repos, so that other client devs can chime in.

If I were submitting again today, I'd submit here:
https://github.com/ethereum/interfaces

I also think EipDAO will improve the clients' ability to agree on how to implement this:
MetaMask/IPFS-Ethereum-Hackathon#14

@kjbbb
Copy link

kjbbb commented Dec 18, 2017

@danfinlay What do you think about adding it as a custom method in metamask alone, outside of Ethereum RPC? (e.g. in some custom namespace, like web3.metamask.encrypt, web3.metamask.decrypt). Considering MetaMask is largely a key-management tool, I will even argue that it's a critical feature. Moreover, as you're aware, it could open up a whole new suite of dApps which require privacy on the blockchain. While it's possible to do now, each dApp needs to design and implement their own crypto protocol - I think MM has the opportunity to really improve the state of things here, especially since it seems like the most popular gateway to Ethereum.

See parity's implementation: https://github.com/paritytech/parity/wiki/JSONRPC-parity-module#parity_encryptmessage

If I create a PR for this, what are the chances of it being merged?

@danfinlay
Copy link
Contributor

Some thoughts on using Parity's implementation as described by @christianlundkvist:

  1. Eth accounts are currently identified by hash(pubkey)
  2. This allows us to use address to sign stuff using recoverable signatures which works pretty well (“verify that this address signed this hash”)
  3. For encryption you need public keys, you can’t say “encrypt this to hash(pubkey)“, so we need to expose pubkeys alongside the addresses in order for encryption to work well
  4. Since ETH uses secp256k1 for keys people automatically want to use that key for encryption through ECEIS (my guess is that Parity is doing this)
  5. secp256k1 is not great for encryption according to most cryptographers (we’ve talked about this before), and ECEIS does not have great implementations
  6. curve25519 is much nicer curve for encryption, and the nacl is a world class library (the js version tweet-nacl is also security reviewed by cure58 and very well tested)
  7. Since the secp256k1 private keys are just random numbers these can be used as curve25519 private keys as well
  8. Thus, to each ETH address we can associate a curve25519 public key (computed from the secp256k1 private key) that can be used for encryption with the nacl library.
  9. This is another way to do “web3 encryption” that I like more

I'm torn on whether or not we should merge the parity method, but I know there is support for this, also from @gleim.

@danfinlay danfinlay removed the blocked label Mar 2, 2018
@danfinlay danfinlay removed this from the New UI Public Release milestone Mar 2, 2018
@kjbbb
Copy link

kjbbb commented Mar 2, 2018

For encryption you need public keys, you can’t say “encrypt this to hash(pubkey)“, so we need to expose pubkeys alongside the addresses in order for encryption to work well

I understand that's the major gotcha to the 'public key infrastructure' that I mentioned - it's missing the public key. BUT! considering the fact that the PK is leaked with every transaction, there is a wide distribution of keys. Moreover, just because a key isn't published, or a transaction hasn't appeared on the blockchain, doesn't mean such encryption isn't useful - infrastructure outside of Ethereum could exist.

Thus, to each ETH address we can associate a curve25519 public key (computed from the secp256k1 private key) that can be used for encryption with the nacl library.

Wouldn't this mean that we couldn't use existing Ethereum transactions (or personalSign) to derive public keys? If so, it disregards the value of the pool of keys that exist as transactions. That's a big downside to me, but not a deal killer, since the value of the pool of keys is speculative for now.

On a side note, I wonder if there are any security downsides using the same private key to derive public keys on different curves. I'm not a cryptographer :)

secp256k1 is not great for encryption according to most cryptographers (we’ve talked about this before)

Interesting, I didn't know that.

and ECEIS does not have great implementations

I agree, that's a major downside to me.

I'm torn on whether or not we should merge the parity method

Cross-platform compatibility is another good argument in favor.

After reading this, I'm thinking a web3 method to grab your public key would be nice so you don't need to sign anything. It could help alleviate the key distribution issue, especially if a different curve is used. There may be security downsides to this, but none are obvious to me.

Anyway, my original wishlist item was to use MM to encrypt private data for yourself. For that, I assume you could skip ECIES altogether and just aes, with a key derived from your PK somehow. When trying to implement in a dapp, I initially thought to use a signature for that, but it's prone to phishing so I aborted that idea :)

@1pocketaces1
Copy link

I have a dapp I have been working on that requires this exact functionality, but I cannot reasonably ask people to input their private key into my dapp.
Metamask is the window to this opportunity. If users could use their one (or many) eth addresses for transactions and encryption it would be a huge benefit.
Also, on the lack of encrypt/decrypt compatibility of current eth key pairs, I have found a workaround for this, but need to try it out after I can use web3 to allow a user to decrypt something with their private key, WITHOUT having to input it into my dapp (like through metamask).

I strongly support adding a metamask.encrypt and metamask.decrypt method. This would definitely open up an entirely new range of privacy dapps, and if integrated would allow a Metamask wallet to be a user's "account" to access and participate in all dapps everywhere!
It already is, but to stay that way we need to look past simple transactions to what the next steps of dapps will be, and provide the way to get there!

Thanks for the work guys

@kjbbb
Copy link

kjbbb commented Mar 15, 2018

After some more thought and some recent developments, I think it would be a good idea to add OpenPGP support to MetaMask. https://github.com/openpgpjs/openpgpjs. It was recently released, audited, and it supports a number of encryption algorithms (including secp256k1), and considering it's an open standard, it'd play much more nicely with everything else.

Though not specific to this project, I think it would be cool to establish a public key registrar in the form of a smart contract. It could exist, in simple terms, as a key/value store, and associate an address or identity with a public key. This way, any arbitrary asymmetric encryption algorithm could be supported, and people could update and revoke keys. A downside, however, is that it would cost a fee to publish a key, unless people wanted to use secp256k1 derived from signatures.

@danfinlay
Copy link
Contributor

@kjbbb that's a really interesting proposal, I'm a big fan of considering how MetaMask might handle keys of different types. I think it is a very long-sighted consideration, and I invite proposals of how a web3-with-other-key-types might work on issue #2532.

@danfinlay
Copy link
Contributor

danfinlay commented Apr 10, 2018

There is some momentum behind adding an eth_decryptMessage(addressHex, dataHex) method, which should be compatible with the [parity_decryptMessage](https://wiki.parity.io/JSONRPC-parity-module.html#parity_decryptmessage) method.

We would need design for a new confirmation screen to add this feature. The minimum design would be something like:

The site ______ has an encrypted file for you. Would you like to decrypt and download it?

Download Dismiss

@cjeria

@paulotrezentos
Copy link

Fully support a encrypt / decrypt feature, in the line of the sign interface.
This is a strong use case for dapps.

@danfinlay danfinlay added this to the Sprint 04 milestone Apr 16, 2018
@danfinlay
Copy link
Contributor

We have a pull request for an implementation of encrypt/decrypt in our signature management module right now, I'd appreciate input from anyone who'd like this feature over there.

@danfinlay
Copy link
Contributor

The UI requirements for this are:

  • Site X has an encrypted file for you.
  • At account Y.
    Would you like to download it?

@meronym
Copy link

meronym commented Jun 21, 2018

@danfinlay Does the decryption process pose a security risk that justifies the need for a confirmation window (hence an additional step that potentially breaks the UX flow of the app)? I'm considering the following use case:

The Buyer sends a payment tx to the Merchant, who confirms the payment and recovers Buyer's public key from the tx. The Merchant then uses this key to encrypt a passcode and sends it back to the Buyer. This cryptographically guarantees that only the originator of the payment can get access to the passcode and subsequently redeem the product/service.

The ideal UX flow won't expose the Buyer to any of these steps. The client-side app receives the encrypted payload in the background and silently decrypts it to obtain the passcode, then includes this passcode in various other requests to a standard web back-end app (this passcode can either behave like an api key or like a third-party payment receipt).

In this case, confirming the decryption through a popup and downloading the decrypted payload as a file, then uploading the file back into the app is bad UX from the Buyer's perspective.

@danfinlay
Copy link
Contributor

We have not solidified the UX for this feature, but we’ll take this point into account when we next discuss it.

@th-ink
Copy link

th-ink commented Jul 18, 2018

I would love to see this encrypt / decrypt function implemeneted in MetaMask as this would make life for DApp developers so much easier. I am currently developing a privacy document sharing DApp where the sharing is done by encrypting document's file key with other user's public key. But handling the key generation and enc / dec in the app and blockchain is not ideal.

@rymnc
Copy link

rymnc commented Jun 18, 2020 via email

@benjaminaaron
Copy link

@Gudahtt This is awesome, can't wait for v8.0.0 to land in the extension stores. Should be anytime now after the release?
Do you know when support for encryption will be added to MetaMask mobile too? That'd be important for developers (like myself) wanting to use public key encryption in web apps build for desktop and mobile.

@bguiz
Copy link
Contributor

bguiz commented Jul 2, 2020

Should be anytime now after the release?

@benjaminaaron The implementation of this feature started in October 2019, and completed in February 2020 - so we've all been waiting a really really long time for this to get included in a new release. 😞

@dulcedu
Copy link

dulcedu commented Jul 2, 2020

@benjaminaaron @Gudahtt @rekmarks @jennypollack @danfinlay Hello metamask team! :) I have a dapp I have been working on, that requires this functionality. I will appreciate if your help in the next release.
#Privacymatter #Staysafe

@Gudahtt
Copy link
Member

Gudahtt commented Jul 3, 2020

This feature was released in v8.0.0. It is available in Firefox today, and will be available on Chrome as soon as the update is accepted into the Chrome Web store. The Chrome review process can be quite slow; we don't know how long that'll take.

@lastmjs Unfortunately the Ethereum public key is not sufficient to encrypt a message using this API. You'd need to get the encryption public key specifically, which is obtained via eth_getEncryptionPublicKey. This public key is derived from the private key using TweetNaCl.js (which you can see here).

The decision to go with Curve25519 over ECIES was made before I got involved in this issue, and discussion about this occurred in a number of different places, so I'm not sure where the best explanation for this decision is. I found this comment to be useful though, and the draft EIP for these methods is a good reference as well. I'd suggest making your case in the EIP PR if that's something you'd like to pursue.

@benjaminaaron:

Do you know when support for encryption will be added to MetaMask mobile too? That'd be important for developers (like myself) wanting to use public key encryption in web apps build for desktop and mobile.

I'm not sure, but I'd encourage you to create an issue in the metamask-mobile repo to ask about it.

@Amirh24
Copy link

Amirh24 commented Aug 23, 2021

What if I want to encrypt a message and send it to any Ethereum address? I know that I can get the Ethereum public key of any address, as long as that account has signed and sent at least one transaction. Can I use that Ethereum public key to encrypt a message that MetaMask will be able to decrypt?

I'm hoping that MetaMask isn't deriving an encryption public key from the private key somehow, otherwise this might eliminate the use case of encrypting messages and sending them to any address, and allowing the owner of the private key associated with the address to decrypt with MetaMask.

If my thinking above is correct, then my next question would be, could we consider allowing other decryption algorithms to MetaMask, like ECIES? Though the security properties might be worse, can we leave that up to developers to decide what algorithms they should use, and not restrict use cases?

This is very much needed. Is this considered? There are a lot of use cases for this kind of functionality

@pokrovskyy
Copy link

@Gudahtt

@lastmjs Unfortunately the Ethereum public key is not sufficient to encrypt a message using this API. You'd need to get the encryption public key specifically, which is obtained via eth_getEncryptionPublicKey. This public key is derived from the private key using TweetNaCl.js (which you can see here).

This is a huge disappointment. I was hoping for this actual behaviour - to be able to encrypt data with public key retrieved from public blockchain so that Metamask could then use the private key for decryption. Deriving the public key from the private key under the hood eliminates this crucial use-case and locks use-cases solely for encryption/decryption within the Metamask.. I do not understand this decision. I would understand if the two options were offered together though. Please reconsider this. Thank you!

@TJKoury
Copy link

TJKoury commented Mar 9, 2022

I'm building a proof-of-concept app using my own key conversion library, that does exactly what everyone here wants: grab a public key from a transaction and do ECIES encryption for it.

Once the app is build I'll see if I can do a pull request to integrate it into the MetaMask suite of apps.

@omnat
Copy link

omnat commented Mar 9, 2022

Ok, we can review the pull requests @TJKoury. It's not in our immediate radar though and will require some research on our end to review the solution you are suggesting and ensure its secure before accepting it.

@cryptoKevinL
Copy link

I would like add my two cents in as well, the power to encrypt data to an Ethereum wallet and have MM decrypt the data is very useful in many applications. I spent a solid 10 hours searching and building an application using the desired method here only to realize the key differences. Would love this option and help test however needed.

@TJKoury
Copy link

TJKoury commented May 25, 2022

Just FYI, my plan is to submit my implementation to the GameStop wallet team.

@cryptoKevinL
Copy link

Just FYI, my plan is to submit my implementation to the GameStop wallet team.

Hey @TJKoury - do you have a link of where to propose this to GameStop? I have a use case for this which would be cool to propose to them as well.

@TJKoury
Copy link

TJKoury commented May 26, 2022

@cryptoKevinL I just sent a support e-mail and they responded.

@kunalgoyal9
Copy link

@danfinlay @kjbbb @TJKoury Can we do public key encryption from the transaction signature (secp256k1) and decrypt it using eth_decrypt method?

I badly need this

@cryptoKevinL
Copy link

I think they are actually decommissioning the existing functionality if I have heard right?

@omnat
Copy link

omnat commented Aug 8, 2022

@bschorchit are you looking at encrypt/decrypt from Product side?

@cryptoKevinL
Copy link

cryptoKevinL commented Aug 8, 2022

@bschorchit are you looking at encrypt/decrypt from Product side?

@omnat - I have wanted this as well, I have a wallet-to-wallet chat application which currently uses some secondary keys generated by our app for signing and encryption, because MM does not support encryption from the public key which can be derived from an Ethereum address. Asking users via the eth_getEncryptionPublicKey (which I think I saw is being depreciated) is also inconvenient, we want to encrypt data to a user without having to ask this question first.

@foufrix
Copy link

foufrix commented Jan 18, 2023

I see that eth_decrypt will be removed in the future in Metamask docs, so encrypting/decrypting messages using metamask is not something that will ever work?
So sad as it can be a useful feature for exchanging data between wallet in a secure way even if public

@seongyun-ko
Copy link

EIP-5630 is created last year. It resolves the security risks of the current encryption/decryption mechanism;
https://eips.ethereum.org/EIPS/eip-5630.
We should support EIP-5630 :D

@ahmaddradii
Copy link

Are there alternative solutions to dispense with the encryption and decryption of Metamask ?

@ahmaddradii
Copy link

I see that eth_decrypt will be removed in the future in Metamask docs, so encrypting/decrypting messages using metamask is not something that will ever work? So sad as it can be a useful feature for exchanging data between wallet in a secure way even if public

Are there alternative solutions to dispense with the encryption and decryption of Metamask ?

@Oli-art
Copy link

Oli-art commented Jun 18, 2023

EIP-5630 is created last year. It resolves the security risks of the current encryption/decryption mechanism; https://eips.ethereum.org/EIPS/eip-5630. We should support EIP-5630 :D

I'm making EIP for a notications protocol (EIP-7017) and considering EIP-5630, as DMs MUST be encrypted. There's a need for an easy way to encrypt and decrypt data in order for it to be used. Please consider implementing EIP-5630.

@dzimbeck
Copy link

I don't understand why the hesitation? First of all, users don't have to use their actual public key but a derivation of it. This prevents them from financial loss. The obviously threats of cryptoanalysis are thwarted when for example you hash the accounts private key a million times and then just share the public key of that from the thing. So even "irresponsible" developers with poor implementation can be safe. Furthermore, there's various methods that can be used to avoid chosen-plain-text attacks by having some sorts of padding with different strategies and more advanced forms of cryptography. In fact, this "derived" public key (that also uses a weird salt that nobody else would use outside of devs) should be available instantly via a web3 request without needing to do annoying ecrecover on a signature which is already a bit of a turnoff to newbies to ETH. I'm not saying that a signing key is less important(since it compromises identity of a user) but consider that you can always "recover" your identity if lost to cryptoanalysis by proving you have a "prior" hash than the one that was lost. Not a full mitigation of an attack but a nice bonus of using a derivation of the privkey. Am I mistaken in my logic here? This is a trivial thing.

@devYonz
Copy link

devYonz commented Jan 26, 2024

Same, Metamask can be the most widely used customer owned KMS and it's being prevented from being used as such. The argument in the video can be mitigated with key derivation but it doesn't even seem like it was considered.

Various applications:

  • Offline login by having sqlite.db encrypted with an app specific public key
  • DMs
  • GPG like solution for emails
  • etc.

@jacquesvcritien
Copy link

What is the status on this please?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs-design Needs design support. type-discussion
Projects
None yet
Development

No branches or pull requests