GetScatter / ScatterDesktop

Connect to applications on EOS, Ethereum, and Tron. Exchange tokens with ease. Manage your assets safely. All in a simple to use interface.
https://get-scatter.com/
GNU General Public License v3.0
500 stars 198 forks source link

Create new funcionality to encrypt and decrypt data with private key. #43

Open mmcs85 opened 6 years ago

mmcs85 commented 6 years ago

This is a continuation of the issue: https://github.com/GetScatter/ScatterWebExtension/issues/39#

There is two distinct use cases that are important.

For the first case you can sign some TEXT with account private key and then use it on symmetrical encryption algorithms like AES-256. Or you can generate a sharedSecret between the account private key / public key. In both cases a nonce is recommended to be used for generating the resulting symmetrical key to add entropy. Think of it as disposable keys per message.

For the second case the only option so far is to generate a sharedSecret between Alice private key / Bob public key. (indirect asymmetrical encryption). This also requires the nonce entropy.

My solution Proposal

In order to avoid using encryption inside scatter I propose to only calculate the disposable symmetrical key inside scatter and let the dapp developers use the symmetrical algorithms they choose. AES-XXX or any other.

Requirements to implementation Note that so far the sharedSecret implemented in eosjs is based on secp256k1 curve that happens to be the default one used to create account keys in crypto. It returns 64 bytes from a sha512 hash.

The encryption key for AES-256 is 32 bytes + 16 bytes for a IV(initialization vector).

Is there a need to generate bigger encryption keys? If so a hashing algorithm that returns more bytes is needed.

check here: https://github.com/EOSIO/eosjs-ecc/blob/master/src/key_private.js#L81

In order to eosjs-ecc play nice with this implementation the crypt function would need receive as parameter the encryption key.

check here: https://github.com/EOSIO/eosjs-ecc/blob/master/src/aes.js#L56

Example:

scatter.getEncryptionKey(fromPubKey, toPubKey, nonce) NOTE: the fromPubKey to obtain the privateKey inside scatter domain.

Returns the encryptionKey.

Advantages:

Disadvantages:

ChenLi0830 commented 6 years ago

Hi @mmcs85 thanks for migrating the issue. A couple of quick questions with regards to the proposal

  1. how would you keep the record of the encryption key/keys on the client side?
  2. if there are multiple clients (web & mobile) how are you planning to sync the encryption key between those clients?

Thanks!

mmcs85 commented 6 years ago

1 - You can store with indexDB or localstorage. 2 - You can always reconstruct the same encryption key with the same parameters (fromPubKey, toPubKey, nonce) or with the inverse Pubkey and Privkey like asymmetric encryption. You just need to share this information between clients using a traditional server or the blockchain.

For example lets assume you want to do a scatter private chat dapp: Lets assume you create a SmartContract with action:

sendprivmessage(account_name from, account_name to, string msg, nonce, checksum)

Use case, Alice sends msg to Bob:

Then Bob in another client can fetch the msg from Contract state or actions log and do:

I hope you get the ideia.

Regards

mlockett42 commented 6 years ago

Would also like this to be implement and have projects that could use it immediately if it was. But the proposed solutions I feel are too complex, we only need the decryption functions. The encryption can be done with the standard eosjs functions and the target users publicly available key, it doesn't need to be in scatter. Just something like decrypted_data = scatter.decrypt(encrypted_data); will do it. The only thing needed to decrypt the data is the private key which scatter has. Also scatter would need to ask the user permission and throw an exception if the data cant be decrypted. This link shows how to do this with eosjs if you have the private key. https://github.com/EOSIO/eosjs-ecc/issues/19#issuecomment-392941963

mmcs85 commented 6 years ago

Encryption inside scatter already was proposed before but that may create a maintenance issue that if scatter has to update the algorithm for security reasons. It may break dapp's before they have time to update.

