[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Tue, 10 Mar 2015 01:02:19 +0100
From: magnum <john.magnum@...hmail.com>
To: john-dev@...ts.openwall.com
Subject: Re: format method spec.
On 2015-03-09 11:55, Solar Designer wrote:
> On Sun, Mar 08, 2015 at 02:33:23AM +0100, magnum wrote:
>> We just noticed a problem which stems from the following (line numbers
>> here are from john proper) in crk_salt_loop()
>>
>> 331 crk_methods.clear_keys();
>> 332
>> 333 if (ext_abort)
>> 334 event_abort = 1;
>> 335
>> 336 if (ext_status && !event_abort) {
>> 337 ext_status = 0;
>> 338 event_status = 0;
>> 339 status_print();
>> 340 }
>>
>> The problem is we call clear_keys() and after that we call get_key() via
>> status_print(). The bleeding-edge mssql12 format currently crashes when
>> this happens.
> You're right - the code snippet above is inconsistent with the comment
> in formats.h. I think we should fix the code. Simply move line 331 to
> below line 340. Please test this in jumbo, and after confirming it
> works as intended remind me to make the same change in core.
It turns out we still call status_print() from john.c after job is finished:
597 do_external_crack(&database);
598 else
599 if (options.flags & FLG_BATCH_CHK)
600 do_batch_crack(&database);
601
602 status_print();
603
604 #if OS_FORK
605 if (options.fork && john_main_process)
606 john_wait();
607 #endif
...so problem remains. I'll try to zoom out and find a better place for
the clear_keys() in cracker.c (eg. right before setting key 0). I think
the one for Single mode is better placed.
> I think there are no uses of clear_keys() in the core tree right now,
> but since jumbo uses it we continue to provide it.
>
> I think many (possibly most) of the jumbo formats that use clear_keys()
> now are simply being sloppy/lazy. Doing a memset() on every
> clear_keys() is probably sub-optimal. Also, some formats define empty
> clear_keys() instead of simply using fmt_default_clear_keys - that's a
> trivial cleanup you can make right now.
We moved away from things like "if (index == 0)" in set_key() because
the self-test code played tricks on us, not behaving like a real crack.
The uses I know of clear_keys() are because the alternatives were either
too complex (bug prone) or slower.
I wasn't aware of any format with empty functions - I fix them as I see
them. But now that I looked for it I see a few, will fix them right away.
magnum
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.
