7
\$\begingroup\$

I created a small program that hides arbitrary files in png images using steganography - it encodes the files in the two least significant bits of every channel, so using a 4-channel png we can encode one byte per pixel.

I also wanted to hide the presence of secret data by encrypting it, and, in effect, making it indistinguishable from random noise to make it impossible to prove there is any data at all hidden in the image without a valid key.

I would like to know if the method I'm using is safe and robust, and if there is anything I should have done better. I'm using a python library called cryptography to do the encryption. It would be also nice to know what attacks (if any) my code could be vulnerable to.

The whole code of my program can be found here.

Here are the functions that do the encryption and decryption. The idea is that the password string you pass to the program is turned into a large key using a key derivation function.

def encrypt(data, password, padding=0):
    """Encrypts data using the password.
    Encrypts the data using the provided password using the cryptography module.
    The password is converted into a base64-encoded key which is then used in a
    symmetric encryption algorithm.
    """

    if padding < 0:
        print "Image too small to encode the file. \
You can store 1 byte per pixel."
        exit()

    password = bytes(password)

    #Use key stretching to generate a secure key
    kdf = PBKDF2HMAC(
        algorithm=hashes.SHA256(),
        length=32,
        salt=bytes(password),
        iterations=100000,
        backend=default_backend())

    key = kdf.derive(bytes(password))

    nonce = os.urandom(16)

    cipher = Cipher(algorithms.AES(key),\
                    modes.CTR(nonce), backend=default_backend())
    enc = cipher.encryptor()
    ct = enc.update(data) + enc.finalize()

    #Add padding if needed
    ct += os.urandom(padding-16)

    #add nonce to data to allow decryption later (nonce does not need to be kept
    #secret and is indistinguishable from random noise)
return bytes(nonce) + ct

def decrypt(data, password):
    """Decrypts data using the password.
    Decrypts the data using the provided password using the cryptography module.
    If the pasword or data is incorrect this will return None. 
    """

    password = bytes(password)

    #Salt is equal to password as we want the encryption to be reversible only
    #using the password itself
    kdf = PBKDF2HMAC(algorithm=hashes.AES(),
                     length=32,
                     salt=bytes(password),
                     iterations=100000,
                     backend=default_backend())

    key = base64.urlsafe_b64encode(kdf.derive(password))
    f = Fernet(key)
    token = f.decrypt(data)
return token

I expect that those two functions are where vulnerabilities and/or otherwise faulty code could be. If anyone feels like this is not enough to determine the quality of the code, feel free to read the rest of the program (at the link at the top) and review it as well.

Cheers

\$\endgroup\$
  • \$\begingroup\$ You should test your program with PNGs having no alpha channel. Python currently throws an IndexError at a+(pixel[3]&252)) when I try it with ftp.netbsd.org/pub/pkgsrc/distfiles/LOCAL_PORTS/…. Furthermore, when encrypting your README.md and decrypting it again, it has four 00 bytes at the beginning, and the last four bytes are cut off. \$\endgroup\$ – Roland Illig Jul 31 '16 at 21:01
2
\$\begingroup\$

For cases like this:

if padding < 0:
        print "Image too small to encode the file. \
You can store 1 byte per pixel."
        exit()

It's probably better to define a custom exception and raise that:

class SizeError(Exception):
    pass

...

if padding < 0 :
    raise SizeError("Image too small to encode the file. \
    You can store 1 byte per pixel.")

I'm no security expert, but is it ok to use the string you want to encode as salt in kdf? This looks like security through obscurity (because it means you just double the input before encrypting).

\$\endgroup\$
  • \$\begingroup\$ The thing is I had no better ideas about how to store the salt inside the encoded file. Normally the salt should be random for every hash and stored separately, but i can't really store it anywhere. \$\endgroup\$ – nukeop Jul 31 '16 at 12:21
  • \$\begingroup\$ Yeah, I saw the comment in the decrypt function after I wrote that. But, according to this answerer here, at best a salt chosen like this (e.g. doubling the password) gives at best a factor 2 more entropy and even less if done systematically (i.e. either you do it or not, either an attacker checks for it or not). So you either keep it for that factor of 2 or just don't choose a salt? Or maybe use a semi-hardcoded salt, like an application key (which gets generated for every installation of your code)? \$\endgroup\$ – Graipher Jul 31 '16 at 12:42
  • \$\begingroup\$ Missing link in previous comment: security.stackexchange.com/questions/29083/… \$\endgroup\$ – Graipher Jul 31 '16 at 13:03

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.