Fix long non-ambiguous passwords (and insertion of numerals/symbols if forced)#14
Open
MichiK wants to merge 1 commit intovinces1979:masterfrom
Open
Fix long non-ambiguous passwords (and insertion of numerals/symbols if forced)#14MichiK wants to merge 1 commit intovinces1979:masterfrom
MichiK wants to merge 1 commit intovinces1979:masterfrom
Conversation
If no_ambiguous is set, the ambiguous characters are now replaced one by one instead of throwing away the password and trying to generate a new one. This was a problem when generating long, non-ambiguous passwords. In addition, when symbols, numerals or capital letters are enforced, these characters will no longer be inserted at random positions in the password. Instead, they will only replace lowercase letters, if available. Otherwise, it was possible to e.g. overwrite the single numeral with a symbol if numerals and symbols are set and only one numeral but no symbols were present in the password (closes vinces1979#13).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
I noticed that the creation of passwords could take a long time if
no_ambiguousis set toTrueand the password length is increased sufficiently (this becomes really noticable at around 30 characters). Generation of non-ambiguous short passwords, while it already takes a lot longer than the genration of passwords that may contain ambiguous characters, is generally quite fast:However, the longer the password becomes, the higher the chance that it contains an ambiguous character. In this case, the algorithm would throw away the password entirely and generate a new one (again with a high chance of containing an ambiguous character):
In case of 10 characters, generating non-ambiguous passwords took around 16 times longer than generating normal ones. At 35 characters, this factor already increased to 12000 times longer (and it gets worse, the longer the passwords get). After this point, generating non-ambiguous is no longer practical.
Therefore I changed the password generation so that in case
no_ambiguousis set, it does not regenerate the password entirely but instead just replaces the ambiguous characters by a new, random one until all of them have been eliminated. Therefore, non-ambiguous password generation gets a lot faster and even the generation of extremely long passwords is possible:As
replaceRandomChar()now can be fed a list of positions where it should insert a new character, I also changed the mechanism that enforces caps, numerals or symbols so that it will only replace a random lowercase letter instead of any letter in the password. Therefore, if the user enforces multiple classes of characters, the chance is smaller that they will cancel each other out (see #13). Theoretically, this might still happen though, if the password does not contain lowercase letters at all that can be replaced. In that case, the algorithm falls back to replacing a random character.