The solution that I proposed is to avoid doing the encryption inside scatter. Instead the symmetric encryption key is calculated inside scatter and then eosjs OR any other lib can use it to feed many symmetrical algorithms. This way the encryption algorithm is deferred to the dapp devs and they have time to update their dapps.

NOTE: It works as long the new algorithms support the returned encryption key size.

cppfuns commented 5 years ago

@nsjames I also need this feature urgently. I feel that this function has a higher priority. Do you have any plans to support it?

nsjames commented 5 years ago

https://github.com/GetScatter/ScatterDesktop/commit/c8b8252d4f69853674f70d8e767a6a83932bfb1c

See this branch. I'm not too sure if this would actually work yet as I haven't had time to test the encryption/decryption.

mmcs85 commented 5 years ago

Did this https://github.com/GetScatter/ScatterDesktop/pull/141 PR to fix the public key format on the sharedSecret function

DebugFuture commented 5 years ago

@mmcs85 I think #141 fixed it. I did a test in my dapp.

@nsjames when will you have time to test this and release?

Thank you.

nsjames commented 5 years ago

Sorry this didn't make it into the most recent update, i'll have to start revisiting it for next release. This looks good though in general. Extrapolating it into plugin specific should make it far stabler into the future ( albeit more work when adding blockchains ).

utilsites commented 5 years ago

Created another issue on eos-transit project to push on a standard functionality for multiple wallets on this subject: https://github.com/eosnewyork/eos-transit/issues/2

angelol commented 5 years ago

I see a huge security issue in the proposed solution. The problem is that ECIES is only secure if you can guarantee that a unique nonce is used every time. If you re-use a nonce, you leak your shared secret.

Now the problem is that the developer has to remember to request a new shared secret with a different nonce every time they want to encrypt a message. Since Scatter will ask the user for permission every time, the developer will also be tempted to minimise the number of confirmations shown to the user.

Considering the above, it doesn't take much imagination to see that this will happen rather sooner than later and that it will be used incorrectly more often than correctly. We can't expect the typical programmer to have extensive cryptography training to recognise this. I'd rather not see the FUD media articles about Scatter & EOS encryption having been hacked.

