Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Does “KeyProtectedByPassword” weaken password hashing otherwise done with “bcrypt”? #426

Closed
ocram opened this issue Nov 8, 2018 · 4 comments

Comments

@ocram
Copy link

ocram commented Nov 8, 2018

In a common use case, PHP’s password_hash function is employed to hash the user’s login password, e.g. with the current default options of “bcrypt” as the algorithm and 10 as the cost or work factor.

Now, as per the second scenario of your excellent tutorial in docs/Tutorial.md, arbitrary messages and content shall be encrypted using that same login password – using this library here.

To do that, as described in your tutorial, a random key is generated and protected with the user’s login password upon registration:

$myProtectedKey = KeyProtectedByPassword::createRandomPasswordProtectedKey($myPassword);
$myProtectedKeyAscii = $myProtectedKey->saveToAsciiSafeString();
// store $myProtectedKeyAscii

Upon every login, the key is loaded again and unlocked using the password:

// retrieve $myProtectedKeyAscii
$myProtectedKey = KeyProtectedByPassword::loadFromAsciiSafeString($myProtectedKeyAscii);
$myKey = $myProtectedKey->unlockKey($myPassword);
// use $myKey

Internally in this library, it seems, the KeyProtectedByPassword is generated and the password is protected as follows:

Crypto::encryptWithPassword(
    Key::createNewRandomKey()->saveToAsciiSafeString(),
    \hash('sha256', $password, true),
    true
);

Now if an attacker got access to the protected keys (i.e. $myProtectedKeyAscii) and also to the separately hashed passwords, both stored in the database, what would be the faster way to crack the passwords and expose the user’s original login passwords – targeting the protected keys (or even messages encrypted with those) or the hashed passwords?

In other words, does KeyProtectedByPassword introduce another target for the attacker which is easier to attack and yield the user’s login password than attacking the “bcrypt” password hashes?

As we know, “bcrypt” has explicitly been designed for the purpose of protecting passwords. But if the attacker could instead attempt to call KeyProtectedByPassword#unlockKey with varying guesses for the password until the key “opens”, this would circumvent the password hashing done separately and reduce the target from bcrypt(password) to sha256(password) (?).

Regarding context separation, you mentioned slides which say:

Reusing a secret key for different purposes can have catastrophic implications.

http://archive.hack.lu/2017/hacklu-crypto-api.pdf

Further, you mention the following:

WARNING: Because of the way KeyProtectedByPassword is implemented, knowing
SHA256($password) is enough to decrypt a KeyProtectedByPassword. To be
secure, your application MUST NOT EVER compute SHA256($password) and use or
store it for any reason. You must also make sure that other libraries your
application is using don't compute it either.

docs/Tutorial.md and docs/classes/KeyProtectedByPassword.md

So is all this a valid concern?

Or do the IVs (or something else) prevent KeyProtectedByPassword from being an easier target for password cracking?

@paragonie-scott
Copy link
Collaborator

In other words, does KeyProtectedByPassword introduce another target for the attacker which is easier to attack and yield the user’s login password than attacking the “bcrypt” password hashes?

As we know, “bcrypt” has explicitly been designed for the purpose of protecting passwords. But if the attacker could instead attempt to call KeyProtectedByPassword#unlockKey with varying guesses for the password until the key “opens”, this would circumvent the password hashing done separately and reduce the target from bcrypt(password) to sha256(password) (?).

Regarding context separation, you mentioned slides which say:

Reusing a secret key for different purposes can have catastrophic implications.
http://archive.hack.lu/2017/hacklu-crypto-api.pdf

Further, you mention the following:

WARNING: Because of the way KeyProtectedByPassword is implemented, knowing
SHA256($password) is enough to decrypt a KeyProtectedByPassword. To be
secure, your application MUST NOT EVER compute SHA256($password) and use or
store it for any reason. You must also make sure that other libraries your
application is using don't compute it either.
docs/Tutorial.md and docs/classes/KeyProtectedByPassword.md

So is all this a valid concern?

Or do the IVs (or something else) prevent KeyProtectedByPassword from being an easier target for password cracking?

So here's the issue: In order to side-step DoS attacks, the password is prehashed with SHA256, since encryptWithPassword() used PBKDF2 to expand the password into a key. You don't want to disclose the SHA256 hash of your plaintext password anywhere since the SHA256 hash itself can be used with decryptWithPassword(). That's the limited scope of the relevant cross-protocol attacks.

If you, separately, have a bcrypt hash of the password, this isn't a concern. It's totally orthogonal, since the SHA256 hash of the password then goes through 100k rounds of PBKDF2-SHA256, which won't afford attackers a significantly more viable target than bcrypt.

That being said, if you can, please don't let your users reuse passwords.

@ocram
Copy link
Author

ocram commented Nov 8, 2018

Thanks, @paragonie-scott!

That being said, if you can, please don't let your users reuse passwords.

Well, they just have a single password, as with most services. It’s just that – other than the login where “bcrypt” is used – now another use case for that single password is added, which is the encryption of the key, which is a suggestion from docs/Tutorial.md. Or are you referring to something else? Re-use across services? Re-use after password change?

If you, separately, have a bcrypt hash of the password, this isn't a concern. It's totally orthogonal, since the SHA256 hash of the password then goes through 100k rounds of PBKDF2-SHA256

Does that mean, though, that the described use case changes (perhaps reduces) the cost of password cracking from bcrypt(password) to min(bcrypt(password), pbkdf2-sha256-100000(password)), assuming each of these expressions describes the costs?

@paragonie-scott
Copy link
Collaborator

Does that mean, though, that the described use case changes (perhaps reduces) the cost of password cracking from bcrypt(password) to min(bcrypt(password), pbkdf2-sha256-100000(password)), assuming each of these expressions describes the costs?

I think that's a fair assessment, assuming that the same password is used in both places.

@ocram
Copy link
Author

ocram commented Nov 10, 2018

Thanks!

Yes, the same password is used, which is what the use case is all about, as per the tutorial.

Anyway, min(bcrypt(password), pbkdf2-sha256-100000-rounds(password)) really sounds acceptable. I just wanted to make sure the cost is not just min(bcrypt(password), pbkdf2-sha256-1-round(password)), which would render the second scenario from the tutorial practically useless, as it would break the security of a separate password hashing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants