Follow @Openwall on Twitter for new release announcements and other news
[<prev] [next>] [thread-next>] [day] [month] [year] [list]
Message-ID: <20150817152200.GA405@openwall.com>
Date: Mon, 17 Aug 2015 18:22:00 +0300
From: Solar Designer <solar@...nwall.com>
To: john-dev@...ts.openwall.com
Subject: BENCHMARK_LENGTH bugs

magnum, Kai -

The "cq" format, presumably like many others, wrongly set
BENCHMARK_LENGTH to -1, thereby inhibiting the Many salts vs. Only one
salt benchmarks.  Yet it's a fast, salted format, for which separate
benchmarks are needed.  The attached patch corrects this.

Before:

[solar@...er run]$ OMP_NUM_THREADS=1 ./john -test -form=cq
Warning: OpenMP is disabled; a non-OpenMP build may be faster
Benchmarking: cq, ClearQuest [CQWeb]... DONE
Raw:    86376K c/s real, 86376K c/s virtual

[solar@...er run]$ OMP_NUM_THREADS=10 ./john -test -form=cq
Will run 10 OpenMP threads
Benchmarking: cq, ClearQuest [CQWeb]... (10xOMP) DONE
Raw:    548929K c/s real, 54838K c/s virtual

After:

[solar@...er run]$ OMP_NUM_THREADS=1 ./john -test -form=cq
Warning: OpenMP is disabled; a non-OpenMP build may be faster
Benchmarking: cq, ClearQuest [CQWeb]... DONE
Many salts:     86245K c/s real, 85391K c/s virtual
Only one salt:  26733K c/s real, 27000K c/s virtual

[solar@...er run]$ OMP_NUM_THREADS=10 ./john -test -form=cq
Will run 10 OpenMP threads
Benchmarking: cq, ClearQuest [CQWeb]... (10xOMP) DONE
Many salts:     546963K c/s real, 54696K c/s virtual
Only one salt:  33816K c/s real, 3374K c/s virtual

Luckily, this does not affect our decision-making for FMT_OMP_BAD, since
we're looking at "Many salts" benchmarks only in such cases, and this is
the same as "Raw" before the fix.

However, this highlights another robustness task for Kai: identify which
formats wrongly set BENCHMARK_LENGTH to -1 when they actually need to
report separate "Many salts" vs. "Only one salt" speeds (that is,
formats that are fast and salted, yet wrongly have this set to -1).

Also, identify formats that wrongly set BENCHMARK_LENGTH to 0 when they
don't actually need to report separate "Many salts" vs. "Only one salt"
speeds (that is, formats that are very slow or/and saltless, so the two
speeds reported are nearly the same anyway).

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.

And we'll need to proceed to correct these settings in formats that
currently got them wrong.

For now, magnum, please commit the attached patch.

Alexander

View attachment "john-cq-benchmark.diff" of type "text/plain" (1327 bytes)

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.