Follow @Openwall on Twitter for new release announcements and other news
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <ef49827df70e8c760c05809e9242ce62@smtp.hushmail.com>
Date: Thu, 14 Feb 2013 19:44:53 +0100
From: magnum <john.magnum@...hmail.com>
To: john-dev@...ts.openwall.com
Subject: Re: mschapv2-bitsliced conversion

On 14 Feb, 2013, at 11:14 , deepika dutta <deepikadutta_19@...oo.com> wrote:
> Good this worked with the previous patch. I think the changes you did should be fine as they are in sync with what already existed in those files. Assembly language is hard to understand without experience. Anyways, there is a small problem in the previous patch w.r.t. multiple definitions of P. 
> 
> -- P is extern in DES_bs.h, 
> -- it is defined in DES_bs.c when C code is to be used and defined in assembly files when assembly code is to be used.
> -- it is defined in DES_bs_b.c also for both cases.
> 
> Though it didn't create any problem but I feel it is not a good approach to programming. Therefore, I had used extern in assembly file and did all the stuff which didn't work.

Apparently, extern is not recognized by all assemblers.

> But just now I re-looked the code, the latest patch should actually be discarded as just removing the DES_bs_vector P[64] from DES_bs_b.c in previous patch will work fine with keeping one definition of P.

OK, so it will be declared from including the header. I haven't tried this yet, will do.

> I think now after addition of your changes in the patch along with the above small change, things should work fine for this bitsliced conversion on all platforms. What you say?

I have merged your code with the latest mschapv2-naive version (it now has Freeradius support too) and dropped the old version in favor of this bitslice format. I have tested on x86 32/64 SSE and generic, as well as Sparc32 big-endian. And I ran it through out test suite. Everything seems fine. You can see the changes in unstable-jumbo branch (soon to be Jumbo 8) and bleeding-jumbo branch (future) here:
https://github.com/magnumripper/JohnTheRipper.git

Note that your code is now named --format=mschapv2-naive and file is MSCHAPv2_bs_fmt_plug.c. The other (default) format now exploits a weakness so it doesn't use DES at all in the inner loop (massive speedup). But that format loads very slow so the BS format is better in some situations. It is also an excellent example for future DES stuff. We should do the same to NETNTLM_old_fmt_plug.c for a starter - it is almost identical to MSCHAPv2.

There are some whitespace changes, so diff it with -w to see the changes. Please base any future contributions on latest Git. Thanks!

Solar, I committed this to unstable too. We can revert it if problems surface. Old code is moved to unused/ and modifed so it will happily co-exist with the two others if moved back for testing.


magnum



