[<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
Confused about mailing lists and their use?
Read about mailing lists on Wikipedia
and check out these
guidelines on proper formatting of your messages.
