Use proper authenticated encryption instead of insecure self-baked scheme

package.json

@@ -1,5 +1,5 @@
 {
-  "name": "eosjs-ecc",
+  "name": "eosjs-ecc-priveos",
   "version": "4.0.4",
   "description": "Elliptic curve cryptography functions",
   "keywords": "ECC, Private Key, Public Key, Signature, AES, Encryption, Decryption",
@@ -30,13 +30,12 @@
   },
   "dependencies": {
     "bigi": "^1.4.2",
-    "browserify-aes": "^1.0.6",
     "bs58": "^4.0.1",
-    "bytebuffer": "^5.0.1",
     "create-hash": "^1.1.3",
     "create-hmac": "^1.1.6",
     "ecurve": "^1.0.5",
-    "randombytes": "^2.0.5"
+    "randombytes": "^2.0.5",
+    "tweetnacl": "^1.0.1"
   },
   "license": "MIT",
   "devDependencies": {

src/aes.js

@@ -1,34 +1,30 @@
 const randomBytes = require('randombytes')
-const ByteBuffer = require('bytebuffer')
-const crypto = require('browserify-aes')
 const assert = require('assert')
 const PublicKey = require('./key_public')
 const PrivateKey = require('./key_private')
 const hash = require('./hash')
-
-const Long = ByteBuffer.Long;
+const nacl = require("tweetnacl/nacl-fast")
 
 module.exports = {
   encrypt,
   decrypt
 }
 
+const nonceLength = 24
+const checkLength = 8
 /**
     Spec: http://localhost:3002/steem/@dantheman/how-to-encrypt-a-memo-when-transferring-steem
 
     @throws {Error|TypeError} - "Invalid Key, ..."
 
     @arg {PrivateKey} private_key - required and used for decryption
     @arg {PublicKey} public_key - required and used to calcualte the shared secret
-    @arg {string} [nonce = uniqueNonce()] - assigned a random unique uint64
 
     @return {object}
-    @property {string} nonce - random or unique uint64, provides entropy when re-using the same private/public keys.
-    @property {Buffer} message - Plain text message
-    @property {number} checksum - shared secret checksum
+    @property {Buffer} message - Secret
 */
-function encrypt(private_key, public_key, message, nonce = uniqueNonce()) {
-    return crypt(private_key, public_key, nonce, message)
+function encrypt(private_key, public_key, message) {
+    return crypt(private_key, public_key, message, true)
 }
 
 /**
@@ -44,16 +40,17 @@ function encrypt(private_key, public_key, message, nonce = uniqueNonce()) {
 
     @return {Buffer} - message
 */
-function decrypt(private_key, public_key, nonce, message, checksum) {
-    return crypt(private_key, public_key, nonce, message, checksum).message
+function decrypt(private_key, public_key, box) {
+    return crypt(private_key, public_key, box, false)
 }
 
 /**
     @arg {Buffer} message - Encrypted or plain text message (see checksum)
     @arg {number} checksum - shared secret checksum (null to encrypt, non-null to decrypt)
     @private
 */
-function crypt(private_key, public_key, nonce, message, checksum) {
+function crypt(private_key, public_key, box, encrypt) {
+    let nonce, checksum, message
     private_key = PrivateKey(private_key)
     if (!private_key)
         throw new TypeError('private_key is required')
@@ -62,7 +59,13 @@ function crypt(private_key, public_key, nonce, message, checksum) {
     if (!public_key)
         throw new TypeError('public_key is required')
 
-    nonce = toLongObj(nonce)
+    if(encrypt) {
+      nonce = uniqueNonce()
+      message = box
+    } else {
+      ({nonce, checksum, message} = deserialize(box))
+    }
+
     if (!nonce)
         throw new TypeError('nonce is required')
 
@@ -71,96 +74,76 @@ function crypt(private_key, public_key, nonce, message, checksum) {
             throw new TypeError('message should be buffer or string')
         message = new Buffer(message, 'binary')
     }
-    if (checksum && typeof checksum !== 'number')
-        throw new TypeError('checksum should be a number')
 
     const S = private_key.getSharedSecret(public_key);
-    let ebuf = new ByteBuffer(ByteBuffer.DEFAULT_CAPACITY, ByteBuffer.LITTLE_ENDIAN)
-    ebuf.writeUint64(nonce)
-    ebuf.append(S.toString('binary'), 'binary')
-    ebuf = new Buffer(ebuf.copy(0, ebuf.offset).toBinary(), 'binary')
+    const ekey_length = S.length + nonce.length
+    let ebuf = Buffer.concat([nonce, S], ekey_length)
     const encryption_key = hash.sha512(ebuf)
 
-    // D E B U G
-    // console.log('crypt', {
-    //     priv_to_pub: private_key.toPublic().toString(),
-    //     pub: public_key.toString(),
-    //     nonce: nonce.toString(),
-    //     message: message.length,
-    //     checksum,
-    //     S: S.toString('hex'),
-    //     encryption_key: encryption_key.toString('hex'),
-    // })
-
-    const iv = encryption_key.slice(32, 48)
+    const iv = encryption_key.slice(32, 56)
     const key = encryption_key.slice(0, 32)
 
-    // check is first 64 bit of sha256 hash treated as uint64_t truncated to 32 bits.
+    // check is first 64 bit of sha256 hash
     let check = hash.sha256(encryption_key)
-    check = check.slice(0, 4)
-    const cbuf = ByteBuffer.fromBinary(check.toString('binary'), ByteBuffer.DEFAULT_CAPACITY, ByteBuffer.LITTLE_ENDIAN)
-    check = cbuf.readUint32()
+    check = check.slice(0, 8)
 
     if (checksum) {
-        if (check !== checksum)
-            throw new Error('Invalid key')
-        message = cryptoJsDecrypt(message, key, iv)
+        if (!check.equals(checksum)) {
+            throw new Error('Invalid checksum')
+        }
+        return cryptoJsDecrypt(message, key, iv)
     } else {
         message = cryptoJsEncrypt(message, key, iv)
+        return serialize(nonce, check, message)
     }
-    return {nonce, message, checksum: check}
 }
 
-/** This method does not use a checksum, the returned data must be validated some other way.
+function serialize(nonce, check, message) {
+  const len = nonceLength + checkLength + message.length
+  return Buffer.concat([nonce, check, message], len)
+}
+
+function deserialize(buf) {
+  const nonce = buf.slice(0, nonceLength)
+  const checksum = buf.slice(nonceLength, nonceLength + checkLength)
+  const message = buf.slice(nonceLength + checkLength)
+  return {nonce, checksum, message}
+}
+/** This method both decrypts and checks the authenticity of the messsage.
 
     @arg {string|Buffer} message - ciphertext binary format
     @arg {string<utf8>|Buffer} key - 256bit
-    @arg {string<utf8>|Buffer} iv - 128bit
+    @arg {string<utf8>|Buffer} iv - 192bit
 
     @return {Buffer}
 */
-function cryptoJsDecrypt(message, key, iv) {
-    assert(message, "Missing cipher text")
-    message = toBinaryBuffer(message)
-    const decipher = crypto.createDecipheriv('aes-256-cbc', key, iv)
-    // decipher.setAutoPadding(true)
-    message = Buffer.concat([decipher.update(message), decipher.final()])
-    return message
+function cryptoJsDecrypt(box, key, nonce) {
+    assert(box, "Missing cipher text")
+    box = toBinaryBuffer(box)
+    const decrypted = nacl.secretbox.open(box, nonce, key)
+    if(decrypted === null) {
+      throw new Error('Secretbox refused to open (most likely corrupted or tampered message)')
+    }
+    return Buffer.from(decrypted)
 }
 
-/** This method does not use a checksum, the returned data must be validated some other way.
+/** This method both encrypts and authenticates the message.
     @arg {string|Buffer} message - plaintext binary format
     @arg {string<utf8>|Buffer} key - 256bit
-    @arg {string<utf8>|Buffer} iv - 128bit
+    @arg {string<utf8>|Buffer} iv - 192bit
 
     @return {Buffer}
 */
-function cryptoJsEncrypt(message, key, iv) {
+function cryptoJsEncrypt(message, key, nonce) {
     assert(message, "Missing plain text")
     message = toBinaryBuffer(message)
-    const cipher = crypto.createCipheriv('aes-256-cbc', key, iv)
-    // cipher.setAutoPadding(true)
-    message = Buffer.concat([cipher.update(message), cipher.final()])
-    return message
+    return nacl.secretbox(message, nonce, key)
 }
 
-/** @return {string} unique 64 bit unsigned number string.  Being time based, this is careful to never choose the same nonce twice.  This value could be recorded in the blockchain for a long time.
+/** @return {string} 192bit random nonce. Long enough to be unique.  This value could be recorded in the blockchain for a long time.
 */
 function uniqueNonce() {
-    if(unique_nonce_entropy === null) {
-        const b = new Uint8Array(randomBytes(2))
-        unique_nonce_entropy = parseInt(b[0] << 8 | b[1], 10)
-    }
-    let long = Long.fromNumber(Date.now())
-    const entropy = ++unique_nonce_entropy % 0xFFFF
-    // console.log('uniqueNonce date\t', ByteBuffer.allocate(8).writeUint64(long).toHex(0))
-    // console.log('uniqueNonce entropy\t', ByteBuffer.allocate(8).writeUint64(Long.fromNumber(entropy)).toHex(0))
-    long = long.shiftLeft(16).or(Long.fromNumber(entropy));
-    // console.log('uniqueNonce final\t', ByteBuffer.allocate(8).writeUint64(long).toHex(0))
-    return long.toString()
+    return randomBytes(nonceLength)
 }
-let unique_nonce_entropy = null
-// for(let i=1; i < 10; i++) key.uniqueNonce()
 
-const toLongObj = o => (o ? Long.isLong(o) ? o : Long.fromString(o) : o)
 const toBinaryBuffer = o => (o ? Buffer.isBuffer(o) ? o : new Buffer(o, 'binary') : o)

src/aes.test.js

@@ -0,0 +1,36 @@
+/* eslint-env mocha */
+const assert = require('assert')
+const ecc = require('.')
+
+const alice = {
+  public_key: 'EOS81xEWcDyZCxACZcYQekiWXLjuSoPMwmRv16nZMuqm2BtQMvXbg',
+  private_key: '5JxhzyqYERz5MRSswNnDUXL1gFyM2m5Zxde9gGWfMkndbnjB8kD',
+}
+const bob = {
+  public_key: 'EOS7jAEWX9d4nZJWNckkaxBsHyqbe6yrVH6VUoCzP6DLxHAEvsBKM',
+  private_key: '5HrR1D5UbeeMETVR6Ud3Xc6PchVKbtAHmHiPmkmMQDqXY53bQKZ',
+}
+
+describe('encrypt/decrypt', () => {
+  it('Decrypt should recover the original message', async function() {
+    const message = Buffer.from("My first message")
+    let box = ecc.Aes.encrypt(alice.private_key, bob.public_key, message)
+    const decrypted = ecc.Aes.decrypt(bob.private_key, alice.public_key, box)
+    assert.deepEqual(decrypted, message)
+  })
+
+  /* The following test fails with the normal eosjs-ecc */
+  it('Tampered message should throw', async function() {
+    const message = Buffer.from("My first message")
+    let box = ecc.Aes.encrypt(alice.private_key, bob.public_key, message)
+
+    // a little tampering
+    box = Buffer.concat([box, box])
+
+    assert.throws(function() {
+      ecc.Aes.decrypt(bob.private_key, alice.public_key, box)
+    })
+  })
+
+})
+