Best practice for a simple encryption class. Am I using crypt and mcrypt well?

Question

These days I read a lot here on SO about password hashing and data encryption. It's a real mess, I mean, deciding what the best practice is. I need a very simple class that can be reused anywhere and that provide a decent-but-not-paranoic security level for my PHP applications (I do not handle bank data). Additionally, I want to rely as much as possible on PHP standard libs. I came up with this:

class Security {

    public static function hashPassword($plain) {
        $salt = md5(rand(0, 1023) . '@' . time()); // Random salt
        return crypt($plain, '$2a$07$' . $salt); // '$2a$07$' is the Blowfish trigger
    }

    public static function checkPassword($plain, $hash) {
        return (crypt($plain, $hash) === $hash);
    }

    public static function generateIv() {
        $iv_size = mcrypt_get_iv_size(MCRYPT_RIJNDAEL_256, MCRYPT_MODE_CBC); // It's 32
        return mcrypt_create_iv($iv_size, MCRYPT_RAND);
    }

    public static function encrypt($key, $data, $iv = null, $base64 = true) {
        if (is_null($iv)) $iv = md5($key);
        $ret = mcrypt_encrypt(MCRYPT_RIJNDAEL_256, $key, $data, MCRYPT_MODE_CBC, $iv);
        return ($base64 ? base64_encode($ret) : $ret);
    }

    public static function decrypt($key, $data, $iv = null, $base64 = true) {
        if (is_null($iv)) $iv = md5($key);
        return rtrim(mcrypt_decrypt(MCRYPT_RIJNDAEL_256, $key, $base64 ? base64_decode($data) : $data, MCRYPT_MODE_CBC, $iv), "\0");
    }

}

As you can see, I choose to hash passwords with crypt() using Blowfish hashing algorithm. The return value of hashPassword() is the salt + hash that then I store in the DB. I made this choice because crypt() is available on every server, provides a confortable way to check hash regardless of algorithm used (it's based on salt prefix) and, I read, bcrypt is a decent hashing method.

Then, for data encryption I used mcrypt() Rijndael 256 algorithm with CBC mode. As you can see, I can use encryption methods in two way. I can pass a IV (and generateIv() helps me to create one) that I will store in the DB along crypted data, or, if I don't, a basic IV is derived from key in both crypt and decrypt process.

What do you think about it? Am I missing something? Can I be finally relaxed about hashing and encryption in my PHP aplications?!?

This is not really a single question. You could use code review instead, or use crypto if you separate the protocol from the implementation. — Maarten Bodewes
Maybe leave it be for now, crossposting is frowned upon too, and I don't know the level of security experts at code review, transfer to crypto would be the best option, but it would require rewriting the question. Just store the recommendation for future use :) — Maarten Bodewes

Maarten Bodewes Maarten Bodewes · Accepted Answer · 2012-05-17T00:35:39

You are using Rijndael 256 bit encryption, which is not AES standard. Try to use AES (MCRYPT_RIJNDAEL_128) using 256 bit keys instead.
A random IV should be kept with cipher text if the derived key is also used to encrypt other data.
You are using out of date functions, you might want to use bcrypt and SHA-256 for the IV (only use the 16 - blocksize - left most bytes) .

Note that this list may not be complete.

Best practice for a simple encryption class. Am I using crypt and mcrypt well?

1 Answers