AES CTR mode using pycrypto 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) &gt; 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))
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 &ndash; 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 &ndash; 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 &ndash; 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 &ndash; 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:

&gt;&gt;&gt; x = IVCounter('\xff\xff\xff\xff\xff\xff\xff\xff\xff\xfe')
&gt;&gt;&gt; x.increment()
'\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff'
&gt;&gt;&gt; x.increment()
Traceback (most recent call last):
  File "&lt;stdin&gt;", line 1, in &lt;module&gt;
  File "&lt;stdin&gt;", 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 &ndash; 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 &ndash; 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 &ndash; 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?