Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Added support for AES IGE #1095

Open
wants to merge 19 commits into
base: master
Choose a base branch
from
Open

Added support for AES IGE #1095

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
12dc23d
Added support for AES IGE
danog Feb 10, 2017
e2f91cc
Use protected iv_size attribute to store required IV size
danog Feb 11, 2017
200c2a9
Merge branch 'master' of https://github.com/phpseclib/phpseclib
danog May 11, 2017
6d714a6
Merge
danog Oct 5, 2017
52c068a
Fixes
danog Oct 5, 2017
1a730d1
Add missing primes static attribute
danog Oct 5, 2017
273ad17
Bugfix
danog Oct 6, 2017
4e367c9
Fixes for phar archives
danog Feb 20, 2018
0d9dc46
Merge main origin
danog Feb 20, 2018
a74a5f7
Fix syntax error
danog Feb 20, 2018
27370df
Fixes for eval engine
danog Feb 20, 2018
3097d29
Merge remote
danog Jun 14, 2019
0aff25e
Fix bad merge
danog Jun 14, 2019
c85ddd3
Use length multiplier
danog Jun 14, 2019
2dc68c8
Rewrite IGE
danog Jun 15, 2019
c62fc0c
Enable openssl w/ IGE
danog Jun 16, 2019
2dc143a
Performance improvements
danog Jun 17, 2019
5e7d391
Fixes to tests
danog Jun 17, 2019
f715b29
Merge
danog Dec 12, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
156 changes: 152 additions & 4 deletions phpseclib/Crypt/Common/SymmetricKey.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,10 @@ abstract class SymmetricKey
* Encrypt / decrypt using streaming mode.
*/
const MODE_STREAM = 5;
/**
* Encrypt / decrypt using the Infinite Garble Extension mode.
*/
const MODE_IGE = 6;
/**#@-*/

