fix: use xor streaming for encryption

Author: jacobheun

package.json

@@ -25,11 +25,11 @@
   ],
   "license": "MIT",
   "dependencies": {
+    "debug": "^3.1.0",
     "interface-connection": "^0.3.2",
     "pull-handshake": "^1.1.4",
-    "pull-length-prefixed": "^1.3.0",
     "pull-stream": "^3.6.7",
-    "tweetnacl": "^1.0.0"
+    "xsalsa20": "^1.0.2"
   },
   "devDependencies": {
     "aegir": "^13.1.0",

src/crypto.js

@@ -1,14 +1,14 @@
 'use strict'
 
 const pull = require('pull-stream')
-const lp = require('pull-length-prefixed')
-const nacl = require('tweetnacl')
+const debug = require('debug')
 const Errors = require('./errors')
+const xsalsa20 = require('xsalsa20')
+const KEY_LENGTH = require('./key-generator').KEY_LENGTH
 
-const lpOpts = {
-  fixed: true,
-  bytes: 4
-}
+const log = debug('libp2p:pnet')
+log.trace = debug('libp2p:pnet:trace')
+log.err = debug('libp2p:pnet:err')
 
 /**
  * Creates a pull stream to encrypt messages in a private network
@@ -18,14 +18,12 @@ const lpOpts = {
  * @returns {PullStream} a through stream
  */
 module.exports.createBoxStream = (nonce, psk) => {
+  const xor = xsalsa20(nonce, psk)
   return pull(
     ensureBuffer(),
     pull.map((chunk) => {
-      const encryptedChunk = nacl.secretbox(Buffer.from(chunk), nonce, psk)
-
-      return Buffer.from(encryptedChunk)
-    }),
-    lp.encode(lpOpts)
+      return xor.update(chunk, chunk)
+    })
   )
 }
 