In my opinion, the best way to solve this, is for scatter simply implement the eosjs_ecc.encrypt and eosjs_ecc.decrypt functions (https://github.com/EOSIO/eosjs-ecc/blob/2cd505ccc717913b813488bcd652981203bb7586/src/aes.js#L30). These implement the ECIES standard and are identical for Bitcoin and other cryptocurrencies that use elliptic curve cryptography. The nonce has to be generated inside scatter to make sure it cannot be reused.

Does this create a maintenance issue for scatter if the encryption gets broken? I would argue that if ECIES gets broken, scatter maintenance issues are the least of our worries. The security of Bitcoin and other chains rely on the fact that the underlying ECC is secure. So replacing a know-to-be-secure implementation of cryptography with something the every developer has to roll on its own is not a very good trade-off. In fact we are violating the golden rule of cryptography to never roll your own.

mmcs85 commented 5 years ago

I see a huge security issue in the proposed solution. The problem is that ECIES is only secure if you can guarantee that a unique nonce is used every time. If you re-use a nonce, you leak your shared secret.

Now the problem is that the developer has to remember to request a new shared secret with a different nonce every time they want to encrypt a message. Since Scatter will ask the user for permission every time, the developer will also be tempted to minimise the number of confirmations shown to the user.

Considering the above, it doesn't take much imagination to see that this will happen rather sooner than later and that it will be used incorrectly more often than correctly. We can't expect the typical programmer to have extensive cryptography training to recognise this. I'd rather not see the FUD media articles about Scatter & EOS encryption having been hacked.

In my opinion, the best way to solve this, is for scatter simply implement the eosjs_ecc.encrypt and eosjs_ecc.decrypt functions (https://github.com/EOSIO/eosjs-ecc/blob/2cd505ccc717913b813488bcd652981203bb7586/src/aes.js#L30). These implement the ECIES standard and are identical for Bitcoin and other cryptocurrencies that use elliptic curve cryptography. The nonce has to be generated inside scatter to make sure it cannot be reused.

Does this create a maintenance issue for scatter if the encryption gets broken? I would argue that if ECIES gets broken, scatter maintenance issues are the least of our worries. The security of Bitcoin and other chains rely on the fact that the underlying ECC is secure. So replacing a know-to-be-secure implementation of cryptography with something the every developer has to roll on its own is not a very good trade-off. In fact we are violating the golden rule of cryptography to never roll your own.

I'm glad you giving a good though about this and more eyes on this discussion the better.

1 - Can you provide more detailed information how reusing the same nonce leaks the shared secret? Its important to understand exactly how it happens.

Also symmetric key that goes to AES is based on a 512hash of the shared secret combined with nonce. Shared secret is never used directly to construct the symmetric key nor you can inverse the symmetric key and obtain the shared secret. Also shared secret is always calculated inside the wallet. https://github.com/EOSIO/eosjs-ecc/blob/2cd505ccc717913b813488bcd652981203bb7586/src/aes.js#L82

2 - This is a valid concern and the question is should scatter show a popup allowing per dapp encryption? Should user be allowed to whitelist? This is a big question indeed...

3 - Dapp dev can still use the same standard no issue there. But can as well change to a better one battle tested Symmetric Encryption scheme.

4 - Please note that ECIES can use multiple Symmetric Encryption schemes. So what we talking about is security vulnerabilities on symmetric encryption scheme's. Not about ECC security related to every crypto out there. If ECC is vulnerable then every transaction can be forged for example.

Finally nonce needs to be know between the 2 users of scatter. Is input to obtain the same symmetrical key on both ends. Thats the reason it is a input to the api.

angelol commented 5 years ago

Thank you for bearing with me on this. Now to your questions:

1) The problem is that some block cipher modes fail catastrophically if the same IV is used twice. The one used by eosjs-ecc (AES-256-cbc) thankfully isn't one of them. So while reusing an IV would not lead to a catastrophe in this case, it would still weaken the encryption scheme. But since the developer in this case has to implement their own encryption, we can't know what they will do. My thinking was: eosjs-ecc already implements a standard way of encryption native to EOS, why not use it rather than having every developer to use their own encryption? It would also make it interoperable.

2) Yes. If the app gets hold of the shared secret, as in your proposal, it can encrypt and sign arbitrary data as many times as it wants (with the same nonce every time which weakens the security of the encryption, which makes it even worse). I'm not sure if that is consistent with scatter's security model of requiring confirmation for every action (@nsjames ?).

3) No, they can't really use the standard encryption scheme without writing custom code, because eosjs-ecc only exports the encrypt and decrypt functions.

4) Yeah, I was talking about the implementation in eosjs-ecc. It's somewhat of a standard to encrypt memos from back in the steem days. Using it would make it interoperable.

In order to prevent problems 2) and 3), the best way would be to perform the encryption and authentication inside scatter. I totally agree with you that it's not ideal either, but to me, it's the least evil of the two options.

By the way: While looking at the code of eosjs-ecc again, I noticed that it uses the 'aes-256-cbc' block cipher mode without authentication 🤯. While 'aes-256-cbc' is a pretty robust choice, it's only secure when used together with a message authentication code. So upon close examination, eosjs-ecc turns out not to be so great either.

🤔🤔🤔

mmcs85 commented 5 years ago

This is a great discussion topic thanks for taking your time to look into it! Some notes about the topic:

1 - I agree that reusing same IV can weaken the security but that does not mean it leaks the shared secret. Developers can use existing implementations or use their own still is up to them to audit and make sure is secure enough by generating a nonce each message.

2 - Again there is a difference between shared secret and encryption key. encryption_key = sha512(nonce+shared_secret) <-- not reversible

