Bug #68238 [Nab]: mcrypt_encode tests are broken

Edit report at https://bugs.php.net/bug.php?id=68238&edit=1 ID: 68238 User updated 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: @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... Previous Comments: ------------------------------------------------------------------------ [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. ------------------------------------------------------------------------ [2014-10-15 17:16:27] gm dot outside+php at gmail dot com Description: ------------ There was a recent commit (http://git.php.net/?p=php-src.git;a=commit;h=a861a3a93d89a50ce58e1ab1abef1eb501f97483) that changed behaviour of the mcrypt_encode() function. After that commit the key is required to be at least the expected key length long, otherwise a warning message is issued and the mcrypt_encode() routine returns a failure. The corresponding test in ext/mcrypt/tests/bug62102_rfc2144.php supplies 10 bytes key instead of 16 for cast-128 80-bit encryption and 5 bytes key instead of 10 for cast-128 40-bit encryption. A quick fix to the test would be to pad the keys with '\0' manually (RFC-2144 B.1), e.g. mcrypt_encrypt('cast-128', "\x01\x23\x45\x67\x12\x34\x56\x78\x23\x45\0\0\0\0\0\0", $plaintext, 'ecb') but unfortunately due to the way changed code treats key data (as a null terminated string) and due to calculating the key size as strlen() of that string there is no way to satisfy the RFC-2144 B.1 since all trailing '\0' will be ignored. Expected result: ---------------- That the RFC-2144 test would be passed with the explicitly specified vector and that mcrypt_encrypt() would honour the key argument as a binary string that can include '\0' anywhere in the string. Actual result: -------------- All trailing '\0' in the key argument are ignored, therefore it's impossible to pass RFC-2144 test to match section B.1. ------------------------------------------------------------------------ -- Edit this bug report at https://bugs.php.net/bug.php?id=68238&edit=1