Follow @Openwall on Twitter for new release announcements and other news
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CABtNtWE4Ke-jXC4k8sGZyKtCW2_L2cppRh4xicQXAeFM3bBvJw@mail.gmail.com>
Date: Mon, 24 Aug 2015 01:11:45 +0800
From: Kai Zhao <loverszhao@...il.com>
To: john-dev@...ts.openwall.com
Subject: Re: testing every index (Re: more robustness)

Hi Alexander,

On Sun, Aug 23, 2015 at 9:53 AM, Solar Designer <solar@...nwall.com> wrote:
> Kai,
>
> 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.

Updated as you said.

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

I do not know well on the "#else", since it only for debug, so I 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).

I add a function: has_same_ciphertext() to cover the case of bfegg and
mysql.

The patch is here:

https://github.com/loverszhaokai/JohnTheRipper/commit/3ef39a3a26d6cf00333d3ad5e56001c0ff0fe318


Thanks,

Kai

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.