efs_fmt_plug.c: iteration count again

[frank-dittrich]

efs_fmt_plug.c also has a component iterations that isn't uses in crypt_all.

valid() tests for a component "iterations", get_salt() has

cs.iterations = atoi(p);

crypt_all calls:

pbkdf2_sha1_shit(out2, 20, cur_salt->iv, 16, 4000, 32, out);

an aptly named function which is defined like this:

// XXX this is most likely crap!
static void pbkdf2_sha1_shit(unsigned char *password, size_t plen,
    const unsigned char *salt, size_t slen,
    const unsigned long iteration_count, const unsigned long key_length,
    unsigned char *output)

I couldn't agree more.

[frank-dittrich]

BTW, since Dhiru seems to use // XXX as a synonym for "FIXME", let's see where other, similar problems might be hidden:

$ git grep -c "// XXX" 7z_fmt_plug.c:3 SybasePROP_fmt_plug.c:1 blackberry_ES10_fmt_plug.c:2 broken/opencl_7z_fmt.c:4 cloudkeychain_fmt_plug.c:1 dmg_fmt_plug.c:1 dynamic_fmt.c:1 ecryptfs_fmt_plug.c:1 efs_fmt_plug.c:2 gpg2john.c:2 gpg_fmt_plug.c:1 kwallet_fmt_plug.c:1 lotus85_fmt_plug.c:3 luks2john.c:1 md2_fmt_plug.c:1 net_md5_fmt_plug.c:2 net_sha1_fmt_plug.c:2 opencl_dmg_fmt.c:1 opencl_gpg_fmt.c:1 ripemd_fmt_plug.c:1 snefru_fmt_plug.c:1 tcp_md5_fmt_plug.c:2 tiger_fmt_plug.c:1 unused/bwtdt_fmt_plug.c:2

But we also have:

$ git grep -c "FIXME" 7z_fmt_plug.c:8 BFEgg_fmt_plug.c:1 NETNTLMv2_fmt_plug.c:1 androidfde_fmt_plug.c:1 bitcoin_fmt_plug.c:5 c3_fmt.c:1 cloudkeychain_fmt_plug.c:9 cuda_xsha512_fmt.c:1 django_fmt.c:2 django_scrypt_fmt_plug.c:1 dmg_fmt_plug.c:2 dynamic_fmt.c:1 gpg_fmt_plug.c:3 lastpass_sniffed_fmt_plug.c:1 listconf.c:2 md2_fmt_plug.c:1 mkv.c:1 odf_fmt_plug.c:1 office_fmt_plug.c:1 opencl_dmg_fmt.c:1 opencl_gpg_fmt.c:3 openssl_enc_fmt_plug.c:2 panama_fmt_plug.c:1 pbkdf2-hmac-sha512_fmt_plug.c:1 pkzip_fmt_plug.c:1 rar_fmt.c:2 skein_fmt_plug.c:1 snefru_fmt_plug.c:1 x86-64.S:1 x86-sse.S:1

[magnumripper]

Good old @kholia :laughing:

[magnumripper]

Prio one for all these cases is to reject hashes that does not use the hardcoded iteration count. Not doing that is a serious bug because it silently accepts hashes and produces false negatives (shrug).

So format2john often needs fixes, and then format's valid(). Once format2john is fixed it's easy to actually honor the iteration count instead of rejecting. But it must AT LEAST reject.

[frank-dittrich]

blackberry_ES10_fmt_plug.c where I removed iterations from the salt definition (also has 2 // XXX comments) has this in crypt_all:

for (j = 0; j < 99; j++) {

not sure, whether this is a fixed value, or if that is a tunable cost parameter.

And androidfde_fmt_plug.c where I removed iterations and other unused salt components (no // XXX comment) has this:

        pbkdf2_sha1((const uint8_t*)password, strlen(password),
                (const uint8_t*)(cur_salt->salt),
                16,
                2000,
                keycandidate,
                cur_salt->keysize + 16, 0);

The 2000 also looks af this shold be a variable...

[magnumripper]

I think removing the unused struct members was wrong. They indicate needs for fixes.

[frank-dittrich]

Yes, now I know that. That's why I re-checked what those formats were really doing. But when I first saw this, I assumed some copy&paste problem. I used separate commits for those, so that they can easily be reverted. I reverted them locally, will send a pull request soon. (Done now)

[kholia]

Fixing this EFS stuff has been on my list for a while now. The bigger problem is the lack of a good efs2john program. We have one (in a different repository) but it only supports older Windows operating systems.

It seems @magnumripper has resorted to naming and shaming ;(

[magnumripper]

Sorry! No offense.

[frank-dittrich]

Unfortunately, EFS wasn't the only format with hard coded iteration count mentioned here (I know, I might as well have created separate issues for these). At least these two should be checked (see my comments from April, 11th):

blackberry_ES10_fmt_plug.c androidfde_fmt_plug.c

[frank-dittrich]

lastpass_fmt_plug.c is another candidate. custom_salt has a component iterations which is never filled and never used. crypt_all apparently uses a hard coded iteration count of 500.

[magnumripper]

At least these two should be checked (see my comments from April, 11th): blackberry_ES10_fmt_plug.c androidfde_fmt_plug.c

lastpass_fmt_plug.c is another candidate.

This issue is about EFS. Please add any other problems as separate issues or they will almost certainly be forgotten.

[kholia]

I am in favor of moving this ticket to the next milestone.

[jfoug]

How big of a issue is it getting a proper efs2john made? How important is this hash? Is it something highly used? The format is now about 7x faster than it was yesterday.

$ ../run/john -test=5 -form=efs
Will run 8 OpenMP threads
Benchmarking: EFS [PBKDF2-SHA1-crap 3DES 32/64]... (8xOMP) DONE
Raw:    793 c/s real, 123 c/s virtual

$ ../run/john -test=5 -form=efs
Will run 8 OpenMP threads
Benchmarking: EFS [PBKDF2-SHA1-crap 3DES  128/128 SSE4.1 2x]... (8xOMP) DONE
Raw:    5490 c/s real, 789 c/s virtual

The non SIMD build also improved about 2x (due to the 1/2 crypt pbkdf2 code)

[magnumripper]

Perhaps we should change the algo name to something serious. Perhaps "PBKDF2-SHA1 (variant)"

[jfoug]

I agree. Or even PBKDF2-SHA1 (efs-variant) since there could be other variant out there also.

I think this will cause a change to benchunify, correct, or is this a new format?

[frank-dittrich]

On 12/09/2014 06:06 AM, magnum wrote:

At least these two should be checked (see my comments from April, 11th):
blackberry_ES10_fmt_plug.c
androidfde_fmt_plug.c

lastpass_fmt_plug.c is another candidate.

This issue is about EFS. Please add any other problems as separate issues or they will almost certainly be forgotten.

Done. https://github.com/magnumripper/JohnTheRipper/issues/881 https://github.com/magnumripper/JohnTheRipper/issues/882 https://github.com/magnumripper/JohnTheRipper/issues/883

[magnumripper]

Hmm but this one is fixed!

[jfoug]

I was leaving this one open in the hopes that @kholia was going to finish the efs2john prior to J1.

[kholia]

@jfoug right, I have been on it for a while now but finding spare time these days is very hard.

[magnumripper]

Ah, OK. We should open a new ticket for that.

[kholia]

Done in commit 92e6cdde397c906d8b7d8a2a119051ba71c34579 already ;)

[magnumripper]

Excellent! :+1: