When is padding required for encryption?

Question

I asked a question here why AES java decryption return extra characters? about getting extra characters when I decrypt the encrypted data. Thanks to a comment by user "Ebbe M. Pedersen" I now understand that the problem is not using the same padding mechanism in both the PHP and Android Java code. So I changed the Java code to

Java code

 public class encryption {

    private String iv = "fedcba9876543210";//Dummy iv (CHANGE IT!)
    private IvParameterSpec ivspec;
    private SecretKeySpec keyspec;
    private Cipher cipher;

    private String SecretKey = "0123456789abcdef";//Dummy secretKey (CHANGE IT!)

    public encryption()
    {
        ivspec = new IvParameterSpec(iv.getBytes());

        keyspec = new SecretKeySpec(SecretKey.getBytes(), "AES");

        try
        {
            cipher = Cipher.getInstance("AES/CBC/PKCS5PADDING");//"AES/CBC/NoPadding"
        }
        catch (NoSuchAlgorithmException e)
        {
            // TODO Auto-generated catch block
            e.printStackTrace();
        }
        catch (NoSuchPaddingException e) {
            // TODO Auto-generated catch block
            e.printStackTrace();
        }
    }

    public byte[] encrypt(String text) throws Exception
    {
        if(text == null || text.length() == 0)
            throw new Exception("Empty string");

        byte[] encrypted = null;

        try {
            cipher.init(Cipher.ENCRYPT_MODE, keyspec, ivspec);

            encrypted = cipher.doFinal(padString(text).getBytes());
        } catch (Exception e)
        {
            throw new Exception("[encrypt] " + e.getMessage());
        }

        return encrypted;
    }

    public byte[] decrypt(String code) throws Exception
    {
        if(code == null || code.length() == 0)
            throw new Exception("Empty string");

        byte[] decrypted = null;

        try {
            cipher.init(Cipher.DECRYPT_MODE, keyspec, ivspec);

            decrypted = cipher.doFinal(hexToBytes(code));
        } catch (Exception e)
        {
            throw new Exception("[decrypt] " + e.getMessage());
        }
        return decrypted;
    }



    public static String bytesToHex(byte[] data)
    {
        if (data==null)
        {
            return null;
        }

        int len = data.length;
        String str = "";
        for (int i=0; i<len; i++) {
            if ((data[i]&0xFF)<16)
                str = str + "0" + java.lang.Integer.toHexString(data[i]&0xFF);
            else
                str = str + java.lang.Integer.toHexString(data[i]&0xFF);
        }
        return str;
    }


    public static byte[] hexToBytes(String str) {
        if (str==null) {
            return null;
        } else if (str.length() < 2) {
            return null;
        } else {
            int len = str.length() / 2;
            byte[] buffer = new byte[len];
            for (int i=0; i<len; i++) {
                buffer[i] = (byte) Integer.parseInt(str.substring(i*2,i*2+2),16);
            }
            return buffer;
        }
    }



    private static String padString(String source)
    {
        char paddingChar = ' ';
        int size = 16;
        int x = source.length() % size;
        int padLength = size - x;

        for (int i = 0; i < padLength; i++)
        {
            source += paddingChar;
        }

        return source;
    }
}

Then I added the same PKCS5padding functions to my PHP mcrypt class:

PHP mcrypt class

class MCrypt
{
    private $iv = 'fedcba9876543210'; #Same as in JAVA
    private $key = '0123456789abcdef'; #Same as in JAVA


function MCrypt()
{
}

function encrypt($str) {

    //$key = $this->hex2bin($key);
    $iv = $this->iv;

    $td = mcrypt_module_open('rijndael-128', '', 'cbc', $iv);

    mcrypt_generic_init($td, $this->key, $iv);
    $encrypted = mcrypt_generic($td, $str);

    mcrypt_generic_deinit($td);
    mcrypt_module_close($td);

    return bin2hex($encrypted);
}

function decrypt($code) {
    //$key = $this->hex2bin($key);
    $code = $this->hex2bin($code);
    $iv = $this->iv;

    $td = mcrypt_module_open('rijndael-128', '', 'cbc', $iv);

    mcrypt_generic_init($td, $this->key, $iv);
    $decrypted = mdecrypt_generic($td, $code);

    mcrypt_generic_deinit($td);
    mcrypt_module_close($td);

    return utf8_encode(trim($decrypted));
}

protected function hex2bin($hexdata) {
    $bindata = '';

    for ($i = 0; $i < strlen($hexdata); $i += 2) {
        $bindata .= chr(hexdec(substr($hexdata, $i, 2)));
    }

    return $bindata;
}

function pkcs5_pad ($text, $blocksize)
{
    $pad = $blocksize - (strlen($text) % $blocksize);
    return $text . str_repeat(chr($pad), $pad);
}

function pkcs5_unpad($text)
{
    $pad = ord($text{strlen($text)-1});
    if ($pad > strlen($text)) return false;
    if (strspn($text, chr($pad), strlen($text) - $pad) != $pad) return false;
    return substr($text, 0, -1 * $pad);
}}

