Integrity check causes AES decryption to fail in Python 3

Question

I'm currently working on a simple encryption and decryption tool using pycryptodome and its AES cipher. But there is a problem when padding the plaintext to the size of a block.
Usually people do it by adding zeros. But what happens when the last byte of the plaintext is a zero as well? I played around with some other pad and unpad functions I found online, but nothing worked for me.

def _pad(self, s):
    rest = (AES.block_size - len(s)) % AES.block_size
    if not s:
        rest = AES.block_size
    return s + (rest * bytes([rest]))
def _unpad(self, s):
    return s[:-ord(s[len(s) - 1:])]

This was another function I saw quite a few times, but it does not work.

Encryption

def encrypt(self, plaintext, check_integrity=False):
    if check_integrity is True:
        plaintext += struct.pack('L', zlib.crc32(plaintext))

    plaintext = self._pad(plaintext)
    iv = Random.new().read(AES.block_size)
    cipher = AES.new(self.key, AES.MODE_CBC, iv)
    ciphertext = iv + cipher.encrypt(plaintext)
    return ciphertext

I'm using zlib.crc32() to create a checksum in order to check integrity after decryption. I just append it at the the end of the plaintext. Maybe it has something to do with this, I'm not sure.

Decryption

def decrypt(self, ciphertext, check_integrity=False):
    iv = ciphertext[:AES.block_size]
    cipher = AES.new(self.key, AES.MODE_CBC, iv)
    plaintext = cipher.decrypt(ciphertext[AES.block_size:])
    plaintext = self._unpad(plaintext)

    if check_integrity is True:
        crc, plaintext = (plaintext[-4:], plaintext[:-4])
        if not crc == struct.pack('L', zlib.crc32(plaintext)):
            return False
    return plaintext

After the ciphertext has been decrypted and unpadded the last 4 bytes should be equal to zlib.crc36(plaintext). But this is not the case. My guess is that the pad and/or unpad function is messing with some bytes, which causes the integrity check to fail.

Thank you for reading!

Added as answer by mistake: I get all kind of conversion errors (at line: `return s + (rest * bytes([rest]))`: `TypeError: Can't convert 'bytes' object to str implicitly)`, then, after I fixed it(them), I get `TypeError("Only byte strings can be passed to C code")`. _Python3.5_ on _Win_. — CristiFati, Sep 01 '17 at 01:19
You need to use byte-strings. You can either write a `b` in front of the string (`plaintext = b'my plaintext'`) or use the `encode()` function (`plaintext = plaintext.encode()`). — chrizator, Sep 01 '17 at 06:25

score 1 · Accepted Answer · answered Sep 01 '17 at 20:02

I'm not going to correct your code, instead I'm going to tell you that you are doing things the wrong way. This is a classic example of a bad crypto API causing problems for a developer. Crypto APIs should be doing the padding for you, not requiring you to implement it yourself.

The right way to do padding is use a standard like pkcs #7.

The right way to do integrity checking is with a mode of operation that does it for you (such as GCM), or using a cryptographic integrity check such as HMAC. The integrity checking should be on the cipher text, and it should be done before decryption. Otherwise, you open yourself up to padding oracle attacks. I'm willing to bet that your code is vulnerable to such attacks.

score 0 · Answer 2 · answered Sep 01 '17 at 19:17

I finally found the solution to my problem:

If the there is no padding required (rest = 0) the _unpad() method takes the plaintext and unpads it nevertheless. That is why I added:

if rest == 0:
    rest = AES.block_size

to the _pad() method.
This way everything works fine!

Integrity check causes AES decryption to fail in Python 3

2 Answers2