Is using bcrypt on existing SHA1 hashes good enough when switching password implementation?

Question

I'm working on improving a CMS where the current implementation of storing password is just sha1(password). I explained to my boss that doing it that way is incredibly insecure, and told him that we should switch to bcrypt, and he agreed.

My plan was to just run all the existing hashes through bcrypt and store those in the password field, and then use the following psudo-code to check the password: correctPassword = bcrypt_verify(password, storedHash) or bcrypt_verify(sha1(password), storedHash).

This way, new users, or users who change their passwords will get "real" bcrypt hashes, while existing users won't all have to change their passwords. Are there any disadvantages to doing this? While it would probably be ideal to ask all users to choose a new password, do we lose much in the way of security by doing this?

I was thinking that even if an attacker got access to both the database and the code, cracking won't be substantially faster even if the majority of the "input" to bcrypt was a 40 character hex string, since the slow part (bcrypt_verify()) still has to be invoked for each password attempt on each user.

Answer 1

Actually this is a good way to protect the otherwise unsecurely stored passwords. There is one weak point in this scheme though, which can be overcome easily in marking old hashes, so I would prefer this solution:

if (checkIfDoubleHash(storedHash))
  correctPassword = bcrypt_verify(sha1(password), storedHash)
else
  correctPassword = bcrypt_verify(password, storedHash)

Imagine an attacker getting hold of an old backup. He would see the SHA hashes, and could use them directly as passwords if you test with bcrypt_verify(...) or bcrypt_verify(sha1(...)).

Most bcrypt libraries add a mark of the used algorithm themselves, so it is not a problem if you add your own "double hash mark", but of course you can also use a separate database field for this:

$2y$10$nOUIs5kJ7naTuTFkBy1veuK0kSxUFXfuaOKdOKf9xYT0KKIGSJwFa
 |
 hash-algorithm = 2y = BCrypt

Answer 2

Why not simply use bcrypt(sha1(password)) for all passwords both old and new? This avoids the problem of people using your old hashes as passwords and is also simpler than your proposal.

Answer 3

It's a good strategy, you won't loose any security unless a user decided to generate a truely random password longer than 160-bits as it will be truncated. So the difference is minimal. (in which case it would still take a significant amount of time to bruteforce the original text)

You might opt to implement some logic to migrate the passwords next time a user changes it, but I don't see any risk that would require immediate change of passwords, unless you believe the hashes have leaked.

Answer 4

I recently implemented a similar system for migrating passwords across to bcrypt. However instead of SHA1 we were using SHA256(password + salt) hashes originally.

This salt is regenerated when we switch the user to bcrypt as they login (optional) or upon changing their password. So the hash wouldn't be based on the original. We then primary use this nonce as an IV to encrypt the bcrypt hash in the database with a key stored outside the database.

Doing this only really prevents injection attacks and data pulled solely from the database providing any useful password information. But the overhead wasn't a concern for us and we can change this external key whenever we need to.

Using the SHA256 hash as input to bcrypt also keeps the length of the input password below the maximum for bcrypt (related articles)

The only concern that I saw any reference too with using SHA1 in this manner when I was looking around for advice and problems with anything I was doing was from Thomas Pornin in the second of those two links:

Using a secure hash function to preprocess the password is secure; it can be shown that if bcrypt(SHA-256(password)) is broken, then either the password was guessed, or some security characteristic of SHA-256 has been proven false. There is no need to fiddle with the salt at that level; just hash the password, then use bcrypt on the result (with the salt, as bcrypt mandates). SHA-256 is considered to be a secure hash function.

So it's possible that holding onto SHA1 may not be a good choice - why else migrate from using it solely in the first place. That being said it's improbable there would be an kind of attack that would provide any practical value in attacking a bcrypt(SHA1(password)) in the near future that wouldn't involve a compromise of some sort with bcrypt itself.

Answer 5

Initial idea

bcrypt(sha1(password));

Problem #1 - Null Termination Problem

The first reason you don't want to do that is because SHA, or any hashing algorithm, puts out bytes. And many programming languages do not have proper String types; and instead simulate strings with a series of characters followed by a null (i.e. \0) terminator. If your hash digest contains a null, the bcrypt algorithm might see the \0 character, and assume that's the end of the string:

• bcrypt(sha1("fsdf3hgfh2faff32f"))
• bcrypt(96 87 0f 9e 71 ff 62 57 55 00 b6 5c 91 07 64 6f b5 81 13 a9)

And with C and PHP, if you blindly treated the digest as a "string", then your "string" would look like:

• bcrypt("–‡žqÿbWU\0¶\‘doµ©")

causing some bcrypt implemetations to cut off at the \0 null terminator:

• bcrypt("–‡žqÿbWU")

This is known as the null termination problem

Solution

Your implementation may be immune to this; or it may not. So lets not tempt fate. You can pre-hash the password, but be sure to base-64 encode the digest first:

• bcrypt(base64(sha1("fsdf3hgfh2faff32f")))
• bcrypt(base64(96 87 0f 9e 71 ff 62 57 55 00 b6 5c 91 07 64 6f b5 81 13 a9))
• bcrypt("locPnnH/YldVALZckQdkb7WBE6k=")

Problem 2 - Hash Shucking

The next issue is has to do with dictionary attacks.

An attacker isn't going to bruteforce every possible password:

• aaaaaaaa
• aaaaaaab
• aaaaaaac
• ...

Instead they're going to use dictionaries, previous password breeches, and password that follow the rules that certain stupid corporations insist upon (e.g. password complexity policies).

• hunter2
• password
• Tr0ub4dor&3
• 12345
• qazxsw
• zxcvbn

The whole point of bcrypt is that it is still hard to brute-force all these dictionary words. But the fact remains that there are still these lists, and it can dramatically shorten the search space.

But imagine there was a password database breech, and fortunately the web-site used SHA-1 to store all their passwords, and one of the breeched SHA-1 hashes was:

• 96 87 0f 9e 71 ff 62 57 55 00 b6 5c 91 07 64 6f b5 81 13 a9

They don't know what the original password is, but at least it's something they can add to their dictionary list. And if your web-site does pre-hash with SHA-1, then suddenly they can try:

• bcrypt(base64(96 87 0f 9e 71 ff 62 57 55 00 b6 5c 91 07 64 6f b5 81 13 a9))

If it matches, it means that they have the SHA-1 hash of someone's password. And since SHA-1 is so easy to compute in hardware, they now have an SHA-1 hash they can try to bruteforce.

This problem is known as 'Hash Shucking'.

Solution

What you want to do is be sure to salt the password hash:

• bcrypt(base64(sha1(password+salt)))

This way the "password hash" will never appear in any other global database.