john-dev - Re: auditing our use of FMT_* flags (was: more robustness)

Follow @Openwall on Twitter for new release announcements and other news

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CABtNtWEroQe47CtmjEN3ik+Yyi0TWqydc1kmtButiHzDTqkSsA@mail.gmail.com>
Date: Fri, 14 Aug 2015 21:12:51 +0800
From: Kai Zhao <loverszhao@...il.com>
To: john-dev@...ts.openwall.com
Subject: Re: auditing our use of FMT_* flags (was: more robustness)

Hi Alexander,

On Fri, Aug 14, 2015 at 9:03 PM, Solar Designer <solar@...nwall.com> wrote:
> On Fri, Aug 14, 2015 at 10:30:43AM +0800, Kai Zhao wrote:
>> On Fri, Aug 7, 2015 at 2:28 AM, Solar Designer <solar@...nwall.com> wrote:
>> > On Tue, Aug 04, 2015 at 08:57:29AM +0800, Kai Zhao wrote:
>> >> 2. FMT_CASE
>> >>
>> >>     2.2 formats have set FMT_CASE but all passwords are
>> >>           case-insensitive
>> >>
>> >>         OpenVMS
>> >
>> > In uaf_encode.c: uaf_test_password() we have:
>> >
>> >     if ( pwd->flags & UAI$M_PWDMIX ) {  /* take password verbatim */
>> >         password_dx.dsc$a_pointer = (char *) password;
>> >     } else {
>> >         /*
>> >          * Upcase password.
>> >          */
>> >         password_dx.dsc$a_pointer = uc_password;
>> >         if ( password_dx.dsc$w_length > sizeof(uc_password) )
>> >                 password_dx.dsc$w_length = sizeof(uc_password);
>> >         for ( i = 0; i < password_dx.dsc$w_length; i++ )
>> >                 uc_password[i] = toupper ( ARCH_INDEX(password[i]) );
>> >     }
>> >
>> > So it's not that simple.  Can you try to come up with and add test
>> > vectors that would exercise the "else" code path?
>>
>> The current test vectors DO go through the "else" code path. Do you
>> want me to test the "if" code path ?
>
> Oh, you're right.  If this format initially appeared to you to be
> case-insensitive, then only the "else" path was being used.
>
>> I add one test vector for vms which goes through the "if" code path.
>>
>> https://github.com/magnumripper/JohnTheRipper/pull/1661/files
>>
>> #define MAX_KEYS_PER_CRYPT 1
>> static struct fmt_tests tests[] = {
>> + {"$V$S44zI913bBx-UJrcFSC------D", "President#44"},
>> {"$V$9AYXUd5LfDy-aj48Vj54P-----", "USER"},
>> {"$V$p1UQjRZKulr-Z25g5lJ-------", "service"},
>> {NULL}
>>
>> The old test vectors go through the "else" code path, so I thought
>> it should not set FMT_CASE, the new test vector goes through
>> the "if" code path, and it does case-sensitive. So vms should set
>> FMT_CASE, no need to change.
>
> This looks good to me, thanks!
>
> Maybe just add a comment near the test vectors, explaining that some are
> case-insensitive and some are case-sensitive, and that we're setting
> FMT_CASE because for some of the supported hashes the passwords are
> case-sensitive?
>

Are the comments ok ?

 static struct fmt_tests tests[] = {
+/*
+ * The following two test vectors: "USER" and "service" are case-insensitive
+ */
        {"$V$9AYXUd5LfDy-aj48Vj54P-----", "USER"},
        {"$V$p1UQjRZKulr-Z25g5lJ-------", "service"},
+/*
+ * The following one test vectors: "President#44" is case-sensitive, so this
+ * format should set FMT_CASE
+ */
        {"$V$S44zI913bBx-UJrcFSC------D", "President#44"},
        {NULL}
 };


Thanks,

Kai

Confused about mailing lists and their use? Read about mailing lists on Wikipedia and check out these guidelines on proper formatting of your messages.