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 web3.eth.encrypt and web3.eth.decrypt functions to JSON-RPC #1098

Closed
wants to merge 8 commits into from
Closed
Changes from 7 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
102 changes: 102 additions & 0 deletions EIPS/eip-1024.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,102 @@
---
eip: 1024
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Where is this number coming from?

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
eip: 1024
eip: <to be assigned>

title: Add web3.eth.encrypt and web3.eth.decrypt functions
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would make sense including "to JSON-RPC" here.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
title: Add web3.eth.encrypt and web3.eth.decrypt functions
title: Add web3.eth.encrypt and web3.eth.decrypt functions to JSON-RPC

author: Tope Alabi <[email protected]>
status: Draft
type: Interface Track
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is no 'interface track'. Please use a type and category from EIP 0.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is still not fixed.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
type: Interface Track
type: Standards Track
category: Interface

created: 2018-05-14
---
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems to be an extension of the RPC protocol, so a requires: 1474 field would be appropriate.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
---
requires: 1474
---



### Abstract
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All these sections seem to be indented one level too deep, it should be ## Abstract.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
### Abstract
## Abstract

This EIP proposes a cross-client method for requesting encryption/decryption. This method will include a version parameter, so that different encryption methods can be added under the same name. Nacl is a cryptographically complete and well audited library that works well for this by implementers are free to choose their crypto. Ethereum keypairs should not be used directly for encryption, instead we should derive an encryption keypair from the account's private key for decryption and generate a random ephemeral keypair for encryption.

