5
\$\begingroup\$

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))
\$\endgroup\$
2
\$\begingroup\$

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?
\$\endgroup\$
0
\$\begingroup\$

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.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.