Restore new_keys logic for HMAC-SHA1 part in RSVP format

[kholia]

This is related to #2989.

After this patch,

$ ./jtrts.pl -noprelims  --type rsvp
-------------------------------------------------------------------------------
- JtR-TestSuite (jtrts). Version 1.13, Dec 21, 2014.  By, JimF & others
- Testing:  John the Ripper v3-3677-g21e29cd5f OMP [linux-gnu 64-bit AVX2-ac]
--------------------------------------------------------------------------------

John Jumbo build detected.

form=rsvp-md5                     guesses: 1500 0:00:00:00 DONE  [PASSED]
.pot CHK:rsvp-md5                 guesses: 1500 0:00:00:00 DONE  [PASSED] (1500 val-pwd)

form=rsvp-sha1                    guesses: 1500 0:00:00:01 DONE  [PASSED]
.pot CHK:rsvp-sha1                guesses: 1500 0:00:00:00 DONE  [PASSED] (1500 val-pwd)

All tests passed without error.  Performed 2 tests.  Time used was 3 seconds

[magnumripper]

Ideally we'd add similar code to the SHA-2 types. OTOH we should also use SIMD (for all types) - not sure how hard it'd be to do both.

[kholia]

I had new_keys logic working for SHA-256 but I couldn't get it working for SHA-512 in the time I had. So I decided to use the simpler approach for now.

[kholia]

@magnumripper Please don't merge this yet. I will have new_keys working for all HMAC types very soon.

Update: All set now. This needs a review. Thanks!

[kholia]

It would be nice to collect the repeated padding strings into one place.

[magnumripper]

Cool, but I think you now have this bug: As long as you have different new_keys for eg. SHA-224 and SHA-256, they need their own opad_ctx and ipad_ctx. So you probably need to introduce ipad_ctx_224, ipad_ctx_384 and same for opad.

Unfortunately I do not think the self-tests will catch this bug.

[magnumripper]

It would be nice to collect the repeated padding strings into one place.

Yeah you could make them two constant char arrays, with length enough to support SHA-512 need.

[kholia]

Both done now. Thanks for the quick reviews 👍

[jfoug]

Good call. I had initially looked at your code that was using the _hmac() functions, and did a quick/dirty change to SHA1 to do the hmac inside the crypt_all (to get the 2x speed improvements on multi-salt) and was going to post a diff, BUT it looks like you beat me to it.

Good stuff!

[jfoug]

Ideally we'd add similar code to the SHA-2 types. OTOH we should also use SIMD (for all types) - not sure how hard it'd be to do both.

Are SIMD builds going to be worth it for this hash? Is this a hash that is VERY MUCH wanted? Doing SIMD in hmac is not a small undertaking. And what you are doing in this format, is doing SIMD allocation, key updating, binary complexity, hashing complexity for ALL hash types. So you will have to add SIMD buffers (all of them), for md5, sha1, sha224, sha256, sha384, sha512, and KEEP THEM ALL UPDATED, even if there are no hashes of a certain type. So if you had a file of SHA256 hashes, you would still be doing the key setting work for all SIMD buffer hash types. Ugly, slow, and not a good fit.

If these hashes were NOT all contained within a single format, then possibly it would be worth the effort. But as a unified format, my vote is to not do this, OR to do this for only the most see (IRL) hash. Say SHA1 and MD5 are the 'defaults', and 95%-99% of all hashes ever seen would be of those 2 flavors. Well, then it may make improvements to add SIMD logic for those types. BUT if IRL it is a pretty even mix of all of the hash types (ignore sha224/sha384, IMHO they are simply afterthoughts and few people will use them IRL), then the fit for SIMD, and getting it ported to BE, etc, is not worth the effort, and actually may be slower in the end.

But I think the idea of adding SIMD to this already complex hash is something which should be greatly questioned.

[magnumripper]

You are probably right. We could make shared HMAC-XX functions with SIMD support, much like the PBKDF2 ones. But it would be hard (or messy) to combine with this new_keys speedup.

[jfoug]

The problem with making shared functions, is HMAC is a single operation. The speed gains we got from doing the shared model of PBKDF2, is that we are doing MANY HMAC calls.

The main gains you were able to squeeze out of the HMAC raw code, was in the key setting. But in this format, our key setting would have to be done into 6 separate key buffers, so it is prepped and ready to be used inside the crypt_all. Now in crypt all, we would only load and generated the opad/ipad for each hash type, as we saw them within the salts. But outside, we just have to be ready.

now, we could add 'state' variables, listing that we have seen data of this type (in our input file). To do this, we would need to watch and ONLY track these salt sets once we are out of TS mode. But even then, if we have a input file with 6 salts, 1 record of each type, then on the set_key, we are still placing the key into 6 SIMD buffers, prepping the buffers for step 2 of the HMAC. I just do not see performance gains (at least not big ones), adding SIMD to this format. I could be wrong, but I really think this one is ugly enough that it should be a non-SIMD format.

