[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <4DB8A8E7.6040406@bredband.net>
Date: Thu, 28 Apr 2011 01:38:15 +0200
From: magnum <rawsmooth@...dband.net>
To: john-dev@...ts.openwall.com
Subject: Re: "SSH private keys cracker" patch for JtR [first cut
for GSoC]
On 2011-04-27 00:17, Dhiru Kholia wrote:
> I have tried to address these issues in the new version of the patch
> (attached as well as uploaded to the wiki).
Hi Dhiru, thanks for your work. I was just going to report a segfault
problem but it seems it was fixed with this revision. I have some
suggestions though, if I may:
1. I reverted all the openssl #includes and init/cleanup stuff that you
added to john.c and moved the init stuff into an init() in ssh_fmt.c
where (I think) it belongs. This worked right away with no further
tweaks. I'm not sure if there is (or should be introduced) a good place
we could put the OpenSSL cleanup stuff, but I just dropped those lines
and let the OS take care of it.
2. I reverted ALL your changes to loader.c *without* moving anything of
it elsewhere. None of it is needed because the unique $ssh$ tag will
take care of it! Auto-detection works fine, forced format too. --show
and --show=left works like expected (without removing any newlines).
There are a few formats that bloat loader.c (for example to support
input files in other formats, like L0phtcrack) but the majority of all
formats do not touch this file.
3. I can't see why you encode the base64 blob (further) into hex,
doubling its size? I'm not sure how much info you need to save, but I
think you should just re-pack the data into something like the following
(here with linebreaks, but there should be none). You can put the
original file name in the "user" field:
dsa_test.key:$ssh$AES-128-CBC,5A8068279C0F46E98207150A3C2EC614$/
uiTinJ4RUjoZv0+pY1iMv1ciVarCi4z/b6ZiLAaVYpyJ1hXT2tci+Y2f3LaaExco
5wr1aAFM47xmRmGo82I/vCHGA9Rxg5waGC9pWOGZVuUQrDsU6tcUkCMB+2Z4GS95
ODGCdDK2gNetDnb2JvHsqATsm4Cc63GdhiZ0sCFYLqym+We1wCYalxsO21W+OgoB
3ogFFIw2r2F+qJzqM7AUCyLFhi5zGmu6SNeXvU4GzxGPFJNG0mAOUIwaaN1aDjcN
2k4bCrfybq3z6nCe3DBs8K22iN+8uRnSAbCOqzZXEdYqUYY5Kkj2neCTRTXINdgQ
+aSSYg3y5Y7boKkjIOrvPUWHeOyduQ+tersAEw7nCVJzr9N8tRg2qV4PUv1CKfpO
IFwB7/9somjYIjqWoaSzjxK0c8Rwz0S1pmr+uqrbwy+Peou5M3semHlBkGiU27fw
ohKy+MUNHbsNjysSZSElNKsMIPqTIVzZE1mVFA/0uMGqdpQ3bzBOjX2Zo6edFCOm
J4wYavW5wM+jmuVKPVVNy99Zxyep0FEdLP5Kb2c4Zl0S9f14/b6h6APixVe7zuvV
bSkByfJnyzhIOYBIyT7Mdw14r4AXJV6+ZoEw09wiwM=
This would more than half the need for your huge increase of
LINE_BUFFER_SIZE. Maybe you need the Proc-Type field too? If so, just
add it (only what is needed from it) with another $ separator.
Last, there are still some hunks in your patch that just did not belong
there, like spurious whitespace changes and even a reference to
SybaseASE. Not a big deal but it makes your patch a pita to combine with
other ones.
> For a while, I am switching to "adding support for FileVault and
> archive files cracking" activity. Will revisit this patch later.
I look forward to that too!
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.