Now the current problem is sending/receiving UTF-8 characters, not how to decode/encode UTF-8 characters. When i send Arabic/Persian words which contain for example more than 3 or less than 3 characters it returns nothing. For example: If I send the word "خوب" (which has exactly 3 characters) I get "خوب" which is correct; but if I send مچکرم (which has 5 characters) I get nothing.

I found that the problem is that I was not using the unpadding function after decrypting the data in my php code, so I fixed that:

PHP code

<?php
$data =file_get_contents('php://input');
$block_size=mcrypt_get_block_size("rijndael-128",'cbc');
require_once "encryption.php";
$etool=new MCrypt();
$data =$etool->decrypt($data);
$data=$etool->pkcs5_unpad($data);//  <------ using unpad function
$data =json_decode($data, true);



$data=$data["request"];
$etool=new MCrypt();
$data=$etool->pkcs5_pad($data,$block_size);
$data=$etool->encrypt($data);
$array=array('data'=>$data);
echo  json_encode($array);

And here is the Java code to get it

JSONObject j=new JSONObject(sb.toString());//sb is string builder
result=j.get("data").toString();
result= new String(etool.decrypt( result ),"UTF-8");
result = new String(result.getBytes("ISO-8859-1"));
Log.d("success remote ",result);

Now the problem is reversed!! I can get words containing more than or less than 3 Persian/Arabic characters, but not words containing exactly 3 characters.

I think I should check "does unpadding required?" but how if so?

Answer 1

the problem is about not properly using un-padding function in php code. actually when encrypting data in java, some times because the text does not fit the block it use padding for it and if the text fits, it does not do so. in PHP code, if padding was used in java encryption process, un-padding is done correctly. but if text does not need padding in java encryption process, PHP un-padding function returns null and null was passed to $data variable. so i get nothing!!! by changing few lines of php code every think start working correctly.

The code that cause the problem:

$data=$etool->pkcs5_unpad($data);// in some cases null is retuned
$data =Json_decode($data,true);

the correct version of the code: issue is fixed.

$padding=$etool->pkcs5_unpad($data);
if($padding!="")
{
  $data=$padding;
}
$data =json_decode($data, true);

Answer 2

function pkcs5_pad ($text, $blocksize)
{
    $pad = $blocksize - (strlen($text) % $blocksize);
    return $text . str_repeat(chr($pad), $pad);
}

function pkcs5_unpad($text)
{
    $pad = ord($text{strlen($text)-1});
    if ($pad > strlen($text)) return false;
    if (strspn($text, chr($pad), strlen($text) - $pad) != $pad) return false;
    return substr($text, 0, -1 * $pad);
}

This code is using PHP's string functions, which operate on raw binary by default and ignores encoding.

Blending your decrypted messages through utf8_encode() doesn't actually make sense. That function remaps ISO-8559-1 codepoints (i.e. 0x00 through 0xFF) to UTF-8 codepoints (i.e. 0x00 through 0x7f, then 0xc280 through 0xc2bf and 0xc380 through 0xc3bf).

Since PHP operates on raw binary by default, you don't need to apply this transformation at all.

Note: I said by default. There's a very stupid feature in PHP called function overloading which is controlled by the mbstring.func_overload PHP.ini directive. If you're using this feature, you need to rewrite every piece of cryptography code that needs to measure and/or slice strings to not use strlen(), substr(), etc.

Defuse Security published and maintains a secure authenticated encryption PHP library, which contains replacements for these functions that resist function overloading.

---

Security Notice

First: Your crypto code is broken. In other words: NOT SECURE.

Second: Avoid ever using mcrypt for anything if you can help it.

If you just need your data to be encrypted over the wire, just use TLS. Trying to reinvent the wheel here will just lead to disaster.

However, if (for example) you need peer-to-peer encryption on top of TLS (e.g. so your server never sees the data), don't roll your own. Pick a secure PHP cryptography library instead. If you need one that works cross-platform, use libsodium.