Bug #68238 [Com]: mcrypt_encode tests are broken

Edit report at https://bugs.php.net/bug.php?id=68238&edit=1 ID: 68238 Comment by: gm dot outside+php at gmail dot com Reported by: gm dot outside+php at gmail dot com Summary: mcrypt_encode tests are broken Status: Not a bug Type: Bug Package: Testing related Operating System: Linux PHP Version: 5.6.1 Block user comment: N Private report: N New Comment: Re: bug #67286 - well, the change to the behaviour as it is now is incompatible with the documentation at http://php.net/manual/en/function.mcrypt-encrypt.php, so there people will report bugs against this behaviour. Moreover, the change is breaking things -- a warning message is one thing, but the function was changed to bail out with a failure upon a discovery of incorrect key size. This is the problem, actually. Re: test case. Yes, the test case was added on Mar 5th (you committed it 2 days later), but it's currently broken in the HEAD, so I'm curious how one could do proper unit testing there :). Re: 5.5.12 - no, I didn't run the exact same test, but OctoberCMS I'm running on that box stopped to work after switching to 5.6.1. By default that application was using the MCRYPT_RIJNDAEL_128 cipher and that cipher was no longer working (the same application, the same mcrypt library, and the only change was PHP). This is the reason I started to investigate. Finally, I'm going to run the same test case on my PHP 5.5.12 and will report back shortly. Still, it's strange to have a broken test for something that important as encryption. Previous Comments: ------------------------------------------------------------------------ [2014-10-15 18:43:11] nikic@php.net The bug you linked has nothing to do with issue, it is a generic complaint from someone using broken crypto code, which is rejected by the stricter input requirements in PHP 5.6. When you say that mcrypt_encrypt on your PHP 5.5.12 build works correctly, do you mean that it provides the correct output for this test script (whereas PHP 5.6 does not)? This test was only added in PHP 5.6, so just running make test will not be enough to check it - you have to manually run the file from the 5.6 tree. ------------------------------------------------------------------------ [2014-10-15 18:36:06] gm dot outside+php at gmail dot com Related To: Bug #67286 ------------------------------------------------------------------------ [2014-10-15 18:30:09] gm dot outside+php at gmail dot com @nikic, I'm running the latest available mcrypt, which is 2.5.8. The library passed all its internal tests. Also, PHP 5.5.12 is working on the same box and does not have any issues with mcrypt_encode(). Additionally to this there are other bug reports pointing to the same issue, e.g. bug #67286 , so I would not dismiss this bug this easy. Finally, the gcov test at php.net itself is failing the test on mcrypt. The following tests was performed less than 2 days ago: http://gcov.php.net/viewer.php?version=PHP_HEAD&func=tests&file=ext%2Fmcrypt%2Ftests%2Fbug62102_rfc2144.phpt So, let's try to pinpoint the issue? :) Or at least let's fix the test case... ------------------------------------------------------------------------ [2014-10-15 18:12:49] nikic@php.net This is a bug in your mcrypt library, which does not properly support Cast-128 with keys <= 80 bits. Padding the keys with NUL bytes is not the same, because Cast-128 uses only 12 rounds (instead of 16) if the key is <= 80 bits. Updating your libmcrypt version will probably fix this. ------------------------------------------------------------------------ [2014-10-15 17:32:24] gm dot outside+php at gmail dot com I did more testing, and I was a bit wrong about the strlen() part. Manually padding the keys with '\0' actually works, but the result does not match the ciphertext provided in RFC-2144 B.1 anymore. Additionally to that I was wrong re: the keysize requirement for that cipher should be 16 bytes (128-bit) as cipher's name 'cast-128' suggests. Once the keys are properly padded with '\0' to be 128-bit the test returns the following differences: 002+ 80-bit: 753de29f5d167d03 003+ 40-bit: f00b0530833d7444 002- 80-bit: eb6a711a2c02271b 003- 40-bit: 7ac816d16e9b302e So, something else was also changed that the mcrypt extension no longer conforms to RFC-2144 B.1. ------------------------------------------------------------------------ The remainder of the comments for this report are too long. To view the rest of the comments, please view the bug report online at https://bugs.php.net/bug.php?id=68238 -- Edit this bug report at https://bugs.php.net/bug.php?id=68238&edit=1