Now, what 'may' work, is if someone wants them BAD ENOUGH, that we write 6 independent formats to do the SIMD work. If each format (only built in SIMD mode), only accepts 1 hash flavor, and someone is working on 200 SHA1 RSVP hashes, then SIMD is a vast improvement if they used --format=rsvp-simd-sha1 format, not simply the rsvp format (the main rsvp format simply being CTX model). But again, getting the proper thought design to utilize SIMD logic for this hash, is NOT an easy undertaking.

The reason we have gotten by with jamming SIMD into some of these other overly complex mega formats (office is a great example), is that they have a HOT loop, which we can simply do the SIMD code there, and get 99% of the possible improvements, while ignoring a lot more. For Office, we simply set the key into a char* buffer, and then within the hot spots (the PBKDF2 crap), we convert to SIMD, do the SIMD logic, and then revert back to flat mode. For the simplistic work of a single HMAC, we do not get that luxury. To make things work, you have to squeeze performance out of every nook and cranny, setting keys, hashing, crypting, etc. it all has to be done, like was done in the raw-hmac formats.

So my vote, is if we WANT to do simd, that we create SIMD only formats, and each only does 1 hash type. Now we 'could' do this with something like how dyna did 'flat' logic, or building a 'generic header', that we set certain defines, and include the thing, 6 times with 6 different settings on the defines. This would allow just a single SIMD format source file (well 2, since we would need the main format, and then the 'real' code in the header file. But at this time (only thinking about it for an hour or so), I really do not see how this can be done within a single format.

[kholia]

Are SIMD builds going to be worth it for this hash?

I don't think so. I am quite happy with the current speed. We could invest that effort somewhere else perhaps?

[jfoug]

Yup. If we had unlimited monkeys typing on unlimited keyboards, that would be a different thing. But there is only so many hours our limited set volunteers have to put into the project at all. And I agree, there are areas where the time is better spent.

[kholia]

I am super interested in seeing support for arbitrary (but reasonable) expressions in the dynamic format with OpenSSL backend.

I vaguely remember reading an article which mentioned an "enhanced hash scheme description" language. Something like our dynamic expression language but with support for sub-strings, and iterations. I can't find the article now ;(

Update: Found the article! https://s3inlc.wordpress.com/2017/11/10/algorithms-on-hashes-org/.

[jfoug]

Yes, the dyna compiler certainly 'could' really become dynamic. using the existing code in dynamic when it can (the flat code), and using other logic, when it can not (such as iterations, hmacs, pbkd2 algorithm, etc). I do like how the compiler does allow 'optimizations' into using appropriate dynamic_flat code, when it can, and I think that is a great way to optimize. But also having it actually fall back into a new format, that does recursive decent parsing on hashes which fall outside of capabilities of the existing dynamic, and then this new format simply does that, would be a great gain. The format would not be the fastest thing possible, BUT it would allow usage of john anyway, and allow you to prototype a 'new' hash easily, get it working, and then if it is something important, write a 'fat' format that optimizes it.

Lemme dig into that article a bit.

[jfoug]

I really like the additions, such as iteration counts, substrings, and rotations. my plan was to do use the '^' character for iterations, such as md5($p)^1000 or sha256((sha256($s.$p).sha256($p))^1500), etc. As for sub and rot[lr] The can be added. Currently, the name of the function specifies things like casing, base64, etc (for internal, while still hashing, re-hash work).

Some other things to think about are talk of algos, such as:

h1=md5($p) count=1000+substr(h1,0,3) hash=sha256($s.$p)^count

within the existing expression syntax, we have no way of INTERPRETING the prior hash results for anything other than feed into the next hashing. but here, we could add something like:

hash=sha256($s.$p)^(binary_value(substr(md5($p),0,3))+1000) (we could also build a hex_value function)

so this type usage 'can' be converted into our single expression model, albeit the expression parser just grew a LOT more complex, but CERTAINLY doable. The SunMD5 fits this model (sort of), where it does some computations on the data returned from a subhash test (heck, all of the algo-crypt functions do that). But those hashes are probably beyond trying to handle generically. They simply have too much 1-off uniqueness to them.

[magnumripper]

This should probably be discussed elsewhere or we'll never find it again... I think an extremely interesting idea is OpenCL dynamic (#1564). It could re-use many ideas from current dynamic and/or dynamic compiler but in the end it'd be combining the needed pre-made functions or blocks of code at run-time to a temporary kernel just for the occasion. Unlike on CPU, we'll actually be compiling the result at run-time, which means the optimizers can do a lot of difference (normally, most functions will end up inlined).

[kholia]

I guess this PR can be merged now.

[jfoug]

OT post information placed in #2800, since it is good information. I would hate to lose it.