Coordinated Disclosure Timeline

Summary

keypair implements a lot of cryptographic primitives on its own or by borrowing from other libraries where possible, including node-forge. An issue was discovered where this library was generating identical RSA keys used in SSH. This would mean that the library is generating identical P, Q (and thus N) values which, in practical terms, is impossible with RSA-2048 keys. Generating identical values, repeatedly, usually indicates an issue with poor random number generation, or, poor handling of CSPRNG output.

Product

keypair

Tested Version

v1.0.3

Details

Issue 1: Poor random number generation (GHSL-2021-1012)

The library does not rely entirely on a platform provided CSPRNG, rather, it uses it’s own counter-based CMAC approach. Where things go wrong is seeding the CMAC implementation with “true” random data in the function defaultSeedFile. In order to seed the AES-CMAC generator, the library will take two different approaches depending on the JavaScript execution environment. In a browser, the library will use window.crypto.getRandomValues(). However, in a nodeJS execution environment, the window object is not defined, so it goes down a much less secure solution, also of which has a bug in it.

It does look like the library tries to use node’s CSPRNG when possible:

https://github.com/juliangruber/keypair/blob/87c62f255baa12c1ec4f98a91600f82af80be6db/index.js#L1016

Unfortunately, it looks like crypto is null because a variable was declared with the same name, and set to null:

https://github.com/juliangruber/keypair/blob/87c62f255baa12c1ec4f98a91600f82af80be6db/index.js#L759

So the node path is never taken.

However, when window.crypto.getRandomValues() is not available, a Lehmer LCG random number generator is used to seed the CMAC counter, and the LCG is seeded with Math.random. While this is poor and would likely qualify in a security bug in itself, it does not explain the extreme frequency in which duplicate keys occur.

Main flaw

The output from the Lehmer LCG is encoded incorrectly. The specific line with the flaw is:

b.putByte(String.fromCharCode(next & 0xFF))

The definition of putByte is

util.ByteBuffer.prototype.putByte = function(b) {
  this.data += String.fromCharCode(b);
};

Simplified, this is String.fromCharCode(String.fromCharCode(next & 0xFF)). The double String.fromCharCode is almost certainly unintentional and the source of weak seeding. Unfortunately, this does not result in an error. Rather, it results most of the buffer containing zeros. An example generated buffer:

(Note: truncated for brevity)

\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00
\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00
\x00\x00\x00\x00\x04\x00\x00\x00....\x00\x00\x00\x00\x00\x00\x00\x00\x00
\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00
\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00

Since we are masking with 0xFF, we can determine that 97% of the output from the LCG are converted to zeros. The only outputs that result in meaningful values are outputs 48 through 57, inclusive.

The impact is that each byte in the RNG seed has a 97% chance of being 0 due to incorrect conversion. When it is not, the bytes are 0 through 9.

In summary, there are three immediate concerns:

  1. The library has an insecure random number fallback path. Ideally the library would require a strong CSPRNG instead of attempting to use a LCG and Math.random.
  2. The library does not correctly use a strong random number generator when run in NodeJS, even though a strong CSPRNG is available.
  3. The fallback path has an issue in the implementation where a majority of the seed data is going to effectively be zero.

Impact

Due to the poor random number generation, keypair generates RSA keys that are relatively easy to guess. This could enable an attacker to decrypt confidential messages or gain authorized access to an account belonging to the victim.

CVE

Credit

This issue was reported to GitHub Security Lab by Ross Wheeler of Axosoft. It was discovered by Axosoft engineer Dan Suceava, who noticed that keypair was regularly generating duplicate RSA keys. GitHub security engineer @vcsjones (Kevin Jones) independently investigated the problem and identified the cause and source code location of the bug.

Contact

You can contact the GHSL team at securitylab@github.com, please include a reference to GHSL-2021-1012 in any communication regarding this issue.