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

Edit report at https://bugs.php.net/bug.php?id=62453&edit=1 ID: 62453 Updated by: gwynne@php.net Reported by: maarten dot bodewes at gmail dot com Summary: Terrible sample code for mcrypt_encrypt -Status: Assigned +Status: Closed Type: Documentation Problem Package: Documentation problem PHP Version: 5.4.4 Assigned To: ircmaxell Block user comment: N Private report: N New Comment: This bug has been fixed in the documentation's XML sources. Since the online and downloadable versions of the documentation need some time to get updated, we would like to ask you to be a bit patient. Thank you for the report, and for helping us make our documentation better. Previous Comments: ------------------------------------------------------------------------ [2013-03-27 23:48:44] gwynne@php.net Automatic comment from SVN on behalf of gwynne Revision: http://svn.php.net/viewvc/?view=revision&revision=329941 Log: Fix bug #62453 ------------------------------------------------------------------------ [2012-08-13 00:45:39] maarten dot bodewes at gmail dot com Hello? Anybody? Are you just going to let this fester? ------------------------------------------------------------------------ [2012-07-15 20:54:20] maarten dot bodewes at gmail dot com I've tried to contact the dev. to apply this change for me. If you want I can apply it myself, but I quickly got lost in the interface where I should apply the change (and I don't have enough time to get fully acquainted with it). If anybody wants the new code applied, please do so, or guide me through the process. ------------------------------------------------------------------------ [2012-07-03 12:47:54] ircmaxell@php.net Maarten, Sounds good to me. I would apply it for you, but I'd rather give credit where credit is due. Could you make the change on http://edit.php.net ? All you need to do is make a patch from that console (I believe you right click your work in progress tab, and there should be an option to submit a patch). Then just post back here with the username that you used and I'll commit it for you. That way you get credit for the change instead of me. If that's too much, let me know and I'll apply it for you. But I would rather you get the credit for it. Thanks, Anthony ------------------------------------------------------------------------ [2012-07-01 10:13:42] maarten dot bodewes at gmail dot com The requirements to add integrity protection and correct padding are very much dependent on the use case. If you store encrypted strings in a database (which is a pretty common use case) then neither are required. That said, adding a integrity validation or PKCS#7 padding makes a lot of sense and will seldom hurt; it's probably better regarding security than using 256 bit keys over 128 bit ones. The following observations can be made regarding PKCS#7: - PKCS#7 padding, if programmed correctly, will allow for any length messages with any content (including bytes valued 00h at the end, which is where zero byte padding fails). - PKCS#7 will *always* pad the message, so it may grow it with 16 bytes if the plain text is a multiple of 16. So approx. 1 out of 16 times it will take 16 bytes more than zero padding now deployed. - padding/unpadding should be almost instantaneous and can be performed after decryption - PKCS#7 should *not* be implemented without taking the block size in mind, and should check all the padding bytes, exiting with an error if both restraints are not met The following observations can be made regarding integrity checks: - currently the only integrity check that seems to be present in PHP is HMAC in http://php.net/manual/en/function.hash-hmac.php so it would currently make sense to point in that direction (if you need integrity checks, see HMAC). HMAC is still a viable solution. - AES CMAC and implementing GCM or possibly EAX modes of encryption would make a lot of sense; GCM will be used more and more, e.g. in upcoming TLS and XML encryption standards. These will add integrity checking and authentication within the encryption protocol itself (and won't require padding). Note that stream ciphers do have more stringent requirements on the IV though (well, actually the Nonce, but they play a similar role). - MAC / HMAC require the use of a second key (!), the key used for encryption should not be used. - MAC / HMAC require a full pass (using either the cipher or the hash algorithm) over the cipher text. This will approx. double the CPU time required for the encryption (for larger messages). - The MAC/HMAC should be over the cipher text, not the plain text (even though this has its drawbacks too, more reason to switch to GCM mode) if only to stop padding oracle attacks. - Padding oracle attacks (and other attacks in this regard), if applicable, can only be stopped by using a stream cipher (no padding) or indeed integrity protection of the cipher text. The PKCS#7 padding algorithms does not seem to be present in PHP - currently it seems that users are required to copy or write their own code (incorrectly of course, see notes above). Note that most other API's integrate padding within their encrypt/decrypt implementations. PKCS#7 is pretty much standard for CBC/ECB. I think this is a bit much to put into the sample code :) In the end, you cannot apply cryptographic algorithms without knowing at least the cornerstones of cryptography. My advice would be to point to the PHP HMAC functionality with a warning to not use the same key used for encryption. As for PKCS#7, I would add (the relatively simple) implementation to PHP and point to that when present - keep the warning in for now. Regards, Maarten PS although I have 10 years experience in the practical *application* of cryptography I am not a professor in cryptography, feel free to have this advice looked at by more knowledgeable persons in the community (http://crypto.stackexchange.com seems to host a surprising amount of them, or try an actual professor in cryptography). ------------------------------------------------------------------------ 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=62453 -- Edit this bug report at https://bugs.php.net/bug.php?id=62453&edit=1