CODEWITHSUNDEEP
pythoncryptographyaespycrypto
chrizator
chrizator

Reputation: 123

Integrity check causes AES decryption to fail in Python 3


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!

Upvotes: 0

Views: 1378

Answers (2)

TheGreatContini
TheGreatContini

Reputation: 6629

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.

Upvotes: 1

chrizator
chrizator

Reputation: 123

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!

Upvotes: 0

Related Questions