Follow @Openwall on Twitter for new release announcements and other news
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Sat, 22 Aug 2015 05:42:05 +0300
From: Solar Designer <solar@...nwall.com>
To: john-dev@...ts.openwall.com
Subject: Re: testing every index (Re: more robustness)

Kai,

On Sat, Aug 22, 2015 at 10:29:55AM +0800, Kai Zhao wrote:
> VNC is special. When I test a wrong password, it seldom changes crypt_out[index]
> which is used by cmp_all() and cmp_one(). This introduces a problem???once you
> test a correct password, the cmp_one() will return 1 when you test incorrect
> passwords. Is this a bug ?

Yes, looks like a bug.

>                 if(memcmp(encrypted_challenge, cur_salt->response, 8) == 0) {
>                         DES_cbc_encrypt(&cur_salt->challenge[8],
> &encrypted_challenge[8], 8, &schedule, &ivec, DES_ENCRYPT);
>                         if(memcmp(encrypted_challenge,
> cur_salt->response, 16) == 0)
>                                 memcpy((unsigned
> char*)crypt_out[index], encrypted_challenge, 16);
>                 }

Out of the two if's above, the second one is unneeded: we'll have an
equivalent of it in cmp_*(), and it will actually affect things there.
So the memcpy() within the first "if" block should be performed all the
time.  And then you need to add an "else" branch for the (only
remaining) outer "if", and simply zeroize the 16 bytes of
crypt_out[index] there.

Please make these changes.  Does the format still pass tests with the
changes?  Does your --test-full problem go away, with no new problems
appearing?  If so, please make a pull request.

Thanks,

Alexander

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.