/**#@+
Expand Down Expand Up @@ -452,6 +456,8 @@ abstract class SymmetricKey
*
* - self::MODE_OFB
*
* - self::MODE_IGE
*
* @param int $mode
* @access public
* @throws \InvalidArgumentException if an invalid / unsupported mode is provided
Expand All @@ -468,6 +474,7 @@ public function __construct($mode)
case self::MODE_CFB:
case self::MODE_OFB:
case self::MODE_STREAM:
case self::MODE_IGE:
$this->paddable = false;
break;
default:
Expand Down Expand Up @@ -498,10 +505,12 @@ public function setIV($iv)
throw new \InvalidArgumentException('This algorithm does not use an IV.');
}

if (strlen($iv) != $this->block_size) {
throw new \LengthException('Received initialization vector of size ' . strlen($iv) . ', but size ' . $this->block_size . ' is required');
if (strlen($iv) != $this->block_size*($this->mode === self::MODE_IGE ? 2 : 1)) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's introduce a new variable: expected_iv_length

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done!

throw new \LengthException('Received initialization vector of size ' . strlen($iv) . ', but size ' . $this->block_size*($this->mode === self::MODE_IGE ? 2 : 1). ' is required');
}



$this->iv = $iv;
$this->changed = true;
}
Expand Down Expand Up @@ -840,6 +849,21 @@ public function encrypt($plaintext)
return $result;
case self::MODE_CTR:
return $this->openssl_ctr_process($plaintext, $this->encryptIV, $this->enbuffer);
case self::MODE_IGE:
$ciphertext = '';
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This block seems to be (largely) duplicated a bunch of times below.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, but I wasn't sure whether it was a good idea to create a separate function just to avoid duplicating a few lines of code.
Should I do that?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, what about the problems in the native module?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

bump

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've been on the fence with this. My initial searches led me to conclude that this was a one off mode (and not even a very good one at that) used for only one app:

https://www.mgp25.com/AESIGE/
http://www.cryptofails.com/post/70546720222/telegrams-cryptanalysis-contest

Keeping that in mind, I don't really want to add every homegrown block cipher algorithm under the sun.

If Joe Schmoe made his own shitty symmetric key algorithm that no one other than he used should I include that in phpseclib as well if he did a PR? No. There has to be some sort of bar for inclusion.

That said, I did a Google search just now. I guess OpenSSL supports this mode:

https://lists.gnupg.org/pipermail/gcrypt-devel/2015-September/003572.html
https://stackoverflow.com/questions/18171973/aes-aes-ige-128-aes-ige-192-aes-ige-256-encryption-decryption-with-openssl-c

So I guess that fact merits reconsideration of this PR.

Copy link
Member

@terrafrost terrafrost Oct 8, 2017 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree with bantu's comment on the code duplication. Seems like you could just call a new method - handleIGEEncrypt:

function handleIGEEncrypt($plaintext) {
    $ciphertext = '';
    $iv_part_1 = substr($this->encryptIV, 0, $this->block_size);
    $iv_part_2 = substr($this->encryptIV, $this->block_size);
    for ($i = 0; $i < strlen($plaintext); $i+= $this->block_size) {
        $indata = substr($plaintext, $i, $this->block_size);
        switch ($this->engine) {
            self::ENGINE_OPENSSL:
                $outdata = openssl_encrypt($indata ^ $iv_part_1, $this->cipher_name_openssl, $this->key, OPENSSL_RAW_DATA | OPENSSL_ZERO_PADDING) ^ $iv_part_2;
                break;
            self::ENGINE_MCRYPT:
                $outdata = @mcrypt_generic($this->enmcrypt, $indata ^ $iv_part_1) ^ $iv_part_2;
                break;
            self::ENGINE_INTERNAL:
                $outdata = $this->encryptBlock($indata ^ $iv_part_1) ^ $iv_part_2;
        }
        $iv_part_1 = $outdata;
        $iv_part_2 = $indata;
        $ciphertext.= $outdata;
    }
    if ($this->continuousBuffer) {
        $this->encryptIV = $iv_part_1 . $iv_part_2;
    }
    return $ciphertext;
}

I mean, I suppose it's somewhat hypocritical for me to criticize it here when phpseclib does it with CFB mode, but... I just pushed a commit, in a branch, aimed at reducing some of the duplication for CFB mode:

https://github.com/terrafrost/phpseclib/tree/cfb-optimization

Of course, in this case, I suppose the old addage applies: "if it ain't broke don't fix it". We'll see where that branch goes.

$iv_part_1 = substr($this->encryptIV, 0, $this->block_size);
$iv_part_2 = substr($this->encryptIV, $this->block_size);
for ($i = 0; $i < strlen($plaintext); $i += $this->block_size) {
$indata = substr($plaintext, $i, $this->block_size);
$outdata = openssl_encrypt($indata ^ $iv_part_1, $this->cipher_name_openssl, $this->key, OPENSSL_RAW_DATA | OPENSSL_ZERO_PADDING) ^ $iv_part_2;
$iv_part_1 = $outdata;
$iv_part_2 = $indata;
$ciphertext .= $outdata;
}
if ($this->continuousBuffer) {
$this->encryptIV = $iv_part_1.$iv_part_2;
}
return $ciphertext;
case self::MODE_CFB:
// cfb loosely routines inspired by openssl's:
// {@link http://cvs.openssl.org/fileview?f=openssl/crypto/modes/cfb128.c&v=1.3.2.2.2.1}
Expand Down Expand Up @@ -958,7 +982,22 @@ public function encrypt($plaintext)

return $ciphertext;
}

if ($this->mode === self::MODE_IGE) {
$ciphertext = '';
$iv_part_1 = substr($this->encryptIV, 0, $this->block_size);
$iv_part_2 = substr($this->encryptIV, $this->block_size);
for ($i = 0; $i < strlen($plaintext); $i += $this->block_size) {
$indata = substr($plaintext, $i, $this->block_size);
$outdata = @mcrypt_generic($this->enmcrypt, $indata ^ $iv_part_1) ^ $iv_part_2;
$iv_part_1 = $outdata;
$iv_part_2 = $indata;
$ciphertext .= $outdata;
}
if ($this->continuousBuffer) {
$this->encryptIV = $iv_part_1.$iv_part_2;
}
return $ciphertext;
}
$ciphertext = @mcrypt_generic($this->enmcrypt, $plaintext);

if (!$this->continuousBuffer) {
Expand Down Expand Up @@ -986,6 +1025,20 @@ public function encrypt($plaintext)
$ciphertext.= $this->encryptBlock(substr($plaintext, $i, $block_size));
}
break;
case self::MODE_IGE:
$iv_part_1 = substr($this->encryptIV, 0, $this->block_size);
$iv_part_2 = substr($this->encryptIV, $this->block_size);
for ($i = 0; $i < strlen($plaintext); $i += $this->block_size) {
$indata = substr($plaintext, $i, $this->block_size);
$outdata = $this->encryptBlock($indata ^ $iv_part_1) ^ $iv_part_2;
$iv_part_1 = $outdata;
$iv_part_2 = $indata;
$ciphertext .= $outdata;
}
if ($this->continuousBuffer) {
$this->encryptIV = $iv_part_1.$iv_part_2;
}
break;
case self::MODE_CBC:
$xor = $this->encryptIV;
for ($i = 0; $i < strlen($plaintext); $i+=$block_size) {
Expand Down Expand Up @@ -1139,9 +1192,26 @@ public function decrypt($ciphertext)
$this->decryptIV = substr($ciphertext, -$offset, $this->block_size);
}
break;
case self::MODE_IGE:
$plaintext = '';
$iv_part_1 = substr($this->decryptIV, 0, $this->block_size);
$iv_part_2 = substr($this->decryptIV, $this->block_size);
for ($i = 0; $i < strlen($ciphertext); $i += $this->block_size) {
$indata = substr($ciphertext, $i, $this->block_size);
$outdata = openssl_decrypt($indata ^ $iv_part_2, $this->cipher_name_openssl, $this->key, OPENSSL_RAW_DATA | OPENSSL_ZERO_PADDING) ^ $iv_part_1;
$iv_part_1 = $indata;
$iv_part_2 = $outdata;
$plaintext .= $outdata;
}
if ($this->continuousBuffer) {
$this->decryptIV = $iv_part_1.$iv_part_2;
}
break;

case self::MODE_CTR:
$plaintext = $this->openssl_ctr_process($ciphertext, $this->decryptIV, $this->debuffer);
break;

case self::MODE_CFB:
// cfb loosely routines inspired by openssl's:
// {@link http://cvs.openssl.org/fileview?f=openssl/crypto/modes/cfb128.c&v=1.3.2.2.2.1}
Expand Down Expand Up @@ -1242,6 +1312,22 @@ public function decrypt($ciphertext)

return $plaintext;
}
if ($this->mode === self::MODE_IGE) {
$plaintext = '';
$iv_part_1 = substr($this->decryptIV, 0, $this->block_size);
$iv_part_2 = substr($this->decryptIV, $this->block_size);
for ($i = 0; $i < strlen($ciphertext); $i += $this->block_size) {
$indata = substr($ciphertext, $i, $this->block_size);
$outdata = @mcrypt_generic($this->demcrypt, $indata ^ $iv_part_2) ^ $iv_part_1;
$iv_part_1 = $indata;
$iv_part_2 = $outdata;
$plaintext .= $outdata;
}
if ($this->continuousBuffer) {
$this->decryptIV = $iv_part_1.$iv_part_2;
}
return $plaintext;
}

$plaintext = @mdecrypt_generic($this->demcrypt, $ciphertext);

Expand Down Expand Up @@ -1282,6 +1368,20 @@ public function decrypt($ciphertext)
$this->decryptIV = $xor;
}
break;
case self::MODE_IGE:
$iv_part_1 = substr($this->decryptIV, 0, $this->block_size);
$iv_part_2 = substr($this->decryptIV, $this->block_size);
for ($i = 0; $i < strlen($ciphertext); $i += $this->block_size) {
$indata = substr($ciphertext, $i, $this->block_size);
$outdata = $this->decryptBlock($indata ^ $iv_part_2) ^ $iv_part_1;
$iv_part_1 = $indata;
$iv_part_2 = $outdata;
$plaintext .= $outdata;
}
if ($this->continuousBuffer) {
$this->decryptIV = $iv_part_1.$iv_part_2;
}
break;
case self::MODE_CTR:
$xor = $this->decryptIV;
if (strlen($buffer['ciphertext'])) {
Expand Down Expand Up @@ -1553,6 +1653,7 @@ protected function openssl_translate_mode()
{
switch ($this->mode) {
case self::MODE_ECB:
case self::MODE_IGE:
return 'ecb';
case self::MODE_CBC:
return 'cbc';
Expand Down Expand Up @@ -1906,6 +2007,7 @@ private function setupMcrypt()
if (!isset($this->enmcrypt)) {
static $mcrypt_modes = [
self::MODE_CTR => 'ctr',
self::MODE_IGE => MCRYPT_MODE_ECB,
self::MODE_ECB => MCRYPT_MODE_ECB,
self::MODE_CBC => MCRYPT_MODE_CBC,
self::MODE_CFB => 'ncfb',
Expand All @@ -1919,7 +2021,7 @@ private function setupMcrypt()
// we need the $ecb mcrypt resource (only) in MODE_CFB with enableContinuousBuffer()
// to workaround mcrypt's broken ncfb implementation in buffered mode
// see: {@link http://phpseclib.sourceforge.net/cfb-demo.phps}
if ($this->mode == self::MODE_CFB) {
if ($this->mode === self::MODE_CFB) {
$this->ecb = @mcrypt_module_open($this->cipher_name_mcrypt, '', MCRYPT_MODE_ECB, '');
}
} // else should mcrypt_generic_deinit be called?
Expand Down Expand Up @@ -2520,6 +2622,52 @@ protected function createInlineCryptFunction($cipher_code)
$decrypt = $init_decrypt . '
$_plaintext = "";
'.$decrypt_block.'
return $_plaintext;
';
break;
case self::MODE_IGE:
$encrypt = $init_encrypt . '
$_ciphertext = "";
$_plaintext_len = strlen($_text);

$_iv_part_1 = substr($this->encryptIV, 0, '.$block_size.');
$_iv_part_2 = substr($this->encryptIV, '.$block_size.');
for ($_i = 0; $_i < $_plaintext_len; $_i+= '.$block_size.') {
$in = ($indata = substr($_text, $_i, '.$block_size.')) ^ $_iv_part_1;
'.$encrypt_block.'
$outdata = $in ^ $_iv_part_2;
$_iv_part_1 = $outdata;
$_iv_part_2 = $indata;
$_ciphertext .= $outdata;
}
if ($this->continuousBuffer) {
$this->encryptIV = $_iv_part_1.$_iv_part_2;
}

return $_ciphertext;
';

$decrypt = $init_decrypt . '
$_plaintext = "";
$_text = str_pad($_text, strlen($_text) + ('.$block_size.' - strlen($_text) % '.$block_size.') % '.$block_size.', chr(0));
$_ciphertext_len = strlen($_text);

$_iv_part_1 = substr($this->decryptIV, 0, '.$block_size.');
$_iv_part_2 = substr($this->decryptIV, '.$block_size.');

for ($_i = 0; $_i < $_ciphertext_len; $_i+= '.$block_size.') {
$in = ($indata = substr($_text, $_i, '.$block_size.')) ^ $_iv_part_2;
'.$decrypt_block.'
$outdata = $in ^ $_iv_part_1;
$_iv_part_1 = $indata;
$_iv_part_2 = $outdata;
$_plaintext .= $outdata;
}

if ($this->continuousBuffer) {
$this->decryptIV = $_iv_part_1.$_iv_part_2;
}

return $_plaintext;
';
break;
Expand Down
7 changes: 5 additions & 2 deletions tests/Unit/Crypt/AES/TestCase.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ public function continuousBufferCombos()
BlockCipher::MODE_CTR,
BlockCipher::MODE_OFB,
BlockCipher::MODE_CFB,
BlockCipher::MODE_IGE,
);
$plaintexts = array(
'',
Expand All @@ -61,6 +62,7 @@ public function continuousBufferCombos()
foreach ($modes as $mode) {
foreach ($plaintexts as $plaintext) {
foreach ($ivs as $iv) {
if ($mode === BlockCipher::MODE_IGE) $iv .= strrev($iv);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suppose it doesn't matter as much in a unit test but, technically, this is a violation of the PSR-2 coding standards. That should read more like this:

if ($mode === BlockCipher::MODE_IGE) {
    $iv.= strrev($iv);
}

foreach ($keys as $key) {
$result[] = array($mode, $plaintext, $iv, $key);
}
Expand Down Expand Up @@ -139,6 +141,7 @@ public function continuousBufferBatteryCombos()
BlockCipher::MODE_CTR,
BlockCipher::MODE_OFB,
BlockCipher::MODE_CFB,
BlockCipher::MODE_IGE,
);

$combos = array(
Expand Down Expand Up @@ -176,7 +179,7 @@ public function continuousBufferBatteryCombos()
*/
public function testContinuousBufferBattery($op, $mode, $test)
{
$iv = str_repeat('x', 16);
$iv = str_repeat('x', 16*($mode === BlockCipher::MODE_IGE ? 2 : 1));
$key = str_repeat('a', 16);

$aes = new AES($mode);
Expand Down Expand Up @@ -227,7 +230,7 @@ public function testNonContinuousBufferBattery($op, $mode, $test)
return;
}

$iv = str_repeat('x', 16);
$iv = str_repeat('x', 16*($mode === BlockCipher::MODE_IGE ? 2 : 1));
$key = str_repeat('a', 16);

$aes = new AES($mode);
Expand Down