-
Notifications
You must be signed in to change notification settings - Fork 2k
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
rar -p salt #868
Comments
Note, this bug SHOWS UP in the older jtrts input file (rar). I am making a new one. The old one was built with VERY few salt values (I am not 100% sure how that file was created). If run, this is the ouput (note most of the passwords ARE NOT right)
If I re-run (with the already created .pot file there, I get this:
This is the wc of the pot file after the 2nd run
I am going to see about adding a hash, it should not be hard at all. |
After adding a simple hash (half an MD5 of the buffer), I reran (3rd run), and this time, it worked like expected (much slower, and it finds 'proper' passwords).
The change is trivial. I think I will make it. There is a tiny loadtime hit (whatever time it takes to md5 the input data), and a small increase in memory usage, BUT this is rar, whcih already has a HUGE memory footprint, so this is not even visible. |
Before the change, the 130 password check was running at 5s / 2s. BUT it was not finding more than a handful of 'real' cracked passwords. It reported that it found 130, so TS was 'happy'. NOTE, this may point to a bug in JtR, where multiple hashes in the same salt bucket may not be getting removed when a password is found. Some of the salt logic still baffles me, so I am not fully sure what needs to be looked at. So, what I have changed (in the format), is that even though we have stopped using the data buffer in the hash (since it is variable size, we now DO have a hash of that data buffer that IS part of the salt. |
08e2a5c is the patch. This is pretty trivial, but makes a big difference on some of this contrived data, and causes no impact on speed or anything else. |
There still are 'some' issues. It is MUCH better, but possibly not 100% better;
immediate re-run
I have to dig in and find out what is up here. It may be that there are dupe hashes in the file, BUT those should get removed at load time. Looks like these 6 were -hp format (the shorter one). Now I need to figure that one out. I am pretty sure the -p have been 'fixed' with the dummy hash being added. I am not sure why the -hp is having a problem. We do not put any of the -hp data into the memory buffer, do we?? |
Found. Same type issue in the -hp mode. Fixed by doing the hash at proper point (and setting packed size for -hp even though there is nothing 'packed'). ce40a26 Is the final fix. This should hash the encrypted data, so that the buffer data can also play part in the salt dupe detection logic. |
NOTE, pass_gen was not detecting this problem, and I am not sure that it can. I will see. Possibly we can run john again, with -w=empty_file after the current 2 runs, and make sure that john lists @magnumripper has this string changed over the versions, or is it different in non-jumbo john ? |
Same issue on opencl: 07964d6 |
The old one was built using a proper rar binary with a script. I strongly suspect the salt is from a clock with a one second resolution or something very similar to that.
It's the same in core and it hasn't changed at all for many years (1.7.8 had it). |
so rar is likely very possible to generate same salt. The 'fix' is not only a theoretical fix then. I have opened a issue about this on TS. I am disturbed that TS did not catch this. |
I have run a test (edited jtrts.pl), and only a few hashes have any problems. The code change is if crack_count and show_count do not match, then I print something out. Here is where we are at:
The dyna_0, dyna_71 both have dupe hashes, so 1502 is expected. crc is a non-exact format. lm is a split format. The rar check was prior to the recent update. So it looks like rar is the only one where we have this distinct salt being viewed as duplicate bug problem (at least of the formats in the TS). rar probably had this issue prior to going to dyna-salt, since I think the prior ONLY looked at the 8 byte salt, or am I not correct here? NOTE, it is possible there are other large item hashes that have this type issue. |
I have updated jtrts. openwall/john-tests@3f31fe7 Now, rar is fixed, and jtrts would detect the problem. Moving on. |
This may be a non issue in real world, but this IS what is for real.
The data type for rar-p must be part of the salt. I saw issues when working on pass_gen. In that case, I was using fixed salts (so that I could get the same hash to know my code was working), and put in different passwords. Well in this case, JtR would find only 1 password, but would crack ALL hashes with this password. What was happening is that the salt was all identical, SINCE the packed encrypted buffer was not being checked for salt. We might be able to fix this in dyna_salt, OR we can simply add a fixed sized buffer to the salt record for rar, do an MD5 on the packed, enc buffer at load time, and store that.
Now, in real world this likely will not make ANY impact. Here is what has to be for this bug to show up.
Now where this 'could' happen, is if there are multiple rar files, all encrypted differently, all with same file inside (say a short disclaimer, or some canned file). However, they would all have to have the same 8 byte salt value. I think under 'normal' situation this would be the case. In pass_gen.pl I have taken away the ability to pass in a salt on command line, since there is NO WAY to have many hashes with same salt, since the data itself is encrypted.
At this time, I am making no move to correct this. But I wanted it listed here, since I have seen this behavior, and want to make sure that somewhere else, this type problem is not biting us.
The text was updated successfully, but these errors were encountered: