Follow @Openwall on Twitter for new release announcements and other news
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20150820214125.GB29462@openwall.com>
Date: Fri, 21 Aug 2015 00:41:25 +0300
From: Solar Designer <solar@...nwall.com>
To: john-dev@...ts.openwall.com
Subject: Re: BENCHMARK_LENGTH bugs

Kai,

On Fri, Aug 21, 2015 at 01:13:15AM +0800, Kai Zhao wrote:
> On Mon, Aug 17, 2015 at 11:22 PM, Solar Designer <solar@...nwall.com> wrote:
> > For a start, maybe --test-full should warn about formats that have:
> >
> > (benchmark_length == -1 && salt_size != 0) ||
> > (benchmark_length == 0 && salt_size == 0)
> >
> > The latter combination is strictly an error (can't benchmark for
> > different salt counts when there are no salts), so perhaps even the
> > non-full --test should complain when it sees it.
> >
> > The former is sometimes valid: we use it on very slow hashes (and
> > non-hashes), where the results would be effectively the same anyway (key
> > setup overhead is negligible).  Perhaps we'll need a (growing) whitelist
> > of formats for which --test-full should mute this warning.

Note that I had suggested a whitelist to mute a warning, but what you
implemented is totally different:

> I think I should add more slow hashes. The top slow hashes are in the
> attached file.

You implemented a list of formats for which an extra check is performed.
This isn't useful at all.

So please just drop the "else if (is_slow_hash(format->params.label))"
for now.  You may later replace it with a benchmark, as discussed in the
previous message I just sent.

For now:

You may instead complete "if (format->params.benchmark_length == -1 &&
format->params.salt_size != 0)", but not with a slow hash check (as you
have in commented out code in there).  Rather, you first have it report
warnings unconditionally, and then start building a whitelist of formats
for which this warning should be muted.  Or you can have the code run a
benchmark and automatically mute the warning if the two results are too
similar (difference below 1%).

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.