[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CABtNtWHC0D+LFBHkg_=er0piYB-HBYV2hH5+1UR9b2n1Qw7V=A@mail.gmail.com>
Date: Tue, 8 Sep 2015 23:42:15 +0800
From: Kai Zhao <loverszhao@...il.com>
To: john-dev@...ts.openwall.com
Subject: Re: auditing our use of FMT_* flags
Hi Alexander，
On Mon, Sep 7, 2015 at 2:25 AM, Solar Designer <solar@...nwall.com> wrote:
>
> As to your current patch above, I didn't review it closely, but I guess
> you can just get it in (submit a pull request). You'll add to it later,
> with separate commits.
Created.
https://github.com/magnumripper/JohnTheRipper/pull/1736/
>
>> The result is almost the same as the previous test. There are 4 formats:
>> dynamic=md5($p), MediaWiki, PHPS and PHPS2 which do not contain
>> the flag and their split do not change case. But the 4 formats finally has
>> the flag FMT_SPLIT_UNIFIES_CASE set. This probably caused by the
>> following code:
>>
>>
>> Is this ok for the 5 formats ? Should I add these 4 formats to whitelist ?
>
> Probably not. I expect that we have many formats that don't handle
> FMT_SPLIT_UNIFIES_CASE right, in one of several ways described
> above.
Since JimF has add the flag for MediaWiki, PHPS and PHPS2, I think
I should add these formats to whitelist. Maybe also includes
dynamic=md5($p). Should I ?
https://github.com/magnumripper/JohnTheRipper/commit/cc5ae475bad53ca46b9c74a82848bc86c6b9c314
> That's at least 3 possible errors:
>
> 1. Setting the flag, but not implementing case unification in split().
>
> 2. Not setting the flag, but implementing case unification in split().
>
> 3. Needing to implement case unification in split() and set the flag,
> but not doing any of this. Alternatively, needing to refuse to handle
> wrong-case encodings (e.g., insist on all-lowercase hex if that's what
> the target system uses).
>
> We probably have some of each of these 3.
>
> There might also be:
>
> 4. Not needing to implement case unification in split() and set the
> flag, yet doing both of this.
>
> But I find #4 unlikely, except for cases where the alternative solution
> to #3 could have been implemented.
>
I think the current patch can detect the #1, #2 error. For #3, I wrote
a pseudo code based on your ideas.
original:
original_split_ret = split(ciphertext, 0, format);
original_binary_ret = binary(ciphertext);
original_salt_ret = salt(ciphertext);
set_key();
original_crypt_ret = crypt_all(&count, NULL);
original_cmp_ret = cmp_all(binary, original_crypt_all);
copy original binary -> original binary
copy original salt -> original salt
change ciphertext case:
new_split_ret = split(ciphertext, 0, format);
new_binary_ret = binary(ciphertext);
new_salt_ret = salt(ciphertext);
set_key();
new_crypt_ret = crypt_all(&count, NULL);
new_cmp_ret = cmp_all(binary, original_crypt_all);
copy new binary -> new binary
copy new salt -> new salt
compare:
if (original_split_ret != new_split_ret)
goto should_set_fmt_split;
if (original_binary_ret != new_binary_ret)
goto should_set_fmt_split;
if (original_salt_ret != new_salt_ret)
goto should_set_fmt_split;
if (original_crypt_ret != new_crypt_ret)
goto should_set_fmt_split;
if (original_cmp_ret != new_cmp_ret)
goto should_set_fmt_split;
if (original bianry != new binary)
goto should_set_fmt_split;
if (original salt != new salt)
goto should_set_fmt_split;
should_set_fmt_split and implement it in split()
Do you think this conform to your ideas ?
Thanks,
Kai
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.
