Follow @Openwall on Twitter for new release announcements and other news
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20120627083740.GA16300@openwall.com>
Date: Wed, 27 Jun 2012 12:37:40 +0400
From: Solar Designer <solar@...nwall.com>
To: john-dev@...ts.openwall.com
Subject: Re: mschap-v2 conversion

On Wed, Jun 27, 2012 at 01:16:42AM -0700, deepika dutta wrote:
> From what I have understood, benchmarking for many salts case measures time for 'setting salt', 'crypt_all' and 'cmp_all' wheres for single salt also includes 'set_keys'. The various performance overheads therefore are:
> 
> 1. In set_salt: Conversion of 'challenge' to bitsliced form 'Plaintext'.
> 2. In crypt_all: MD4 computations for key setup key are being done here (and not in mschapv2_set_key() as i belive you maybe assuming).

Correct so far.  I was aware of this implementation detail, but thanks
for mentioning it anyway.

> It thus is causing overhead in both single and many salt cases.

No, this is avoided with the keys_prepared flag.

> The other overhead is of generate_output() which converts from bitsliced form to output[] and also of setup_des_key().
> 
> I don't think we can remove overhead from set_salt(), we have to convert to bitsliced plaintext.

We can perform this conversion in get_salt() instead, which is not
performance-critical.  If the conversion is fully moved like that, the
salt size would be pretty large, though.  It might be quicker (due to
smaller reads) to do DES IP in get_salt(), but expansion of bits to
words (and only it) in set_salt().

> From crypt_all(), MD4 computation and setup_des_key() can be removed and put in mschapv2_set_key().

They should be kept inside crypt_all() in order for us to be able to
parallelize them with OpenMP later.  However, they should be protected
from unnecessary recomputation with the keys_prepared flag.  IIRC, this
is currently done for MD4, but not for DES key setup - we should do it
for both.

> generate_output() is required before cmp_all() so i think we cannot take it out from crypt_all.

generate_output() should be avoided entirely.  See how DES_bs_cmp_all()
is implemented.

Then, for the case when cmp_all() is not used but one of the get_hash*()
functions is, you do in fact need to extract a few individual bits - but
there are already DES_bs_get_hash_*() functions that do that, and you
don't need to do it in crypt_all().

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.