rfc:mcrypt-viking-funeral

Differences

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

Link to this comparison view

Both sides previous revision Previous revision
Next revision
Previous revision
rfc:mcrypt-viking-funeral [2016/03/15 17:35]
sarciszewski
rfc:mcrypt-viking-funeral [2017/09/22 13:28] (current)
Line 3: Line 3:
   * Date: 2016-01-09   * Date: 2016-01-09
   * Author: Scott Arciszewski, security@paragonie.com   * Author: Scott Arciszewski, security@paragonie.com
-  * Status: Voting+  * Status: Implemented (PHP 7.1)
   * 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 =====
Line 50: 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" closed="false">+<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 patches to deprecate ext/mcrypt.+Patches are available:  
 +  * <del><https://github.com/php/php-src/pull/1995></del> 
 +  * <https://github.com/php/php-src/pull/1996> (merged)
  
 ===== References ===== ===== References =====
Line 65: 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.1458063313.txt.gz · Last modified: 2017/09/22 13:28 (external edit)