[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20160610190608.GA31513@openwall.com>
Date: Fri, 10 Jun 2016 22:06:08 +0300
From: Solar Designer <solar@...nwall.com>
To: john-dev@...ts.openwall.com
Subject: Re: dmg2john's dmg file header parsing
On Fri, Jun 10, 2016 at 09:53:03PM +0300, Solar Designer wrote:
> Our dmg2john.c and dmg2john.py assume that the lengths of some dmg file
> header fields are fixed, even though the actual lengths are encoded in
> the header and are propagated into the output "hash". I got a couple of
> dmg header v2 files where:
>
> kdf_salt_len = 20
> blob_enc_iv_size = 8
> encrypted_keyblob_size = 64
>
> However, dmg2john.py blindly assumes blob_enc_iv_size = 32 and
> encrypted_keyblob_size = 48. dmg2john.c appears to similarly assume the
> former, but not the latter (so it encodes 32 and 64 instead of 8 and 64).
> kdf_salt_len appears to be handled correctly by both.
Upon a second thought, maybe padding the IV to 32 bytes with zeroes, and
reporting its size in the "hash" as 32, is actually correct for our
cracker?
The obvious bug is different output by dmg2john.c vs. dmg2john.py for
encrypted_keyblob_size = 64, where .py would replace 16 bytes that are
non-zero in the dmg file with zeroes.
> I have no idea whether any such assumptions are also present in the
> actual dmg crackers. Either way, we clearly have this bug in dmg2john*,
> where they blindly output erroneous "hashes" instead of processing the
> input files correctly or reporting an error.
Per the above, I am not so sure about dmg2john.c's handling of IV.
encrypted_keyblob_size other than 48 appears to be supported in the
cracker, as there are two checks like this:
if (cur_salt->encrypted_keyblob_size == 48)
AES_set_decrypt_key(aes_key_, 128, &aes_decrypt_key);
else
AES_set_decrypt_key(aes_key_, 128 * 2, &aes_decrypt_key);
I am not sure if my specific case is supported, though.
> For dmg2john.py, fixing this problem correctly would require changing
> the approach it uses. Right now, it parses the entire header with:
>
> v1_header_fmt = '> 48s I I 48s 32s I 296s I 300s I 48s 484s'
> v1_header_size = struct.calcsize(v1_header_fmt)
> v2_header_fmt = '> 8s I I I I I I I 16s I Q Q 24s I I I I 32s I 32s I I I I I 48s'
> v2_header_size = struct.calcsize(v2_header_fmt)
>
> It will need to separately parse the length fields and the actual data.
> In my case, the "48s" in v2_header_fmt should become "64s", but it would
> probably be wrong to patch it in the v2_header_fmt line, because some
> other file could in fact need "48s" or something else.
Since the cracker seems to only support one other value besides 48,
maybe an acceptable workaround would be to support only 48 and 64 in
dmg2john.py, and error out on anything else. This is a much smaller
change from the current code.
> Dhiru, will you fix this properly, please? Thanks!
Alexander
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.
