[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20141007030703.GA2517@openwall.com>
Date: Tue, 7 Oct 2014 07:07:03 +0400
From: Solar Designer <solar@...nwall.com>
To: crypt-dev@...ts.openwall.com
Subject: Re: BCrypt $2b$ support in PHP
Hi Leigh,
On Mon, Oct 06, 2014 at 03:06:47PM +0100, Leigh wrote:
> I've submitted a patch to the PHP project to support revision 'b' of bcrypt.
Yeah. This is something I've been meaning to do, but never got around to.
> PHPs key-expansion is currently performed using an 18 x 4 nested loop,
> rather than explicitly assigning the key length anywhere, which I am
> assuming sidesteps the 8-bit length wrapping issue altogether.
>
> The crypt 1.3 release states: Version 1.3 adds support for the $2b$
> prefix introduced in OpenBSD 5.5+, which behaves exactly the same as
> crypt_blowfish's $2y$.
>
> As PHP already supports the 'y' revision, I again making an assumption
> that supporting 'b' is as simple as using the same code path as 'y'.
That's correct.
> Comments and/or review are both welcome and very much appreciated. The
> patch is on github: https://github.com/php/php-src/pull/868
While I like the simplicity of this patch, I'd prefer to merge
crypt_blowfish's more elaborate changes into its revision in PHP instead
of deviating from crypt_blowfish farther.
The additional changes would include, in patch context order:
- Updates to the lengthy comment that starts crypt_blowfish.c. I think
PHP's version of it should differ only as it relates to actual specifics
of the code in PHP.
- flags_by_subtype[] moved to global scope in that source file (but kept
static, so not exported), and used by the self-test wrapper as well.
- The more invasive changes to the self-test code, which avoid leaking
the subtype via overall timings - an extremely subtle point, probably
with no practical relevance, but it's not good for the code in PHP to
be different in this respect. In your patch, the comparison against
'x' is more or at least differently leaky.
- Extra test vectors from wrappers.c. Your added test vectors do not
check that 'b' is implemented as equivalent specifically to 'y', rather
than possibly to 'a' or 'x'.
Basically, please diff crypt_blowfish-1.2 to crypt_blowfish-1.3 and port
those changes. If any changes made between older versions of
crypt_blowfish haven't made it into the PHP tree yet, port those as well -
with a separate patch. We'll also need to take a look at how the final
code in PHP would differ from crypt_blowfish-1.3's, and make sure only
the actually intentional changes are in there. This will ease further
updates, if those are needed.
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.
