Thomas - Sorry I didn't get this review back to you sooner, my day job ...
10 years, 11 months ago
(2013-06-03 21:53:00 UTC)
#1
Thomas -
Sorry I didn't get this review back to you sooner, my day job is pretty busy
right now. I really like the new strength formula, particularly the
alphabet_size() function -- that's an awesome idea. I've made a couple of notes
about some minor ideas here and there, but overall I really like it.
https://codereview.appspot.com/9142045/diff/2002/pwcheck.py
File pwcheck.py (right):
https://codereview.appspot.com/9142045/diff/2002/pwcheck.py#newcode24
pwcheck.py:24: SPECIAL = set(u"""_ .,:;!?+-*/|\\<>(){}[]#@$%&="'`^~""")
I think SPACE should get it's own category... for example, a series of
space-separated words would be pretty common, and not necessarily correlate with
the use of punctuation (or vice versa).
Also, perhaps if there was some way to detect if only lowercase-hex was in use
as a special case, so it would result in an alphabet size of 16, rather than 36;
and similar for upper case? Then again, that might be an edge case not worth
checking.
https://codereview.appspot.com/9142045/diff/2002/pwcheck.py#newcode33
pwcheck.py:33: # fine-grained to give better estimates about the alphabet
size.
If you could note a canonical url where this list came from, in case it needs
updating in the future.
https://codereview.appspot.com/9142045/diff/2002/pwcheck.py#newcode217
pwcheck.py:217: if symbols:
At this point, I think converting `symbols` into a sorted list of ordinal
integers, and tracking the current index, would allow much more efficient
searching, without having to iterate over `symbols` every pass or having to
rebuild the set down on line 222.
https://codereview.appspot.com/9142045/diff/2002/pwcheck.py#newcode331
pwcheck.py:331: return ld(alphabet_size(symbols)) * entropy(symbols)
I really like this new strength formula, though I'm going to have to play with
it a bit to get a feel for what exactly it's measuring, and how it flexs in
relation to various characteristics. I may post some more feedback on it.
https://codereview.appspot.com/9142045/diff/2002/pwcheck.py#newcode341
pwcheck.py:341: def strength_classify(symbols, classifications=CLASSIFICATIONS):
Unless there was a particular reason, I think it might make it easier for users
if `classifications` was just a list of breakpoints, e.g. `[100,180]`, and just
return the right index into the list.
https://codereview.appspot.com/9142045/diff/2002/pwcheck.py File pwcheck.py (right): https://codereview.appspot.com/9142045/diff/2002/pwcheck.py#newcode24 pwcheck.py:24: SPECIAL = set(u"""_ .,:;!?+-*/|\\<>(){}[]#@$%&="'`^~""") not sure about the space. ...
10 years, 11 months ago
(2013-06-07 22:16:26 UTC)
#2
Issue 9142045: pw checking
Created 10 years, 11 months ago by Thomas.J.Waldmann
Modified 10 years, 11 months ago
Reviewers: Eli Collins
Base URL:
Comments: 9