Bug #72362 [Com]: OpenSSL Blowfish encryption is incorrect for short keys

Edit report at https://bugs.php.net/bug.php?id=72362&edit=1 ID: 72362 Comment by: ian at oshaughnessy dot cc Reported by: sachavav at tut dot by Summary: OpenSSL Blowfish encryption is incorrect for short keys Status: Closed Type: Bug Package: OpenSSL related Operating System: Linux/Ubuntu PHP Version: 5.6.22 Assigned To: bukka Block user comment: N Private report: N New Comment: Is there any way to have this fix backported to 7.0? Since mcrypt was deprecated in 7.1, it makes it more difficult to transition to OpenSSL given this issue. Previous Comments: ------------------------------------------------------------------------ [2017-07-02 17:35:13] bukka@php.net A new constant OPENSSL_DONT_ZERO_PAD_KEY has been introduced to address that and it will be part of 7.1.8. For example it can be used like: openssl_encrypt("this is a test string","bf-ecb","12345678", OPENSSL_RAW_DATA | OPENSSL_DONT_ZERO_PAD_KEY)); ------------------------------------------------------------------------ [2017-04-25 13:10:52] bukka@php.net So this is happening for exactly the same reason as https://bugs.php.net/bug.php?id=71917 (we don't try to set key length for variable length ciphers if the key is shorter than the default key size for the algorithm). Unfortunately changing that as default would have exactly the same implication in terms of BC. It would stop working for people using shorter keys and relying on the fact that is filled with \0. So I would be inclined to introduce a new option for that which could possibly go to 7.1 if RM is fine with that or master (7.2) otherwise. I will create a PR. ------------------------------------------------------------------------ [2016-06-08 12:35:29] sachavav at tut dot by Description: ------------ According to description of Blowfish algorithm it can use variable key starting from 32bits. If the key is shorter than block, it should be cycled over. https://www.schneier.com/academic/archives/1994/09/description_of_a_new.html Keys A - AA - AAA should be equivalent. This is true in openssl extension for keys, longer than 128 bits, but for shorter keys extension works incorrectly. It padds short keys with zeros up to 128 bits instead of cycling them. Please check sample code for explanation. If you can compare behavior of mcrypt extension - it works correctly there. Test script: --------------- > php -r 'echo bin2hex(openssl_encrypt("this is a test > string","bf-ecb","12345678" , OPENSSL_RAW_DATA));' 0b30345b335e2ca5ba4d12c0077768c99680ca260b07d693 > php -r 'echo bin2hex(openssl_encrypt("this is a test > string","bf-ecb","12345678\0\0\0\0\0\0\0\0" , OPENSSL_RAW_DATA));' 0b30345b335e2ca5ba4d12c0077768c99680ca260b07d693 > php -r 'echo bin2hex(openssl_encrypt("this is a test > string","bf-ecb","1234567812345678" , OPENSSL_RAW_DATA));' e3214d1b16e574828c8a3e222202dde81afd1ad2cb165ab3 Expected result: ---------------- Short keys are cycled over. Actual result: -------------- Short keys are padded with zeros ------------------------------------------------------------------------ -- Edit this bug report at https://bugs.php.net/bug.php?id=72362&edit=1