A encryption_key leak means you can read only 1 message per nonce. If app dev reuses the nonce then you can think of a open channel leak between 2 peers. A shared secret leak means a leak for every possible open channel you can create between 2 peers. I don't believe my proposal leaks the shared secret.

There is also the possibility to not provide a nonce and make scatter generate it by default and return it to the dapp.

3 - how so? you can use any standard symmetrical encryption algorithm implementation in Javascript. eosjs-cc is not mandatory to use. Whats the added value for making it inter operable?

I agree on the point that using encrypt and decrypt on scatter indeed has the advantage of enforcing generating a ephemeral key each time by generating a nonce and hiding the encryption key.

Did not know that aes-256-cbc was not robust enough. But that kinda proves the point of scatter maintenance issue. In the future any other mode or AES can just get broken. and a new symmetrical algorithm can be developed and you can still use the same encryption_key above as input to this new algorithm.

nsjames commented 5 years ago

Right, we don't even use CBC inside of Scatter since it has a few "hmmmm" things in it, we default to GCM.

I think the larger benefit here is that if we can create a forward secrecy pattern which is more dynamic then we could even do things like EOS <> ETH encrypted chats, or even EOS <> BTC. Which should be the real goal there.

angelol commented 5 years ago

Alright guys, I've changed my mind. Thank you for letting me challenge the proposal.

Since eosjs-ecc is not the as foolproof as I hoped it would be, it's probably better to let the developer choose their own encryption. There are good libraries out there like tweetnacl-js that are suited for this.

If it's compatible with Scatter's security model, then requiring only one confirmation to set up a "secure channel" as you describe it per public/private key pair should be fine. The resulting shared secret can then be used together with a random nonce and tweetnacl's secretbox to implement a secure system.

The key derivation is the same as on ETH and BTC, so it could be used to implement EOS <> BTC communication schemes, which is awesome. I love the idea.

friedger commented 5 years ago

Another use case for this function is user storage for preferences, personal data, etc.

dApps could use storage that is address-based access controlled (like https://github.com/blockstack/gaia). The encryption key returned by 'getEncryptionKey' can be used as the private key for the gaia bucket/address.

(All the blockstack apps on app.co/blockstack could then authenticate with eos accounts)

medatlas commented 5 years ago

I see a huge security issue in the proposed solution. The problem is that ECIES is only secure if you can guarantee that a unique nonce is used every time. If you re-use a nonce, you leak your shared secret.

Now the problem is that the developer has to remember to request a new shared secret with a different nonce every time they want to encrypt a message. Since Scatter will ask the user for permission every time, the developer will also be tempted to minimise the number of confirmations shown to the user.

Considering the above, it doesn't take much imagination to see that this will happen rather sooner than later and that it will be used incorrectly more often than correctly. We can't expect the typical programmer to have extensive cryptography training to recognise this. I'd rather not see the FUD media articles about Scatter & EOS encryption having been hacked.

In my opinion, the best way to solve this, is for scatter simply implement the eosjs_ecc.encrypt and eosjs_ecc.decrypt functions (https://github.com/EOSIO/eosjs-ecc/blob/2cd505ccc717913b813488bcd652981203bb7586/src/aes.js#L30). These implement the ECIES standard and are identical for Bitcoin and other cryptocurrencies that use elliptic curve cryptography. The nonce has to be generated inside scatter to make sure it cannot be reused.

Does this create a maintenance issue for scatter if the encryption gets broken? I would argue that if ECIES gets broken, scatter maintenance issues are the least of our worries. The security of Bitcoin and other chains rely on the fact that the underlying ECC is secure. So replacing a know-to-be-secure implementation of cryptography with something the every developer has to roll on its own is not a very good trade-off. In fact we are violating the golden rule of cryptography to never roll your own.

The solution would be to make Scatter generate a random nonce and return a mp with the shared key and nonce. But still, developers may not use the returned nonce. So, the ideal is to do the encryption inside Scatter.