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.