PKCS#7 padding problem

[SungjunKim1]

Hi, In pkcs # 7 padding, If the size of the raw text is divided by the block size, should the raw text be padded to the size of the block size? If you divide by block size, you are not padding. (in getPadding (), if size == 0, return QByteArray ())

[SungjunKim1]

In pkcs # 7 padding, We know that the value for padding is the size we need to handle padding. If the encrypted bytes are to be passed to another location (eg java) and decrypted, you must check the padding value and remove it by that value. If the raw text is divided by block size and not padded, an error will occur when unpadding the encrypted text is received. Is my idea wrong?

[bricke]

I am not sure I understand what operations you are doing to pass the encrypted buffer to Java.

Are you splitting up the encrypted buffer before passing it up?

[SungjunKim1]

In my case, I need to pass the encrypted bytes to the server and decrypt it on the server. When using the javax.crypto.cipher package on the server, the error "Given final block not properly padded. Such issues can arise if a bad key is used during decryption" occurred intermittently.

Also, if you look at the wiki for PKCS # 7 padding, there is the phrase "If the length of the original data is an integer multiple of the block size B, then an extra block of bytes with value is added". Does this mean that raw text should be padded by block size even though it is divided into block sizes?

[bricke]

Yes, that's how I interpret it. If your data size is a multiple then a padded block is added. I have to check if I do it, I thought I was doing it

[bricke]

I believe the issue is here:

QByteArray QAESEncryption::getPadding(int currSize, int alignment)
{
    int size = (alignment - currSize % alignment) % alignment;
     if (size == 0) return QByteArray();

That return doesn't take into consideration a perfect block for PKCS#7, in case that is the chosen padding style it should return a byte array of the size of the block.

I should look into it more, let me know if you can test it against other AES implementations, I would gladly accept any PR that solves this issue

[SungjunKim1]

Yes ! This is the first point I have pointed out!

If you divide by block size, you are not padding. (in getPadding (), if size == 0, return QByteArray ())

I am glad that my opinion has been delivered.

[hmoffatt]

I just discovered this problem also. The case of the perfect block size is not handled correctly.

[hmoffatt]

See https://en.wikipedia.org/wiki/Padding_(cryptography)#PKCS#5_and_PKCS#7

If the length of the original data is an integer multiple of the block size B, then an extra block of bytes with value B is added

[hmoffatt]

Here is my hack to the source to fix this. I didn't think about it too much so I won't submit a PR unless you want me to.

Index: Qt-AES/qaesencryption.cpp
===================================================================
--- Qt-AES/qaesencryption.cpp   (revision 19131)
+++ Qt-AES/qaesencryption.cpp   (revision 19132)
@@ -129,6 +129,8 @@
 QByteArray QAESEncryption::getPadding(int currSize, int alignment)
 {
     int size = (alignment - currSize % alignment) % alignment;
+    if (size == 0 && m_padding == Padding::PKCS7)
+        size = alignment;
     if (size == 0) return QByteArray();
     switch(m_padding)
     {

[bricke]

Please check out this PR #25

The branch fix_PKCS7_blocklen should fix your issue.

Let me know

[bricke]

Pr #25 was merged into master