Cryptography implementation for a web-based file server

Question

I'm not a dedicated cryptographer, so I'm looking for someone to look over these functions I wrote and let me know if there are any implementation errors leading to security vulnerabilities or just anything I should improve. Any other comments about improving performance, etc. would be great too.

This is used in a PHP-based file server I'm writing, where users can optionally encrypt their files individually through their web browser. These are the filesystem functions that are called in the middle of Ajax request.

I had to separate the decryption into 3 parts because it's used not only for statically decrypting files but also for doing it on the fly during a file download. So it's not always used the same way.

It's designed to be very modular so that ciphers and other settings can be changed readily. I also took a cautious approach to security, using cascading ciphers, a 512-bit salt, 4000-iteration whirlpool-based key derivation and multiple keys and initialization vectors per file, which also saves on memory usage.

public static function EncryptFile($owner, $id, $password) { // This requires PHP 5.5.0 or higher!
    global $mysqli, $settings;
    if (empty($owner) || empty($id) || !isset($password)) {
        return false;
    }
    $crypto_settings = unserialize($settings['crypto']);
    $secure = false;
    openssl_random_pseudo_bytes(1, $secure);
    if (!$secure) {
        return false;
    }
    if (!($file = mysqli_fetch_assoc(mysqli_query($mysqli, 'SELECT * FROM content_files WHERE owner="' . $owner . '" AND id="' . $id . '"')))) {
        return false;
    }
    if ($file['crypto_binary'] || $file['crypto_settings']) {
        return false;
    }
    $chunks = ceil($file['size'] / $crypto_settings['chunk_size']);
    $keys = array();
    $vectors = array();
    $keys_string = "";
    $vectors_string = "";
    for ($i = 0; $i < $chunks; $i++) {
        $keys[$i] = array();
        for ($n = 0; $n < count($crypto_settings['ciphers']); $n++) {
            $key = openssl_random_pseudo_bytes($crypto_settings['key_lengths'][$n] / 8);
            $keys[$i][$n] = $key;
            $keys_string. = $key;
        }
        $vectors[$i] = array();
        for ($n = 0; $n < count($crypto_settings['ciphers']); $n++) {
            $vector = openssl_random_pseudo_bytes($crypto_settings['vector_lengths'][$n] / 8);
            $vectors[$i][$n] = $vector;
            $vectors_string. = $vector;
        }
    }
    $salt = openssl_random_pseudo_bytes($crypto_settings['salt_length'] / 8);
    $master_key_length = 0;
    for ($i = 0; $i < count($crypto_settings['ciphers']); $i++) {
        $master_key_length += ($crypto_settings['key_lengths'][$i] + $crypto_settings['vector_lengths'][$i]) / 8;
    }
    $master_key = openssl_pbkdf2($password, $salt, $master_key_length, $crypto_settings['pbkdf2_iterations'], $crypto_settings['pbkdf2_algorithm']);
    $master_key_used = 0;
    $crypto_binary = "TRUE".$keys_string.$vectors_string;
    for ($i = 0; $i < count($crypto_settings['ciphers']); $i++) {
        $crypto_binary = openssl_encrypt($crypto_binary, $crypto_settings['ciphers'][$i], substr($master_key, $master_key_used, $crypto_settings['key_lengths'][$i] / 8), OPENSSL_RAW_DATA, substr($master_key, $master_key_used + $crypto_settings['key_lengths'][$i] / 8, $crypto_settings['vector_lengths'][$i] / 8));
        $master_key_used += ($crypto_settings['key_lengths'][$i] + $crypto_settings['vector_lengths'][$i]) / 8;
    }
    $crypto_binary = $salt.$crypto_binary;
    if (!mysqli_query($mysqli, 'UPDATE content_files SET crypto_binary="'.mysqli_real_escape_string($mysqli, $crypto_binary).
        '", crypto_settings="'.mysqli_real_escape_string($mysqli, serialize($crypto_settings)).
        '" WHERE id="'.$id.
        '" AND owner="'.$owner.
        '"')) {
        return false;
    }
    for ($chunk = 0; $chunk < $chunks; $chunk++) {
        $file = fopen("content/$id", 'rb+');
        if (!$file) {
            return false;
        }
        fseek($file, $chunk * $crypto_settings['chunk_size']);
        $data = fread($file, $crypto_settings['chunk_size']);
        for ($i = 0; $i < count($crypto_settings['ciphers']); $i++) {
            $data = openssl_encrypt($data, $crypto_settings['ciphers'][$i], $keys[$chunk][$i], OPENSSL_RAW_DATA, $vectors[$chunk][$i]);
        }
        fseek($file, $chunk * $crypto_settings['chunk_size']);
        fwrite($file, $data);
        fclose($file);
    }
    return true;
}
public static function DecryptHeader($crypto_settings, $crypto_binary, $filesize, $password) {
    $chunks = ceil($filesize / $crypto_settings['chunk_size']);
    $salt = substr($crypto_binary, 0, $crypto_settings['salt_length'] / 8);
    $header = substr($crypto_binary, $crypto_settings['salt_length'] / 8);
    $master_key_length = 0;
    for ($i = 0; $i < count($crypto_settings['ciphers']); $i++) {
        $master_key_length += ($crypto_settings['key_lengths'][$i] + $crypto_settings['vector_lengths'][$i]) / 8;
    }
    $master_key = openssl_pbkdf2($password, $salt, $master_key_length, $crypto_settings['pbkdf2_iterations'], $crypto_settings['pbkdf2_algorithm']);
    $master_key_used = 0;
    for ($i = count($crypto_settings['ciphers']) - 1; $i >= 0; $i--) {
        $header = openssl_decrypt($header, $crypto_settings['ciphers'][$i], substr($master_key, -1 * $master_key_used - ($crypto_settings['key_lengths'][$i] + $crypto_settings['vector_lengths'][$i]) / 8, $crypto_settings['key_lengths'][$i] / 8), OPENSSL_RAW_DATA, substr($master_key, -1 * $master_key_used - ($crypto_settings['vector_lengths'][$i] / 8), $crypto_settings['vector_lengths'][$i] / 8));
        $master_key_used += ($crypto_settings['key_lengths'][$i] + $crypto_settings['vector_lengths'][$i]) / 8;
    }
    if (substr($header, 0, 4) != "TRUE") {
        return false;
    } else {
        $header = substr($header, 4);
    }
    $keys = array();
    $vectors = array();
    $header_position = 0;
    for ($c = 0; $c < $chunks; $c++) {
        $keys[$c] = array();
        for ($n = 0; $n < count($crypto_settings['ciphers']); $n++) {
            $keys[$c][$n] = substr($header, $header_position, $crypto_settings['key_lengths'][$n] / 8);
            $header_position += $crypto_settings['key_lengths'][$n] / 8;
        }
    }
    for ($c = 0; $c < $chunks; $c++) {
        $vectors[$c] = array();
        for ($n = 0; $n < count($crypto_settings['ciphers']); $n++) {
            $vectors[$c][$n] = substr($header, $header_position, $crypto_settings['vector_lengths'][$n] / 8);
            $header_position += $crypto_settings['vector_lengths'][$n] / 8;
        }
    }
    return array('keys' = > $keys, 'vectors' = > $vectors);
}
public static function DecryptChunk($file, $crypto_settings, $header, $chunk) {
    fseek($file, $chunk * $crypto_settings['chunk_size']);
    $data = fread($file, $crypto_settings['chunk_size']);
    for ($i = count($crypto_settings['ciphers']) - 1; $i >= 0; $i--) {
        $data = openssl_decrypt($data, $crypto_settings['ciphers'][$i], $header['keys'][$chunk][$i], OPENSSL_RAW_DATA, $header['vectors'][$chunk][$i]);
    }
    fseek($file, $chunk * $crypto_settings['chunk_size']);
    return $data;
}
public static function DecryptFile($owner, $id, $password) { // This requires PHP 5.5.0 or higher!
    global $mysqli;
    if (empty($owner) || empty($id) || !isset($password)) {
        return false;
    }
    if (!($file = mysqli_fetch_assoc(mysqli_query($mysqli, 'SELECT size, crypto_settings, crypto_binary FROM content_files WHERE owner="'.$owner.
        '" AND id="'.$id.
        '"')))) {
        return false;
    }
    if (!$file['crypto_settings'] || !$file['crypto_binary']) {
        return false;
    }
    $crypto_settings = unserialize($file['crypto_settings']);
    $crypto_binary = $file['crypto_binary'];
    $header = Filesystem::DecryptHeader($crypto_settings, $crypto_binary, $file['size'], $password);
    if (!$header) {
        return false;
    }
    for ($chunk = 0; $chunk < count($header['keys']); $chunk++) {
        $handle = fopen("content/$id", 'rb+');
        if (!$handle) {
            return false;
        }
        $data = Filesystem::DecryptChunk($handle, $crypto_settings, $header, $chunk);
        fwrite($handle, $data);
        fclose($handle);
    }
    if (!mysqli_query($mysqli, 'UPDATE content_files SET crypto_binary="", crypto_settings="" WHERE id="'.$id.
        '" AND owner="'.$owner.
        '"')) {
        return false;
    }
    return true;
}

