Python image encryption security analysis

Question

I'm currently seeking someone to review the source code for an image encryption program in Python 2.7. The code is complete and should run provided you have installed the correct packages. For this project I used Anaconda 2 as it had scipy which was convenient for its write access.

I've spent some time on cryptography stack exchange and they have specific rules preventing the analysis of cryptographic functions on that site. I'm well aware that my programming abilities are elementary, but I enjoy cryptography! I know that you're not supposed to roll your own cryptography, but that is why I'm here! I would like some security advice regarding the actual implementation. Like what aspects of my code would make it vulnerable to side channel attacks (ex differential power analysis)? Or is there information that is leaked and embedded in the actual photo? (Yikes!) Any help is appreciated!

About the code:

• I know that it's slow, but it's not an issue for me. Any comments that show how to increase the speed WITHOUT the addition of the ^ operator would be appreciated. I personally like my home brewed XOR function.

• The particular encryption style uses cryptographic block chaining. And no, for the love of all things intelligent, I'm not talking about bit coin. This is the particular mode of operation that I used, where by SHA-512 is initially seeded with a password of the users choice the SHA-512 output is recursively passed back into SHA-512 while simultaneously being stored in the actual key_string. This is repeated until the key_string reaches the length of the image to be encrypted, and is then is trimmed if necessary. The key trimming could be an issue from a cryptographic perspective so any comments on that would be appreciated. Everything is converted to bit_strings and XOR'd and then packed back into an image to be transferred.

The commented code is below! Rip it apart!

#Precooked packages
#Running Anaconda2 

from PIL import Image
import numpy as np
import hashlib
import scipy.misc

def L_P(SET,n):
    #Splits an iterable object into n len chunks
    to_return=[]
    j=0
    k=n
    while k<len(SET)+1:
        to_return.append(SET[j:k])
        j=k
        k+=n 
    return(to_return)

def xo(bit_string_1,bit_string_2):
    #This is the exclusive or function I used, I know there is
    #the ^ function that will execute the same for 0b and 0x
    #encoded values, but I don't things I can't "see".  
    xor_str=''
    for i in range(len(bit_string_1)):
        if bit_string_1[i]=='0' and bit_string_2[i]=='0':
            xor_str+='0'
        if bit_string_1[i]=='1' and bit_string_2[i]=='1':
            xor_str+='0'
        if bit_string_1[i]=='0' and bit_string_2[i]=='1':
            xor_str+='1'
        if bit_string_1[i]=='1' and bit_string_2[i]=='0':
            xor_str+='1'
    return(xor_str)

def bin_n_bit(dec,n):
    #Used for encoding int values to a binary string.
    return(str(format(dec,'0'+n+'b')))

def list_concat(list_of_lists):
    #As in the name will concatinated a list of lists
    to_return=[]
    for i in range(len(list_of_lists)):
        to_return+=list_of_lists[i]
    return(to_return)

def str_concat(list_of_strings):
    #As in name it will concatinated a list of strings to a single string
    to_return=''
    for i in range(len(list_of_strings)):
        to_return+=list_of_strings[i]
    return(to_return)

def array_to_list(a_array):
    #Function will take a nested numpy array and return and convert
    #each element of the numpy array to a  python list type
    to_return=[]
    for i in range(len(a_array)):
        to_return.append(a_array[i].tolist())
    return(to_return)

def image_mount(file_string_name):
    return(Image.open(file_string_name + '.png').convert('L'))

def itl(file_string_name):
    #From the file directory will convert image to list, black and white.
    return(list(np.array(Image.open(file_string_name + '.png').convert('L'))))

def lti(a_list):
    #In python shell will convert list to an image, black and white only.
    return(Image.fromarray(np.array(a_list)))

def image_dim(image_list):
    #Function will take and image and return the dimensions of the photo
    #in pixel locations.
    return(len(image_list),len(image_list[0]))

def int_list(a_image_str_name):
    #Function will take a .png image and convert the image to a set
    #of pixel integers.
    to_return=[]
    to_iter=list_concat(array_to_list(itl(a_image_str_name)))
    for i in range(len(to_iter)):
        to_return.append(to_iter[i])
    return(to_return)

def int_list_bin(int_list):
    #Function will take an int list and convert each list value to its
    #binary representaion.
    for i in range(len(int_list)):
        int_list[i]=bin_n_bit(int_list[i],'8')
    return(int_list)

def image_encryption(password_str,image_str_name):
    #Images must be png files types, type is a string without the .png handle
    #Password is any string, no length restrictions 
    key_to_build=''
    int_pic_list=int_list_bin(int_list(image_str_name))
    split_len=image_dim(array_to_list(itl(image_str_name)))[-1]
    while len(key_to_build) < (len(int_pic_list))*2:
        insert=hashlib.sha512(password_str.encode('utf-8')).hexdigest()
        #password_str.encode('utf-8')
        #MUST HAVE .encode('utf-8') can have silent failure FUCK!
        key_to_build+=insert
        password_str=insert
    to_xor=L_P(key_to_build,2)
    for i in range(len(int_pic_list)):
        int_pic_list[i]=int(xo(int_pic_list[i],bin_n_bit(int(to_xor[i],16),'8')),2)
    int_pic_list.append(split_len)
    image_list_split=L_P(int_pic_list,int_pic_list[-1])
    list_of_arrays=[]
    for i in range(len(image_list_split)):
        list_of_arrays.append(np.asarray(image_list_split[i]))
    array_main=np.asarray(list_of_arrays)
    scipy.misc.toimage(array_main).save(image_str_name + '_encryption' + '.png')

