emschwartz / ilp3

An implementation of Interledger V3
5 stars 3 forks source link

Call crypto.randomBytes() asynchronically #6

Closed dappelt closed 6 years ago

dappelt commented 6 years ago

In edge cases, crypto.randomBytes() will take longer to complete, which would make your connector unresponsive. Excerpt from the node docs:

The crypto.randomBytes() method will not complete until there is sufficient entropy available. This should normally never take longer than a few milliseconds. The only time when generating the random bytes may conceivably block for a longer period of time is right after boot, when the whole system is still low on entropy.

It's better to be on the safe side and call .randomBytes() asynchronically. Here is a snippet:

  return new Promise((resolve, reject) => {
    crypto.randomBytes(4, (err, buf) => {
      if (err) reject(err)
      resolve(buf.readUInt32BE(0))
    })
})
michielbdejong commented 6 years ago

else resolve(buf.readUInt32BE(0))? Or would trying to resolve the promise fail silently if it's already rejected?

dappelt commented 6 years ago

Should be ok without else. Whatever is called first of resolve/reject will be the result of the promise, see https://jsfiddle.net/0y5ovcwp/1/

dappelt commented 6 years ago

... and resolve(buf.readUInt32BE(0) fails silently if reject() was called before.

dappelt commented 6 years ago

The synchronous version of crypto.randomBytes() is also used in ilp3/lib/psk.js

emschwartz commented 6 years ago

That's not used in the connector though

On Tue, Nov 28, 2017, 5:42 AM dappelt notifications@github.com wrote:

The synchronous version of crypto.randomBytes() is also used in ilp3/lib/psk.js

— You are receiving this because you modified the open/close state.

Reply to this email directly, view it on GitHub https://github.com/emschwartz/ilp3/issues/6#issuecomment-347483512, or mute the thread https://github.com/notifications/unsubscribe-auth/ADHIkk1D18c4rRIng6CN00uTcKSiVbZgks5s6-OIgaJpZM4QrlTg .

--

Evan Schwartz Software Engineer