search

EOSIO / eosjs-ecc

Elliptic curve cryptography functions: Private Key, Public Key, Signature, AES, Encryption, Decryption
288 stars 119 forks source link

Use proper authenticated encryption instead of insecure self-baked scheme #61

Open angelol opened 5 years ago

angelol commented 5 years ago

We at privEOS are using eosjs-ecc and found a number of issues that this pull request fixes:

1) Replace insecure use of AES-256-CBC, which is an unauthenticated mode, with tweetnacl. Tweetnacl has been audited and found to be secure and it implements proper authenticated encryption.

2) Removes insecure use of checksum that leaks a hash of the private key to the public.

3) Ability to encrypt data using a pre-existing shared secret. This adds interoperability with a huge number of tools, including hardware security modules and Scatter (@nsjames).

jcalfee commented 5 years ago

From what I see, there are several different encrypted message schemes. How about deprecating this in eosjs-ecc then removing it? The shares secret can still come from eoejs-ecc.

angelol commented 5 years ago

From what I see, there are several different encrypted message schemes. How about deprecating this in eosjs-ecc then removing it? The shares secret can still come from eoejs-ecc.

Suggestion:

For compatibility with messages that were encrypted with the old version, it may make sense to detect the old format end decrypt it using the old algorithm. New messages should be encrypted using the new algorithm. That way, the change would be backwards compatible.

jcalfee commented 5 years ago

TweetNaCl has its own signing, sha implementation, keypair object, etc.. so that is all code that eosjs-ecc has already. Seems like you just need the well-known Encrypt-then-MAC code for message encryption (I assume that is what TweetNaCl is doing: Encrypt-then-MAC).

angelol commented 5 years ago

TweetNaCl has its own signing, sha implementation, keypair object, etc.. so that is all code that eosjs-ecc has already. Seems like you just need the well-known Encrypt-then-MAC code for message encryption (I assume that is what TweetNaCl is doing: Encrypt-then-MAC).

No, it's a streaming cypher with built-in authentication. The tweetnacl-js library has already been audited (report here). That's why it would be much more secure than a self-made encrypt-then-mac implementation. Such a self-made implementation would have to be audited again.

jcalfee commented 5 years ago

Agreed, a streaming cipher is better. This library and its direction is out of my hands. I helped to bring it to b1's attention though. Let see what they say.

For my own curiosity: did you look comparability for other platforms like and Java/Android and iOS?

angelol commented 5 years ago

Agreed, a streaming cipher is better. This library and its direction is out of my hands. I helped to bring it to b1's attention though. Let see what they say.

Thank you!

For my own curiosity: did you look comparability for other platforms like and Java/Android and iOS?

It's a 1:1 port of tweetnacl to javascript. So it should be compatible to tweetnacl libraries on other platforms.

jcalfee commented 5 years ago

I got some feedback (and permission to repost it): We're working on replacing eosjs-ecc with elliptic. I was directed to the eosjs develop branch. I see they did remove eosjs-ecc https://github.com/EOSIO/eosjs/commit/81d2323549bcf602a87fa78950143302712bd7b2 .. I did not find any code to encrypt or decrypt data in that library.