[<prev] [next>] [day] [month] [year] [list]
Message-ID: <95d6d8d5032f923ce35bf43b71ef42f4@smtp.hushmail.com>
Date: Wed, 09 Jul 2014 04:31:04 +0200
From: magnum <john.magnum@...hmail.com>
To: john-dev@...ts.openwall.com
Subject: Re: Salt dupe removal logic and format changes (Comments wanted)
On 2014-07-06 09:35, Solar Designer wrote:
> On Sun, Jul 06, 2014 at 12:51:11AM +0200, magnum wrote:
>> On 2014-07-05 15:40, Solar Designer wrote:
>>> Here's another idea: add a new FMT_SALT_STRUCT flag that would indicate
>>> that the "binary salt" as used by the format is actually of a specific
>>> struct type containing a pointer to the actual salt value and a size
>>> (just these two fields, so it's 16 bytes on a 64-bit arch). When the
>>> flag is set, memcmp() would be called after the extra pointer
>>> dereference and using the actual salt size read from that struct. With
>>> this approach, you don't need to make any changes to existing formats.
>>> You start to use this new feature in formats that need it, one by one.
>>
>> I really like this idea, for being KISS and easy to experiment with. I'd
>> vote for trying this first.
>
> Great. Here's yet another idea: instead of the above, have a flag like
> FMT_DYNAMIC_SALT that would tell the rest of JtR that salt() returns a
> dynamically allocated salt of arbitrary size and encodes the actual size
> in first size_t of the returned buffer. It's almost the same as above,
> except that it has less pointer indirection. Since we intend to use
> this for cases where salts are large, I think the dynamic allocation
> should be made in a way that can be free()d, and the rest of JtR should
> actually be free()ing these as appropriate.
These ideas might come handy in the future but after some poking around
I'm not sure they solve the problems at hand: zip and rar. Both are "non
hashes" and their "salts" are not just salts but structs like this:
typedef struct {
unsigned char salt[8];
int type; /* 0 = -hp, 1 = -p */
unsigned char *raw_data;
/* for rar -p mode only: */
union {
unsigned int w;
unsigned char c[4];
} crc;
unsigned long long pack_size;
unsigned long long unp_size;
char *archive_name;
long pos;
int method;
} rarfile;
This is actually fixed length as written now, it's just that it contains
pointers so the usable compare size (which is 8 now - it could be larger
if we move things around so both pointers are placed at the end) is
smaller than SALT_SIZE (which is obviously sizeof(rarfile)). We don't
really have to look at the data pointed to by the pointers when
comparing salts. It would be more "right" to do so, but in practice it's
not needed. Arguably we could use FMT_DYNAMIC_SALT as above to create
something that would work, but I think it would be very quirky.
Zooming out, the root problem is that each call to get_salt() with the
same ciphertext will allocate new buffers and return new pointers. We
could do the allocations and fill the buffers in set_salt() instead but
that would hit performance. These might be huge buffers, even gigabytes.
The problem only surfaces in two cases though: One is dupe rejection. If
you duplicate a RAR file "hash line" in your input file, it will be seen
as two different salts because the pointers are different. The other is
pot sync. Each time a new pot line is seen with a RAR entry, get_salt()
will be called with it, and allocate a possibly huge buffer with data
which won't ever be used nor freed. And again, if we compare SALT_SIZE
it won't be recognized as the same salt as one we are attacking so the
pot sync will fail even for the correct "hash". This latter problem is
currently handled by a quirk in cracker.c:
if (!strcasecmp(crk_params.label, "rar") ||
!strcasecmp(crk_params.label, "rar-opencl"))
/* Actual salt is first 8 bytes */
potcheck_salt_size = 8;
else
potcheck_salt_size = crk_params.salt_size;
I guess some more thinking is needed here.
magnum
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.
