From e02f4c47fab1b5db0dab879e07546d01b42f381e Mon Sep 17 00:00:00 2001 From: Technion Date: Sat, 10 May 2014 10:03:43 -0400 Subject: [PATCH 1/6] There is no value in a high work order here. An HMAC-SHA256 is used in a single run in most other protocols. If there is a need for a work order, it should be used on the key. Completion of previous commit --- lib/phpSec/Crypt/Crypto.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/phpSec/Crypt/Crypto.php b/lib/phpSec/Crypt/Crypto.php index bdcba45..102ed68 100644 --- a/lib/phpSec/Crypt/Crypto.php +++ b/lib/phpSec/Crypt/Crypto.php @@ -111,7 +111,7 @@ public function encrypt($data, $key) { $encrypted['iv'] = base64_encode($iv); /* Initialization vector, just a bunch of randomness. */ $encrypted['cdata'] = base64_encode(mcrypt_generic($td, $serializedData)); /* The encrypted data. */ $encrypted['mac'] = base64_encode( /* The message authentication code. Used to make sure the */ - $this->pbkdf2($encrypted['cdata'], $key, 1000, 32) /* message is valid when decrypted. */ + $this->pbkdf2($encrypted['cdata'], $key, 1, 32) /* message is valid when decrypted. */ ); return json_encode($encrypted); } @@ -149,7 +149,7 @@ public function decrypt($data, $key) { $block = mcrypt_enc_get_block_size($td); /* Check MAC. */ - if(base64_decode($data['mac']) != $this->pbkdf2($data['cdata'], $key, 1000, 32)) { + if(base64_decode($data['mac']) != $this->pbkdf2($data['cdata'], $key, 1, 32)) { throw new \phpSec\Exception\GeneralSecurityException('Message authentication code invalid'); return false; } From ebcc18ebb49257174e59b000322b019dce044243 Mon Sep 17 00:00:00 2001 From: Technion Date: Sat, 10 May 2014 10:21:11 -0400 Subject: [PATCH 2/6] Break key into encryption key and HMAC key using pbkdf --- lib/phpSec/Crypt/Crypto.php | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/lib/phpSec/Crypt/Crypto.php b/lib/phpSec/Crypt/Crypto.php index 102ed68..a4ad86f 100644 --- a/lib/phpSec/Crypt/Crypto.php +++ b/lib/phpSec/Crypt/Crypto.php @@ -84,12 +84,17 @@ public function encrypt($data, $key) { } } + /* Using PBKDF with constant salts dedicated to each purpose + * can securely derivce two keys from one */ + $key1 = $this->pbkdf2($key, "encrypt", 1, $keySize); + $key2 = $this->pbkdf2($key, "HMAC", 1, $keySize); + /* Create IV. */ $rnd = $this->psl['crypt/rand']; $iv = $rnd->bytes(mcrypt_enc_get_iv_size($td)); /* Init mcrypt. */ - mcrypt_generic_init($td, $key, $iv); + mcrypt_generic_init($td, $key1, $iv); /* Prepeare the array with data. */ $serializedData = serialize($data); @@ -111,7 +116,7 @@ public function encrypt($data, $key) { $encrypted['iv'] = base64_encode($iv); /* Initialization vector, just a bunch of randomness. */ $encrypted['cdata'] = base64_encode(mcrypt_generic($td, $serializedData)); /* The encrypted data. */ $encrypted['mac'] = base64_encode( /* The message authentication code. Used to make sure the */ - $this->pbkdf2($encrypted['cdata'], $key, 1, 32) /* message is valid when decrypted. */ + $this->pbkdf2($encrypted['cdata'], $key2, 1, 32) /* message is valid when decrypted. */ ); return json_encode($encrypted); } @@ -148,14 +153,19 @@ public function decrypt($data, $key) { $td = mcrypt_module_open($data['algo'], '', $data['mode'], ''); $block = mcrypt_enc_get_block_size($td); + /* Using PBKDF with constant salts dedicated to each purpose + * can securely derivce two keys from one */ + $key1 = $this->pbkdf2($key, "encrypt", 1, $keySize); + $key2 = $this->pbkdf2($key, "HMAC", 1, $keySize); + /* Check MAC. */ - if(base64_decode($data['mac']) != $this->pbkdf2($data['cdata'], $key, 1, 32)) { + if(base64_decode($data['mac']) != $this->pbkdf2($data['cdata'], $key2, 1, 32)) { throw new \phpSec\Exception\GeneralSecurityException('Message authentication code invalid'); return false; } /* Init mcrypt. */ - mcrypt_generic_init($td, $key, base64_decode($data['iv'])); + mcrypt_generic_init($td, $key1, base64_decode($data['iv'])); $decrypted = rtrim(mdecrypt_generic($td, base64_decode($this->stripPadding($block, $data['cdata'])))); From def1203f4d5b614cc7327686a568c97ef7eb3641 Mon Sep 17 00:00:00 2001 From: Technion Date: Sat, 10 May 2014 10:34:27 -0400 Subject: [PATCH 3/6] Typo in comment --- lib/phpSec/Crypt/Crypto.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/phpSec/Crypt/Crypto.php b/lib/phpSec/Crypt/Crypto.php index a4ad86f..28aef38 100644 --- a/lib/phpSec/Crypt/Crypto.php +++ b/lib/phpSec/Crypt/Crypto.php @@ -77,7 +77,7 @@ public function encrypt($data, $key) { return false; } } else { - /* No spsecific size is needed. */ + /* No specific size is needed. */ if($keySize == 0 || $keySize > mcrypt_enc_get_key_size($td)) { throw new \phpSec\Exception\InvalidKeySpecException('Key is out of range. Should be between 1 and ' . mcrypt_enc_get_key_size($td).' bytes.'); return false; From fcaa7614a62fac4e0dc2111e7335a4cda355a0f5 Mon Sep 17 00:00:00 2001 From: Technion Date: Sat, 10 May 2014 10:44:15 -0400 Subject: [PATCH 4/6] Added additional test case - incorrect key. --- tests/phpSec/Crypt/CryptoTest.php | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/tests/phpSec/Crypt/CryptoTest.php b/tests/phpSec/Crypt/CryptoTest.php index 298ebb4..06e3a96 100644 --- a/tests/phpSec/Crypt/CryptoTest.php +++ b/tests/phpSec/Crypt/CryptoTest.php @@ -7,12 +7,15 @@ public function testCrypto() { $str = 'foobaz'; $key = '123abc12123abc12'; + $badkey = '123abcR77123abc12'; + $encrypted = $crypto->encrypt($str, $key); $decrypted = $crypto->decrypt($encrypted, $key); $this->assertEquals($decrypted, $str); + $this->assert(!($crypto->decrypt($encrypted, $badkey))); } -} \ No newline at end of file +} From e09b852c9b53e25181bbe4dab0bce44187d7e27a Mon Sep 17 00:00:00 2001 From: Technion Date: Sat, 10 May 2014 10:46:46 -0400 Subject: [PATCH 5/6] Regression - define keysize in decrypt. --- lib/phpSec/Crypt/Crypto.php | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/phpSec/Crypt/Crypto.php b/lib/phpSec/Crypt/Crypto.php index 28aef38..573de07 100644 --- a/lib/phpSec/Crypt/Crypto.php +++ b/lib/phpSec/Crypt/Crypto.php @@ -155,6 +155,7 @@ public function decrypt($data, $key) { /* Using PBKDF with constant salts dedicated to each purpose * can securely derivce two keys from one */ + $keySize = strlen($key); $key1 = $this->pbkdf2($key, "encrypt", 1, $keySize); $key2 = $this->pbkdf2($key, "HMAC", 1, $keySize); From 85d08aa29bd046ae00e301b7a6cdfb35c608b80a Mon Sep 17 00:00:00 2001 From: Technion Date: Sat, 10 May 2014 10:50:14 -0400 Subject: [PATCH 6/6] Correct use of PHPUnit Catch exception correctly. --- tests/phpSec/Crypt/CryptoTest.php | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/tests/phpSec/Crypt/CryptoTest.php b/tests/phpSec/Crypt/CryptoTest.php index 06e3a96..ae11faf 100644 --- a/tests/phpSec/Crypt/CryptoTest.php +++ b/tests/phpSec/Crypt/CryptoTest.php @@ -15,7 +15,11 @@ public function testCrypto() { $decrypted = $crypto->decrypt($encrypted, $key); $this->assertEquals($decrypted, $str); - $this->assert(!($crypto->decrypt($encrypted, $badkey))); + try { + $ret = $crypto->decrypt($encrypted, $badkey); + }catch(Exception $e){ + $this->assertEquals("Message authentication code invalid",$e->getMessage()); + } } }