3

I am trying to understand if my database password encryption structure is good, or not much use.

So when i register, the php will generate a 50 character random key, and that key will be put into a db table called hash tagged against the user id, then the hash will be used to AES encrypt the password and the password will be stored in another table.

When a user login, the hash will be pulled out and use for decryption. And if the login is successful, the hash key will be replaced with a newly generated one for next use etc.....

I am not sure if this is good idea or not, any feedback is good thanks!

John
  • 197
  • 2
  • 9
  • 17
    Normally, you don't encrypt passwords, but hash them. Maybe you should read this question: http://security.stackexchange.com/questions/211/how-to-securely-hash-passwords/ – Lukas Sep 12 '16 at 10:05
  • 2
    I'm not sure what the situation you're in, but where possible you should really look for existing open-source security systems. See http://security.stackexchange.com/questions/18197/why-shouldnt-we-roll-our-own – Jezzamon Sep 12 '16 at 13:41
  • 2
    Like my mate stated, you should : 1. store the hashed password using `password_hash()` in a table. 2. In case you need to check the password : 2.1 get the hashed password 2.2 hash the actual password 2.3 compare the 2 hashed strings using `password_verify()` 2.4. make some decisions if they are equal or not. – Anwar Sep 12 '16 at 14:36
  • 1
    Is your application a password manager of some form? If not: you don't need encryption on the passwords, just hash them. – Bakuriu Sep 12 '16 at 16:42
  • This is wrong on so many levels ... – Num Lock Sep 13 '16 at 09:05
  • It's incredible what lengths some people will go to when `password_hash()` is literally designed for this... – Niet the Dark Absol Sep 13 '16 at 11:17

4 Answers4

33

No, this is not a good idea! As Lukas pointed out in an comment, you want to hash a password, never encrypt it! With your schema, it would be possible to get the plaintext passwords from an database dump without any effort. Don't let that happen!

Josef
  • 5,903
  • 25
  • 33
  • 1
    Cool, sweet thanks for the advices, i was just reading it up! I am totally new to this security area! Does it mean on php just using password_hash() is good enough? – John Sep 12 '16 at 10:24
  • 11
    password_hash uses bcrypt and should be secure enough. – Josef Sep 12 '16 at 10:28
  • 4
    @John Don't forget to use `password_verify()` and `password_needs_rehash()`. The PHP documentation explains pretty well how you should use it. Use the pre-defined constant to use the default algorithm. Also, benchmark your server, to see which cost parameter you can use. Always go for the highest that you can afford. I would say that 11 or 12 is a good value, but you can find loads of codes to benchmark it, that return the ideal parameter. – Ismael Miguel Sep 13 '16 at 08:32
9

No, this isn't good at all.

First of all, there is a huge confusion about terminology.

A hash function maps arbitrary input to a fixed set. The benefit here is that they are one way, so they can be used to store passwords for authentication without storing the actual password, and without the possibility to recover the actual password.

Encryption on the other hand is two-way: If you have the key, decryption is trivial.

What you call "hash" doesn't seem to be a hash at all, it seems to be a symmetric key, used to encrypt and decrypt.

Your whole scheme doesn't provide any benefits to storing passwords in plaintext. If an attacker has read access to the database and can thus read the encrypted passwords, they can also read the key, making encryption useless.

Generally, when you encrypt something stored in a database, you do not want to store the encryption key in the same database. But in this case it doesn't matter, as passwords are hashed, not encrypted.

tim
  • 29,018
  • 7
  • 95
  • 119
  • Cool thanks! Yer i was confused...pretty new to all these. Quick question, then would encrypting a hashed password bring more security benefit to it? Or not – John Sep 12 '16 at 10:34
  • 1
    @John Theoretically, yes (assuming you store the key eg in the source code, not the database), for the same reason that a pepper does add a bit of security: It isn't enough to read the database to crack passwords, the source code needs to be read as well. So if you eg find a backup disk of the database, but not the source code, getting the passwords will be more difficult with additional encryption. But if you need the additional security, using a pepper is enough, so there is no need for the overhead of encryption (also, a pepper avoids possible encryption/hash confusion). – tim Sep 12 '16 at 10:39
1

There's something called "decryption" for encryption but there's no such thing as "unhashing". So you can salt the password or use the built-in password_hash function.

mzcoxfde
  • 585
  • 2
  • 5
  • 12
  • 2
    This shouldn't be a case of 'you can salt the password or use the built-in password_hash function', it should be a 'you should salt the password and use a built in has function, like `password_hash`'. Passwords should always be salted and hashed. – gabe3886 Sep 13 '16 at 14:12
1

Another reason for why this might not be safe, in addition to the whole hash != encryption problem, is the manner of the password generation. Your "key" or "hash" as you call it, is basically this: A auto-generated password.
Now, if the algorithm generating said password does this in a predictable manner, and the attacker can access this. Then it becomes possible for an attacker to predict the password for all users, even if it's properly hashed!

I recommend reading up a bit more on timing attacks, and ensure that you use a proper random source for these kind of password/key generations.

ChristianF
  • 826
  • 6
  • 8