crypto.pseudoRandomBytes

[douxing]

Hi, i am a little confused with this line of your code: rnd = crypto.pseudoRandomBytes(howMany);

but i cannot find this function in node.js document about crypto.

i don't if i am wrong, this is just a kindly remindness

[raszi]

I bet this was changed since the commit made to the code.

If you check this answer on stackoverflow it describes the situation, although it seems like that the node authors just modified the crypto library and kept the pseudoRandomBytes for backward compatibility.

See the following short snippet:

var crypto = require('crypto');
crypto.randomBytes(1); // <Buffer 44>
crypto.pseudoRandomBytes(1); // <Buffer 45>

See also:

• nodejs/node-v0.x-archive#6372
• nodejs/node-v0.x-archive#7724

[douxing]

Sorry for the late, thank you for the explanation.

[3cp]

pseudoRandomBytes api has been removed from Nodejs since v4. That's a long long time ago. It should be removed from here, since tmp only supports Nodejs v8+.

"engines": {
    "node": ">=8.17.0"
  },

[silkentrance]

While it seems to have been deprecated, it is still available in Node v16.15.1.

λ node                                
Welcome to Node.js v16.15.1.          
Type ".help" for more information.    
> const crypto = require('crypto')    
undefined                             
> crypto.pseudoRandomBytes(10)        
<Buffer 07 c1 62 de b1 20 90 06 f7 4c>
> crypto.pseudoRandomBytes            
[Function: randomBytes]                                                    

Which is why we never changed it.

[3cp]

  try {
    rnd = crypto.randomBytes(howMany);
  } catch (e) {
    rnd = crypto.pseudoRandomBytes(howMany);
  }

The code branch is never reachable in Nodejs v8+.

[raszi]

You are probably right in most of the cases. However, it could happen that crypto.randomBytes() throws an error and in that case, we are fallbacking to the crypto.pseudoRandomBytes().

I'll need to investigate deeper before we can safely remove this fallback mechanism.

[3cp]

My encounter of the issue is Nodejs's EMS export.

When I use esbuild to bundle my lib, it transpile this lib into esm. Something like this:

import {randomBytes, pseudoRandomBytes} from 'crypto';

Then at runtime, Nodejs rejects it because the ESM exports (I guess Nodejs internal) of crypo doesn't have pseudoRandomBytes.

import {randomBytes, pseudoRandomBytes} from 'crypto';
                     ^^^^^^^^^^^^^^^^^
SyntaxError: The requested module 'crypto' does not provide an export named 'pseudoRandomBytes'
    at ModuleJob._instantiate (node:internal/modules/esm/module_job:128:21)
    at async ModuleJob.run (node:internal/modules/esm/module_job:194:5)
    at async Promise.all (index 0)
    at async ESMLoader.import (node:internal/modules/esm/loader:409:24)
    at async loadESM (node:internal/process/esm_loader:85:5)
    at async handleMainPromise (node:internal/modules/run_main:61:12)

[raszi]

It is there, but I guess the type definitions are not showing that anymore.

Welcome to Node.js v18.1.0.
Type ".help" for more information.
> const crypto = require('crypto')
undefined
> crypto.pseudoRandomBytes(200)
<Buffer 70 39 22 06 93 67 f4 25 dc e7 43 a4 3b 82 ac 4b f8 61 43 23 4e 81 c1 52 fa 6f 18 44 92 13 d1 69 99 a4 34 82 79 a8 ac b0 f8 38 39 30 27 3d b5 63 d4 cc ... 150 more bytes>

[3cp]

It's there for commonjs access, not esm access.

> cat > file.mjs
import {randomBytes, pseudoRandomBytes} from 'crypto';

> node file.mjs
file:///Users/huocp/playground/file.mjs:1
import {randomBytes, pseudoRandomBytes} from 'crypto';
                     ^^^^^^^^^^^^^^^^^
SyntaxError: The requested module 'crypto' does not provide an export named 'pseudoRandomBytes'
    at ModuleJob._instantiate (node:internal/modules/esm/module_job:128:21)
    at async ModuleJob.run (node:internal/modules/esm/module_job:194:5)
    at async Promise.all (index 0)
    at async ESMLoader.import (node:internal/modules/esm/loader:409:24)
    at async loadESM (node:internal/process/esm_loader:85:5)
    at async handleMainPromise (node:internal/modules/run_main:61:12)

Node.js v18.4.0

[3cp]

FYI, Node.js v12.22.12 also gives the same error.

[raszi]

I don't really understand your point. tmp is importing the crypto module using require():

https://github.com/raszi/node-tmp/blob/4b51e90248cef684a38803304c7e8dc68b1d06e2/lib/tmp.js#L15

How is your example relevant to tmp?

[raszi]

When I use esbuild to bundle my lib, it transpile this lib into esm. Something like this:

Sorry, I missed this part. I don't think this is an issue of tmp but rather an esbuild issue.

[3cp]

Not esbuild, did you see my example above?

The cat and node commands, without esbuild.

[raszi]

This source code is not from tmp. How is this related then?

[3cp]

Sorry. I misrepresented myself. I am not saying this is tmp code issue.

Rather just a point for consideration to remove the usage of pseudoRandomBytes, since you said "I'll need to investigate deeper".

The other point is Nodejs removed pseudoRandomBytes from its public API document.

[3cp]

https://github.com/nodejs/node/blob/main/lib/crypto.js

FYI pseudoRandomBytes is just an alias to randomBytes, it's not the real pseudo anymore. So the try-catch fallback here is actually not doing anything more.

[raszi]

Thanks for the link. This definitely helps. I'll open a PR as soon as I get some free time.

[silkentrance]

randomBytes will still generate an error. So we need a replacement pseudoRandomBytes generator in case that there is not enough entropy in the system and the "true" random bytes generator runs out of data.

[3cp]

From nodejs doc, crypto.randomUUID does not throw, but it's only available in v15.6.0+.

[silkentrance]

reopening since this is definitely an issue and we might run out of entropy, especially during system start.

[silkentrance]

@3cp Good point. I also thought about having a UUIDv4 based name generation strategy. However, this will require an additional dependency for node < v15.6.0, as you pointed out.

Maybe we will revive the old 'naive' random name generation code that simply used random ints and a lookup table as a fallback.

[silkentrance]

I will create a new issue and link to this one in the description.

[silkentrance]

Please continue the conversation over at #280