Weird addition - output character distribution probably not inform.

[divVerent]

https://github.com/paulgreg/UniquePasswordBuilder/blob/67fc98394e6c7ce33bb4b3e87512e1b213bec632/src/passwordgeneration.js#L11

Why this weird addition? The result, asuming the inputs go from 0 to 255, will have a triangle probability distribution. You then are taking the mod with availableCharacters.length.

I think the intent is making the distribution more uniform, but byte1 * 256 + byte2 would be actually uniform at 16 bits and thus a lot better.

Probably a minor issue and not relevant in practice, and not worth an incompatible change (except when adding a new hash type and fixing this only for that one). Can you maybe just add a comment explaining how high the entropy per output character is (in bits) given this distribution?

[divVerent]

b=new Array(availableChars.length).fill(0); for(var i = 0; i < 255; ++i) for(var j = 0; j < 255; ++j) ++b[(i+j)%b.length]; b
(80) [811, 812, 813, 814, 815, 816, 817, 818, 819, 820, 821, 822, 823, 824, 825, 824, 823, 822, 821, 820, 819, 818, 817, 816, 815, 814, 813, 812, 811, 810, 810, 810, 810, 810, 810, 810, 810, 810, 810, 810, 810, 810, 810, 810, 810, 810, 810, 810, 810, 810, 810, 810, 810, 810, 810, 810, 810, 810, 810, 810, 810, 810, 810, 810, 810, 810, 810, 810, 810, 810, 810, 810, 810, 810, 810, 810, 810, 810, 810, 810]
entropy = 0; for (var n of b) { var p = n / 65536.0; entropy += -p * Math.log2(p); } entropy
6.28381765535784

So the distribution is actually pretty good, we get 6.28 bits of entropy per password character (ideal would be 6.32). Not doing this addition BTW would yield "only" 6.27 bits ;)

Anyway, can you just add a comment there saying that this yields 6.28 bits per character of entropy?

[paulgreg]

Here the PR including that comment : https://github.com/paulgreg/UniquePasswordBuilder/pull/18 What do you think ?

[divVerent]

Looks good, thanks!

Well, almost - sorry for wrong computation, I had a bug (< rather than <= 255). Correct entropy values are, from:

b=new Array(availableChars.length).fill(0); for(var i = 0; i <= 255; ++i) for(var j = 0; j <= 255; ++j) ++b[(i+j)%b.length]; entropy = 0; for (var n of b) { var p = n / 65536.0; entropy += -p * Math.log2(p); } [entropy, ...b]

• 6.3112781 bits when using naive modulo, causing a group of 1024 and a group of 768 out of 65536 combinations.
• 6.3219024 bits with your code, causing from 816 to 832 possibilities to reach each character.
• 6.3219279 bits with (i+256*j)%length.
• 6.3219281 bits maximum achievable entropy when taking "many" input bytes (simply -log2(1/80)).

So please just make the comment say 6.32 bits, and I think then this should be clear.

And yes, this is nitpicking - the damage done by the suboptimal operation is less than a single bit per password - but for the next person reviewing this, having the number 6.32 bits written there will be useful.

Thanks!