4
\$\begingroup\$

I have been using this function locally to encrypt strings for example a username.

If the user successfully logged in, place the encrypted username into a session, decrypt the username in the admin section and use for various things.

Is it a strong method and are there any security concerns with the way I'm using it?

Cipher Function:

    public function cipher($string, $method) {
        $key = pack('H*', "bcb04b7e103a0cd8b54763051cef08bc55abe029fdebae5e1d417e2ffb2a00a3");
        $iv_size = mcrypt_get_iv_size(MCRYPT_RIJNDAEL_128, MCRYPT_MODE_CBC);
        $iv = mcrypt_create_iv($iv_size, MCRYPT_RAND);

        if($method === 1) {
            $string = mcrypt_encrypt(MCRYPT_RIJNDAEL_128, $key, $string, MCRYPT_MODE_CBC, $iv);
            $string = $iv . $string;
            $encryptedString = base64_encode($string);
    return trim($encryptedString, "\0");
    }
        if($method === 2) {
            $string = base64_decode($string);
            $iv_dec = substr($string, 0, $iv_size);
            $ciphertext_dec = substr($string, $iv_size);
            $decryptedString = mcrypt_decrypt(MCRYPT_RIJNDAEL_128, $key, $ciphertext_dec, MCRYPT_MODE_CBC, $iv_dec);
    return trim($decryptedString, "\0");
        }
    }

Function Usage:

//Encrypt
$_SESSION['username'] = $backend->cipher($username, 1)

// Decrypt
echo $backend->cipher($_SESSION['username'], 2)
\$\endgroup\$
2
\$\begingroup\$

It looks great, however, you can improve it as follows:

  1. Break out the function into two - encrypt and decrypt. Your functions should do the least amount of things.
  2. return on trim - no need to set $encryptedString.
  3. In the encrypt, no need to have the second $string when you concat $iv. Just concat on the initial mcrypt call.

In the end, we're left with three functions. One setupCipher which creates the key, iv_size and iv. One encrypt and one decrypt to encrypt and decrypt, as follows:

private function setupCipher(){
    $iv_size = mcrypt_get_iv_size(MCRYPT_RIJNDAEL_128, MCRYPT_MODE_CBC);

    return array(
        pack('H*', "bcb04b7e103a0cd8b54763051cef08bc55abe029fdebae5e1d417e2ffb2a00a3"),
        $iv_size, 
        mcrypt_create_iv($iv_size, MCRYPT_RAND)
    );
} 

public function decrypt($string){
    list($key, $iv_size) = $this->setupCipher();

    $string = base64_decode($string);
    $iv_dec = substr($string, 0, $iv_size);
    $ciphertext_dec = substr($string, $iv_size);
    return trim(mcrypt_decrypt(MCRYPT_RIJNDAEL_128, $key, $ciphertext_dec, MCRYPT_MODE_CBC, $iv_dec), "\0");
}

public function encrypt($string) {
    list($key, $iv_size, $iv) = $this->setupCipher();

    $string = $iv . mcrypt_encrypt(MCRYPT_RIJNDAEL_128, $key, $string, MCRYPT_MODE_CBC, $iv);
    return trim(base64_encode($string), "\0");
}

You would use them nearly identically to how you used them before, except no need to pass in the $method:

# Encrypt
$_SESSION['username'] = $backend->encrypt($username);

# Decrypt
echo $backend->decrypt($_SESSION['username']);
\$\endgroup\$

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.