Follow @Openwall on Twitter for new release announcements and other news
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Mon, 17 Aug 2015 13:51:21 +0300
From: Solar Designer <solar@...nwall.com>
To: john-dev@...ts.openwall.com
Subject: Re: testing every index (Re: more robustness)

Hi Kai,

On Mon, Aug 17, 2015 at 06:10:52PM +0800, Kai Zhao wrote:
> I tried to implement this in the following patch:
> 
> https://github.com/loverszhaokai/JohnTheRipper/commit/794f5ffa998b122e9457c566ee8860d456fe01af
> 
> There is check_all_even_index() and check_all_odd_index().
> I set the correct password to all the even index in check_all_even_index().
> 
>         for (i = 0; i < max; i++) {
>                 if (i % 2 == 0)
>                         key = plaintext;
>                 else
>                         key = longcand(format, i, ml);
>                 fmt_set_key(key, i);
>         }
> 
> And then check all the even index by cmp_one() in check_all_index()
> 
>         for (i = 0; i < max; i++)
>         if ((is_even && i % 2 == 0) ||
>             (!is_even && i % 2 == 1)) {
>                 if (!format->methods.cmp_one(binary, i)) {
>                         snprintf(err_buf, sizeof(err_buf), "cmp_one(%d) failed",
>                                 i);
>                         return err_buf;
>                 }
>         }
> 
> 
> Does this what you mean by testing every index ?

This is much closer to what is needed, but you're not quite there yet.
You're currently checking only the indices holding expected-correct
passwords, and only with cmp_one().  You should be checking all indices,
including the expected-incorrect ones to make sure they are in fact
detected as incorrect.  For the expected-correct ones, you should also
be checking all get_hash() outputs.  And you should also be checking
cmp_exact() and get_key() for all indices.

You "can't" check get_hash() outputs for the expected-incorrect indices,
because those partial hashes are small, and thus occasional false
positives are expected.  Yet you can try that anyway for the largest
hash table size, for formats supporting it, since getting a 27-bit match
in a relatively quick test is fairly unlikely... and we may simply
adjust the test vectors if this does happen somewhere, so that it won't.
You'll need to add a source code comment on the possibility of such
false positives if you implement this.

Also, it is preferable that you make use of all test vectors at once
rather than use just one plaintext for all expected-correct indices.

And even/odd is too simple a pattern that might match some format
implementation internals.  Preferably, you'd replace it with calls to a
decent hash function of "i" and check whether that function's return
value is even or odd.  OTOH, this could result in long sequential runs
of expected-correct or expected-incorrect indices.  So maybe it's better
to run 4 tests: even i, odd i, even h(i), odd h(i).  In fact, you could
include some (time-based?) randomization into h(): it needs to stay the
same within a john invocation, but it may differ between invocations.
That way, the likelihood to spot a bug only triggerable by some
correct/incorrect pattern will be growing as --test-full is invoked
multiple times.

Your code can be made shorter: there should be no need to have separate
functions (and thus separate calls to them) for even vs. odd.  Rather,
you can have an outer loop iterating over test numbers (0 to 1, or maybe
0 to 3 as I suggested above).

The more extensive checks for indices 0 or 1 should be merged into the
main testing loop, since they mostly shouldn't be limited to those two
indices.

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.