1
\$\begingroup\$

Ahead of the General Data Protection Regulation (GDPR) I begin to crypt more than ever before, in fact, all I can crypt become crypt.

However, I wonder if anybody can tell me if this code is secure enough or can make it even better, optimize or just have some tips about the code. and not sure about the OpenSSL are secure anymore so if any could give me some feedback about it I will love you forever.

The $keyis defined in the config.php and are a base64 encrypt and look like this e.g bRuD5WYw5wd0rdHR9yLlM6wt2vteuiniQBqE70nAuhU

class myCrypt {

    public function my_encrypt($data, $key) {
        // Remove the base64 encoding from our key
        $encryption_key = base64_decode($key);
        // Generate an initialization vector
        $iv = openssl_random_pseudo_bytes(openssl_cipher_iv_length('aes-256-cbc'));
        // Encrypt the data using AES 256 encryption in CBC mode using our encryption key and initialization vector.
        $encrypted = openssl_encrypt($data, 'aes-256-cbc', $encryption_key, 0, $iv);
        // The $iv is just as important as the key for decrypting, so save it with our encrypted data using a unique separator (::)
        return base64_encode($encrypted . '::' . $iv);
    }

    public function my_decrypt($data, $key) {
        // Remove the base64 encoding from our key
        $encryption_key = base64_decode($key);
        // To decrypt, split the encrypted data from our IV - our unique separator used was "::"
        list($encrypted_data, $iv) = explode('::', base64_decode($data), 2);
        return openssl_decrypt($encrypted_data, 'aes-256-cbc', $encryption_key, 0, $iv);
    }   
}

I like the fact that the same encrypt is never the same, but if a hacker hacks the code will he/she just use the decrypt method to decrypt the data??

PS: This is not for passwords

\$\endgroup\$
2
\$\begingroup\$

I guess my first suggestion is that, unless you really understand cryptography, you really should not be writing your own encryption libraries. Use an existing one like libsodium (or something based on it) , or, if you can't install the libsodium extension in your environment, perhaps the defuse/php-encryption library which is well-regarded.

Now on to your specific code...

I would start by not having your $key in your codebase at all. Any sensitive information - passwords, encryption keys, etc. should never be placed in your code base where these items can be visible in your revision control systems. Ideally, you can derive this from environmental configuration and inject the secure material during environmental deployment.

Stylistically, you should consider starting all PHP class names with uppercase letter (which is pretty much de facto standard) and stay away from mixing camelCase and snake_case in your code libraries as you are doing here with camelCase class name and snake_case method and variable names. I know PHP is not good about this within it's built-in classes and functions, but that doesn't mean you shouldn't be in your own code.

From a use standpoint, is there really reason for a class like this to be instantiated, or should these be static methods?

\$\endgroup\$
  • \$\begingroup\$ Just wondering if you say that variables shouldn't be named with _ and case then all lowercase? And yes I'm working on to learn the PR standard for coding PHP. A other thing is what to do with the key then? As I know you should always have a key to the crypt \$\endgroup\$ – TheCrazyProfessor Apr 9 '17 at 17:20
  • \$\begingroup\$ @TheCrazyProfessor Oftentimes, one might have some sort of environmental setup process whereby a key will be placed at some location on the server, and the application would only know the path to the key file. \$\endgroup\$ – Mike Brant Apr 9 '17 at 19:49
  • \$\begingroup\$ @TheCrazyProfessor The second (unaccepted) answer at this link also gives some good suggestions for key deployment - stackoverflow.com/questions/38319765/… \$\endgroup\$ – Mike Brant Apr 9 '17 at 19:59
  • \$\begingroup\$ how will you store encrypted data in the database there become decrypted when the user with the permission to see there own data login with out using a key in your code?? beside mcrypt are removed from PHP 7.2 \$\endgroup\$ – TheCrazyProfessor Apr 10 '17 at 16:44

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.