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.