This is a static dump of issues in the old "Flyspray" bugtracker for DokuWiki. Bugs and feature requests
are now tracked at the issue tracker at Github.
Closed
Fixed
FS#2685 Blowfish ciphertext depends on architecture/PHP version
Security
2013-01-10Michitux
Our blowfish implementation generates different/incompatible ciphertext when it is run on different systems. This should not happen as blowfish is a clearly defined, deterministic block cipher. The two systems I've tested on are Arch Linux with PHP 5.4.10 and Gentoo with 5.3.18-pl0-gentoo, both x86_64 systems, both PHP versions report 8 byte integer size. In addition to that blowfish uses 32bit integers with overflows etc. in most places and I think our PHP implementation assumed a 32bit system, too. I'm not sure if this influences the results of the encryption as in many places the result is correctly interpreted as 32bit integer so it's actually possible that 32/64bit are not important but that something else causes these problems, especially as both systems I've tested on were 64bit.
In other words: it's very likely that at least on some systems our blowfish implementation isn't actually blowfish and this might influence the security of the encryption, that's why I've filled this under security. I'm not sure this is really security relevant for authentication cookies, but it might be relevant for plugins like the crypto plugin that allow a user to encrypt text in a page.
Is there any documentation for this implementation what it should do (apart from the standard blowfish documentation)? While most of the algorithm looks like a straightforward blowfish implementation I'm really wondering why the key is set more than once without resetting the P/S values first. The XOR at the beginning of the setKey()-function will at least partially remove the key again on the second invocation, however the P-values are also replaced with the encrypted values after these first steps so I'm not sure this is that bad but I'm relatively sure that this is not the original blowfish algorithm or at least not just blowfish but some special block cipher mode that only exists for blowfish. This means that actually each block is encrypted with a different secret which might be more secure than simple ECB but I wouldn't bet on this until I have seen some security reviews of this algorithm. Actually from a security point of view it might also be good to use some random initialization vector and CBC so two identical inputs that are encrypted with the same secret result in different ciphertexts.
We also don't have any test cases that check if the encryption of a certain input results in a certain ciphertext or if a certain ciphertext can be decrypted, in the current state such test cases would fail on certain systems. I haven't created any test cases yet as I'm not sure which ciphertext is actually correct. I have already tried to compare the results with the results of the OpenSSL implementation of blowfish but OpenSSL seems to always use a salt so I have the feeling that even in ECB mode the first block doesn't match (because of this strange key setting I don't expect that any other block will match).
If there is nobody who knows more I'll try to do more detailed testing in order to see what's actually happening in our blowfish implementation. My goal would be to implement CBC mode with a random initialization vector such that the results are identical to the results of the OpenSSL library so we have a reference that the implementation is actually working.
2013-01-10andi
I can not really say much about the implementation. It came from PHPMyAdmin so we should check if they have any updates. If I remember correctly PHPMyAdmin took the code from some other project (maybe Horde?).
2013-01-11Michitux
I have already looked at PHPMyAdmin, they don't have any updates and do the exactly same thing and Horde is no longer using the class, they are using Crypt_Blowfish from PEAR now which seems to be a different implementation that is dead, too (last update of the actual code in 2008, some update of the directory structure in 2010), but it contains for example a custom XOR implementation with the comment "Workaround for XOR on certain systems" - and it has test cases with plaintext/ciphertext pairs.
2013-02-16Michitux
The basic problem is that this algorithm is adding large integers which result in an overflow that does not result in a real overflow but in a float conversion. This float doesn't have enough precision to contain the 32 least significant bits of the integer we wanted to have which results in an undefined float -> integer conversion. This happens already in the calculation of the S-boxes. This means that these S-boxes are most probably a lot less random than they were thought to be. I haven't done any analysis how badly they are affected but I could imagine that the security of the algorithm is flawed. I see differences in the last 5 digits in the first differing value that add up to all digits being different. Assuming that 5 digits are missing we have to assume that 13-17 bits are missing, leading to 15-23 remaining random bits. I'm not sure that in fact so many bits are always the same and I'm also not sure that all of these S-boxes are affected as I couldn't find any differences in the s1-values. Actually after some thoughts I think this is not easily exploitable as you can't guess the S-boxes separately but still this might be a serious problem.
2013-02-16andi
I suggest to deprecate the use of this library now and ship with a different library (and maybe a different algorithm) in the next release (autumn 2013).