[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <20190407164919.GA16942@openwall.com>
Date: Sun, 7 Apr 2019 18:49:19 +0200
From: Solar Designer <solar@...nwall.com>
To: john-dev@...ts.openwall.com
Subject: Re: ldr_split_line() performance regression
On Sun, Apr 07, 2019 at 07:31:25PM +0300, Aleksey Cherepanov wrote:
> Solar Designer <solar@...nwall.com> writes:
> > On Wed, Apr 03, 2019 at 11:47:35PM +0300, Aleksey Cherepanov wrote:
> >> Solar Designer <solar@...nwall.com> writes:
> >> > On Wed, Sep 16, 2015 at 02:02:34PM -0500, jfoug wrote:
> >> >>
> >> >> On 9/16/2015 1:52 PM, Solar Designer wrote:
> >> >> >strlen(*ciphertext) < 10 && strncmp(*ciphertext, "$dummy$", 7)) {
> >> >>
> >> >> These should be reversed, since strncmp should short circuit out much
> >> >> earlier than the length check.
> >
> > I now think Jim was wrong about that: we're checking for _not_ matching
> > "$dummy$", and that condition will usually be true, so with these checks
> > reversed the strlen() will be reached almost all of the time anyway.
>
> (*ciphertext)[0] != '$' check just passes everything with $ to full
> handling of lines, so $dummy$ would be handled fully. So the check might
> be suboptimal and against the intention, yet correct globally.
Right.
> Further there is the following code:
>
> if (((*ciphertext)[0] != '$' ||
> (strncmp(*ciphertext, "$dummy$", 7) &&
> strncmp(*ciphertext, "$0$", 3))) &&
> p - *ciphertext != 10 /* not tripcode */) {
>
> The code seems to do a similar check. Comparing shows that the NIS check
> (written as intended) would skip short $0$cleartext hashes (that's for
> --format=plaintext).
Yes, maybe the NIS check should be revised in jumbo to allow "$0$" of
any length similarly to how it allows "$dummy$" of any length. I think
this was missed because "$0$" is a jumbo-specific addition.
I doubt this will matter in practice, though. It isn't expected that
NIS-like login names will actually be seen along with those "hashes".
magnum, please feel free to take care of this or not.
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.
