rfc:mcrypt-viking-funeral

Differences

This shows you the differences between two versions of the page.

Link to this comparison view

Both sides previous revisionPrevious revision
Next revision
Previous revision
Next revisionBoth sides next revision
rfc:mcrypt-viking-funeral [2016/03/15 16:10] sarciszewskirfc:mcrypt-viking-funeral [2016/07/10 18:23] – Add link to original patch by Scott cmb
Line 1: Line 1:
-====== PHP RFC: Deprecate Mcrypt ======+====== PHP RFC: Deprecate (then Remove) Mcrypt ======
   * Version: 1.0   * Version: 1.0
   * Date: 2016-01-09   * Date: 2016-01-09
   * Author: Scott Arciszewski, security@paragonie.com   * Author: Scott Arciszewski, security@paragonie.com
-  * Status: Under Discussion+  * Status: Accepted
   * First Published at: http://wiki.php.net/rfc/mcrypt-viking-funeral   * First Published at: http://wiki.php.net/rfc/mcrypt-viking-funeral
  
Line 17: Line 17:
  
 This RFC does not concern itself with the concept of shims or compatibility layers, and those topics are out of scope. If this RFC passes, another RFC could be drafted by interested parties to propose such a feature at a later date. This RFC does not concern itself with the concept of shims or compatibility layers, and those topics are out of scope. If this RFC passes, another RFC could be drafted by interested parties to propose such a feature at a later date.
 +
 +===== Background Information (Regarding Security) =====
 +
 +A survey of websites where people share cryptography code reveals a lot about how people use mcrypt. You'll most likely find gems like this:
 +
 +<code>
 +/**
 + * Don't use this. It was copied from StackOverflow to demonstrate how
 + * unsuccessful developers are at using the mcrypt extension:
 + */
 +function fnEncrypt($sValue, $sSecretKey)
 +{
 +    return rtrim(
 +        base64_encode(
 +            mcrypt_encrypt(
 +                MCRYPT_RIJNDAEL_256,
 +                $sSecretKey, $sValue,
 +                MCRYPT_MODE_ECB,
 +                mcrypt_create_iv(
 +                    mcrypt_get_iv_size(
 +                        MCRYPT_RIJNDAEL_256,
 +                        MCRYPT_MODE_ECB
 +                    ),
 +                    MCRYPT_RAND)
 +                )
 +            ), "\0"
 +        );
 +}
 +
 +function fnDecrypt($sValue, $sSecretKey)
 +{
 +    return rtrim(
 +        mcrypt_decrypt(
 +            MCRYPT_RIJNDAEL_256,
 +            $sSecretKey,
 +            base64_decode($sValue),
 +            MCRYPT_MODE_ECB,
 +            mcrypt_create_iv(
 +                mcrypt_get_iv_size(
 +                    MCRYPT_RIJNDAEL_256,
 +                    MCRYPT_MODE_ECB
 +                ),
 +                MCRYPT_RAND
 +            )
 +        ), "\0"
 +    );
 +}
 +</code>
 +
 +Problems with this code:
 +
 +  * It's using [[https://blog.filippo.io/the-ecb-penguin|ECB Mode]]
 +  * It's attempting to generate an IV for ECB mode (which is a waste of CPU since IVs aren't used in ECB mode)
 +  * It's using MCRYPT_RAND for IV generation, which isn't a CSPRNG
 +  * fnEncrypt() will rtrim() null bytes off the encrypted value before base64 encoding it, which means a 1/256 chance of data corruption that prevents decryption
 +  * fnDecrypt() will rtrim() null bytes off the decrypted plaintext, which means if your plaintext message was raw binary (e.g. gzip compressed), it's now corrupted
 +  * There is no MAC, so you transmit this over a network, [[https://tonyarcieri.com/all-the-crypto-code-youve-ever-written-is-probably-broken|it's vulnerable to chosen-ciphertext attacks]]
 +
 +Mcrypt has a lot of design warts and puts a lot of burden on the implementer to choose the right components (and stitch them together correctly). Blaming the implementer leads to error-prone cryptographic designs. Error-prone cryptographic designs leads to insecure applications.
 +
 +If libmcrypt were still being maintained, we could work with the libmcrypt team to improve it. Unfortunately, it was [[https://sourceforge.net/projects/mcrypt/files/Libmcrypt/2.5.8/|abandoned in 2007]], and contains [[https://sourceforge.net/p/mcrypt/bugs/|unfixed bugs]] and [[https://sourceforge.net/p/mcrypt/patches/|patches that will never be merged]].
 +
 +Everything libmcrypt can do, openssl can do too (and often better), either out-of-the-box or via its support for pluggable ciphers.
 +
 +For example: OpenSSL's AES implementation (which mcrypt still called MCRYPT_RIJNDAEL_128 before it was abandoned) uses AES-NI on modern processors to deliver very fast AES that withstands cache-timing attacks. Mcrypt's AES still used S-boxes.
 +
 +Comparing libmcrypt to openssl, you will find that it is:
 +
 +  * Less well-maintained
 +  * Less standards compliant (NULL byte padding can cause data loss on decryption; PKCS#7 padding, which openssl_encrypt() uses, does not)
 +  * More confusing for end-users (not that OpenSSL's API and documentation are stellar)
 +  * Slower on modern hardware
 +  * Less resistant to active attackers
 +  * Less feature-rich (no public-key cryptography)
  
 ===== Backward Incompatible Changes ===== ===== Backward Incompatible Changes =====
  
-Any cryptography code that depends on mcrypt will need to be refactored against openssl. This isn't as difficult as it sounds, provided you're using a trustworthy cipher (e.g. MCRYPT_RIJNDAEL_128). Based on [[https://3v4l.org/m4P2C|this 3v4l]], I can generally conclude that the following MCRYPT ciphers are not supported by openssl:+Any cryptography code that depends on mcrypt will need to be refactored against openssl. This isn't as difficult as it sounds, provided you're using a trustworthy cipher (e.g. MCRYPT_RIJNDAEL_128). Based on [[https://3v4l.org/m4P2C|this 3v4l]], I can generally conclude that the following MCRYPT ciphers are not currently supported by openssl:
  
   * GOST   * GOST
Line 33: Line 107:
   * XTEA   * XTEA
   * Enigma   * Enigma
 +
 +This is an acceptable loss: Most of the ciphers in the list above should not be used in new software anyway. Most cryptography experts would consider their inclusion in any software written in 2016 to be a code smell and indicative of a bad protocol design. Some of them (e.g. Enigma) are outright insecure and should not be used at all.
  
 ===== Proposed PHP Version(s) ===== ===== Proposed PHP Version(s) =====
Line 48: Line 124:
 Since this would break backwards compatibility, a 2/3 majority is required. Since this would break backwards compatibility, a 2/3 majority is required.
  
-<doodle title="Deprecate then Remove Mcrypt from the PHP Core?" auth="sarciszewski" voteType="single">+<doodle title="Deprecate then Remove Mcrypt from the PHP Core?" auth="sarciszewski" voteType="single" closed="true">
    * Yes    * Yes
    * No    * No
 </doodle> </doodle>
  
-This vote is opened on March 15th, 2016 and will close March 22th at 17:00 UTC as announced on list.+This vote is opened on March 15th, 2016 and will close March 22nd at 17:00 UTC as announced on list.
  
 ===== Patches and Tests ===== ===== Patches and Tests =====
  
-If this RFC is accepted, I will author the patch to expunge ext/mcrypt.+Patches are available:  
 +  * <https://github.com/php/php-src/pull/1995> 
 +  * <https://github.com/php/php-src/pull/1996>
  
 ===== References ===== ===== References =====
Line 63: Line 141:
   * [[http://blog.remirepo.net/post/2015/07/07/About-libmcrypt-and-php-mcrypt|Remi on mcrypt]]   * [[http://blog.remirepo.net/post/2015/07/07/About-libmcrypt-and-php-mcrypt|Remi on mcrypt]]
   * [[https://paragonie.com/blog/2015/05/if-you-re-typing-word-mcrypt-into-your-code-you-re-doing-it-wrong|If you're typing the word MCRYPT into your PHP code, you're doing it wrong]]   * [[https://paragonie.com/blog/2015/05/if-you-re-typing-word-mcrypt-into-your-code-you-re-doing-it-wrong|If you're typing the word MCRYPT into your PHP code, you're doing it wrong]]
 +  * [[http://swiftonsecurity.tumblr.com/post/98675308034/a-story-about-jessica|A story about Jessica (regarding the usability and accessibility of computer security)]] 
 +  * [[http://www.gaudior.net/alma/johnny.pdf|Why Johnny Can't Encrypt]] 
 +  * [[https://cr.yp.to/talks/2015.01.07/slides-djb-20150107-a4.pdf|Error-prone cryptographic designs]] 
 +  * [[http://blog.ircmaxell.com/2014/11/its-all-about-time.html|More about cache-timing attacks]]
  
 ===== Rejected Features ===== ===== Rejected Features =====
  
rfc/mcrypt-viking-funeral.txt · Last modified: 2017/09/22 13:28 by 127.0.0.1