-
Notifications
You must be signed in to change notification settings - Fork 2.2k
rules.c: Simplify the latest bugfix to stacked rules #5725
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
rules.c: Simplify the latest bugfix to stacked rules #5725
Conversation
4073598
to
995c5d9
Compare
995c5d9
to
a6b35de
Compare
Among other things, this is great for debugging.
Just drop a couple of redundant log_event and move the last one so it covers all three. See openwall#5722
a6b35de
to
7f2e468
Compare
If we're logging all of them, a large-ish wordfile run with many stacked rules may produce petabytes worth of log file, not to mention the performance penalty involved with that.
Also added a commit requiring debug verbosity in order to log all stacked rules (otherwise only the first run of the stacked rules are logged, then a "Some rule logging suppressed. Re-enable with --verbosity=6 or greater"). Without this, we can very easily end up with petabytes worth of log file, and performance should plummet. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me assuming you've tested it, but there's a lot more room for simplification and optimization.
@@ -1947,6 +1948,7 @@ static void john_done(void) | |||
/* We already printed to stderr from signals.c */ | |||
log_event("%s", abort_msg); | |||
} else if (children_ok) { | |||
log_event("Candidates tried: %"PRIu64"p", status.cands); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd also log suppressor's hits and misses, which is info that is otherwise impossible to access at session end (no way to press s
at exactly the right moment).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That would be a new function in status.c, right? You could create a ticket for it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That would be a new function in status.c, right?
No, it's status.suppressor_hit
and status.suppressor_miss
just like status.cands
that you use.
Just drop a couple of identical
log_event()
and move the last one so it covers all threeAlso as a separate commit, add a log call just before session end that says how many candidates were processed.
See #5722