Let me first say that I know it is a bad idea to store sensitive information in a mysql database, so please don't respond by saying "DON'T DO IT" or something to that effect. I am building a website for which it is absolutely essential to store Social Security Numbers, and I have to be able to retrieve that data back out of the DB (no hashing).

That said, I have researched the best way to encrypt/decrypt the data, and built a custom function to handle the encryption. Here is my encrypting function:

So basically I am generating a random string for my salt, then appending to the salt a private key MY_PRIVATE_KEY which is defined in a separate config file. I then use mcrypt_encrypt to encrypt the data, then base64_encode to make the encryption safe to store in the DB. The encrypted string and the unique salt are then returned and stored in the DB together.

My thinking was that throwing a "private key" that is stored in the config file (not the DB) into the mix would add another level of security to the encryption, that way even if someone hacks the database and gets the encrypted value and the salt, they still wouldn't have everything they need to decrypt the data.

Can you security experts review my function and let me know if/how my data could be hacked and if there is anything else I could do to improve it?

The fact you are asking "is this idea secure?" means you don't have the proper knowlege on how to properly store PII currently. Please look up the laws that surround storing this sort of information. Please don't use a "custom" encryption schema use something proven to be sure. The biggest problem I have with your code is your using a MD5 hasing method.
–
RamhoundMay 10 '13 at 11:19

Using salt makes it difficult crack a list of passwords.If its just one key you are adding a salt to and then encrypting it will not make the cracking difficult.
–
ShurmajeeMay 10 '13 at 11:25

@MayankSharma: Using MD5 leaves him open to collision and chosen attacks. Using double MD5 does not increase entropy.
–
Sébastien RenauldMay 10 '13 at 11:26

1

However, collision attacks only apply in contexts where the attacker chooses what is hashed, and may gain something through a collision; neither property applies to the present situation.
–
Thomas PorninMay 10 '13 at 11:28

1

In addition to what others said - your setup is as secure as is your config file. If someone broke in to your webserver he would be able to get the data - he would have probably both username/password for db, means pass the firewall to do it and the password for the encryption.
–
Maciej PiechotkaMay 10 '13 at 15:18

2 Answers
2

You are using a 128-bit key (output of MD5, hence 128 bits) with a block cipher which expects a 256-bit key (Rijndael-256). If PHP was properly implemented, it would blow in your face at that point. However, the documentation states that the key is merely padded with zeros up to the required length. If you use a 128-bit key, you'd better use a cipher with expects a 128-bit key, i.e. Rijndael-128.

The "salt" is either useless, or poorly applied. Salts are meant to deal with weak keys, which can be realistically recovered by exhaustive search, i.e. when keys are passwords. If your key is a real, properly generated, fat and random key, then the salt is useless. On the other hand, if your key is a password, then you are in the real of password hashing, which will require more effort.

On the other hand, the IV should be random, instead of being the hash of the key. Appropriate use of the block cipher would be:

When encrypting a new message, create a new random IV with a strong PRNG (i.e. openssl_random_pseudo_bytes(), which internally feeds on /dev/urandom or equivalent).

Forget about any "salt". Encrypt with your main key and the randomly generated IV. You then have to store the IV along with the encrypted message (for a SSN, you will end up with 32 bytes: 16 for the IV, and 16 for the encrypted block).

Then the following caveats apply:

This is encryption; it ensures confidentiality, not integrity. Someone who can modify parts of your database may play havoc with your SSN (if only by switching them around between users). Database integrity is a hard problem and the best known solutions are not good; they imply significant overhead.

Confidentiality is maintained only as long as the key is not revealed. But we envisioned an attacker who got an unusually privileged read access to the server contents, and the server, generally speaking, knows the key. If you keep the key in a file, separated from the database, then you may get some extra protection against basic SQL injection attacks, which work on the database contents only; but be aware that you have fixed only some vulnerabilities.

Sorry to break it to you - you're taking really dangerous chances with those SSNs. Let's quickly define something according to PCI-DSS standards:

Q: What is defined as ‘cardholder data’?
A: Cardholder data is any personally identifiable data associated with a cardholder. This could be an account number, expiration date, name, address, social security number, etc. All personally identifiable information associated with the cardholder that is stored, processed, or transmitted is also considered cardholder data.

PCI DSS is the standard for secure information storage (preferably financial). The rules are strict, and the fines for not complying are massive ($5k - $100k per month). SSNs, however, can be stored according to it, but again, the guidelines are strict (page 14 of this document). So, even before discussing your current implementation, I'd strongly suggest you know what you are getting into.

Let's lay a few things down as well:

If you are retransmitting the SSNs (on a page or otherwise) and your transport layer is not secure, it's as though all the levels of encryption you put the data through on the back end did not matter. Sniffing is dead-easy these days.

If you are designing your own "encryption suite" because you think it will work - do you remember all those companies that had breaches? They thought their stuff would work, too.

Let's go into the details. Anyone who is not familiar with PHP (other answer providers), the function mcrypt_encrypt takes as parameters:

Cipher - here, 256bit AES

PK

plaintext

Mode (here, CBC)

IV

Let's break your function down a bit. The fist obvious point of "WTF is this shit" is your use of md5 to generate keys and IVs. You are aware that MD5 has serious vulnerabilities, right?

The second obvious WTF is the choice of cipher. AES 512 is usable on PHP. This would double your key size as well. Not just this, but your use of salt is quite amusing - this is what an IV is for!

The third obvious WTF is the use of a constant for your private key. You do realize that this constant will persist from initialization to closure of your script, right? Not just that, but I bet it's defined in clear text in one of the files. This means that:

If anyone manages to find a SINGLE include/file-read vuln on your site, they have your private key

If anyone manages to find a SINGLE code execution vuln on your site, they have your private key

Consider storing your private key securely (or even better, only storing a hash of it and requiring user validation of the key to decrypt - effectively forcing the user to know the key to decrypt sensible info).

TL;DR: read up on PCI-DSS, stop making stuff up and actually understand the tools you're using. I'd recommend a primer in cryptography to start with - any objections to Bruce Schneier? If not, pick up Practical Cryptography (or if you prefer softer books, Secrets & Lies, assuming there is a more recent edition than 2001).

At least somebody says what I was thinking.
–
RamhoundMay 10 '13 at 12:08

Wow. Thanks for the butt-whooping! In case you can't tell, I am new to this kind of security, which is why I posted this question here. I will look into the items you mentioned and work on improving my script.
–
BWDesignMay 10 '13 at 13:39

1

@BWDesign: Speaking further of protecting the key, you might (if this project has a big enough budget) want to invest in HSMs. They are quite secure, being custom-built for the task of protecting keys. SSNs are a really dangerous thing to store, especially if they are linked with other private information, so you probably want to at least find out if HSMs are an option for you.
–
ReidMay 10 '13 at 15:50