Follow @Openwall on Twitter for new release announcements and other news
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Mon, 4 Mar 2013 23:56:12 +0100
From: magnum <john.magnum@...hmail.com>
To: john-users@...ts.openwall.com
Subject: Re: dmg2john used and password cracked, hdiutil fails to accept it

On 4 Mar, 2013, at 20:59 , Milen Rangelov <gat3way@...il.com> wrote:

>> Yes, but I meant the verification during cracking. I don't quite get the
>> apple_des3_ede_unwrap_key1() function in our format but it does not compare
>> the decrypted stuff with anything known. It just rejects hashes when
>> certain EVP decrypt calls return non-zero. I presume this means we are
>> accepting any candidate that happens to pass the padding check? This should
>> merely pose as early reject and we need more stringent tests following it.
>> 
> 
> 
> Hmmmm the devil is in the details and it seems we've missed one here
> (rather stupid I admit).
> 
> Basically the checks are OK - yes, they are meant not to compare the
> decrypted stuff with anything known, at least not explicitly. Implicitly,
> we do that - because by default, the EVP_Decrypt* routines by default check
> PKCS#5 padding. Here is how it works:
> 
> *PKCS padding works by adding n padding bytes of value n to make the total
> length of the encrypted data a multiple of the block size. Padding is
> always added so if the data is already a multiple of the block size n will
> equal the block size. For example if the block size is 8 and 11 bytes are
> to be encrypted then 5 padding bytes of value 5 will be added.*
> 
> *When decrypting the final block is checked to see if it has the correct
> form.*
> 
> So when we decrypt 40 bytes, "real" payload is just 32 bytes (block size
> with 3des is 8 bytes). We can explicitly check if the final 8 bytes are
> "\x08\x08\x08\x08\x08\x08\x08\x08", but EVP_DecryptFinal already does that
> for us and returns error when it's not properly structured.
> 
> So this check is not that promiscuous at all :)

Sounds good. So provided wrapped_key_len is always some multiple of 8, we have 64 bits of verification - that's plenty.

> However there is something stupid, which is completely not needed and
> should be removed (and yes, it was in my code too). This check:
> 
> if (cur_salt->len_wrapped_aes_key == 48)
> 
> Will not work and we will never exit when padding is bad. This might be
> "corrected" on the second decryption operation, but we already increase our
> chances for false positive a lot.
> 
> Just removing this bad check will fix it for you.


Except for this test vector:

{"$dmg$1*20*f615ec6c463799eccc6a2dfbedf12c6bdc422a2a*56*a595f4a81a490e7aa6378034661da57a424f922c971d3db3f856f8d54b0784bcc5d7182905c4237153c5d250b8aee1d26410b1dca7b1cb73*48*74a060efbaf2c79d5523219d8162c425befbb2094fb46e7ffaedc7cd4f192e6f0c47d8aa91e0a3201346725d3ddadfff*1000", "vilefault"}

This corresponds to testimage.AES-256.64k.header_v1.dmg which has a len_wrapped_aes_key of 56. The padding test failed for this file as soon as I removed the bogus check. After quite a lot of head scratching and debugging I found out it was because of hard-coded key lengths in the caller:

if ((apple_des3_ede_unwrap_key1(cur_salt->wrapped_aes_key, 40, derived_key) == 0) && (apple_des3_ede_unwrap_key1(cur_salt->wrapped_hmac_sha1_key, 48, derived_key) == 0)) {

The "40" should be cur_salt->len_wrapped_aes_key and the "48" should be cur_salt->len_hmac_sha1_key. Maybe you already got that right in hashkill.

Fixes committed now. I think all is good now! Thanks for your help. BTW I found a silly bug in dmg2john (my bad again), the iterations count *is* declared as 1000 in all v1 images I have found. But I'll still fall back to 1000 if it reads as zero. I figure it can't hurt.

magnum

Powered by blists - more mailing lists

Confused about mailing lists and their use? Read about mailing lists on Wikipedia and check out these guidelines on proper formatting of your messages.