Follow @Openwall on Twitter for new release announcements and other news
[<prev] [next>] [thread-next>] [day] [month] [year] [list]
Date: Wed, 18 Jul 2012 07:42:22 -0500
From: "jfoug" <jfoug@....net>
To: <john-dev@...ts.openwall.com>
Subject: Pwsafe, is our algorithm right?

I am only looking at the cpu file  pwsafe_fmt.c    I question the algorithm.
I recently added this to pass_gen.pl, and will list the function here, since
it is easier to see than the C code:

 

sub pwsafe {

      my $salt=randstr(32);

      my $pass=$_[0];

      my $digest = sha256$pass.$salt);

      my $i;

      for ($i = 0; $i <= 2048; ++$i) {

            $digest = sha256($digest);

      }

      print "u$u-pwsafe:\$pwsafe\$\*3\*", unpack('H*', $salt), "\*2048\*",
unpack('H*', $digest), ":$u:0:$_[0]::\n";

}

 

Notice the for (i=0; i<=iter; ++i)

 

That will do 2049 sha256's if iter is 2048.  I know this is the way the CPU
versoin of the code is (at least), because I have created a TS file, and the
CPU version detects it perfectly.   BUT is the hashing wrong?  Any time I
see a for loop like this:  for (i = 0; i <= MAX; ++i)  I reaslly think long
and hard about if it is right or not.

 

I went out to pwsafe site, and downloaded the code.  Here is the stretchkey
function, directly from their source tree:

 

void PWSfileV3::StretchKey(const unsigned char *salt, unsigned long saltLen,

                           const StringX &passkey,

                           unsigned int N, unsigned char *Ptag)

{

  /*

  * P' is the "stretched key" of the user's passphrase and the SALT, as
defined

  * by the hash-function-based key stretching algorithm in

  * http://www.schneier.com/paper-low-entropy.pdf (Section 4.1), with
SHA-256

  * as the hash function, and N iterations.

  */

  size_t passLen = 0;

  unsigned char *pstr = NULL;

 

  ConvertString(passkey, pstr, passLen);

  unsigned char *X = Ptag;

  SHA256 H0;

  H0.Update(pstr, passLen);

  H0.Update(salt, saltLen);

  H0.Final(X);

 

#ifdef UNICODE

  trashMemory(pstr, passLen);

  delete[] pstr;

#endif

 

  ASSERT(N >= MIN_HASH_ITERATIONS); // minimal value we're willing to use

  for (unsigned int i = 0; i < N; i++) {

    SHA256 H;

    // The 2nd param in next line was sizeof(X) in Beta-1

    // (bug #1451422). This change broke the ability to read beta-1

    // generated databases. If this is really needed, we should

    // hack the read functionality to try both variants (ugh).

    H.Update(X, SHA256::HASHLEN);

    H.Final(X);

  }

}

 

NOTICE, for (I = 0; I < N; ++i)

 

I do not think our version is proper.  Has anyone tested this against REAL
hashes of known passwords, and not simply against self generated data???


Content of type "text/html" skipped

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.