Openwall GNU/*/Linux - a small security-enhanced Linux distro for servers
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Fri, 9 Aug 2013 08:28:32 +0530
From: Sayantan Datta <std2048@...il.com>
To: john-dev@...ts.openwall.com
Subject: Re: Busted OpenCL formats

Hi Lukas,

On Fri, Aug 9, 2013 at 5:56 AM, Lukas Odzioba <lukas.odzioba@...il.com>wrote:

> 2013/8/8 Sayantan Datta <std2048@...il.com>:
> > Hi magnum, Alexander, Lukas,
> > Here's the commit:
> >
> https://github.com/magnumripper/JohnTheRipper/commit/0965d1a42355b4fa4c48b12adcb858a86bb8dce5
> >
> > Please take a look. Magnum already mention a few changes which I'll
> > introduce in my next commit. Hopefully nothing is broken too seriously
> due
> > to this commit.
>
> rpp.h
> - * Copyright (c) 1996-98,2009,2010 by Solar Designer
> + * Copyright (c) 1996-98,2009,2010,2013 by Solar Designer
>
> Did solar propose those changes?
>
>
Yes. He modified the rpp_processor to make it suitable for mask mode.


> Copyright notes for mask.h/c looks weird to me:
> +/*
> + * This file is part of John the Ripper password cracker,
> + * Copyright (c) 2013 by Solar Designer
> + *
> + * Redistribution and use in source and binary forms, with or without
> + * modification, are permitted.
> + *
> + * There's ABSOLUTELY NO WARRANTY, express or implied.
> + */
> +
> +/*
> + * This software is Copyright (c) 2013 Sayantan Datta <std2048 at
> gmail dot com>
> + * and it is hereby released to the general public under the following
> terms:
> + * Redistribution and use in source and binary forms, with or without
> modification, are permitted.
> + * There's ABSOLUTELY NO WARRANTY, express or implied.
> + */


How is it supposed to be, like  what we have in md5_kernel.cl ?


>

mask.c:
> +int checkRange(struct mask_context *ctx, int rangePos) {
> AFAIK core does not use camelCase notation and I guess we should try
> to stick with that.
> What's worse you're mixing use of it in this file:
>
> +static void set_mask(struct rpp_context *rpp_ctx, struct db_main *db,
> unsigned char flg_wrd) {
> +void combinationUtil(void *arr, int data[], int start, int end, int
> index, int r, int target, int *isOptimal) {
>
> We were not rigorous enough and now this "problem" is present in some
> other files, not only yours.
>

> formats.c:
> +  if(db) db -> max_int_keys = 0;
> I guess this is the prefered indentation:
> >       if (db)
> >               db->max_int_keys = 0;
>
> mask.h:
> I think this file should contain all nonstatic functions declarations
> from mask.c.
>
> opencl_shared_mask.h:
> +  /* Range of charcters for a placeholder in the mask */
> +  /* Charchters in the range */
> +  /* Number of charchters in the range */
> +  /* Postion of the charcters in mask */
> +  /* Set of mask pacholders selected for processing inside the format */
>
> I am not confident to check grammar correctness of your comments, but
> so much typos hurts my eyes.
>
> options.c:
> What this change does?
> -  {"wordlist", FLG_WORDLIST_SET, FLG_CRACKING_CHK,
> +  {"wordlist", FLG_WORDLIST_SET, FLG_WORDLIST_CHK,
>
> Please try format your changes by: (only code you are adding, not all
> files where other people contributed)
> indent -kr -i8 -nlp -nbbo -ncs -l79 -lc79
> This way it will be consistent at least.
>
> Also you are overusing brackets.
> +  if((db -> format))
> +    while ((mask_word = msk_next(&rpp_ctx, &msk_ctx, &flag))) {
> +  if((*flag)) return NULL;
>
> That's all without looking into logic - tomorrow I'll print out your
> code and try to look deeper into it.
>
> Lukas
>

Thanks, I'll fix them. However I'll wait for magnum to give a go, no-go for
my next commit to bleeding. I think we should fork another repo PG-test
that it receives all the commits the main bleeding gets along with my
commits. Then we could just revert back all broken stuffs in main bleeding
and push those to the PG-test branch.

Regards,
Sayantan

[ CONTENT OF TYPE text/html SKIPPED ]

Powered by blists - more mailing lists

Your e-mail address:

Powered by Openwall GNU/*/Linux - Powered by OpenVZ