HiveBrain v1.2.0
Get Started
← Back to all entries
patternphpMinor

String encryption function in admin section

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
encryptionadminsectionfunctionstring

Problem

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)

Solution

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

  • Break out the function into two - encrypt and decrypt. Your functions should do the least amount of things.



  • return on trim - no need to set $encryptedString.



  • 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']);

Code Snippets

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");
}
# Encrypt
$_SESSION['username'] = $backend->encrypt($username);

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

Context

StackExchange Code Review Q#61369, answer score: 2

Revisions (0)

No revisions yet.