@@ -37,18 +35,11 @@ module.exports.createBoxStream = (nonce, psk) => {
  * @returns {PullStream} a through stream
  */
 module.exports.createUnboxStream = (nonce, psk) => {
+  const xor = xsalsa20(nonce, psk)
   return pull(
     ensureBuffer(),
-    lp.decode(lpOpts),
     pull.map((chunk) => {
-      let decryptedChunk = nacl.secretbox.open(Buffer.from(chunk), nonce, psk)
-
-      // If it's null, there's most likely a psk mismatch
-      if (decryptedChunk === null) {
-        throw new Error(Errors.INVALID_PEER)
-      }
-
-      return Buffer.from(decryptedChunk)
+      return xor.update(chunk, chunk)
     })
   )
 }
@@ -72,7 +63,7 @@ module.exports.decodeV1PSK = (pskBuffer) => {
     const codec = metadata.shift()
     const psk = Buffer.from(metadata.shift(), 'hex')
 
-    if (psk.byteLength !== nacl.box.sharedKeyLength) {
+    if (psk.byteLength !== KEY_LENGTH) {
       throw new Error(Errors.INVALID_PSK)
     }
 

src/handshake.js

@@ -1,13 +1,17 @@
 'use strict'
 
 const series = require('async/series')
-const lp = require('pull-length-prefixed')
-const nacl = require('tweetnacl')
 const pull = require('pull-stream')
 const handshake = require('pull-handshake')
+const debug = require('debug')
 
 const crypto = require('./crypto')
 const Errors = require('./errors')
+const NONCE_LENGTH = require('./key-generator').NONCE_LENGTH
+
+const log = debug('libp2p:pnet')
+log.err = debug('libp2p:pnet:err')
+log.trace = debug('libp2p:pnet:trace')
 
 /**
  * Exchanges nonces with a connected peer. The nonce read from the peer
@@ -18,21 +22,13 @@ const Errors = require('./errors')
  * @returns {void}
  */
 function exchangeNonce (state, callback) {
-  pull(
-    // Write the nonce
-    pull.values([state.local.nonce]),
-    // Append 4 bytes
-    lp.encode({fixed: true, bytes: 4}),
-    // Collect the data and write it to the handshake
-    pull.collect((err, res) => {
-      if (err) { return callback(err) }
-      state.shake.write(res[0])
-    })
-  )
+  // Write the nonce
+  state.shake.write(state.local.nonce)
 
   // Read the nonce from the handshake
-  lp.decodeFromReader(state.shake, {fixed: true, bytes: 4}, (err, nonce) => {
+  state.shake.read(NONCE_LENGTH, (err, nonce) => {
     state.remote.nonce = nonce || null
+    log.trace('nonce exchange complete')
     callback(err)
   })
 }
@@ -46,11 +42,7 @@ function exchangeNonce (state, callback) {
  */
 function protectConnection (state, callback) {
   const stream = state.shake.rest()
-  const shake = handshake({ timeout: state.timeout }, (err) => {
-    if (err) {
-      callback(err)
-    }
-  })
+  const shake = handshake({ timeout: state.timeout })
 
   // Bridge the handshakes to form the encrypted connection
   pull(
@@ -61,23 +53,8 @@ function protectConnection (state, callback) {
     stream
   )
 
-  // Exchange nonces via the handshake
-  shake.handshake.write(state.local.nonce)
-  shake.handshake.read(nacl.box.nonceLength, (err, nonceBack) => {
-    if (err) {
-      state.secure.resolve({
-        source: pull.error(err),
-        sink: () => { }
-      })
-      return callback(err)
-    }
-
-    // Resolve the deferred connection with the completed handshake
-    // This will be set as the inner connection in the final protected
-    // connection result
-    state.secure.resolve(shake.handshake.rest())
-    callback()
-  })
+  state.secure.resolve(shake.handshake.rest())
+  callback()
 }
 
 /**
@@ -88,6 +65,8 @@ function protectConnection (state, callback) {
  * @returns {PullStream} a handshake stream
  */
 module.exports = function handshake (state, callback) {
+  log.trace('begin handshake')
+
   series([
     (cb) => exchangeNonce(state, cb),
     (cb) => protectConnection(state, cb)

src/index.js

@@ -8,6 +8,9 @@ const Errors = require('./errors')
 const handshake = require('./handshake')
 const State = require('./state')
 const decodeV1PSK = require('./crypto').decodeV1PSK
+const debug = require('debug')
+const log = debug('libp2p:pnet')
+log.err = debug('libp2p:pnet:err')
 
 /**
  * Takes a Private Shared Key (psk) and provides a `protect` method
@@ -16,11 +19,12 @@ const decodeV1PSK = require('./crypto').decodeV1PSK
 class Protector {
   /**
    * @param {Buffer} keyBuffer The private shared key buffer
+   * @param {any} options Options for the protector
    * @constructor
    */
-  constructor (keyBuffer) {
+  constructor (keyBuffer, options) {
     const decodedPSK = decodeV1PSK(keyBuffer)
-
+    this.options = options || {}
     this.psk = decodedPSK.psk
     this.tag = decodedPSK.tag
   }
@@ -38,18 +42,22 @@ class Protector {
     assert(connection, Errors.NO_HANDSHAKE_CONNECTION)
 
     const protectedConnection = new Connection(undefined, connection)
-    const state = new State(this.psk, callback)
+    const state = new State(this.psk, { timeout: this.options.timeout }, callback)
+
+    log('protecting the connection')
 
     // Run the connection through an encryption handshake
     pull(
       connection,
       handshake(state, (err) => {
         if (err) {
+          log.err('There was an error attempting to protect the connection', err)
           return callback(err)
         }
 
         connection.getPeerInfo(() => {
           protectedConnection.setInnerConn(new Connection(state.secure, connection))
+          log('the connection has been successfully wrapped by the protector')
           callback()
         })
       }),

src/key-generator.js

@@ -1,8 +1,7 @@
 'use strict'
 
 const crypto = require('crypto')
-const nacl = require('tweetnacl')
-const KEY_LENGTH = nacl.box.sharedKeyLength
+const KEY_LENGTH = 32
 
 /**
  * Generates a PSK that can be used in a libp2p-pnet private network
@@ -15,6 +14,8 @@ function generate (writer) {
 }
 
 module.exports = generate
+module.exports.NONCE_LENGTH = 24
+module.exports.KEY_LENGTH = KEY_LENGTH
 
 if (require.main === module) {
   generate(process.stdout)

src/state.js

@@ -2,23 +2,31 @@
 
 const handshake = require('pull-handshake')
 const deferred = require('pull-defer')
-const nacl = require('tweetnacl')
+const crypto = require('crypto')
+const NONCE_LENGTH = require('./key-generator').NONCE_LENGTH
 
 /**
  * Keeps track of the state of a given connection, such as the local psk
  * and local and remote nonces for encryption/decryption
  */
 class State {
-  constructor (psk, callback) {
+  /**
+   * @param {Buffer} psk The key buffer used for encryption
+   * @param {any} options Options for the state, including timeout of the handshake
+   * @param {function(Error)} callback Called on completion of the handshake
+   * @constructor
+   */
+  constructor (psk, options, callback) {
     this.local = {
       nonce: Buffer.from(
-        nacl.randomBytes(nacl.box.nonceLength)
+        crypto.randomBytes(NONCE_LENGTH)
       ),
       psk: psk
     }
+    options = options || {}
 
     this.remote = { nonce: null }
-    this.timeout = 60 * 1000
+    this.timeout = options.timeout || 5e3
     this.secure = deferred.duplex()
     this.stream = handshake({ timeout: this.timeout }, callback)
     this.shake = this.stream.handshake

test/pnet.spec.js

@@ -14,7 +14,6 @@ const pull = require('pull-stream')
 
 const Protector = require('../src')
 const Errors = require('../src').errors
-const doneAfterN = require('./utils').doneAfterN
 const generate = require('../src/key-generator')
 
 const swarmKeyBuffer = Buffer.alloc(95)
@@ -42,20 +41,6 @@ describe('private network', () => {
     expect(protector.psk.byteLength).to.equal(32)
   })
 
-  describe('invalid psks', () => {
-    it('when the psk file is wrong', () => {
-      expect(() => {
-        return new Protector(Buffer.from('not-a-key'))
-      }).to.throw(Errors.INVALID_PSK)
-    })
-
-    it('when the psk isnt the proper length', () => {
-      expect(() => {
-        return new Protector(Buffer.from('/key/swarm/psk/1.0.0/\n/base16/\ndffb7e'))
-      }).to.throw(Errors.INVALID_PSK)
-    })
-  })
-
   it('protects a simple connection', (done) => {
     const p = pair()
     const protector = new Protector(swarmKeyBuffer)
@@ -86,18 +71,9 @@ describe('private network', () => {
     const p = pair()
     const protector = new Protector(swarmKeyBuffer)
     const protectorB = new Protector(wrongSwarmKeyBuffer)
-    const multiDone = doneAfterN(2, done)
 
-    const aToB = protector.protect(new Connection(p[0]), (err) => {
-      expect(err).to.exist()
-      expect(err.message).to.equal(Errors.INVALID_PEER)
-      multiDone()
-    })
-    const bToA = protectorB.protect(new Connection(p[1]), (err) => {
-      expect(err).to.exist()
-      expect(err.message).to.equal(Errors.INVALID_PEER)
-      multiDone()
-    })
+    const aToB = protector.protect(new Connection(p[0]), () => { })
+    const bToA = protectorB.protect(new Connection(p[1]), () => { })
 
     pull(
       pull.values([Buffer.from('hello world'), Buffer.from('doo dah')]),
@@ -106,9 +82,24 @@ describe('private network', () => {
 
     pull(
       bToA,
-      pull.collect(() => {
-        expect.fail()
+      pull.collect((values) => {
+        expect(values).to.equal(null)
+        done()
       })
     )
   })
+
+  describe('invalid psks', () => {
+    it('when the psk file is wrong', () => {
+      expect(() => {
+        return new Protector(Buffer.from('not-a-key'))
+      }).to.throw(Errors.INVALID_PSK)
+    })
+
+    it('when the psk isnt the proper length', () => {
+      expect(() => {
+        return new Protector(Buffer.from('/key/swarm/psk/1.0.0/\n/base16/\ndffb7e'))
+      }).to.throw(Errors.INVALID_PSK)
+    })
+  })
 })