1

Disclaimer: English isn't my mother tongue so feel free to ask if something isn't clear.

Hi there,

I have to encrypt files using AES as soon as they are uploaded on the server and send the key needed to decrypt them via mail to the client (not storing it anywhere server side). Files can be as big as 2GB and are deleted 7 days after their upload.

Here is what I'm using to encrypt/decrypt files :

function encrypt_file($source, $destination, $key) {
    $iv = md5("\x1B\x3C\x58".$key, true);
    $ivsize = openssl_cipher_iv_length('aes-256-cbc');
    $fp = fopen($destination, 'wb') or die("Could not open file for writing.");
    $handle = fopen($source, "rb");
    while (!feof($handle)) {
        $e = 0;
        $contents = fread($handle, 4 * 1024 * 1024);
        $ciphertext = openssl_encrypt($contents, 'aes-256-cbc', $key, OPENSSL_RAW_DATA, $iv);
        $iv = substr($ciphertext, -$ivsize);
        while (!fwrite($fp, $ciphertext)) {
            $e++;
            if ($e == 5) {
                die("Couldn't write to file.");
                break 2;
            }
        }
    }
    fclose($handle);
    fclose($fp);
}

function streamdecrypt_file($source, $key) {
    $iv = md5("\x1B\x3C\x58".$key, true);
    $ivsize = openssl_cipher_iv_length('aes-256-cbc');
    $handle = fopen($source, "rb");
    while (!feof($handle)) {
        $contents = fread($handle, 4 * 1024 * 1024);
        $raw = openssl_decrypt($contents, 'aes-256-cbc', $key, OPENSSL_RAW_DATA, $iv);
        $iv = substr($contents, -$ivsize);
        print $raw; // Printing because it's directly sent to the user to download
    }
    fclose($handle);
}

If you're wondering why 4 * 1024 * 1024 it's just that this is the buffer size with which I got the fastest encryptions. My implementation uses the schema proposed here https://stackoverflow.com/a/30742644/3857024

I also made those 2 little functions to encrypt a string to a file using a passphrase :

function encrypt_string($source, $destination, $passphrase) {
    $iv = md5("\x1B\x3C\x58".$passphrase, true);
    $key = md5("\x2D\xFC\xD8".$passphrase, true);
    $ciphertext = openssl_encrypt($source, 'aes-256-cbc', $key, OPENSSL_RAW_DATA, $iv);
    $fp = fopen($destination, 'wb') or die("Could not open file for writing.");
    fwrite($fp, $ciphertext) or die("Could not write to file.");
    fclose($fp);
}

function decrypt_string($source, $passphrase) {
    $iv = md5("\x1B\x3C\x58".$passphrase, true);
    $key = md5("\x2D\xFC\xD8".$passphrase, true);
    $contents = file_get_contents($source);
    return openssl_decrypt($contents, 'aes-256-cbc', $key, OPENSSL_RAW_DATA, $iv);
}

And here is what I'm finally doing when an upload is complete :

$skey = /* 32 chars random generated string [a-Z0-9]*/
$ukey = /* 10 chars random generated string [a-Z0-9]*/

encrypt_file($originalFile, $encryptedFile, $skey);
encrypt_string($skey, $encryptedKey, $ukey);

I then delete the original file and send a link containing the $ukey to the user via mail.

When they want to decrypt the file to download it, I first decrypt the file containing the $skey using the $ukey, checking if I end up with a 32-chars 256-bits long string made of [a-Z0-9]. If the $skey doesn't match the regexp, I know the $ukey is invalid, so I stop there.

I did this so that I wouldn't have to decrypt the file to check if the key was correct or not.

Now I hope that my questions fit in SO :

  • Am I doing it right ?
  • Is there anything that could/should be improved ?
  • It takes about 60s to encrypt a 2GB file, is that an "ok" result ?
  • Is it good enough ? The goal is to prevent an attacker gaining access to the server to also gain access to the users files already stored. I know he would then be able to modify the code and access the following uploads, but that should protect the files already stored right ? Am I doing too much for nothing ?

Thank you for your answers !

Community
  • 1
  • 1
Lynesth
  • 81
  • 6
  • Since this question is more of a code review than a specific question, I recommend moving it to [programmers.se](http://programmers.stackexchange.com/) – Matthew Herbst Mar 23 '16 at 00:46
  • You don't specify the system that is performing the encryption that takes 60s/4GB. An iPhone6s can AES encrypt 400MB/s, my laptop 150MB/s. – zaph Mar 23 '16 at 01:06
  • For an iv use random bytes. For password extension use [PBKDF2](https://en.wikipedia.org/wiki/PBKDF2) or equivalent, the derivation needs to be slower. – zaph Mar 23 '16 at 01:08
  • @MatthewHerbst when referring other sites, it is often helpful to point that [cross-posting is frowned upon](http://meta.stackexchange.com/tags/cross-posting/info) – gnat Mar 23 '16 at 08:24

1 Answers1

0

For an IV, use random bytes.

For password expansion, use PBKDF2 or equivalent; the derivation needs to be slower.

Restricting a key to the characters [a-Z0-9] reduces the 256 key to essentially 36 bytes. That is not very secure. You need at least 128-bits of key material.

You need a better method to authenticate the user's password.

Scott Arciszewski
  • 33,610
  • 16
  • 89
  • 206
zaph
  • 111,848
  • 21
  • 189
  • 228
  • Thank you for your answer though there's a part I don't understand. If I use a 32 characters long string pick from [a-Z0-9], doesn't that mean there's 36^32 possibilities, which, even if it's lower than 2^256 is still higher than 2^128 ? (I'm not really good at crypto or math so I'm pretty sure I'm wrong, but I can't see what's wrong) – Lynesth Mar 23 '16 at 05:36
  • Concerning PBKDF2, I don't have access to PHP5.5+, and I found many different implementation for lower versions, not sure which to pick. Do you have a recommandation by any chance ? Or can I use [this](https://github.com/ircmaxell/password_compat) ? – Lynesth Mar 23 '16 at 05:41
  • 1
    You're tasked with implementing a homegrown crypto protocol and they won't even let you use a secure version of PHP? WTF – Scott Arciszewski Mar 23 '16 at 10:55
  • ["Schneier's Law"](https://www.schneier.com/blog/archives/2011/04/schneiers_law.html): Anyone, from the most clueless amateur to the best cryptographer, can create an algorithm that he himself can't break. – zaph Mar 23 '16 at 12:00
  • @ScottArciszewski : I've just been told that they're migrating the project and I'll get to use PHP 5.6.17, good news. zaph, I didn't get your quote... I'm not saying I made something safe and unbreakable, I wouldn't otherwise be there asking about it... (did I get your comment the wrong way ?) – Lynesth Mar 23 '16 at 12:31
  • @Lynesth He was quoting a famous cryptographer, whose words of wisdom are worth repeating. Also, http://www.cryptofails.com/post/75204435608/write-crypto-code-dont-publish-it – Scott Arciszewski Mar 23 '16 at 13:36
  • Secure cryptography is more thn getting an encrypt function to work. It is virtually impossible to get 100% security so it is necessary to examine what level you need. Define the value of what you are protecting, the attacker, the company's reputation, etc. – zaph Mar 23 '16 at 13:45