From 18767e31711656e9e2648fbe051b74ebbefd3f2e Mon Sep 17 00:00:00 2001 From: Andrey Andreev Date: Tue, 4 Mar 2014 22:21:35 +0200 Subject: CI_Encryption: Remove MCrypt 'work-arounds' for CAST-128 compatibility Turns out it's OpenSSL's fault for performing 16 rounds instead of 12 for key sizes of 5-11 bytes. Reference: http://tools.ietf.org/rfc/rfc2144.txt --- system/libraries/Encryption.php | 25 ++++--------------------- tests/codeigniter/libraries/Encryption_test.php | 16 ++++------------ user_guide_src/source/libraries/encryption.rst | 8 +++++++- 3 files changed, 15 insertions(+), 34 deletions(-) diff --git a/system/libraries/Encryption.php b/system/libraries/Encryption.php index bfc1e892a..810b7bf4a 100644 --- a/system/libraries/Encryption.php +++ b/system/libraries/Encryption.php @@ -365,15 +365,6 @@ class CI_Encryption { : NULL; } - // CAST-128 compatibility (http://tools.ietf.org/rfc/rfc2144.txt) - // - // RFC2144 says that keys shorter than 16 bytes are to be padded with - // zero bytes to 16 bytes, but (surprise) MCrypt doesn't do that. - if ($params['cipher'] === 'cast-128' && ($kl = strlen($params['key'])) < 16) - { - $params['key'] .= str_repeat("\x0", 16 - $kl); - } - if (mcrypt_generic_init($params['handle'], $params['key'], $params['iv']) < 0) { if ($params['handle'] !== $this->_handle) @@ -556,15 +547,6 @@ class CI_Encryption { } } - // CAST-128 compatibility (http://tools.ietf.org/rfc/rfc2144.txt) - // - // RFC2144 says that keys shorter than 16 bytes are to be padded with - // zero bytes to 16 bytes, but (surprise) MCrypt doesn't do that. - if ($params['cipher'] === 'cast-128' && ($kl = strlen($params['key'])) < 16) - { - $params['key'] .= str_repeat("\x0", 16 - $kl); - } - if (mcrypt_generic_init($params['handle'], $params['key'], $params['iv']) < 0) { if ($params['handle'] !== $this->_handle) @@ -794,9 +776,10 @@ class CI_Encryption { // - CAST-128/CAST5 produces a longer cipher when encrypted via // OpenSSL, but (strangely enough) can be decrypted by either // extension anyway. - // Also, RFC2144 says that the cipher supports key sizes - // between 5 and 16 bytes by the implementation actually - // zero-padding them to 16 bytes, but MCrypt doesn't do that. + // Also, it appears that OpenSSL uses 16 rounds regardless of + // the key size, while RFC2144 says that for key sizes lower + // than 11 bytes, only 12 rounds should be used. This makes + // it portable only with keys of between 11 and 16 bytes. // // - RC4 (ARCFour) has a strange implementation under OpenSSL. // Its 'rc4-40' cipher method seems to work flawlessly, yet diff --git a/tests/codeigniter/libraries/Encryption_test.php b/tests/codeigniter/libraries/Encryption_test.php index 5f2382482..cb9326d11 100644 --- a/tests/codeigniter/libraries/Encryption_test.php +++ b/tests/codeigniter/libraries/Encryption_test.php @@ -330,18 +330,10 @@ class Encryption_test extends CI_TestCase { array('blowfish', 'cfb', 56), array('blowfish', 'ofb', 56), array('blowfish', 'ecb', 56), - array('cast5', 'cbc', 5), - array('cast5', 'cfb', 5), - array('cast5', 'ofb', 5), - array('cast5', 'ecb', 5), - array('cast5', 'cbc', 8), - array('cast5', 'cfb', 8), - array('cast5', 'ofb', 8), - array('cast5', 'ecb', 8), - array('cast5', 'cbc', 10), - array('cast5', 'cfb', 10), - array('cast5', 'ofb', 10), - array('cast5', 'ecb', 10), + array('cast5', 'cbc', 11), + array('cast5', 'cfb', 11), + array('cast5', 'ofb', 11), + array('cast5', 'ecb', 11), array('cast5', 'cbc', 16), array('cast5', 'cfb', 16), array('cast5', 'ofb', 16), diff --git a/user_guide_src/source/libraries/encryption.rst b/user_guide_src/source/libraries/encryption.rst index cedc8d381..25bf7c12a 100644 --- a/user_guide_src/source/libraries/encryption.rst +++ b/user_guide_src/source/libraries/encryption.rst @@ -133,7 +133,7 @@ AES-256 aes-256 256 / 32 CBC, CT DES des 56 / 7 CBC, CFB, CFB8, OFB, ECB TripleDES tripledes 56 / 7, 112 / 14, 168 / 21 CBC, CFB, CFB8, OFB Blowfish blowfish 128-448 / 16-56 CBC, CFB, OFB, ECB -CAST5 / CAST-128 cast5 40-128 / 5-16 CBC, CFB, OFB, ECB +CAST5 / CAST-128 cast5 88-128 / 11-16 CBC, CFB, OFB, ECB RC4 / ARCFour rc4 40-2048 / 5-256 Stream ======================== ================== ============================ =============================== @@ -176,6 +176,7 @@ Rijndael-192 MCrypt 128 / 16, 192 / 24, 256 / 32 CBC, CTR, CFB, CFB8, OFB Rijndael-256 MCrypt 128 / 16, 192 / 24, 256 / 32 CBC, CTR, CFB, CFB8, OFB, OFB8, ECB GOST MCrypt 256 / 32 CBC, CTR, CFB, CFB8, OFB, OFB8, ECB Twofish MCrypt 128 / 16, 192 / 24, 256 / 32 CBC, CTR, CFB, CFB8, OFB, OFB8, ECB +CAST-128 MCrypt 40-128 / 5-16 CBC, CTR, CFB, CFB8, OFB, OFB8, ECB CAST-256 MCrypt 128 / 16, 192 / 24, 256 / 32 CBC, CTR, CFB, CFB8, OFB, OFB8, ECB Loki97 MCrypt 128 / 16, 192 / 24, 256 / 32 CBC, CTR, CFB, CFB8, OFB, OFB8, ECB SaferPlus MCrypt 128 / 16, 192 / 24, 256 / 32 CBC, CTR, CFB, CFB8, OFB, OFB8, ECB @@ -198,6 +199,11 @@ Seed OpenSSL 128 / 16 CBC, CFB, OFB, ECB important to note that AES-128 and Rijndael-128 are actually the same cipher, but **only** when used with a 128-bit key. +.. note:: CAST-128 / CAST-5 is also listed in both the portable and + driver-specific ciphers list. This is because OpenSSL's + implementation doesn't appear to be working correctly with + key sizes of 80 bits and lower. + .. note:: RC2 is listed as supported by both MCrypt and OpenSSL. However, both drivers implement them differently and they are not portable. It is probably worth noting that we only -- cgit v1.2.3-24-g4f1b