def image_decryption(password_str,image_str_name):
    #Images must be png files types, type is a string without the .png handle
    #Password is any string, no length restrictions 
    key_to_build=''
    encryption_list=int_list_bin(int_list(image_str_name))
    split_len=image_dim(array_to_list(itl(image_str_name)))[-1]
    while len(key_to_build) < len(encryption_list)*2:
        insert=hashlib.sha512(password_str.encode('utf-8')).hexdigest()
        #password_str.encode('utf-8')
        #MUST HAVE .encode('utf-8') can have silent failure FUCK!
        key_to_build+=insert
        password_str=insert
    to_xor=L_P(key_to_build,2)
    for i in range(len(encryption_list)):
        encryption_list[i]=int(xo(encryption_list[i],bin_n_bit(int(to_xor[i],16),'8')),2)
    encryption_list.append(split_len)
    image_list_split=L_P(encryption_list,encryption_list[-1])
    list_of_arrays=[]
    for i in range(len(image_list_split)):
        list_of_arrays.append(np.asarray(image_list_split[i]))
    array_main=np.asarray(list_of_arrays)
    scipy.misc.toimage(array_main).save(image_str_name + '_decryption' + '.png')

Answer 1

I'm going to ignore the security of your application, and focus mostly on how you can improve your code.

• XOr can be defined as !=. Rather than having multiple if statements, I'd just have two.

  if bit_string_1[i] in '01' and bit_string_2[i] in '01':
      xor_str += '1' if bit_string_1[i] != bit_string_2[i] else '0'
  

• You may want to use zip when iterating through multiple things at the same time. Such as in xo.

  for a, b in zip(bit_string_1, bit_string_2):
      # Do a != b check
  

• Adding a char to strings is only guaranteed to have \$O(n)\$, rather than \$O(1)\$, time. It's advised to build a list and use ''.join. Such as:

  def xo(bit_string_1, bit_string_2):
      return ''.join([
          '1' if a != b else '0'
          for a, b in zip(bit_string_1, bit_string_2)
          if a in '01' and b in '01'
      ])
  

• I'd suggest changing xo to assume that bit_string_1 and bit_string_2 are actually correct.

• I'd rather maintain ^ as apposed to another function if I were to take on your code. I highly suggest you change to using it.

• Rather than using format with string concatenation, you can use ''.format. Such as:

  def bin_n_bit(dec, n):
      return '{0:0{1}b}'.format(dec, n)
  

• str_concat can be str_concat = ''.join. I'd also suggest completely replacing this with ''.join.

• Don't use for i in range(len(a_array)). There are rare occasions that it's acceptable, but for the most part use for item in a_array.

• I'd rather use an array comprehension for array_to_list:

  def array_to_list(a_array):
      return [item.tolist() for item in a_array]
  

• All the type casting is slow. Just use np.array(Image.open(file_string_name + '.png').convert('L')), and perform operations on that array. Doing anything else is just work for the sake of work.

• Please use better function names, itl is cryptic and confusing, if seen in the wild.

---

I don't have the time to do a full re-write. But most of your code is not needed. I can only see a reason for itl, lti, image_encryption, and image_decryption. The other functions are conversions for the sake of xo. This is slow, hard to maintain, and unpythonic.

Answer 2

Code remarks

The actual encryption is hard to spot, but it's fairly simple: xor the plaintext with a key, where the key for each block is the sha512 hash of the previous key (the given password is used as IV).

I'm not sure why you chose to combine encryption with image loading, but it makes your encryption almost impossible to reuse in other contexts. Functions should (ideally) have a single responsibility: one function for encrypting binary data, another for loading an image and yet another for turning pixel data from an image into a binary form. These functions can be used together to encrypt an image, but the encryption function can also be used on its own, for other kinds of data.

There seems to be some confusion about strings and encodings. I'm not sure about Python, but in most statically typed languages you'll see a clear distinction between text (strings) and binary data (byte arrays). An encoding determines how they can be converted to each other.

There's also some code duplication: xor is commutative, so encryption and decryption are the same. There should be no need for two separate functions.

Finally, there's a lot of useless code: conversions between lists and Numpy arrays, list flattening, bit string conversions, a custom xor implementation, and so on. It's been a while since I used PIL, but it looks like getdata will give you an array of tuples, which can easily be flattened with a list comprehension ([value for tuple in data for value in tuple]), so you don't need Numpy at all. The custom xor implementation only has drawbacks: it's slower, took you more time to write and us more time to review, requires additional conversions, and having more code means higher maintenance costs.

Security

I'm not a cryptography expert, but what you've got here looks like a basic xor cypher, so it wasn't terribly difficult to find some (known) weaknesses.

For example, this Wikipedia article about stream cipher attacks talks about bit-flipping attacks. If an attacker knows part of the message, they can replace it with something else, without having to know the key. I've tried this - it works.

It also talks about a reused key attack: if a key is reused, xor'ing one cyphertext with another gives the same result as xor'ing both plaintexts. That makes it easier to perform statistical analysis. That's a bit too much effort for my taste, but I suspect it can be fairly effective for certain kinds of messages.

If the content of a whole block is known (perhaps an attacker can control part of the input, perhaps they know something about the format of the message, and so on), then the key for that block can easily be recovered. Since block keys depend only on the previous key, all following blocks can now be decrypted as well.

Related to the above point, if the plaintext of a block consists of only 0's, the cyphertext will be equal to the key for that block.