Openwall GNU/*/Linux - a small security-enhanced Linux distro for servers
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Wed, 30 Sep 2015 07:38:58 +0530
From: Sayantan Datta <std2048@...il.com>
To: john-dev <john-dev@...ts.openwall.com>
Subject: Re: Reading loaded password hashes inside crypt-all.

On Tue, Sep 29, 2015 at 8:33 PM, Solar Designer <solar@...nwall.com> wrote:

> Hi Sayantan,
>
> > On Tue, Sep 29, 2015 at 3:05 PM, Sayantan Datta <std2048@...il.com>
> wrote:
> > > Are password hashes removed from database as they are cracked ?
>
> Yes, but only from fields actually used during a given cracking run, and
> not necessarily right away.  (In "single crack" mode, there's postponed
> removal.)
>
> > > Apparently
> > > there are discrepancies between salt->count and number of remaining
> hashes
> > > in salt->list. While salt-count is updated, it seems like salt->list
> is not
> > > updated to reflect the cracked hashes.
> > >
> > > pw = salt -> list;
> > > i = 0;
> > > do {
> > >         bin = (int *)pw -> binary;
> > >         if (bin != NULL) {
> > >           store_binary();
> > >           i++ ;
> > >         }
> > >     } while ((pw = pw -> next)) ;
> > >
> > > Running this loop in the middle of cracking results in 'i' being
> greater
> > > than 'salt->count' !!
>
> Yes, and that's "normal".  See crk_remove_hash().  It always updates
> crk_db->password_count and salt->count, but it only updates the list
> when there's no bitmap, and it only updates the bitmap and sometimes
> the hash table when there is a bitmap.
>
> OK, you don't actually rely on the list being updated - you only rely on
> the removed entries being marked with "binary = NULL".  That's easier
> for cracker.c to support, and it happened to support this until recently.
>
> > > Some recent change might have triggered this issue.
> > > Earlier, such behavior were not noticed.
> > >
> > > This broke an optimization where hashes are re-loaded onto GPU after
> some
> > > of them are cracked, resulting in better speed!!
>
> Oh.  We probably need to do something about this.
>
> On Tue, Sep 29, 2015 at 04:18:57PM +0530, Sayantan Datta wrote:
> > This one line patch fixes the issues but I'm not sure to commit it.
>
> There was a good reason for this change (minimizing writes to the
> database when running with --fork), so I'd rather not revert it.
>
> > diff --git a/src/cracker.c b/src/cracker.c
> > index 0456114..3276e2f 100644
> > --- a/src/cracker.c
> > +++ b/src/cracker.c
> > @@ -308,7 +308,7 @@ static void crk_remove_hash(struct db_salt *salt,
> struct db_password *pw)
> >   * "single crack" mode, so mark the entry for removal by "single crack"
> mode
> >   * code if that's what we're running, instead of traversing the list
> here.
> >   */
> > -     if (crk_guesses)
> > +     //if (crk_guesses)
> >               pw->binary = NULL;
> >  }
>
> To summarize, the issue is that cracker.c makes assumptions about how
> the database is used in a given cracking run, and those assumptions
> might not hold true for formats' own use of the database from
> crypt_all().  Maybe we need to introduce a format flag and check it
> here?  Like this:
>
>         if (crk_guesses || (crk_params.flags & FMT_REMOVE))
>                 pw->binary = NULL;
>
> and your formats that need this would set FMT_REMOVE.
>

Fine by me.

Regards,
Sayantan

[ CONTENT OF TYPE text/html SKIPPED ]

Powered by blists - more mailing lists

Your e-mail address:

Powered by Openwall GNU/*/Linux - Powered by OpenVZ