Hi, Your Encryption Sucks

This is a result of programmers trying to reinvent the wheel, more often than not, when existing tools are more that sufficient to get the job done. But since these are efforts by those who either don’t understand what they’re doing or think they can do better, let me clarify one glaringly obvious fact that a lot of them overlook.

Anything you create will be worse

Not that someone isn’t clever to come across a better algorithm or a better technique. It’s all down to a simple matter of logistics and probability (another thing inherent to encryption). Your own concoctions will be seen by your eyes, your boss’ (maybe) and a few colleagues.

This is vastly below the scrutiny existing libraries receive. There are countless eyes scanning existing libraries whereas yours won’t and it won’t be a fair comparison. Which do you think is more likely to expose bugs?

So how do you do it properly?

There are countless forum threads out there complaining about decryption being corrupted. This is usually because block ciphers need their input strings in… ready? Specific size blocks. The solution is to pad the data to match the block size of the cipher before encryption and the remove the pad after encryption.

Here are two functions in PHP that do just that when given the string to pad and the block size.

function _pad( $str, $bsize ) {

	// Find the pad size
	$pad = $bsize - ( strlen( $str ) % $bsize );

	// Repeat the equivalent character up to the pad size
	$str .= str_repeat( chr( $pad ), $pad );

	return $str;
}

function _unpad( $str, $bsize ) {

	// Michael Corleone says hello
	$len = mb_strlen( $str );

	// Find the pad character ( last one )
	$pad = ord( $str[$len - 1] );

	// If padding would have been applied to the string...
	if ($pad && $pad < $bsize) {

		// ...find the pad
		$pm = preg_match( '/' . chr( $pad ) . '{' . $pad . '}$/', $str );

		// Pad found, strip it.
		if( $pm )
			return mb_substr($str, 0, $len - $pad);
	}

	return $str;
}

And now of course, you need an encryption function that uses it and gets back the expected result. I’ve seen many examples of multiple functions doing this, but I don’t see why one can’t accomplish both encryption and decryption. The critical aspect of IV based (initialization vector) encryption is packaging it along with the encrypted data and making sure both are in a format that’s not prone to corruption, yet is still easy to decrypt. Base64 will work nicely for this.

So here’s how you don’t reinvent the wheel…

Keep in mind that I may have been suffering from mild sleep-deprivation when I wrote this.

function _crypt( $str, $key, $encrypt = true ) {

	// Open Rijndael-256 in CBC mode (was CFB; code fixed 12/08/2012, comment changed 12/13/2012).
	// Change the encryption mode to 'ecb' if you enjoy Herpes
	$td = mcrypt_module_open('rijndael-256', '', 'cbc', '');

	// Find the IV size for the mcrypt module
	$size = mcrypt_enc_get_iv_size( $td );

	// Block sizes are needed to calculate the padding ( further down )
	$bsize = mcrypt_enc_get_block_size( $td );

	// Hash the key for consistency and use only the key size needed
	$key = substr( hash( 'ripemd160', $key ), 0, mcrypt_enc_get_key_size( $td ) );

	if( $encrypt ) {

		/*
			We're encrypting. Create a new initialization vector.

		 	If you change this to MCRYPT_RAND for any reason other than
			for testing or because your platform doesn't support it,
			I'll hunt you down and kick you in the groin.
		*/
		$iv = mcrypt_create_iv( $size, MCRYPT_DEV_URANDOM);

	} else {

		// Unwrap the combined IV and data packet
		$str = base64_decode( $str );

		// Extract the IV from the front (using IV size)
		$iv = mb_substr( $str, 0, $size );

		/*
			Isolate the data by removing the IV altogether.

			There's some controversy about using str_replace on multibyte
			characters, but as far as I can tell, there seem to be no ill effects.
			Besides, there's no equivalent "mb_replace" in PHP unless it's DIY.
		*/
		$str = str_replace( $iv, '', $str );

		// Decode in preparation for decryption
		$str = base64_decode( $str );
	}

	// Initialize mcrypt
	// I keep calling this "MyCrypt" in my head. Probably because of "MySQL"
	mcrypt_generic_init( $td, $key, $iv );

	if ( $encrypt ) {

		// Prepare string by padding to match block size
		$str = _pad( $str, $bsize );

		/*
			Encrypt the data.

			I'm supposed to say "this is where the magic happens" here, except I'll never say,
			"this is where the magic happens". That is a tired and old cliche with no useful
			purpose other than to engender ire and abject contempt to whoever says it. If
			"this is where the magic happens", then why are you trying desperately to point it
			out to everyone? Shouldn't you be hiding the location "where the magic happens"?
			The phrase "this is where the magic happens" was created to let everyone know that
			the speaker is constantly getting sex and this is their favorite
			[ bed / hooker / other object ] on which they receive it. No one realy believes it
			because people who say it are either never seeing this "magic" or are date-rapists.
			Now, despite the dubious origin of the phrase "this is where the magic happens",
			people have started using it at every inappropriate instance. E.G. While pointing
			to a chair in the office "this is where the magic happens". Unless the tool who says
			it is looking at porn at work, he's not performing "magic". In fact, that's not
			"magic", it's just disturbing. Not just because it's a public place, but people
			will be borrowing his office supplies without knowing he's touched them after he's
			touched something else... repeatedly.

			BTW, this is where the magic happens.
		*/
		$enc = mcrypt_generic( $td, $str );

		// Base64 the encrypted data, add the IV to the front and base64 again.
		$out = base64_encode( $iv . base64_encode( $enc ) );
		/*
			This ensures that the data remains portable because both the IV and the encrypted
			data contain special chracters ( unicode / multibyte ) that may be lost if handled
			as-is. This is especially true if the functions you're passing the data to can only
			handle ASCII data.
		*/

	} else {

		// Decrypt the data ( we removed the IV and decoded above )
		$str = mdecrypt_generic( $td, $str );

		// Remove the padding added to fit the block size
		$out = _unpad( $str, $bsize );
	}

	// Clean up
	mcrypt_generic_deinit( $td );
	mcrypt_module_close( $td );

	// Return encrypted/decrypted string
	return $out;
}

Very simple way to test this is to put it in a loop.

for( $i = 0; $i < 30; $i++ ) {

	$key = md5( $i ); // Simple key generation
	$data = "This is some test data";

	$enc = _crypt( $data, $key );
	$dec = _crypt( $enc, $key, false );

	echo "Test data :  $data <br />";
	echo "Key : $key; <br />";
	echo "Encrypted data : $enc <br />";
	echo "Decrypted data : $dec <br />";
	echo "Match : " . ( ( $data == $dec )? "True" : "False" ) . "<br /><br /><br />";
}

I’ve tested this a couple of times and it worked. I don’t have the time or the energy to keep testing, but trust me, this is how you do encryption properly. If there are bugs, scan through and see where I’ve missed a semicolon or something, but rest assured, this is “standard”.

Now… bedtime!

Advertisement