[Phpmyadmin-devel] Blowfish implementation might be broken

[Marc Delisle]

Michael Hamann a écrit :
> Hi,
> 
> I'm part of the developer team of DokuWiki and we are using the same
> Blowfish implementation in DokuWiki that you are using in PHPMyAdmin
> so I think you might be interested in what I found out about this
> Blowfish implementation.
> 
> I was alerted that something might be wrong in the Blowfish
> implementation when a user complained that he couldn't decrypt a text
> that was encrypted on another system. I found that the ciphertext
> depends on the system (even if both systems are 64 bit systems).
> 
> I dug a bit deeper into the code and added debug output. After some
> tests I found that the problem is that the algorithm is adding large
> integers. The expected result is an integer overflow, however PHP
> gives a float as result. This float is then used in a binary XOR with
> an int, I think this results in a cast back to int, but as it is
> explained in the PHP documentation for floats beyond the integer range
> this cast is undefined as the float doesn't have enough precision.
> 
> This loss of precision happens in the calculation of the S-boxes which
> means that these S-boxes are most probably less random than they
> should be. I think the security of the algorithm might be flawed
> because of this but I'm not sure. However at least one thing is clear:
> this is not Blowfish.
> 
> Another thing I found strange is that the key setup routine is called
> after every chunk. I couldn't find any documentation which suggests
> this as algorithm, what I found is that the p and s-boxes remain the
> same during the encryption of multiple blocks.
> 
> We don't plan to fix the implementation, instead we deprecated it and
> want to replace it after the current release,
> http://phpseclib.sourceforge.net/ (AES) is a possible candidate. I
> assume that regardless which library we choose the ciphertext will be
> different so it doesn't matter if we also switch the block cipher.
> 
> Regards,
> Michael Hamann

Thanks Michael. Note that phpMyAdmin uses the PHP implementation of 
Blowfish (taken from Horde a while ago), only when the mcrypt extension 
is not found.

 From the phpseclib page, I see that they use mcrypt if it's available, 
for speed purpose.

Also, we emit a warning when this extension does not exist.

Moreover, our doc says "When using the cookie authentication (the 
default), the mcrypt extension is strongly suggested for most users and 
is required for 64–bit machines. Not using mcrypt will cause phpMyAdmin 
to load pages significantly slower."

We'll have a look to decide the fate of this PHP Blowfish implementation 
in our code base, but phpseclib looks promising.

-- 
Marc Delisle
http://infomarc.info