Parity wallet already implements a compatible [encrypt/decrypt] https://wiki.parity.io/JSONRPC-parity-module#parity_decryptmessage method and the MetaMask version is on the way. Having a cross-client standard will enable a whole new wave of decentralized applications that will allow users to securely store their private data in public databases such as IPFS.
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
Parity wallet already implements a compatible [encrypt/decrypt] https://wiki.parity.io/JSONRPC-parity-module#parity_decryptmessage method and the MetaMask version is on the way. Having a cross-client standard will enable a whole new wave of decentralized applications that will allow users to securely store their private data in public databases such as IPFS.
Parity wallet already implements a compatible [encrypt/decrypt](https://wiki.parity.io/JSONRPC-parity-module#parity_decryptmessage) method. [MetaMask's implementation](https://github.com/MetaMask/metamask-extension/pull/7831) was released in [v8.0.0](https://github.com/MetaMask/metamask-extension/releases/tag/v8.0.0). Having a cross-client standard will enable a whole new wave of decentralized applications that will allow users to securely store their private data in public databases such as IPFS.

(update to include Metamask impl as of July 2020)


### Motivation
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
### Motivation
## Motivation

Imagine an illegal immigrant named Martha. Martha moved to the United States illegally but then had 2 children there, so her children are citizens. One day Martha gets arrested and deported but her children get to stay. How will Martha pass power of Attorney, bank account info, identification docs, and other sensitive information to her children? Storing that data in a centralized database can be incriminating for Martha, so maybe decentralized databases like IPFS could help, but if the data is not encrypted anyone can see it, which kind of defeats the purpose. If Martha had access to a Dapp with end-to-end encryption connected to her identity, she could save her data in a decentralized, censor-proof database and still have confidence that only her children can access it.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This motivation is bound to piss people off but it gets big ups from me. Nice imagination. Just wanted to leave that here.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed. But it's also a very legit scenario.


More casually, Martha can create a treasure hunt game, or a decentralized chat app etc.

### Specification
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
### Specification
## Specification


```

const nacl = require('tweetnacl')
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this needed?


/**
Comment on lines +24 to +27
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

tl;dr=IMHO this is implementation specific, but is worth including here.

An implementation of web3.eth.getEncryptionPublicKey would require a call to nacl.box.keyPair.fromSecretKey(/* ... */) (or some other equivalent library). tweetnacl is a JS implementation of the library referred to in the abstract: "Nacl is a cryptographically complete and well audited library that works well for this by implementers are free to choose their crypto."

An alternative would be to bundle (parts of) the tweetnacl library (or some other equivalent library) together with web3.js. Probably not appropriate for now, but is something to consider if encryption/ decryption use cases (and the use of the JSON-RPC methods introduced in this EIP) become very common in client implementations.

In the case that implementation specific lines do not belong in this section, here's a patch that removes it:

Suggested change
const nacl = require('tweetnacl')
/**
/**

FWIW, I think that leaving it in there is probably a good thing because it aids in understanding the proceeding sections of the specifications, and is a reference to something mentioned in the abstract.

addresses: https://github.com/ethereum/EIPs/pull/1098/files#r299563507

* Returns user's public Encryption key derived from privateKey Ethereum key
* @param {Account} reciever - The Ethereum account that will be recieving/decrypting the data
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

minor nitpick: reciever, is usually spelt as "receiver"

*/
web3.eth.getEncryptionPublicKey(reciever.privateKey) { /* implementation */ }
Comment on lines +29 to +31
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
* @param {Account} reciever - The Ethereum account that will be recieving/decrypting the data
*/
web3.eth.getEncryptionPublicKey(reciever.privateKey) { /* implementation */ }
* @param {Account} receiver - The Ethereum account that will be receiving/decrypting the data
*/
web3.eth.getEncryptionPublicKey(receiver.privateKey) { /* implementation */ }

addresses: https://github.com/ethereum/EIPs/pull/1098/files#r193246175


/**
* Encrypts plain data.
* @param {string} encryptionPublicKey - The encryption public key of the reciever
* @param {string} version - A unique string identifying the encryption strategy.
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

would it make sense to define an enum, and use it here instead of a raw string?

pros:

  • this would enable tools/IDEs to type-check any arguments here
  • makes it more discoverable which encryption-strategies are available e.g. via autocomplete

cons:

  • I don't know how easy or tough it would be to add new encryption-strategies i.e. new values to the enum, if the enum definition were to be included in the standard itself.
    • Maybe there's an alternate way of defining it?
    • e.g. if we could have a base-enum in the standard and implementations can "extend" it, but I'm not sure if the solidity language supports that. Oh, also this would only work for "string-enums", but solidity seems to only have "int-enums" https://solidity.readthedocs.io/en/develop/types.html#enums

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

An enum would be a good idea when we have more than one option (as of now this is the only one).
Adding more options, and enumerating them, sound like a good thing for a future EIP that specifies this among its requires.

* @param {Object} data - The data to encrypt
* @param {Function} callback - The function to call back when decryption is complete.
*/
web3.eth.encrypt(encryptionPublicKey, version, data, callback) { /* implementation */ }

/**
* Decrypts some encrypted data.
* @param {Account} reciever - The account that will decrypt the message
* @param {Object} encryptedData - The data to decrypt
* @param {Function} callback - The function to call back when decryption is complete.
*/
web3.eth.decrypt = function decrypt (recievier.privatekey, encryptedData, callback) { /* implementation */ }
Comment on lines +42 to +48
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
/**
* Decrypts some encrypted data.
* @param {Account} reciever - The account that will decrypt the message
* @param {Object} encryptedData - The data to decrypt
* @param {Function} callback - The function to call back when decryption is complete.
*/
web3.eth.decrypt = function decrypt (recievier.privatekey, encryptedData, callback) { /* implementation */ }
/**
* Decrypts some encrypted data.
* @param {Account} receiver - The account that will decrypt the message
* @param {Object} encryptedData - The data to decrypt
* @param {Function} callback - The function to call back when decryption is complete.
*/
web3.eth.decrypt = function decrypt (receiver.privatekey, encryptedData, callback) { /* implementation */ }


```

**To Encrypt:**
- Alice requests Bob's publicEncryptionKey
- Bob generates his encryptionKeypair using nacl.box.keyPair.fromSecretKey(bob.ethereumPrivateKey)
- Bob sends Alice his encryptionKeyPair.publicKey
- Alice generates a random ephemeralKeyPair
- Alice uses her ephemeralKeypair.secretKey and Bob's encryptionPublicKey to encrypt the data using nacl.box. She sends him an encrypted blob of the form:
Comment on lines +52 to +57
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
**To Encrypt:**
- Alice requests Bob's publicEncryptionKey
- Bob generates his encryptionKeypair using nacl.box.keyPair.fromSecretKey(bob.ethereumPrivateKey)
- Bob sends Alice his encryptionKeyPair.publicKey
- Alice generates a random ephemeralKeyPair
- Alice uses her ephemeralKeypair.secretKey and Bob's encryptionPublicKey to encrypt the data using nacl.box. She sends him an encrypted blob of the form:
**To Encrypt:**
- Alice requests Bob's `publicEncryptionKey`
- Bob generates his `encryptionKeypair` using `nacl.box.keyPair.fromSecretKey(bob.ethereumPrivateKey)`
- Bob sends Alice his `encryptionKeyPair.publicKey`
- Alice generates a random `ephemeralKeyPair`
- Alice uses her `ephemeralKeypair.secretKey` and Bob's `encryptionPublicKey` to encrypt the data using `nacl.box`. She sends him an encrypted blob of the form:


```
{ version: 'x25519-xsalsa20-poly1305',
nonce: '1dvWO7uOnBnO7iNDJ9kO9pTasLuKNlej',
ephemPublicKey: 'FBH1/pAEHOOW14Lu3FWkgV3qOEcuL78Zy+qW1RwzMXQ=',
ciphertext: 'f8kBcl/NCyf3sybfbwAKk/np2Bzt9lRVkZejr6uh5FgnNlH/ic62DZzy' }
```


**To Decrypt:**
- Bob generates his encryptionPrivatekey using nacl.box.keyPair.fromSecretKey(bob.ethereumPrivateKey).secretKey
- Bob passes his encryptionPrivateKey along with the encrypted blob to nacl.box.open(ciphertext, nonce, ephemPublicKey, myencryptionPrivatekey)
Comment on lines +67 to +69
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
**To Decrypt:**
- Bob generates his encryptionPrivatekey using nacl.box.keyPair.fromSecretKey(bob.ethereumPrivateKey).secretKey
- Bob passes his encryptionPrivateKey along with the encrypted blob to nacl.box.open(ciphertext, nonce, ephemPublicKey, myencryptionPrivatekey)
**To Decrypt:**
- Bob generates his `encryptionPrivatekey` using `nacl.box.keyPair.fromSecretKey(bob.ethereumPrivateKey).secretKey`
- Bob passes his `encryptionPrivateKey` along with the encrypted blob to `nacl.box.open(ciphertext, nonce, ephemPublicKey, myencryptionPrivatekey)`



### Rationale
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
### Rationale
## Rationale

These methods should require user confirmation. We include the versioning to allow different encryption/decryption types to be added under the same method name. For example, it might make sense to have a few kinds of decrypt methods, for different kinds of consent:
- Consent to download a decrypted file.
- Consent to return decrypted file to the current site.
- Consent to return any number of decrypted messages to the current site over a certain period of time. (could enable chat apps)


### Backwards Compatibility
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
### Backwards Compatibility
## Backwards Compatibility

Parity implements an encrypt/decrypt method with a different curve than the one which is intended in this proposal, but that it would be possible to add support for curves to this standard.
https://wiki.parity.io/JSONRPC-parity-module#parity_decryptmessage

### Test Cases
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
### Test Cases
## Test Cases

`getEncryptionPublicKey(7e5374ec2ef0d91761a6e72fdf8f6ac665519bfdf6da0a2329cf0d804514b816)` should return a public encryption key of the form `"C5YMNdqE4kLgxQhJO1MfuQcHP5hjVSXzamzd/TxlR0U="`

`web3.eth.encrypt("C5YMNdqE4kLgxQhJO1MfuQcHP5hjVSXzamzd/TxlR0U=", 'x25519-xsalsa20-poly1305-v1', {data: 'My name is Satoshi Buterin'})` should return a blob of the form `{ version: 'x25519-xsalsa20-poly1305',
nonce: '1dvWO7uOnBnO7iNDJ9kO9pTasLuKNlej',
ephemPublicKey: 'FBH1/pAEHOOW14Lu3FWkgV3qOEcuL78Zy+qW1RwzMXQ=',
ciphertext: 'f8kBcl/NCyf3sybfbwAKk/np2Bzt9lRVkZejr6uh5FgnNlH/ic62DZzy' }`

`web3.eth.decrypt('7e5374ec2ef0d91761a6e72fdf8f6ac665519bfdf6da0a2329cf0d804514b816',
{ version: 'x25519-xsalsa20-poly1305',
nonce: '1dvWO7uOnBnO7iNDJ9kO9pTasLuKNlej',
ephemPublicKey: 'FBH1/pAEHOOW14Lu3FWkgV3qOEcuL78Zy+qW1RwzMXQ=',
ciphertext: 'f8kBcl/NCyf3sybfbwAKk/np2Bzt9lRVkZejr6uh5FgnNlH/ic62DZzy' })` should return plain text/file of the form `{ data:'My name is Satoshi Buterin' }`

### Implementation
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
### Implementation
## Implementation

Parity wallet has already implemented a compatible encryption/decryption method. The Metamask version will be published soon.
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
Parity wallet has already implemented a compatible encryption/decryption method. The Metamask version will be published soon.
Both Parity Wallet and MetaMask have implemented compatible encryption/ decryption methods.

(update to include MetaMask's implementation as of July 2020)

https://github.com/topealabi/eth-sig-util/blob/master/index.js

## Copyright
Copyright and related rights waived via [CC0](https://creativecommons.org/publicdomain/zero/1.0/).