AES CTR mode using pycrypto

Question

I've implemented CTR mode by myself (only decryption for now), using only AES built-in functions from pycrypto. It means that I'm not supposed to use mode=AES.MODE_CTR. However, I know that using AES.MODE_CTR would be more simple, but I'm doing this as a learning experience.

Is it a safe AES CTR??

(non-parallel version)

from Crypto.Cipher import AES
from Crypto.Cipher import AES
ciphers = ["69dda8455c7dd4254bf353b773304eec0ec7702330098ce7f7520d1cbbb20fc3" + \
 "88d1b0adb5054dbd7370849dbf0b88d393f252e764f1f5f7ad97ef79d59ce29f5f51eeca32eabedd9afa9329", \
 "770b80259ec33beb2561358a9f2dc617e46218c0a53cbeca695ae45faa8952aa" + \
 "0e311bde9d4e01726d3184c34451"]
key = "36f18357be4dbd77f050515c73fcf9f2" 
class IVCounter(object):
 def __init__(self, value):
 self.value = value
 def increment(self):
 # Add the counter value to IV
 newIV = hex(int(self.value.encode('hex'), 16) + 1)
 # Cut the negligible part of the string
 self.value = newIV[2:len(newIV) - 1].decode('hex') # for not L strings remove $ - 1 $ 
 return self.value
 def __repr__(self):
 self.increment()
 return self.value
 def string(self):
 return self.value
class CTR():
 def __init__(self, k):
 self.key = k.decode('hex')
 def __strxor(self, a, b): # xor two strings of different lengths
 if len(a) > len(b):
 return "".join([chr(ord(x) ^ ord(y)) for (x, y) in zip(a[:len(b)], b)])
 else:
 return "".join([chr(ord(x) ^ ord(y)) for (x, y) in zip(a, b[:len(a)])])
 def __split_len(self, seq, lenght):
 return [seq[i:i+lenght] for i in range(0, len(seq), lenght)]
 def __AESencryptor(self, cipher):
 encryptor = AES.new(self.key, AES.MODE_ECB)
 return encryptor.encrypt(cipher)
 def decrypt(self, cipher):
 # Split the CT into blocks of 16 bytes
 blocks = self.__split_len(cipher.decode('hex'), 16)
 # Takes the initiator vector
 self.IV = IVCounter(blocks[0])
 blocks.remove(blocks[0]) 
 # Message block 
 msg = []
 # Decrypt
 for b in blocks:
 aes = self.__AESencryptor(self.IV.string())
 msg.append(self.__strxor(b, aes))
 self.IV.increment()
 return ''.join(msg)
def main():
 decryptor = CTR(key)
 for c in ciphers:
 print 'msg = ' + decryptor.decrypt(c)
if __name__ == '__main__':
 main()

This code was supposed to do the same that the code below:

import Crypto.Util.Counter
ctr_e = Crypto.Util.Counter.new(128, initial_value=long(IV.encode('hex'), 16))
decryptor = AES.new(key.decode('hex'), AES.MODE_CTR, counter=ctr_e)
print decryptor.decrypt(''.join(blocks))

Answer 1

I couldn't spot any problems with your implementations, but caveat lector: I'm not a cryptography expert, so take that with an appropriate pinch of salt.

(削除) I'm also concerned about the output:

$ python reinvent.py
msg = CTR mode lets yo▒B▒▒▒c▒?▒N▒▒w▒▒ap▒9▒uz▒▒▒3▒▒▒o▒V_M▒▒A
msg = Always avoid the▒?A▒▒r ▒ά

It looks as if the first block has been decrypted correctly, and then it all falls over. Probably a bug, but I didn't dig into what's causing it. (削除ここまで)

Edit: Turns out this was an indentation problem – the code as posted had lost some indentation, and I guessed wrong when I tried to add it back.

The biggest problem with this code is the lack of docstrings and comments. Especially when you're dealing with cryptography – it's important to explain why you wrote the code you did, and how it relates to the original problem. This makes code easier to read, review and maintain – and would probably make it much easier to spot the bug above.

A variety of comments below.

IVCounter class

• There should be a docstring explaining what this class represents, and what sort of values it expects as input. I had lots of (削除) fun (削除ここまで) hassle trying different inputs and hitting a variety of errors before I got it working.

• Your __repr__ method is mutating the internal state of the object – that seems like a really bad idea. The usual use case for repr() is as a debugging tool; changing the instance you're trying to debug will be a bundle of laughs.

  Here's an alternative repr you could use:

  def __repr__(self):
   return '%s(%r)' % (self.__class__.__name__, self.value)
  

  This returns a string that could be eval'd to get something equivalent to the current object, which is a much more conventional use of this function.

• I'm not sure why you have a string method. It would be better for callers to access self.value directly, and if you want a string representation of an object, you should define __str__ instead.

• What happens when the counter overflows? You get a nasty TypeError:

  >>> x = IVCounter('\xff\xff\xff\xff\xff\xff\xff\xff\xff\xfe')
  >>> x.increment()
  '\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff'
  >>> x.increment()
  Traceback (most recent call last):
   File "<stdin>", line 1, in <module>
   File "<stdin>", line 8, in increment
   File "/usr/lib/python2.7/encodings/hex_codec.py", line 42, in hex_decode
   output = binascii.a2b_hex(input)
  TypeError: Odd-length string
  

  You should think about how you want to handle an overflow like this – at the very least, you should wrap the exception and provide a more intelligible error message.

CTR class.

• Should subclass from object.
• Avoid single letter variable names where possible; try to prefer descriptive and expressive names. For example, __init__(self, key) instead of __init__(self, key). Or for block in blocks.
• In your __strxor method, you have repeated code – the only thing that changes is the arguments to zip(). Would be good to cut down that repetition.
• The variable name lenght is misspelt in the definition of the __split_len method.
• When I hear the word cipher, I think of things like AES, whereas you seem to be using it to describe a message text. That's a little confusing – docstrings would help.

• In the decrypt() method, rather than:

  self.IV = IVCounter(blocks[0])
  blocks.remove(blocks[0])
  

  you could just use:

  self.IV = IVCounter(blocks.pop(0))
  

  which will remove the item and extract it.

Misc

• You appear to be importing Crypto.Cipher.AES twice. Why?

Answer 2

The Pycrpto library has both encrpypt and decrypt functions for the class AES, but you have used the encrypt function for decryption:

For b in blocks:

aes = self.__AESencryptor(self.IV.string())"

It can be corrected by first defining an AES decryptor function as:

def __AESdecryptor(self, cipher):
 dec = AES.new(self.key, AES.MODE_ECB)
 return dec.decrypt(cipher)

and calling it inside your CTR decryptor.