Skip to content
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

--regen-lost-salts option: Bug fixes, closes #4517 #4795

Merged
merged 1 commit into from
Sep 9, 2021

Conversation

magnumripper
Copy link
Member

Since every salt copied the same salt->list pointer, a special fixup is needed after removing the first binary in the list in crk_process_guess(). Also, the generated salts written to the salt database lacked some info and the total number of salts wasn't updated.

Less important updates:
Error messages pointed to a non-existing file in a non-existing directory.
Change the "Remaining xx hashes with yy different salts" output to be more accurate when this feature is used.

@magnumripper
Copy link
Member Author

Still testing this. There's a slight problem, still:

$ ../run/pass_gen.pl dynamic_4 <../run/password.lst | shuf > test.in
$ sed -r 's/\$..:/:/' test.in > test.nosalts.in

$ ../run/john test.nosalts.in --regen-lost-salts='dynamic_4:32:?y?y' -format=dynamic_4 -w
Loaded 1500 password hashes with 9025 possible salts (1805.0x same-salt boost) (dynamic_4 [md5($s.$p) (OSC) 256/256 AVX2 8x3])
Proceeding with wordlist:../run/password.lst
Press 'q' or Ctrl-C to abort, almost any other key for status
integra          (u897:fa)     
indigo           (u260:ga)     
007007           (u1137:ga)     
(...)
dundee           (u520:49)     
frogs            (u1417:79)     
gremlin          (u1438:99)     
1499g 0:00:00:01 DONE (2021-09-01 00:21) 1094g/s 2588p/s 23359Kc/s 48003MC/s Hammer..sss
Remaining 1 password hash with 9025 possible salts
Use the "--show --format=dynamic_4" options to display all of the cracked passwords reliably
Session completed. 

$ ../run/john test.nosalts.in -format=dynamic_4 -show=left
u1290:$dynamic_4$8b2c2e07ac0cb51062832f7210d19e80:1290

1499 password hashes cracked, 1 left

$ grep 8b2c2e07ac0cb51062832f7210d19e80 test.in
u1290:$dynamic_4$8b2c2e07ac0cb51062832f7210d19e80$aa:1290:0:cannon:

It missed the 'aa' salt. This may well be a separate bug, I'll look into it.

@magnumripper magnumripper marked this pull request as draft August 31, 2021 22:26
@magnumripper
Copy link
Member Author

magnumripper commented Aug 31, 2021

Notes to self:

  • Sometimes (different shuffles) we instead get 1501 cracks and a dupe pot entry, that's a data point... and when it misses one it's always the 'aa' salt.
  • If the test file doesn't contain any 'aa' salt, all hashes are cracked.
  • If the test file contain several 'aa' salts, none of them are cracked.
  • I see pot entries aren't written with the cracked salt inserted. Not sure if that was intentionally left out but I think we can do that, using source(), if we want to.
  • With ShowUIDinCracks = Y, I get cracks like this:
aaron            (u456:E9)     
cactus           (u487:E9)     

...and they correspond to these:

$ grep '\$E9:' test.noshuf.in 
u456:$dynamic_4$92acf59298a88ae17c81095ba435deba$E9:456:0:aaron:
u487:$dynamic_4$982f9f8be1e1b0ad91e058bbc25fb984$E9:487:0:cactus:

...so what is supposed to be the uid is actually the brute-forced salt. I don't yet understand why this happens. It should have showed like this:

aaron            (u456:456)     
cactus           (u487:487)     

@magnumripper
Copy link
Member Author

magnumripper commented Aug 31, 2021

It missed the 'aa' salt. This may well be a separate bug, I'll look into it.

This is a separate bug, Jim's code never outputs the 'aa' salt... oh well, I'll fix that too.

@magnumripper
Copy link
Member Author

magnumripper commented Aug 31, 2021

This is a separate bug, Jim's code never outputs the 'aa' salt... oh well, I'll fix that too.

No, it actually does. It's written to the original single-salt at load time, before the generation of all other salts. Still, I confirmed this bug was present before my changes - it always misses the 'aa' salt.

@magnumripper
Copy link
Member Author

More problems found: Some code isn't handling the dynamic compiler format or bare hashes correctly. Also, --show does not work (never did AFAICS). I'm looking into both.

@magnumripper
Copy link
Member Author

magnumripper commented Sep 6, 2021

Hopefully, all problems are now cared for. The code always relied on "bare hashes" (ie. with no format tag) so that's now enforced (and documented). I also added a way to use --show: It simply relies on you to still supply the --regen-lost-salts option with same parameters as the ones used for cracking (or at least ending up same salt length).

Countless of other problems were fixed. I bet some remain but it's definitely better than ever.

@magnumripper
Copy link
Member Author

Note that since the --regen-lost-salts option takes a format as its first parameter (how stupid is that!?) we now infer the --format option from it if not given, or if it was given as well, we enforce they are the same.

Since every salt has a copy of the initial salt->list pointer, a special
fixup is needed after removing the first binary in the list in
crk_process_guess().  This was the actual openwall#4517 issue.

Other fixes:
The generated salts written to the salt database lacked some info and the
total number of salts wasn't updated.  From now we copy the whole initial
database struct, avoiding several current and at least some possible future
problems.
Improved the "Remaining xx hashes with yy different salts" output to be
more accurate/informative when this feature is used - correctly showing
same-salt boost or salt BF penalty.
Error messages referred to a non-existing file in a non-existing directory.
Several comments were bogus, eg. referring to code that was long gone.
General code-style fixes (including a couple of barely related, made while
trying to understand the code paths involved).
Add bodges for --show to work with hashes cracked using this feature. For
this to work you need to supply the --regen-lost-salts option with --show
too.
Since we unfortunately give the exact format as one of the parameters to
--regen-lost-salts (a fairly stupid decision made 8 years ago), add code to
automatically infer that format whenever a --format parameter isn't given,
or complain if it is and they don't match.
Minor updates to doc/Regen-Lost-Salts, eg. dropping references to things
that were deprecated eight years ago.
@magnumripper magnumripper merged commit 65818a4 into openwall:bleeding-jumbo Sep 9, 2021
@magnumripper magnumripper deleted the regen-salts branch September 9, 2021 18:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant