Encrypting a user's file with a key that isn't stored but only known by the user

Question

I want to make some kind of file vault. I want to store encrypted files in a MySQL table, and the key to encrypt/decrypt them is never stored but only known to the user (that's the goal, anyway).

The user has to be logged in to view a list of his files. Then, to view a file, the user has to enter a key to decrypt a file to read it. So assuming a user's account was hacked and his password was discovered, I want the hacker to still be unable to decrypt the files, since the hacker doesn't know the key used to encrypt it.

What are the biggest holes in this? Please help me improve it rather than just saying 'this is terrible', 'use a library', or 'don't do it yourself'. I know the security of this depends on other code that I didn't put here. I just tried to put the most relevant parts.

Here is some code from the controller to add a new file:

public function addFile($request, $response) {

    $user = Security::getUser();
    if (!$user) { $this->denyAccess('add-file'); }

    $unencrypted_content = $request->getParam('content');
    $key = $request->getParam('key');
    if (Security::authenticate($user->username, $key)) {
        $this->flash->addMessage('error', 'You can not use your password as the key to your files.');
        return $this->redirect('new-file');
    }

    $otherFile = $user->files()->first();
    if ($otherFile && !Crypt::decrypt($otherFile->content, $key, $otherFile->iv)) {
        $this->flash->addMessage('error', 'The key you entered failed to decrypt another one of your files. You must always use the same key.');
        return $this->redirect('new-file');
    }

    $iv = Crypt::iv();
    $encrypted_content = Crypt::encrypt($unencrypted_content, $key, $iv);

    $file = new File;
    $file->owner()->associate($user);
    $file->name = $request->getParam('name');
    $file->content = $encrypted_content;
    $file->iv = $iv;
    $file->save();

    return $this->redirect('all-files');
}

Here is the Crypt class:

public static function iv() {
    $letters = 'abcdefghijklmnopqrstuvwxyz0123456789';
    $iv = '';

    for ($i = 0; $i < 16; $i++) {
        $iv .= substr($letters, rand(0, 35), 1);
    }

    return $iv;
}

public static function encrypt($content, $key, $iv) {
    return openssl_encrypt($content, 'aes-256-cbc', $key, 0, $iv);
}

public static function decrypt($content, $key, $iv) {
    return openssl_decrypt($content, 'aes-256-cbc', $key, 0, $iv);
}

And again in the controller to view a file:

public function viewFile($request, $response) {
    $csrf_name = $request->getAttribute('csrf_name');
    $csrf_value = $request->getAttribute('csrf_value');

    $file = File::find($request->getAttribute('id'));
    if ($file->owner->id !== Security::getUser()->id) { $this->denyAccess('view-file/' . $file->id); }

    $key = $request->getParam('key');
    $decrypted_content = Crypt::decrypt($file->content, $key, $file->iv);

    if (!$decrypted_content) {
        $this->flash->addMessage('error', 'The key you entered is not valid.');
        return $this->redirect('authorize', ['id' => $file->id]);
    }

    $file->content = $decrypted_content;
    $file->key = $key;

    return $this->render($response, 'file/view.html.twig', compact('file', 'csrf_value', 'csrf_name'));
}

Answer 1

I honestly disagree with this whole approach. First, storing files IN a relational database is rarely a good strategy, limited almost exclusively to use cases where you want to perform binary searches against the binary artifacts. That is not your use case here. Outside of that, you are just adding a lot of overhead to your typical database management operations (making DB backups/snapshots, replicating data across DB servers, etc.) without really any added value in having the files in the DB.

Your use case is simply one of needing to operate on the file in it's entirety, so I would think typical directory-based disk storage or cloud-based file storage mechanisms would make much more sense in terms of being able to operate against the collection of files as you typically would a collection of files. Your database could then only store reference to the files. I would have a strong bias towards using OS-based mechanisms for archiving, syncing, encrypting/decrypting, etc. PHP obviously can give you entry points to these sorts of operations.

I also VERY much question your encryption approach (and the approach for trying to roll your own logic). You are talking about what would seem to be an assymmetric cryptography use case here, but you are attempting to execute this using a symmetric encryption approach. This approach seems to not make sense, because, as soon as the end user passes their key to the application, the application can now decrypt the data, as it now has the shared symmetric encryption key.

In this case you then need to generate a shared key for each user. This would beg the question as to why the user even needs to pass the shared key, since the application should theoretically already store it. This means the application could decrypt the data without user interaction, which seems to not be what you desire.

If you truly want an assymmetric approach, then you probably need to forfeit the idea of the application performing ANY decryption at all. That should be up to the end user and/or a client-side application where this operation takes place. Outside of that, it seems you are defeating the entire purpose of what you are trying to do, which is storage of an encrypted file without the ability for the application to decrypt it, without the user having a key.