Doc #62453 [Asn]: Terrible sample code for mcrypt_encrypt

Edit report at https://bugs.php.net/bug.php?id=62453&edit=1 ID: 62453 Updated by: ircmaxell@php.net Reported by: maarten dot bodewes at gmail dot com Summary: Terrible sample code for mcrypt_encrypt Status: Assigned Type: Documentation Problem Package: Documentation problem PHP Version: 5.4.4 Assigned To: ircmaxell Block user comment: N Private report: N New Comment: Maarten, I like the patch in general, and completely agree with it. The only thing I'm a bit concerned about is the warning: > # === WARNING === > > # Resulting cipher text has no integrity or authenticity added > # and is not protected against padding oracle attacks. While I completely understand *why* the warning is there, perhaps is there a way that it can be demonstrated how to protect those issues (integrate HMAC, etc). Remember that the documentation is not targeted towards security experts, and many who will use it have no idea of the difference between integrity and authenticity. Would it be worth while to briefly demonstrate a method to do so? Or do you think that's too deep for this example? Additionally, there are several mentions of PKCS#7 padding, yet I don't see an example of that in the patch provided. Is that something worth adding in? Anthony Previous Comments: ------------------------------------------------------------------------ [2012-06-30 23:55:18] nikic@php.net Assigning to ircmaxell in hope that he can review this and apply the changes :) ------------------------------------------------------------------------ [2012-06-30 14:08:11] maarten dot bodewes at gmail dot com Created patch on the original sample code (including PHP header/footer) using the following requirements. Tested against Java code (send to Nikic). Requirements: * use MCRYPT_RIJNDAEL_128 (AES) instead of MCRYPT_RIJNDAEL_256 * use CBC mode encryption * use a 128 bit (16 byte), 192 (24 bit) or 256 bit (32 byte) key specified in hexadecimals consisting of random bytes * use a random IV (already present), and prepend that to the cipher text (so it can be used during decryption) * perform an explicit encoding (UTF-8 would come to mind) on the plain text, or specify that UTF-8 encoding is being used for compatability reasons * base64 encode the resulting cipher text, as most of web programmers use strings Furthermore the following warnings should be present: * PKCS#7 padding should be used on binary plain "text" * this CBC encryption mode only provided confidentiality, not integrity or authentication * padding oracle attacks may reveal the plain text in client/server setups ------------------------------------------------------------------------ [2012-06-30 11:14:01] nikic@php.net Could you maybe provide a better example for the function? I don't know anything about the topic at hand, so I probably can't come up with a good example on my own. ------------------------------------------------------------------------ [2012-06-29 23:49:49] maarten dot bodewes at gmail dot com Description: ------------ --- From manual page: http://www.php.net/function.mcrypt-encrypt#refsect1-function.mcrypt-encrypt-examples --- Hi, I'm a security professional (+10 years experience). I'm wondering which arse wrote that PHP sample this bug is pointing at. The following mistakes are present (at the minimum): * using an IV with ECB encoding * using ECB at all for non random plain text * using ECB within an example in the first place * mistaking a passphrase with a key (keys should be random bytes of data, or at least generated using e.g. PBKDF2, bcrypt or scrypt) * supplying an incorrect number of characters for the key (25 if I'm not mistaken) * using MCRYPT_RIJNDAEL_256 instead of MCRYPT_RIJNDAEL_128 (AES) * not performing PKCS#7 padding by default If this sample is to coax unsuspecting people in writing insecure code which is not compatible with any crypto library out there, keep it in. Otherwise toss it out and start over again. ------------------------------------------------------------------------ -- Edit this bug report at https://bugs.php.net/bug.php?id=62453&edit=1