# Is the random data at the end of my decrypted message supposed to be there?

This is my PKCS#5 & AES/ECB Python 3.3 code:

from Crypto.Cipher import AES

def encrypt_snap(media):
key = bytes('M02cnQ51Ji97vwT4', 'utf8')
cipher = AES.new(key, AES.MODE_ECB)
padded = media + ((16 - len(media) % 16) * chr(16 - len(media) % 16))
return encrypted

def decrypt_snap(media):
key = bytes('M02cnQ51Ji97vwT4', 'utf8')
cipher = AES.new(key, AES.MODE_ECB)
padded = media + ((16 - len(media) % 16) * chr(16 - len(media) % 16)).encode('utf8')
return decrypted

msg = 'hello world! :)'
print(msg)
encrypted = encrypt_snap(msg)
print(encrypted)
decrypted = decrypt_snap(encrypted)
print(decrypted)


This is what it returns:

hello world! :)
b'M\xd0g\xca\xa2x\x91\xf5\xa5\x9ao\x1d\x8ev^@'
b'hello world! :)\x01\x02\xf9K\x95\x01\xb9\xdaG\x03D\x94\x88\x9bi)w'


Should that "random" (?) data be at the end of the decrypted message? Is that all part of the padding?

Please let me know if I've done anything wrong. Thanks! :)

-
I don't know Python but here's my two cents: Comment out the padded=... lines from your code. Does the issue persist? Plus I think the question is better suited for CodeReview.SE or SO. Cheers –  rath Feb 8 at 22:32
Actually, it appears that, during the decryption process, you're adding padding and then decrypting the padded message. Yes, that decrypted padding would look like gibberish... –  poncho Feb 9 at 0:32

As poncho said in his comment, you added padding before decryption as well, which is not correct. AES encryption and decryption are both permutations, so if you decrypt data with a key, it will "look" random (at least, if AES is secure).

from Crypto.Cipher import AES

def encrypt_snap(media):
key = bytes('M02cnQ51Ji97vwT4', 'utf8')
cipher = AES.new(key, AES.MODE_ECB)
padded = media + ((16 - len(media) % 16) * chr(16 - len(media) % 16))
return encrypted

def decrypt_snap(media):
key = bytes('M02cnQ51Ji97vwT4', 'utf8')
cipher = AES.new(key, AES.MODE_ECB)
decrypted = cipher.decrypt(media)

msg = 'hello world! :) how are you?'
print(msg)
encrypted = encrypt_snap(msg)
print(encrypted)
decrypted = decrypt_snap(encrypted)
print(decrypted)


Most notable are the lines:

padding_bytes = decrypted[-1]


In your padding scheme, the last byte of the message represents how many bytes of padding there are in total. So, we strip off that number of characters from the byte array. I also changed the message to be a little longer to make sure it worked correctly.

The above is still not perfect, however: it does not check to see if the padding itself is valid. For example, you might desire to iterate over the stripped bytes and make sure they are monotonically increasing starting from 1.

As a final warning, hopefully the above code is for learning purposes only. If you're actually trying to write secure code, you need to invest in a different mode of operation (e.g. CBC instead of ECB) as well as some authentication (e.g. HMAC). If it is for learning purposes, my apologies; I felt compelled to include this notice just in case.

-
Notice that at least in languages without built-in exception mechanism on index out of range (and arguably all languages), the value of padding_bytes should be checked to be in range [1..16] before use as decrypted[:-padding_bytes]. Same before peeking in decrypted to get padding_bytes as decrypted[-1] (also, both of these different syntaxes are alien to me, even if I get the intend from context). –  fgrieu Feb 10 at 6:55