defuse / php-encryption

Simple Encryption in PHP.
MIT License
3.79k stars 306 forks source link

Derive Key From Password #5

Closed defuse closed 8 years ago

defuse commented 10 years ago

Add a function for deriving a key from a password with PBKDF2.

We have to think really hard about how to make this usable, since we want to make sure they're doing the salt right (by making it hard to do it wrong).

defuse commented 10 years ago

One way to do this might be to derive a key-encryption-key from the password and have it encrypt a master key. Then, we can save the salt inside the master key ciphertext.

defuse commented 10 years ago

So, it would be a new class called KeyLocker or something, with the following methods:

Other things we might want:

defuse commented 10 years ago

Added a warning not to use passwords as the key in 646a94f65e0d4d9ba4a63065c58abac1c3f1b12a

htho commented 10 years ago

Hi,

this is a great library. I am always unsure with my password-related functions. Here is what I am using right now:

/**
 * @return string
 */
private static function gen_salt(){
    return bin2hex((mcrypt_create_iv(16, MCRYPT_DEV_URANDOM)));
}

/**
 * 
 * @param string $cleartext
 * @param string $salt
 * @return string enctypted password
 */
private static function hash_password($cleartext, $salt = null){
    if($salt == null) {
        $salt = State::gen_salt();
    }
    return crypt($cleartext, "$6$" . $salt . "$"); // "$6$" makes sure SHA-512 is used.
}

/**
 * 
 * @param string $cleartext
 * @param string $hashed_password
 * @return boolean
 */
private static function check_auth($cleartext, $hashed_password){
    return (crypt($cleartext, $hashed_password) == $hashed_password);
}

When creating a user, the user entered password string is put into hash_password, which uses a generated salt. This hashed_password is stored in the databas. When a user tries to log in, check_auth returns true, if the password was right.

What do you think about that. Is this a startingpoint for KeyLocker? As I am looking forward to use it. Bye

htho commented 10 years ago

Uh... Ive just read your article: https://crackstation.net/hashing-security.htm I wish it was linked in the php.net manual and at w3schools.

I just found these php functions what do you think about them? http://de1.php.net/manual/en/function.password-hash.php http://de1.php.net/manual/en/function.password-verify.php

sarciszewski commented 9 years ago

@htho hash_pbkdf2 and hash_equals can satisfy this need. Luckily, defuse has previously written an implementation for a PBKDF2 class in PHP, so he knows what to do.

defuse commented 9 years ago

password_hash and password_verify are not what you want to make keys with. You need a KDF on its own, like hash_pbkdf2 (and make sure it's salted, etc).

htho commented 9 years ago

Uh I see, my post was offtopic. I was thinking about password hashing for authentication needs, not for encryption.

Anyway, @defuse Have you had a look at password_hash and password_verify? Are they (in your opinion) convenient for authentication?

pnowosie commented 9 years ago

@htho password_hash uses bcrypt algorithm as such follows OWASP recommendation for authentication / password storage purposes. For password_verify I missed the information in docs that it compares hashes in length-constant time which such function is expected to do.

defuse commented 9 years ago

@htho Yes, password_hash and password_verify are exactly what you should be using for password verification.

defuse commented 9 years ago

This is blocking adoption: https://github.com/Webiny/Framework/issues/39#issuecomment-80982903

SvenAlHamad commented 9 years ago

From what I've read, the KeyLocker should do the trick. It will allow the users to pass a password of any length, and then that can be used to generate a key for the encrypt method.

sarciszewski commented 9 years ago

@defuse Do you want to assign this one to me? :)

defuse commented 9 years ago

Definitely want derive-key-from-password in v2.0.

This should be independently versioned, so that we can start with PBKDF2 and upgrade to Argon2 (or scrypt) later without the user having to special case that themselves.

paragonie-scott commented 9 years ago

Feel free to copypasta this: https://github.com/sarciszewski/php-future/blob/be8ef2346341c90e50066db2e6e7be1ea469889d/src/Security.php#L53-L90

I took your old PBKDF2 code and made it PSR-2 compliant (mostly explicitly adding braces after if statements).

defuse commented 9 years ago

