creating biased random numbers from a cryptographically secure source

[ptrgits]

Describe the bug

keycloak-js/lib/keycloak.js

Lines 1667 to 1670 in fd1fca4

	const randomData = generateRandomData(len)
	const chars = new Array(len)
	for (let i = 0; i < len; i++) {
	chars[i] = alphabet.charCodeAt(randomData[i] % alphabet.length)

Generating secure random numbers can be an important part of creating a secure software system. This can be done using APIs that create cryptographically secure random numbers. however, using some mathematical operations on these cryptographically secure random numbers can create biased results, where some outcomes are more likely than others. such biased results can make it easier for an attacker to guess the random numbers, and thereby break the security of the software system.

The below uses the modulo operator to create an array of 10 random digits using random bytes as the source for randomness.

const crypto = require('crypto');
const digits = [];
for (let i = 0; i < 10; i++) {
    digits.push(crypto.randomBytes(1)[0] % 10); 
}

The random byte is a uniformly random value between 0 and 255, and thus the result from using the modulo operator is slightly more likely to be between 0 and 5 than between 6 and 9. The issue has been fixed in the code below by using a library that correctly generates cryptographically secure random values.

const cryptoRandomString = require('crypto-random-string');
const digits = cryptoRandomString({length: 10, type: 'numeric'});

Alternatively, the issue can be fixed by fixing the math in the original code. In the code below the random byte is discarded if the value is greater than or equal to 250. Thus the modulo operator is used on a uniformly random number between 0 and 249, which results in a uniformly random digit between 0 and 9.

const crypto = require('crypto');

const digits = [];
while (digits.length < 10) {
    const byte = crypto.randomBytes(1)[0];
    if (byte >= 250) {
        continue;
    }
    digits.push(byte % 10); 
}

References

Understanding “randomness”
Rule - Use strong approved cryptographic algorithms

Version

26.2.1

[jonkoops]

Keycloak JS doesn't run in Node.js, so we can't use those APIs you are suggesting here. We could use the Web Crypto API as an alternative, it can generate random bytes. PRs welcome.