bcgit / bc-csharp

BouncyCastle.NET Cryptography Library (Mirror)
https://www.bouncycastle.org/csharp
MIT License
1.67k stars 556 forks source link

Serpent implementation output not match to MCrypt implementation #13

Closed pkorsukov closed 10 years ago

pkorsukov commented 10 years ago

Output from PHP Mcrypt cannot be decrypted with SerpentEngine. For Twofish or Rijndael conversion is working.

using System;
using System.Collections.Generic;
using System.Linq;
using System.Security.Cryptography;
using System.Text;
using System.Threading.Tasks;
using Org.BouncyCastle.Crypto;
using Org.BouncyCastle.Crypto.Engines;
using Org.BouncyCastle.Crypto.Modes;
using Org.BouncyCastle.Crypto.Paddings;
using Org.BouncyCastle.Crypto.Parameters;
using Org.BouncyCastle.Utilities.Encoders;

namespace ConsoleApplication3
{
    public class SERPENT
    {
        private string _key = "anhEWHBNQnRCSHdNYW5jS0w1aEQ=";

        private readonly Encoding _encoding;
        private readonly IBlockCipher _blockCipher;
        private PaddedBufferedBlockCipher _cipher;
        private IBlockCipherPadding _padding;

        public SERPENT()
        {
            //_blockCipher = new CfbBlockCipher(new RijndaelEngine(128), 128);
            _blockCipher = new CfbBlockCipher(new SerpentEngine(), 128);
            _encoding = Encoding.UTF8;
            _padding = new Pkcs7Padding();
        }

        public string Encrypt(string plain, string iv)
        {
            byte[] ivBytes = this.GetIV(iv);
            byte[] keyBytes = this.GetKey();
            byte[] result = BouncyCastleCrypto(true, _encoding.GetBytes(plain), keyBytes, ivBytes);
            return _encoding.GetString(Base64.Encode(result));
        }

        private byte[] GetKey()
        {
            string key = Encoding.UTF8.GetString(Base64.Decode(this._key)).Substring(0, 16);
            return Encoding.UTF8.GetBytes(key);
        }

        private byte[] GetIV(string iv)
        {
            string hash = MD5Hash(iv).Substring(0, 16);
            return Encoding.UTF8.GetBytes(hash);
        }

        public string Decrypt(string cipher, string iv)
        {
            byte[] ivBytes = this.GetIV(iv);
            byte[] keyBytes = this.GetKey();
            byte[] result = BouncyCastleCrypto(false, Base64.Decode(cipher), keyBytes, ivBytes);
            return _encoding.GetString(result);
        }

        private byte[] BouncyCastleCrypto(bool forEncrypt, byte[] input, byte[] key, byte[] iv)
        {
            try
            {
                _cipher = _padding == null ? new PaddedBufferedBlockCipher(_blockCipher) : new PaddedBufferedBlockCipher(_blockCipher, _padding);
                ParametersWithIV param = new ParametersWithIV(new KeyParameter(key), iv);
                _cipher.Init(forEncrypt, param);
                return _cipher.DoFinal(input);
            }
            catch (Org.BouncyCastle.Crypto.CryptoException ex)
            {
                throw ex;
            }
        }

        public string MD5Hash(string input)
        {
            // step 1, calculate MD5 hash from input
            MD5 md5 = System.Security.Cryptography.MD5.Create();
            byte[] inputBytes = System.Text.Encoding.UTF8.GetBytes(input);
            byte[] hash = md5.ComputeHash(inputBytes);

            // step 2, convert byte array to hex string
            StringBuilder sb = new StringBuilder();
            for (int i = 0; i < hash.Length; i++)
            {
                sb.Append(hash[i].ToString("x2"));
            }
            return sb.ToString();
        }
    }
}

PHP code:

<?php
class cryptor {
    private static $key = 'anhEWHBNQnRCSHdNYW5jS0w1aEQ=';

    public static function getKey(){
        $k = substr(base64_decode(cryptor::$key),0,16);
        return $k;
    }

    public static function getIv($iv){
        $k = substr(md5($iv),0,16);
        return $k;
    }

    public static function pkcs7padding($text){
        $block = mcrypt_get_block_size(cryptor::$cipher,cryptor::$mode);
        $padding = $block-(strlen($text)%$block);
        $text .= str_repeat(chr($padding), $padding);
        return $text;
    }

    public static function unpad($text){
        $dec_s = strlen($text); 
        $padding = ord($text[$dec_s-1]); 
        return substr($text, 0, -$padding);
        //return $text;
    }

    public static function encrypt($textArr,$iv) {
        $k = cryptor::getKey();
        return cryptor::encryptSerpent($textArr,$k,$iv);
    }

    public static function decrypt($encryptedArr,$iv) {
        $k = cryptor::getKey();
        return cryptor::decryptSerpent($encryptedArr,$k,$iv);
    }

    private static $cipher = 'serpent';
    private static $mode = 'ncfb';
    public static function encryptSerpent($textArr,$key,$iv) {
        $res = [];
        $cipher = mcrypt_module_open(cryptor::$cipher,'',cryptor::$mode,'');
        foreach($textArr as $ikey => $text){
            if(is_array($text)){
                $res[$ikey] = [];
                foreach($text as $key2 => $value2){
                    if($key2 == 'encoded' || empty($value2)){
                        continue;
                    }
                    //$decrypted = mcrypt_encrypt(cryptor::$cipher,$key,cryptor::pkcs7padding($value2),cryptor::$mode,$iv);
                    mcrypt_generic_init($cipher, $key, $iv);
                    $decrypted = mcrypt_generic($cipher,cryptor::pkcs7padding($value2));
                    $res[$ikey][$key2] = base64_encode($decrypted);
                    mcrypt_generic_deinit($cipher);
                }
            }else{
                mcrypt_generic_init($cipher, $key, $iv);
                $decrypted = mcrypt_generic($cipher,cryptor::pkcs7padding($text));
                //$decrypted = mcrypt_encrypt(cryptor::$cipher,$key,cryptor::pkcs7padding($text),cryptor::$mode,$iv);
                $res[$ikey] = base64_encode($decrypted);
                mcrypt_generic_deinit($cipher);
            }
        }
        mcrypt_module_close($cipher);
        return $res;
    }

