Closed chafey closed 4 years ago
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.
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!: https://github.com/ethereum/EIPs/issues/130
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: https://github.com/MetaMask/IPFS-Ethereum-Hackathon/issues/14
@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?
Some thoughts on using Parity's implementation as described by @christianlundkvist:
nacl
is a world class library (the js version tweet-nacl
is also security reviewed by cure58 and very well tested)nacl
library.I'm torn on whether or not we should merge the parity method, but I know there is support for this, also from @gleim.
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 :)
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
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.
@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.
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
Fully support a encrypt / decrypt feature, in the line of the sign interface. This is a strong use case for dapps.
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.
The UI requirements for this are:
X
has an encrypted file for you.Y
.
Would you like to download it?@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.
We have not solidified the UX for this feature, but we’ll take this point into account when we next discuss it.
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.
I think the key used for this should be typed - there should be a purpose associated with the key, a key for sending money shouldn't be used for encrypting messages and vice versa. Similarly, a key should be bound to the origin and follow the security framework of the browser.
The user might not care or need to care, through key derivations, but an attack where the user is made to decrypt data belonging to another site, decrypt data of the wrong type etc. should not be possible.
There might be a "generic" type, but there should be specific types for things such as messages, auth, money, profile data etc.
The JWT claims standard might be used for inspiration.
Any update on if/when this is coming?
Would deriving a private key like this introduce any issues?:
sign(<constant random string>) % 64 bytes
I have implemented code for this feature, please merge eth_decryptMessage and I will publish PR here. Already 1.5 months passed. Thank you in advance.
@logvik thanks for showing up in this thread :) so I guess encryptMessage
will look similar, except it wraps https://github.com/MetaMask/eth-sig-util/blob/master/index.js#L288 then, right? or are there any caveats?
@razum2um, you are right. Need to encrypt via eth-sig-util then in Metamask interface user can decrypt with own privateKey. That patch for keyring uses the same eth-sig-util for decrypting. By other words, Dapp will encrypt with eth-sig-util some message. Then Dapp can invoke "eth_decryptMessage" that will trigger Metamask UI for decrypting, the user will confirm this operation and Dapp will receive the raw message.
Thank you
On Mon, Sep 9, 2019, 12:05 PM Konstantin notifications@github.com wrote:
@razum2um https://github.com/razum2um, you are right. Need to encrypt via eth-sig-util then in Metamask interface user can decrypt with own privateKey. That patch for keyring uses the same eth-sig-util for decrypting. By other words, Dapp will encrypt with eth-sig-util some message. Then Dapp can invoke "eth_decryptMessage" that will trigger Metamask UI for decrypting, the user will confirm this operation and Dapp will receive the raw message.
— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/MetaMask/metamask-extension/issues/1190?email_source=notifications&email_token=ALNXKRBNBRMGRZMBJX72Z3LQIZX33A5CNFSM4DC352Y2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD6IEVOA#issuecomment-529550008, or mute the thread https://github.com/notifications/unsubscribe-auth/ALNXKRCS5BREX2HWDGN43ATQIZX33ANCNFSM4DC352YQ .
is it just the front end work remaining on this? everything else seems to be in place, right?
@awalias not exactly. Need to merge PR mentioned by mine above - https://github.com/MetaMask/eth-simple-keyring/pull/15. Metamask uses it in the core, without this PR we can't have the right build. There was touched not only UI, but also and metamask-controller.js, for example. Due to circumstance that PR on eth-simple-keyring is stuck already on more than 1.5 month, I obviously need to prepare a new patch for Metamask already. The main reason why I have placed all my comments here is to understand why PR on eth-simple-keyring is stuck. I have written to Metamask support already and to @danfinlay directly on email. No response from @danfinlay. In supporting team they told me that will transmit to developers and nothing happened while.
@logvik I also asked @danfinlay about it, and was directed to this discussion
@razum2um thank you very much, but I don't understand what for we need to wait for "web3 permission features" for merging that PR? Weird situation...
We're working to merge that as soon as possible, but there has been a UX question about how this feature is exposed, and the permissions system provides a clear answer to that question, rather than having to develop a custom view for every single API developers request.
@danfinlay thank you very much, I hope that we can add "web3 login permission" above this feature. Also, I think that we can reuse that code which allows permitting web3 - "privacy mode". But it is another theme. If briefly, then we can create common UI on one screen for all permission, not only for encryption and decryption. P.s. I will learn that theme mentioned by you above.
The login permissions system actually does address privacy mode, with our best isolated permissions we've ever bundled in MetaMask. Combined with encryption, I think this could be the beginning of a much more private web3.
I am currently working on an project, which would heavily rely on encryption and decryption support from metamask. I wanted to ask when this feature is expected to be released as we would need to develop a new startegy to circumvent the abscenece of encryption and decryption support if it isn't expected to be released soon.
@benjaminstrasser I hope that I will be able to provide this feature within a week or two because of the need to rewrite some code for a new Metamask version. But I can't predict a moment when this feature will be included in the extension.
I hope that I will be able to provide this feature within a week or two
I don't think we have any designs for this yet, we don't generally merge any interface changes without a design pass. Were you thinking of user-consent-per-decryption, or a blanket permission to decrypt?
We should either make sure we have a design issue for your proposed approach, and have designs for it, or we should just wait for the permissions system, which is already getting design attention and provides a general and reusable framework for approaching problems like this.
With Devcon approaching very soon, I'm not sure our design team is going to have a chance to review/design for this in the next two weeks, so realistically this is probably a post-devcon feature.
Sorry for always seeming like I'm getting in the way, I'm actually trying to channel you towards our process so that this can actually get published as quickly as possible.
We should either make sure we have a design issue for your proposed approach, and have designs for it, or we should just wait for the permissions system, which is already getting design attention and provides a general and reusable framework for approaching problems like this.
I have wanted to re-use the same approach that the Metamask extension uses for the sign request. I thought that if some new changes from the core team side will be applied, then it will influence and on the decrypting message feature. It will allow avoiding new UI developing and testing because the current flow for the sign request works well.
These flows work almost in the same way. If briefly, then the sign request gives back to Dapp a string in hex format based on a text and decrypting will give a text, based on data.
In any case, let me provide PR and we will review together.
That sounds good, you can probably use one of our sign views as a template.
We will probably want to expose both approaches: One method to decrypt per-message, and another permission to decrypt while logged in.
We need to wait for merging https://github.com/MetaMask/eth-json-rpc-middleware/pull/16, then change a version in package.js and merge to the extension.
For quick testing need to
After encryption via https://github.com/MetaMask/eth-sig-util the text "Hello world!" for PrivateKey mentioned above will be transformed to
{ version: 'x25519-xsalsa20-poly1305',
nonce: '05vld8Ca6n7COoPZHsXftV255BXeBO9t',
ephemPublicKey: 'XqesW3Wz3EyjHCijXKWXuiCyFkanRbktzw1g2Wml4AI=',
ciphertext: 'SoQDkBvVtR6+NsnQd5RZlfyLqC4xluIKZXe+8Q==' }
Need to use JSON.stringify() for translating to hex. For example via web3.toHex we will get:
"0x7b2276657273696f6e223a227832353531392d7873616c736132302d706f6c7931333035222c226e6f6e6365223a223035766c64384361366e37434f6f505a487358667456323535425865424f3974222c22657068656d5075626c69634b6579223a22587165735733577a3345796a4843696a584b575875694379466b616e52626b747a77316732576d6c3441493d222c2263697068657274657874223a22536f51446b4276567452362b4e736e516435525a6c66794c714334786c75494b5a58652b38513d3d227d"
Test code that we can run in the browser console
params = ["0x7b2276657273696f6e223a227832353531392d7873616c736132302d706f6c7931333035222c226e6f6e6365223a223035766c64384361366e37434f6f505a487358667456323535425865424f3974222c22657068656d5075626c69634b6579223a22587165735733577a3345796a4843696a584b575875694379466b616e52626b747a77316732576d6c3441493d222c2263697068657274657874223a22536f51446b4276567452362b4e736e516435525a6c66794c714334786c75494b5a58652b38513d3d227d", window.web3.eth.defaultAccount];
method = 'eth_decryptMessage';
window.web3.currentProvider.sendAsync({
jsonrpc: '2.0',
method: method,
params: params,
from: window.web3.eth.defaultAccount,
}, function(error, rawData) {
console.log(error, rawData); // undefined, Hello world!
})
Added a review @logvik. I could merge now, but it would be better to remove the code that allows negligent param ordering ;)
@whymarrh @Gudahtt @danjm please review https://github.com/MetaMask/metamask-extension/pull/7247 when you will have free time. Just I don't know that you are aware of this PR or not, because I have experienced one case when was no info about PR by key persons. Thank you in advance.
No activity? Can anybody predict the date when there will be possible at least to review PR?
Sorry for slow replies. The last couple weeks we have been representing at Devcon in Japan, we announced a plugin system which among other things, we hope to allow the integration of new encryption strategies without going through this tedious review/merge process ever again.
You could just wait for that to roll out over the next year, but I understand you've taken it this far, you want to get this feature merged.
One last hurdle remains, and it is not a small one: We have to agree on the UX of this feature before we can merge it. This probably requires getting a design sign-off, so you should open an issue and make sure our designers are happy about the feature as it is being proposed.
I will be getting back to normal work next week, until then I recommend posting an issue to the design board with screen shots of your PR branch, so they can take a moment to make any changes they think are important to the design.
Sorry, that forgot to remind you about the needs of review https://github.com/MetaMask/KeyringController/pull/41. It is the necessary pull request for this feature.
@danfinlay you have mentioned here https://github.com/MetaMask/metamask-extension/issues/1190#issuecomment-369777513
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.
But we can't produce a public key for nacl
from a public key for secp256k1
:
const ethUtil = require('ethereumjs-util')
const sigUtil = require('eth-sig-util')
const secp256k1 = require('secp256k1');
const address = '0xbe93f9bacbcffc8ee6663f2647917ed7a20a57bb'
const privateKey = new Buffer('6969696969696969696969696969696969696969696969696969696969696969', 'hex')
const privKeyHex = ethUtil.bufferToHex(privateKey)
console.log(new Buffer(sigUtil.getEncryptionPublicKey(privateKey)).toString('hex'))
//4778754d716f45326f48735a7a635174762f574d4e4233674348325036757a796e75774f3150304d4d31553d
console.log(new Buffer(secp256k1.publicKeyCreate(privateKey, false)).toString('hex'))
//04666bdf2025e32f41088899f2bcb4bf6983187f380e72fc7dee115b1f9957cc729dd976131c4c8e12ab1083ca0654ca5fdbcac8d3198daf90f581b591d56379ca
console.log(new Buffer(secp256k1.publicKeyCreate(privateKey, true)).toString('hex'))
//02666bdf2025e32f41088899f2bcb4bf6983187f380e72fc7dee115b1f9957cc72
As we can see that public keys are not matched, as expected. But then we need to lead them to one format to use it for encryption/decryption features or implement "encryption" feature.
We can extract the public key from signature
for secp256k1, but we can't extract for nacl
. Only via privateKey
.
Sorry, that I have missed this option before. In this case, I have to implement and "encryption" feature because we can touch privateKey
only in Keyring or we need to use secp256k1 as for signature
feature. For Dapp easy to use the public key from secp256k1, because then we can send encrypted
messages from the server-side.
Additional thoughts about how we can cover hardware devices. We need to create "get public key for nacl" feature. Then we can once get another public key for encryption in Dapp and use the current implementation for eth_decryptMessage
feature.
Thanks for your work on this @logvik, excited to get it merged.
We're working on some UI to help make sure this is a smooth experience for folks.
It would be helpful if anyone on this thread can describe their specific usecase / application / user flow so our design team has a better idea of how this will be deployed in the wild. (Thanks to those who have shared already). cc @cjeria @omnat
@bdresser we are developing hybrid DEX exchange and it demands a secure "Talk" system for solving any issues with the escrow process during exchanging fiat-crypto. This "Talk" system demands secure channels. It looks like channels or groups in any modern chat-app with the one key aspect, each channel has its own identity that consists of private/public key. This channel identity encrypted by the public key of each participant in this secure channel. Once a member wants to invite a new participant he/she needs to decrypt this channel identity and encrypt with a new member public key, this way we can build the next diagram for the interaction with this channel for Bob and Alice:
1) Bob needs to obtain a public key for the encryption of the channel identity that he has generated.
2) Dapp invokes encryption_public_key
RCP method.
3) Metamask opens a popup with a confirmation step for this action.
4) Bob either accepts or declines this proposal. At the next step, we assume that he will confirm.
5) Bob uses the obtained public key for encryption of the channel identity for self and for Alice. Alice got it by the same invocation encryption_public_key
before this step.
6) Adding or showing a message on this channel needs to use the channel identity. For decryption of this, you need to invoke eth_decrypt_message
RCP method, by using its own private key in Metamask.
7) Metamask opens a popup with a confirmation step for this action.
8) Bob either accepts or declines this proposal. On the next step, we assume that he will confirm.
9) Bob uses the public key of channel identity for sending a message. For displaying encrypted messages will be used the private key of the identity channel.
10) Alice decrypts the channel identity by the same invocation "eth_decrypt_message" RCP method for itself, by using own private key in Metamask.
Key benefits for this flow:
1) Alice and Bob need to create the public key for encrypting procedures only once. 2) Such public keys will exist parallel to the Ethereum public key. Encrypting curves will be different. 3) Only the channel members can invite another member by the way of encrypting the channel identity. 4) Identity can be stored in a database without any worries about them being stolen.
@bdresser We are working on a voting application with 2 user flows.
Voting Admin(person who created a vote) 1.1 When creating a vote with n options n secrtes have to be encrypted and stored in a contract. ... (some time passes. Computer/Browser may change. Account obviosly has to stay the same) 1.4 The admin gets the enncrypted secrets from the contract decrypts them and shares the decrypted secrets.
An eligible Voter 2.1 The voter registers to a vote by creating a secret and encrypting it and storing in it in a contract. ... (some time passes. Computer/Browser may change. Account obviosly has to stay the same) 2.3 When casting a vote sometimes later the voter gets his encrypted secret from the contract decrypts it and uses the decrypted secret to calculate some stuff then votes based on those calculations.
Encryption and Decryption is just used as a mean to store sensitive information so users don't have to remember a potentially weak password for ever voting or save the secret somewhere on their pc.
Hello @bdresser,
Our use case is quite similar to the one describe by @logvik (https://github.com/MetaMask/metamask-extension/issues/1190#issuecomment-549154196)
We would need:
I am looking forward to having decryption primitives in Metamask :tada: !
Cheers,
@lazaridiscom I will prepare example/template for a test app the link provided by you. Also please keep in mind, that I didn't have time to add getting the public key from "encryption_public_key" due to delaying merging on other packages, such as "keyring controller". Please, don't waste time for testing the current code, because it is only with "decrypt message" implementation. I will try to implement the "encryption_public_key" till 5 December 2019. I hope that by this time all required merging will be completed.
May I ask which other packages you've changed/need to change besides
KeyringController
andeth-simple-keyring
?
Nothing more, only these two packages. Also, I think, that for a quick and comfortable supporting from the community side better to have them in npm
And do you need to make modifications to metamask-extension itself, too (except the UI)?
yes, if you presume that background.js is not UI. But in any case, we need to have some additional code in Metamask extension.
@logvik @vrolland @benjaminstrasser @lazaridiscom Question: Does the following UI design satisfy your use-cases?
In this prototype:
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.