[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <000f01cbe031$bd040e10$370c2a30$@net>
Date: Fri, 11 Mar 2011 15:17:33 -0600
From: "jfoug" <jfoug@....net>
To: <john-dev@...ts.openwall.com>
Subject: RE: New update to md5-gen, AND changes to the john core 'format' structures. (diff posted to Wiki)
>-----Original Message-----
>From: Solar Designer [mailto:solar@...nwall.com]
Thanks for the quick Comments, Solar.
>> 3. Added a 'new' prepare() function. This would be called
>prior to
>> valid(). It will 'build' the string to use as the ciphertext string.
>This
>> also was a change for every format, since it is a new method. The
>> majority, simply use fmt_prepare_default()
>
>Basically, this moves the per-format hacks out of loader.c and takes
>them into the formats themselves. I like this, but I didn't have a
>chance to give this much thought yet, let alone review the specific
>proposal/code.
Hopefully you (and others) can easily critique.
>I could "blindly" apply Jim's patch including the above (after all, it's
>jumbo, where similar "blind" application of contributed patches was done
>in the past), but the loader.c changes I already committed for 1.7.7
>will "conflict" with this change (not fundamentally, but in terms of
>specific source code changes). This means that I'd have a hard time
>updating the jumbo patch for 1.7.7 then, whereas my plan was to put out
>1.7.7 and 1.7.7-jumbo-1 simultaneously. (This is getting delayed,
>sorry.)
>
>A way around this could be to base 1.7.7-jumbo-1's jumbo patch portion
>on 1.7.6-jumbo-12 (the last version that does not include Jim's latest
>changes yet) even if we have a newer revision of the jumbo patch by
>then. After this release, Jim could work on 1.7.7-jumbo-2, which would
>re-introduce his changes.
A 1.7.7-jumbo-x+1 would be acceptable. Once we know that 1.7.7 and Jumbo-x
(which would be equal to todays jumbo-12) are out and working/building
properly for everyone. Then the jumbo-x+1 would be the 'new-features'
release.
>A comment on the implementation:
>
>In struct fmt_tests, why not have a 10- or 9-element array instead of
>these fields:
>
> char *fld1, *fld3, *fld4, *fld5, *fld6, *fld7, *fld8, *fld9,
>*fld10;
>
>I see that fld2 is special, but that's not enough of a reason to replace
>would-be loops with explicit duplicate/similar code for the different
>ones of the above fields.
I have thought about this also. It could be done that way. Right now, the
only place where the code 'building' is done, is in loader, and formats
(testing code). But it is also used in the formats. The 'main' reason I
did it with individual field names, is due to the 2nd one (fld[1] if we had
arrays), is already hard coded to ciphertext.
However, I could do this:
struct fmt_tests {
char *ciphertext, *plaintext;
char *fld[10]
};
Then fld[0] = username (first field) fld[1] points to ciphertext (so it
'stays' proper), fld[2] is the 3rd field, etc. In that way, there are 2
ways to 'view' fld[1], but so what. I did the singleton's because I could
name them 1, 3, 4, 5, 6.... since 2 was 'taken'.
Making it into arrays should be very easy (but I will have to adjust all the
prepares).
>> 7. Added the 'tiny_memory' cleanup.
>> .....
>
>The code looks buggy.
The bug you found, is buggy.
>More importantly, I am missing the rationale behind this "cleanup".
To be able to detect leaks upon program exit. The way that the alloc_tiny
works, there are things which 'appear' to be leaks, and worse yet, they are
scattered throughout the source, at somewhat random locations. With this
change, after all memory usages would be done away with, the memory is free.
Then 'auto' leak detection tools will be usable to spot real memory leaks.
>> Upper case possible for md5 to base-16 conversions.
>
>If so, and if I understood you correctly, you need to have split() unify
>case, and set FMT_SPLIT_UNIFIES_CASE.
No, what this allows is handling a hash where a partial step of
binary-to-base16 is done to upper case. It allows a format to be built for
that 'type'. I have found hashes like this:
md5(MD5($p))
So, password would be:
H1= MD5(password) = 5F4DCC3B5AA765D61D8327DEB882CF99
H3 = md5(H1) = 3b73cca8b7d9d93a834631fb22769334
If you compare to md5(md5($p)) for password, which is
696d29e0940a4957748fe3fc9efd22a3 you see they do not match.
md5(md5($p)) would be:
H1= md5(password) = 5f4dcc3b5aa765d61d8327deb882cf99
H3 = md5(H1) = 696d29e0940a4957748fe3fc9efd22a3
>If you like, I may get this (or rather your next revision of it with at
>least #7 fixed) into 1.7.6-jumbo-13, but then plan to base 1.7.7's
>initial revision of the jumbo patch on 1.7.6-jumbo-12.
>
>What do you say?
This sounds like a plan. So you are freezing jumbo-12 so that 1.7.7 can get
it's jumbo quickly. I can then get the diffs between jumbo-12 against
jumbo-13 (or whatever), into a 177-j2 done pretty easily. It would take
only a single hand patching.
This also gives developers something to 'see' if their patches will likely
need changed or not. If they try to patch against jumbo-13, it will show up
many of the problems, and at least show that the patch will need updated.
If the patch patches against jumbo-13, then it is likely to patch against
177-j2 also (unless 1.7.7 causes the patch to fail).
I have a few things to 'fix' and get to you. #7 was certainly broke. I will
also look at an array in the preload() function.
>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.
