[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Mon, 9 Mar 2015 13:55:29 +0300
From: Solar Designer <solar@...nwall.com>
To: john-dev@...ts.openwall.com
Subject: Re: format method spec.
magnum -
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.
>
> I can easily fix this in mssql12's get_key() but I thought I should
> bring it up here first. Are we supposed to be able to call get_key()
> after a call to clear_keys()? I have no problem with that but then we
> should document it: Current spec in formats.h says keys are undefined
> after a call to clear_keys().
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.
clear_keys() was originally intended as a reliability measure for the
old bitslice DES key setup, which could otherwise be updating the same
"transposed matrix" for a very long time, so that hardware faults could
accumulate. This was gone in the core tree with:
http://cvsweb.openwall.com/cgi/cvsweb.cgi/Owl/packages/john/john/src/DES_bs.c.diff?r1=1.13;r2=1.14
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.
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.