Here's what I currently have in the DB as the starting cipher settings:

a:7:{s:7:"ciphers";a:2:{i:0;s:16:"CAMELLIA-256-CFB";i:1;s:11:"AES-256-CFB";}s:11:"key_lengths";a:2:{i:0;i:256;i:1;i:256;}s:14:"vector_lengths";a:2:{i:0;i:128;i:1;i:128;}s:11:"salt_length";i:512;s:16:"pbkdf2_algorithm";s:9:"whirlpool";s:17:"pbkdf2_iterations";i:3996;s:10:"chunk_size";i:20971520;}

Answer 1

1. fwrite returns FALSE on error. You should handle that to avoid dangerous silent data corruption.

2. If you're using PHP 5.5 I guess you could use exceptions with detailed error messages instead of false return values. Getting different error messages for different type of errors makes development easier since it requires less debugging, developers don't have to dig into the called function to figure out which return false they got and what was its cause.

3. public static function EncryptFile($owner, $id, $password) { // This requires PHP 5.5.0 or higher!
   

   You could put the comment a line before the function to avoid horizontal scrolling:

   // This requires PHP 5.5.0 or higher!
   public static function EncryptFile($owner, $id, $password) { 
   

   On the other hand you could automatize that with version_compare to avoid debugging and support calls by users with older PHP versions.

4. Lines are too long, horizontal scrolling makes the code hard to read:

   for ($i = 0; $i < count($crypto_settings['ciphers']); $i++) {
       $crypto_binary = openssl_encrypt($crypto_binary, $crypto_settings['ciphers'][$i], substr($master_key, $master_key_used, $crypto_settings['key_lengths'][$i] / 8), OPENSSL_RAW_DATA, substr($master_key, $master_key_used + $crypto_settings['key_lengths'][$i] / 8, $crypto_settings['vector_lengths'][$i] / 8));
       $master_key_used += ($crypto_settings['key_lengths'][$i] + $crypto_settings['vector_lengths'][$i]) / 8;
   }
   

   You could create some explanatory variables here:

   for ($i = 0; $i < count($crypto_settings['ciphers']); $i++) {
       $method = $crypto_settings['ciphers'][$i];
       $password = substr($master_key, $master_key_used, $crypto_settings['key_lengths'][$i] / 8);
       $options = OPENSSL_RAW_DATA;
       $iv = substr($master_key, $master_key_used + $crypto_settings['key_lengths'][$i] / 8, $crypto_settings['vector_lengths'][$i] / 8);
   
       $crypto_binary = openssl_encrypt($crypto_binary, $method, $password, $options, $iv);
       $master_key_used += ($crypto_settings['key_lengths'][$i] + $crypto_settings['vector_lengths'][$i]) / 8;
   }
   

   This does not force readers to check the documentation of openssl_encrypt to understand its parameters and expresses the intent of the original author. If he mixed up two parameters it is easy to spot. ($crypto_settings['key_lengths'][$i] / 8 also deserves a local variable here, it's used twice.)

   (Clean Code by Robert C. Martin, G19: Use Explanatory Variables; Refactoring: Improving the Design of Existing Code by Martin Fowler, Introduce Explaining Variable)

5. The following loop is duplicated:

   for ($i = 0; $i < count($crypto_settings['ciphers']); $i++) {
       $master_key_length += ($crypto_settings['key_lengths'][$i] + $crypto_settings['vector_lengths'][$i]) / 8;
   }
   

   You should create a separate function for that to reduce the duplicated maintenance costs.

6. 4 is a magic number here:

   if (substr($header, 0, 4) != "TRUE") {
       return false;
   } else {
       $header = substr($header, 4);
   }
   

   Usually you should crate a named constant for it which expresses the intent but here I'd create a startsWith function for better readability:

   $HEADER_PREFIX = "TRUE";
   if (!startsWith($header, $HEADER_PREFIX)) {
       return false;
   }
   $header = substr($header, strlen($HEADER_PREFIX));
   

   Note the guard clause.