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](http://codereview.stackexchange.com/) instead, or use [crypto](http://crypto.stackexchange.com/) if you separate the protocol from the implementation. — Maarten Bodewes, May 17 '12 at 00:36
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, May 17 '12 at 09:35

score 1 · Accepted Answer · edited May 23 '17 at 12:04

1

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.

edited May 23 '17 at 12:04

Community

1
1

answered May 17 '12 at 00:35

Maarten Bodewes

90,524
13
150
263

**1.** The [`crypt()`](http://us.php.net/manual/en/function.crypt.php) PHP function returns itself a string concatenating the passed salt (prefixed with a code representing the algorithm used) and the hash. The same string can be reused by the same `crypt()` again when you want to check hash matching. **2.** Isn't it? I know Rijndael is the AES standard, maybe the 256 bit version is not? Tell me. **3.** If `::encrypt()` is used with no IV, IV is derived from key so I have not to store it; if IV is passed (preferred case), it will be a calling script problem to store it along encrypted data. – lorenzo-s May 17 '12 at 07:13
1. Ah, did not know that about crypt, removed from recommendations. 2. AES is a subset of Rijndael, and indeed does not include 256 bit blocks. 3. that's fine *as long as you don't use the derived key for other purposes* – Maarten Bodewes May 17 '12 at 09:29
**2.** Is Rijndael 128 or 192 in the AES standard? **3.** No, I don't; I know if I store the IV in that case, then the key is easily derivable because it's a simple MD5. – lorenzo-s May 17 '12 at 09:47
2. Only Rijndael 128. 3. The key is not derivable from MD5, MD5 is not broken for key derivation in general. But using the same key + IV for different CBC encrypts would leak information about the plain text to a potential attacker. You may of course use the same password and a different salt, but it would mean recalculating the key (which has its advantages and disadvantages). – Maarten Bodewes May 17 '12 at 09:54
I obviously mean that storing an MD5 hash of the crypt key is not so much secure. With "easily derivable" I mean that simple keys (few letters, common words, etc) can be derived from thier MD5 quite easily. In that case, MD5 of the key will be not stored (as it's normal in the other case when IV is given and *not derived*) but only used as IV. – lorenzo-s May 17 '12 at 10:00
MD5 is a one way hash, simple keys can be found by using brute force, but you cannot reverse the hash function (even now it is partly broken). You use MD5 *after* `crypt`, which means that you are operating on data that the attacker does not know. MD5 is therefore safe for use in this scenario. I'll update my recommendations though. – Maarten Bodewes May 17 '12 at 10:07
I know it's a one way, but tables and rainbow tables exists and can be used. Passwords like "hello", "tomas 1234", etc can be reversed. http://stackoverflow.com/q/3049070/995958 – lorenzo-s May 17 '12 at 10:09
let us [continue this discussion in chat](http://chat.stackoverflow.com/rooms/11368/discussion-between-owlstead-and-lorenzo-s) – Maarten Bodewes May 17 '12 at 10:16
IIRC, AES specifies a block size of only 128 but a key size may be 128, 196, or 256. As far as the cipher choice, it appears it falls into AES. – Sam May 17 '12 at 11:29
@sam No it doesn't. `MCRYPT_RIJNDAEL_256` is not AES. – Maarten Bodewes May 17 '12 at 11:30
@owlstead, it would be useful if you identified the parameter that you believe invalidates the AES restrictions on the Rijndael algorithm. – Sam May 22 '12 at 04:33
@sam see [here](http://stackoverflow.com/questions/4537099/problem-with-aes-256-between-java-and-php) – Maarten Bodewes May 22 '12 at 07:26

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

1 Answers1