Follow @Openwall on Twitter for new release announcements and other news
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20150823015333.GA14411@openwall.com>
Date: Sun, 23 Aug 2015 04:53:33 +0300
From: Solar Designer <solar@...nwall.com>
To: john-dev@...ts.openwall.com
Subject: Re: testing every index (Re: more robustness)

Kai,

On Sun, Aug 23, 2015 at 12:00:19AM +0800, Kai Zhao wrote:
> A new patch for testing every index:
> 
> https://github.com/loverszhaokai/JohnTheRipper/commit/7fcc07ef25464907105c266fa2b9caae0b598813

I looked through this patch.  I am not immediately sure how close it is
to what I had envisioned - it certainly looks like you did make an
attempt to address my recent criticism/advice, but I'd need to mentally
execute the code (or step through it on a computer) to see what it
actually ends up doing.  I guess it's not quite there yet, but I
really don't know without putting effort into it.  Overall, the code
isn't as clean as I would have liked it to be, but it's consistent with
the rest of jumbo.  I am tired and I need to focus on other things now,
so please just get this in, with these minor corrections:

This for loop's body needs to be indented:

+	for (i = 0; i < max; i++) { 
+	if (n == 0) {
[...]
+	j++; 
+	current++; 
+	}

s/ith passwords/i-th password/g in two places in a comment (wrong use of plural)

s/is wrong passwords/is a wrong password/ (ditto)

The placement of set_salt() in test_every_index() is weird.  I'd prefer
us to set_salt() before doing things, not (seemingly) after having done
things that would use a salt.

In the "#ifndef DEBUG ... #else", have you tested the #else branch?
Does it build, does it work?  You should.  Or drop it.

> There are 3 formats have fatal errors: "cmp_exact() unexpected success"

You need to whitelist these specific tests, or for the case of bfegg and
mysql you might be able to enhance the code such that it sees that the
corresponding ciphertexts are the same and actually expects success from
those comparisons for that reason (and then you don't need to whitelist
these two).

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.