> 
> Cheers,
> Deepika
> From: magnum <john.magnum@...hmail.com>
> To: john-dev@...ts.openwall.com 
> Sent: Thursday, February 14, 2013 2:31 PM
> Subject: Re: [john-dev] mschapv2-bitsliced conversion
> 
> ...and the same UNDERSCORES approach works for 32-bit:
> 
> diff --git a/src/x86-sse.S b/src/x86-sse.S
> index b6c7b1b..23bfdb4 100644
> --- a/src/x86-sse.S
> +++ b/src/x86-sse.S
> @@ -58,7 +58,6 @@
>  #define DES_bs_crypt                   _DES_bs_crypt
>  #define DES_bs_crypt_25                        _DES_bs_crypt_25
>  #define DES_bs_crypt_LM                        _DES_bs_crypt_LM
> -#define P                                              _P
>  #define DES_bs_crypt_plain             _DES_bs_crypt_plain
>  #endif
>  
> @@ -115,7 +114,11 @@ DO_SPACE(nptr(48))
>  #define E(i)                           DES_bs_all_E+nptr(i)
>  #define B(i)                           DES_bs_all_B+nvec(i)
>  #define tmp_at(i)                      DES_bs_all_tmp+nvec(i)
> +#ifdef UNDERSCORES
> +#define P(i)                           _P+nvec(i)
> +#else
>  #define P(i)                           P+nvec(i)
> +#endif
>  #define pnot                           tmp_at(0)
>  
>  #define S1(out1, out2, out3, out4, extra) \
> 
> 
> Benchmarking: MSCHAPv2 C/R MD4 DES [DES_BS_MSCHAPv2]... DONE
> Many salts:	70839K c/s real, 70839K c/s virtual
> Only one salt:	5675K c/s real, 5675K c/s virtual
> 
> 
> I will commit my changes to GitHub for now.
> 
> magnum
> 
> 
> 
> On 14 Feb, 2013, at 9:49 , magnum <john.magnum@...hmail.com> wrote:
> 
>> Oh, I was so close in my previous mail. This works on OSX:
>> 
>> diff --git a/src/x86-64.S b/src/x86-64.S
>> index 5026c6c..e0b2c08 100644
>> --- a/src/x86-64.S
>> +++ b/src/x86-64.S
>> @@ -42,7 +42,6 @@
>>  #define DES_bs_crypt                   _DES_bs_crypt
>>  #define DES_bs_crypt_25                        _DES_bs_crypt_25
>>  #define DES_bs_crypt_LM                        _DES_bs_crypt_LM
>> -#define P                                              _P
>>  #define DES_bs_crypt_plain             _DES_bs_crypt_plain
>>  #endif
>>  
>> @@ -99,7 +98,11 @@ DO_SPACE(nptr(48))
>>  #define E(i)                           DES_bs_all_E+nptr(i)(%rip)
>>  #define B(i)                           DES_bs_all_B+nvec(i)(%rip)
>>  #define tmp_at(i)                      DES_bs_all_tmp+nvec(i)(%rip)
>> -#define P(i)                           P+nvec(i)
>> +#ifdef UNDERSCORES
>> +#define P(i)                           _P+nvec(i)(%rip)
>> +#else
>> +#define P(i)                           P+nvec(i)(%rip)
>> +#endif
>>  
>>  #define pnot                           tmp_at(0)
>>  
>> 
>> This is relative to john-1.7.9-jumbo-5-MSCHAPV2-BS-4.diff.
>> 
>> 
>> Benchmarking: MSCHAPv2 C/R MD4 DES [DES_BS_MSCHAPv2]... DONE
>> Many salts:	83918K c/s real, 83918K c/s virtual
>> Only one salt:	7691K c/s real, 7691K c/s virtual
>> 
>> 
>> magnum
>> 
>> 
>> On 14 Feb, 2013, at 9:40 , magnum <john.magnum@...hmail.com> wrote:
>> 
>>> I think you should revert this latest patch, it was probably less bad before. For OSX 64-bit you need to use relative addressing. If I do this:
>>> 
>>> 1. revert this latest patch
>>> 2. drop the _P define in line 45 (for now)
>>> 3. change to relative addressing like this:
>>> 
>>> -#define P(i)                           P+nvec(i)
>>> +#define P(i)                           P+nvec(i)(%rip)
>>> 
>>> ...now the code assembles on OSX without a warning. However it fails self-test because the addressing is not quite right now I guess. I do not really know assembler but I think that is closer to a solution.
>>> 
>>> magnum
>>> 
>>> 
>>> On 14 Feb, 2013, at 9:10 , magnum <john.magnum@...hmail.com> wrote:
>>> 
>>>> This did not help the OSX assembler:
>>>> 
>>>> x86-64.S:99:0: warning: "P" redefined [enabled by default]
>>>> x86-64.S:45:0: note: this is the location of the previous definition
>>>> x86-64.S:67:Unknown pseudo-op: .extern
>>>> x86-64.S:67:Rest of line ignored. 1st junk character valued 95 (_).
>>>> x86-64.S:1368:32-bit absolute addressing is not supported for x86-64
>>>> x86-64.S:1368:cannot do signed 4 byte relocation
>>>> x86-64.S:1370:32-bit absolute addressing is not supported for x86-64
>>>> x86-64.S:1370:cannot do signed 4 byte relocation
>>>> 
>>>> This uses the same arch.h as Linux, but -DUNDERSCORES, -DBSD and -DALIGN_LOG.
>>>> 
>>>> magnum
>>>> 
>>>> 
>>>> On 14 Feb, 2013, at 8:50 , deepika dutta <deepikadutta_19@...oo.com> wrote:
>>>> 
>>>>> I am attaching a new patch handling the problem of multiple definition of P. I hope it works on all platforms.
>>>>>  
>>>>> Cheers,
>>>>> Deepika
>>>>> From: deepika dutta <deepikadutta_19@...oo.com>
>>>>> To: "john-dev@...ts.openwall.com" <john-dev@...ts.openwall.com> 
>>>>> Sent: Wednesday, February 13, 2013 1:11 PM
>>>>> Subject: Re: [john-dev] mschapv2-bitsliced conversion
>>>>> 
>>>>> I think I can correct the warning of multiple definition of P. Though this warning didn't occur in linux, I feel that P has been defined in multiple locations and need to be externed in the assembly file. I will look into this and give a new patch for your testing. I don't know why the other issues are coming, maybe someone can resolve them.
>>>>> 
>>>>> Cheers,
>>>>> Deepika
>>>>> From: magnum <john.magnum@...hmail.com>
>>>>> To: john-dev@...ts.openwall.com 
>>>>> Sent: Wednesday, February 13, 2013 5:37 AM
>>>>> Subject: Re: [john-dev] mschapv2-bitsliced conversion
>>>>> 
>>>>> Deepika's work can be found in branch "mschapv2-bs" on GitHub, with patch history preserved. It is still based on 1.7.9-jumbo-5. It seems to work just fine on Linux (64-bit works fine on Bull, 32-bit cross compile builds fine too but we have no 32-bit OpenSSL to link against).
>>>>> 
>>>>> On 64-bit OSX I get this though:
>>>>> 
>>>>> x86-64.S:102:0: warning: "P" redefined [enabled by default]
>>>>> x86-64.S:45:0: note: this is the location of the previous definition
>>>>> x86-64.S:1371:32-bit absolute addressing is not supported for x86-64
>>>>> x86-64.S:1371:cannot do signed 4 byte relocation
>>>>> x86-64.S:1373:32-bit absolute addressing is not supported for x86-64
>>>>> x86-64.S:1373:cannot do signed 4 byte relocation
>>>>> x86-64.S:1375:32-bit absolute addressing is not supported for x86-64
>>>>> x86-64.S:1375:cannot do signed 4 byte relocation
>>>>> x86-64.S:1377:32-bit absolute addressing is not supported for x86-64
>>>>> x86-64.S:1377:cannot do signed 4 byte relocation
>>>>> x86-64.S:1379:32-bit absolute addressing is not supported for x86-64
>>>>> x86-64.S:1379:cannot do signed 4 byte relocation
>>>>> ...
>>>>> make[1]: *** [x86-64.o] Error 1
>>>>> make[1]: *** Waiting for unfinished jobs....
>>>>> make: *** [macosx-x86-64] Error 2
>>>>> 
>>>>> And on 32-bit:
>>>>> 
>>>>> x86-sse.S:118:1: warning: "P" redefined
>>>>> x86-sse.S:61:1: warning: this is the location of the previous definition
>>>>> Undefined symbols for architecture i386:
>>>>>   "P", referenced from:
>>>>>       _DES_bs_crypt_plain in x86-sse.o
>>>>> ld: symbol(s) not found for architecture i386
>>>>> collect2: ld returned 1 exit status
>>>>> make[1]: *** [../run/john] Error 1
>>>>> make: *** [macosx-x86-sse2] Error 2
>>>>> 
>>>>> 
>>>>> These are hopefully trivial problems. If we get them solved, I'll see if I can add this as a separate format for a starter, eg. "mschapv2-bs" and merge it to current bleeding tree.
>>>>> 
>>>>> magnum
>>>>> 
>>>>> 
>>>>> On 7 Feb, 2013, at 20:50 , magnum <john.magnum@...hmail.com> wrote:
>>>>> 
>>>>> > Thanks, I'll have a look at your patch within a week.
>>>>> > 
>>>>> > magnum
>>>>> > 
>>>>> > 
>>>>> > On 7 Feb, 2013, at 20:21 , deepika dutta <deepikadutta_19@...oo.com> wrote:
>>>>> > 
>>>>> >> Hi, I am attaching a new patch which is tested to work on both 32 and 64 bit SSE2 linux OS. I have modified both x86-sse.S and x86-64.S.
>>>>> >> I cannot say for sure that this will not break the build on other targets. I think someone should test this since I do not have those target machines. I will try to rectify any problems which arise due to this.
>>>>> >> 
>>>>> >> Regarding working on converting other formats, though I don't think it will be too difficult but I will inform you whenever I get ample time to devote on it :) 
>>>>> >> 
>>>>> >> Cheers,
>>>>> >> Deepika
>>>>> >> From: magnum <john.magnum@...hmail.com>
>>>>> >> To: john-dev@...ts.openwall.com 
>>>>> >> Sent: Thursday, February 7, 2013 2:43 PM
>>>>> >> Subject: Re: [john-dev] mschapv2-bitsliced conversion- openmp support
>>>>> >> 
>>>>> >> On 7 Feb, 2013, at 8:35 , Frank Dittrich <frank_dittrich@...mail.com> wrote:
>>>>> >>> On 02/07/2013 04:47 AM, Solar Designer wrote:
>>>>> >>>> On Wed, Feb 06, 2013 at 07:12:34AM -0800, deepika dutta wrote:
>>>>> >>>>> Actually, I worked on coverting the DES_bs_crypt_plain(), which I had added to do single DES encryption, into SSE implementation.  I have added this function into the x86-sse.S file. Since the code in this file was in assembly language, I tried some copy-pasting from previous code and added some assembly code myself to get the single DES encryption working. I compiled this program using linux-x86-64 option, everything is working fine and the latest banchmarks for bitsliced implementation are given below. You can test for yourself. The patch is attached.
>>>>> >> 
>>>>> >> I'm confused - isn't x86-sse.S only used for 32 bit SSE builds? For 64-bit it should be x86-64.S. Or did you do both? That would be awesome.
>>>>> >> 
>>>>> >>>> This is cool, but (un)fortunately we've just revised the MSCHAPv2 format
>>>>> >>>> such that it does not depend on speed of DES encryption anymore - see
>>>>> >>>> the "NetNTLMv1 and MSCHAPv2" thread in here.  So I'm afraid there's no
>>>>> >>>> point in introducing your changes to that format now (nor would they
>>>>> >>>> apply to the revised code).
>>>>> >>> 
>>>>> >>> But the changes should apply to the new mschapv2-naive format which
>>>>> >>> basically is the renamed old mschapv2 format, intended for shorter
>>>>> >>> cracking runs, because it loads hashes faster than the new mschapv2.
>>>>> >> 
>>>>> >> True, although it wouldn't be much benefit other than as a nice example of code - you'll only use the old naïve version for very short runs where the speed doesn't matter anyway.
>>>>> >> 
>>>>> >> Anyway, the thing that has stopped me from pulling the MSCHAPv2 BS patches into Jumbo is the fact they (so far) b0rked builds other than generic (and now linux-x86-64?). I haven't looked at this latest patch yet but I assume it still breaks some builds. There is no need to implement BS for every single target, but the changes need to be #ifdef'ed so that all builds pick code that works for it. Given that, I'll include it right away (even for mschapv2-naive).
>>>>> >> 
>>>>> >> Also, new patches should ideally be rebased upon the bleeding-jumbo branch on GitHub: https://github.com/magnumripper/JohnTheRipper/tree/bleeding-jumbo
>>>>> >> 
>>>>> >> magnum
>>>>> >> 
>>>>> >> 
>>>>> >> <john-1.7.9-jumbo-5-MSCHAPV2-BS-4.diff>
>>>>> > 
>>>>> > 
>>>>> > 
>>>>> 
>>>>> 
>>>>> 
>>>>> 
>>>>> 
>>>>> 
>>>>> <john-1.7.9-jumbo-5-MSCHAPV2-BS-5.diff>
>>>> 
>>> 
>> 
> 
> 
> 


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.