fireout / keepasssequencer

Word sequencer for KeePass
65 stars 12 forks source link

Entropy calculation for substitutions doesn't match applying substitution #35

Closed fritzophrenic closed 8 years ago

fritzophrenic commented 8 years ago

If I'm reading the code right, a substitution is applied EVERYWHERE it matches in a word, if a SINGLE random roll matches its probability. E.g. if a word has three matches for the "from", all three of them will be substituted, or none of them.

But the entropy value is calculated, as if each match position had a separate roll. E.g. if a word has three matches for the "from", then a separate roll will be made for each match, so you could have from zero to 3 substituted items.

Either the substitution code or the entropy calculation should be modified to match. Right now the entropy is too high for the actual logic.

fritzophrenic commented 8 years ago

I think this should be fixed before the next release. It should be easy enough. How do you want it fixed? I'd suggest changing the substitution code to apply randomly at each match (i.e. update substitution code to match entropy bar), because that would add entropy to the password. But if you prefer the current substitution method I can update the entropy calculation to match the substitution code instead.

fireout commented 8 years ago

I agree, we should fix it before next release. I also tend to think we should update the substitution logic rather than the entropy calculation.