Date: Thu, 10 Sep 2015 19:42:32 +0300 From: Solar Designer <solar@...nwall.com> To: john-dev@...ts.openwall.com Subject: Re: auditing our use of FMT_* flags Kai, On Tue, Sep 08, 2015 at 11:42:15PM +0800, Kai Zhao wrote: > 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/ Thank you. > 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 ? I don't see why you would want to whitelist anything. Do your checks still detect errors after Jim's fixes? If so, why do they? > > 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 ? No, the above is wrong or irrelevant. You've since posted something more relevant in another message - I'll comment on that. 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.