Follow @Openwall on Twitter for new release announcements and other news
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CABh=JRFwyxV8vKjuGSb=1Dq3JCAgZWQMkPbJqdcWwBBYj3PuBw@mail.gmail.com>
Date: Mon, 4 Mar 2013 21:59:09 +0200
From: Milen Rangelov <gat3way@...il.com>
To: john-users@...ts.openwall.com
Subject: Re: dmg2john used and password cracked, hdiutil fails to
 accept it

> 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 :)

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.

Regards,
Milen

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.