Now that we have the Key interface this is absolutely necessary for 2.0, since without it there's no way to use a key derived from a passphrase!

defuse commented 9 years ago

I thought up a few API designs for this. I'm not too happy with their usability (it's hard because of the interaction with a salt that has to be saved, and getting the key loaded into a Key class). Before I pick one, what are your ideas?

defuse commented 9 years ago

(Note that we also want to version the salt/algorithm, so that we can easily upgrade to Argon2 once it's available in PHP)

paragonie-scott commented 9 years ago

Well, there's really no avoiding having ($password, $salt) in the derivation API.

Take a look at what libsodium does:

If we want to follow their example, we need to expose:

/**
 * PBKDF2-SHA256 with iterations >= 86000
 * @param string $password
 * @param string $salt
 * @return Key
 */
function deriveFromPasswordPBKDF2($password, $salt);
/**
 * Reserved for Argon2i:
 * @throws Exception
 */
function deriveFromPassword($password, $salt);

Alternatively, we can store all of the version information with the salt (i.e. the first N bytes of a crypt(3) hash). This makes our API look like this:

/**
 * @param string $password
 * @param string $configString
 * @return Key
 */
function deriveFromPassword($password, $configString);
/**
 * Generates salt, stores it in a formatted string
 * @return string
 */
function generateConfigString();

For example, generateConfigString() might return $pbkdf2$sha256$86000$7xjCjohuIi4QCARN today and $argon2i$m=1024,t=50000,p=4$t003K73k/Bomtg8iHN/K4w later.

defuse commented 9 years ago

I want to abstract away the salt, so that there are two distinct operations:

  1. Creating a key from a password for the first time (we generate a random salt).
  2. Re-createing the key from a password (and some abstracted-away salt state pulled from the DB).

i.e. like the usual password hashing API, where the user is never responsible for generating a salt.

(which looks like the second option above)

Actually... what if we used the exact same API as password hashing, except we rename the verification method to something like verifyPasswordAndGiveMeAKeyIfItIsCorrect which checks the password and returns a key if it's correct. That way we see if the password is correct without having to decrypt anything.

We also have to decide whether to use scrypt(password, salt) as the key for encryption, or generate a random key and then use scrypt(password, salt) to encrypt that key (so that the password can be changed without re-encrypting all the ciphertexts).

paragonie-scott commented 8 years ago

Are we going to use scrypt, or hash_pbkdf2() with a polyfill since that's the best we can do from PHP?

defuse commented 8 years ago

Probably PBKDF2. I don't think there's a fast enough scrypt implementation that we could use without bringing in another dependency like libsodium (in which case why even bother with this library)

defuse commented 8 years ago

Considering kicking this out of 2.0 just to get it out the door.

defuse commented 8 years ago

Oh, I see there's a PR. I guess I'll look at that first.

defuse commented 8 years ago

The PR I merged (#187) doesn't have the nice API I wanted. However, I don't have time to implement that. Let's maybe add the usable API in 2.1.

defuse commented 8 years ago

If I close this ticket, or put it into 2.0, then we don't need to do a 2.1 version!

defuse commented 8 years ago

I put this back in 2.0 since it's easy:

  1. Make a 'Salt' class that abstracts a random 32-byte value (with timing-safe save/load).
  2. Require a 'Salt' object as the $salt parameter.
defuse commented 8 years ago

Also needs tests

defuse commented 8 years ago

Bug: https://travis-ci.org/defuse/php-encryption/jobs/120515501 need to polyfill hash_pbkdf2 or something.

paragonie-scott commented 8 years ago

Yeah, I was going to port it over from https://github.com/sarciszewski/php-future/blob/master/src/Security.php#L53-L90

You could also just switch to 5.5 as a minimum version ;)

defuse commented 8 years ago

Debian Wheezy still has 5.4 :(

paragonie-scott commented 8 years ago

For reference: Wheezy is 7.x, and 8.4 just came out. (Also, not using dotdeb for PHP on Debian is asking for complications.)

But yeah, polyfilling hash_pbkdf2 is probably the best move.

defuse commented 8 years ago

I'm copying the one from defuse/password-hashing as we speak :)

defuse commented 8 years ago

Done!