    public static function decryptSerpent($encryptedArr,$key,$iv){
        $res = [];
        $cipher = mcrypt_module_open(cryptor::$cipher,'',cryptor::$mode,'');
        foreach($encryptedArr as $ikey => $encrypted_text){
            if(is_array($encrypted_text)){
                $res[$ikey] = [];
                foreach($encrypted_text as $key2 => $value2){
                    if($key2 == 'encoded' || empty($value2)){
                        continue;
                    }
                    mcrypt_generic_init($cipher, $key, $iv);
                    $decrypted = mdecrypt_generic($cipher,base64_decode($value2));
                    //$decrypted = mcrypt_decrypt(cryptor::$cipher,$key,base64_decode($value2),cryptor::$mode,$iv);
                    $res[$ikey][$key2] = cryptor::unpad($decrypted);
                    mcrypt_generic_deinit($cipher);
                }
            }else{
                mcrypt_generic_init($cipher, $key, $iv);
                $decrypted = mdecrypt_generic($cipher,base64_decode($encrypted_text));
                //$decrypted = mcrypt_decrypt(cryptor::$cipher,$key,base64_decode($encrypted_text),cryptor::$mode,$iv);
                $res[$ikey] = cryptor::unpad($decrypted);
                mcrypt_generic_deinit($cipher);
            }
        }
        mcrypt_module_close($cipher);
        return $res;
    }
}
timw commented 10 years ago

In general if you can provided a test vector (plaintext, iv, key, ciphertext) that fails it's a lot easier to help with these kinds of issues.

It looks like the Serpent implementation in mcrypt (or the PHP binding of mcrypt) is broken.

Using serpent/ecb test vectors from the Serpent AES submission (cases I=1, I=2, I=3 from floppy4/ecb_vt.txt) the mcrypt Serpent fails all of them. BC Java (and I presume C# as it shares test vectors) passes all of them. The mcrypt Twofish and Rijndael implementations pass the same vectors as BC Java, so this seems isolated to the Serpent implementation.

The PHP snippet below demonstrates the failure on the serpent test cases (these are serpent/ecb, so no CFB mode or padding involved).

Tests were run using mcrypt: stable 2.5.8, php54-mcrypt: stable 5.4.26, and php54: stable 5.4.26 from Homebrew.

<?php
function crypt_test($algo, $mode, $key, $iv, $name, $plaintext, $expected) {
    $cipher = mcrypt_module_open($algo,'',$mode,'');
    mcrypt_generic_init($cipher, $key, $iv);
    $encrypted = mcrypt_generic($cipher, hex2bin($plaintext));

    printf("%s/%s %s:\n expected: %s\n   actual: %s\n", $algo, $mode, $name, bin2hex($encrypted), $expected);

    mcrypt_generic_deinit($cipher);
    mcrypt_module_close($cipher);
}

$key = hex2bin('00000000000000000000000000000000');
$iv = hex2bin('00000000000000000000000000000000');

crypt_test('serpent', 'ecb', $key, $iv, 'zeros', '00000000000000000000000000000000', 'e9ba668276b81896d093a9e67ab12036');
crypt_test('serpent', 'ecb', $key, $iv, 'I1', '80000000000000000000000000000000', '10b5ffb720b8cb9002a1142b0ba2e94a');
crypt_test('serpent', 'ecb', $key, $iv, 'I2', '40000000000000000000000000000000', '91a7847ef1cd87551b5b4bf6f8e96e2c');
crypt_test('serpent', 'ecb', $key, $iv, 'I3', '20000000000000000000000000000000', '5d32aece8383fb2ee22cb4a6061d1429');

?>
pkorsukov commented 10 years ago

Thank you. It seems to be a PHP problem. I have submitted bug for them https://bugs.php.net/bug.php?id=66949. You can close the ticket. Thanks!

drizzt commented 9 years ago

I have the same problem and it's a BouncyCastle bug! gnu-crypto, libgcrypt, mcrypt, etc, returns another value

dghgit commented 9 years ago

Then you need to advise gnu-crypto, libgcrypt, mcrypt, etc, returns another value to fix their implementations. Please see http://www.bouncycastle.org/jira/browse/BMA-52

Unless this is in relation to something new this is not a bug in Bouncy Castle.

dghgit commented 9 years ago

I have confirmed this is the usual problem - just to repeat what has already been said, the AES submission for Serpent is available at:

http://www.cl.cam.ac.uk/~rja14/Papers/serpent.tar.gz

Bouncy Castle's version of Serpent passes these, we also confirmed with the authors of Serpent in 2009 that the AES submission showed the test vectors in correct ordering.

drizzt commented 9 years ago

You are right. The other cryptographics libreries uses the NESSIE format. You are the only one I found using the correct (aka original) format. Since I need compatibity with pre-existant code I'll reverse the input and output arrays of byte.

Thank you for the explanation

bcgit commented 8 years ago

Okay, just to follow up on this one, it turns out that there was some sort of misunderstanding when we first approached the Serpent authors for clarification. Apparently we should be using the NESSIE vectors. Funnily this confusion is not rare, and the official name for the other version is Tnepres. We're trying to sort it out now.