[12.x] Allow limiting bcrypt hashing to 72 bytes to prevent insecure hashes.

[waxim]

This is an updated version of this PR designed to target Laravel 12.

Why This PR?

Recently Okta had a security incident created by them using bcrypt hashes that were over 72 chars in length, bcrypt implementations generally allow strings longer than 72 charachters, but simply omit the characters over that length. This can lead to predictable hashes if you can "stuff" the string with 72 known characters.

Take this simple example from Laravel Framework 11.41.3.

<?php

namespace App\Console\Commands;

use Illuminate\Console\Command;
use Illuminate\Support\Facades\Hash;
class TestBcryptSpillOver extends Command
{
    /**
     * The name and signature of the console command.
     *
     * @var string
     */
    protected $signature = 'bcrypt:test';

    /**
     * The console command description.
     *
     * @var string
     */
    protected $description = 'Test bcrypt spill over';

    /**
     * Execute the console command.
     */
    public function handle()
    {
        // Make our prefixes 72 characters long
        $my_hash_prefix = "some.very.long.prefix-"; // 22 characters
        $my_hash_key = "lnHQkcD7fZdMzJt640xbeOhzDGCI3aki7mePMxQgEQBkuUuwwm"; // 50 characters
        $super_secret_password = "thisIsMySuperSecretPassword";
        $random_password = "thisIsARandomPassword";

        $hash_one = $my_hash_prefix . $my_hash_key . $super_secret_password;
        $hash_two = $my_hash_prefix . $my_hash_key . $random_password;

        $computed_hash_one = Hash::make($hash_one);
        $computed_hash_two = Hash::make($hash_two);

        $this->info("Hash one: " . $computed_hash_one);
        $this->info("Hash two: " . $computed_hash_two);

        $this->info("Checking hashes for match");

        $check_one = Hash::check($hash_one, $computed_hash_one);
        $check_two = Hash::check($hash_two, $computed_hash_one);

        if($check_one && $check_two) {
            $this->info("Hashes match, bcrypt is vulnerable to spill over");
        } else {
            $this->info("Hashes do not match, bcrypt is not vulnerable to spill over");
        }
    }
}

Running this, should return us Hashes do not match, bcrypt is not vulnerable to spill over because $hash_two should not be the same as $computed_hash_one but it doesn't, it returns Hashes match, bcrypt is vulnerable to spill over because of the spill over.

Whats Changed?

This change intoduces a new config value hashing.bcrypt.limit_length in hashing.php to allow you to enable limiting the length of values passed to Hash::make() when using bcrypt.

Running the same code above after my change, throws an exception instead of silently failing.

Resources

1. https://trust.okta.com/security-advisories/okta-ad-ldap-delegated-authentication-username/
2. https://n0rdy.foo/posts/20250121/okta-bcrypt-lessons-for-better-apis/

[valorin]

The changes look good to me, although it would be a good idea to add a note in the docs about adding a max-length validation rule, or users will be hit with a 500 on super long passwords. Plus ensuring the starter kids have the max-length rule by default.

Also, I can see it causing problems with automatic password rehashing: if you bump your rounds and someone with a 72+ byte password logs in, I think it'll attempt to rotate/rehash, hit the limit, and error out.

[Synchro]

I made a PR to achieve the same thing last year, as part of default password validation but it was rejected on maintenance grounds.
With hindsight it would have been a good idea to include a test case to demonstrate Laravel’s default validation failing.

[Synchro]

While this does the job, adding a config property for this seems unnecessary, adding to maintenance overhead. There's no reason not to complain about excess password length, and bcrypt is the only supported password hash function that has input length problems, so hard-coding it shouldn't pose any real issues. Developers should be actively discouraged from suppressing security issues!

[valorin]

Developers should be actively discouraged from suppressing security issues!

Agreed, but I'm still worried about rehashing causing errors and locking folks with 72+ byte passwords out. It's going to be a pain to debug and the fix involves telling the user to pick a shorter password. 🙃 That's gonna be a strange conversation.

[Synchro]

Well, I'd say a better fix is to switch to Argon2ID, as then the problem just goes away.

You only need to enforce the length when setting a password; that will avoid locking anyone out on login. Max password length validations are quite common, though probably unnecessary if you're not using bcrypt. NIST recommends a minimum maximum (ha!) of 64 chars (note not bytes – with 4-byte UTF-8, bcrypt only allows 18 char passwords). But it's not too difficult to change because new passwords don't have any rehashing issues.

Much of this sort of thing came up recently in discussions surrounding WordPress "upgrading" from SHA1 to bcrypt, and a strong faction was in favour of using SHA256 + base64. There's also a problem with bcrypt passwords containing null chars; that's definitely something that should be in test cases. OWASP apparently wants bcrypt to die in a fire, so it seems prudent to push in the direction of Argon2ID as a default anyway, especially since Laravel no longer supports any version of PHP that doesn't have it.

To summarise, I think the best approach is to do a silent upgrade. bcrypt passwords are basically not a significant security risk, but you can transparently rehash everything to Argon2ID on login, and use it for new/changed/reset passwords. This way all new and active users get upgraded, and everyone else is not a major worry.

[valorin]

You only need to enforce the length when setting a password; that will avoid locking anyone out on login.

Not true. When the system identifies a need to rehash, it'll use the password provided for login and attempt to hash it. That'll trigger the exception on login.

OWASP apparently OWASP/CheatSheetSeries#1532 (comment), so it seems prudent to push in the direction of Argon2ID as a default anyway, especially since Laravel no longer supports any version of PHP that doesn't have it.

I'm aware of different reports that say Argon2 isn't actually more secure and bcrypt is still better: https://x.com/TerahashCorp/status/1155119064248913920
This is why PHP still defaults to bcrypt over Argon2.

I haven't really seen anything more recent than that which actually confirms it one way or the other though.

[Synchro]

Not true. When the system identifies a need to rehash, it'll use the password provided for login and attempt to hash it. That'll trigger the exception on login.

No, that's what I was suggesting you don't do. Say someone has a password longer than 72 bytes. Currently it will be bcrypt hashed, truncating it back to 72 bytes, and the verification will pass because of that limitation of bcrypt. Then we rehash it to argon and save to the DB, and the full length of the password will be used (because we have the full password on hand). Next time they log in, the problem no longer exists.

If you were, for some reason like changing work factor, rehashing bcrypt to bcrypt, then you would indeed trigger an exception, but that strikes me as an entirely self-inflicted problem?

Pretty much all the references I found that mention bcrypt > Argon2 refer exclusively to that single tweet and nothing else! Not a very solid basis to build crypto decisions on! A year or so later, the same author posted this more complete comment, effectively saying that in just a year, due to GPU performance improvements, the break point was now at 100ms, not 1s, and that was 4 years ago.

One alternative I've seen as a workaround for the bcrypt problem (this was mentioned several times in the WordPress thread) is to hash the raw password with SHA512 or 384 before giving it to bcrypt. This will weaken it somewhat, but not in a way that's cryptographically significant, and it completely avoids the 72-byte problem.

AFAIK, the only reason PHP has not changed the default to Argon2 is for BC, not any concern about strength.

Either way, the strength aspect makes no difference. Both algorithms are plenty strong enough — you're not going to brute-force either of them — but the 72-byte limit of bcrypt is massively more problematic than any other difference between the two, and is directly vulnerable to practical attacks because of it, as we saw with Okta. bcrypt also fails to meet NIST and FIPS minimum maximum length requirements, if that kind of compliance is a concern.

[valorin]

Not true. When the system identifies a need to rehash, it'll use the password provided for login and attempt to hash it. That'll trigger the exception on login.

No, that's what I was suggesting you don't do.

Agreed, however this PR has been merged and bcrypt is still default, so that is exactly what is going to happen.

If you were, for some reason like changing work factor, rehashing bcrypt to bcrypt, then you would indeed trigger an exception, but that strikes me as an entirely self-inflicted problem?

Only if you're aware of the new length limit and the risk that now exists if you change your work factor. Most folks wouldn't be aware of this, or may even forget, before they get to changing their workfactor.

Also, the work factor changed in 10, so there is the potential for folks upgrading from Laravel 9 -> 12 to be hit with this.

I don't think @taylorotwell tracks conversations on merged or closed PRs, so I might ping him directly.

Pretty much all the references I found that mention bcrypt > Argon2 refer exclusively to that single tweet and nothing else! Not a very solid basis to build crypto decisions on! A year or so later, the same author posted this more complete comment, effectively saying that in just a year, due to GPU performance improvements, the break point was now at 100ms, not 1s, and that was 4 years ago.

Nice find! I've been trying to find more information ever since this conversation started and haven't had much luck. At this point I'm pretty convinced moving to Argon2id is probably the best move. The question is, is there enough time before 12 is released to switch now, and do all modern PHP installations have libargon2 enabled? 🤔

AFAIK, the only reason PHP has not changed the default to Argon2 is for BC, not any concern about strength.

There are no doubt BC concerns with moving to Argon2, but the "bcrypt is better" tweets were also brought up in the discussion, so the idea didn't go any further. See: https://externals.io/message/120993

[Synchro]

Thanks. I don't know whether libargon2 is enabled by default (like libsodium is), but I've not encountered a stock packaged version that doesn't have it. However, that doesn't stop some less-enlightened shared hosting providers from disabling things in their custom PHP builds, just as they sometimes disable extension_loaded() 🤷. Frankly, I don't think we should be accommodating their misguided idiosyncrasies! Keep doing that, and we'll be in WordPress-land! Laravel has a history of dropping support (quite rightly) for entire PHP versions as they become obsolete, so it shouldn't come as any kind of surprise.

As of PHP 8.4, the argon2 implementation is provided by openssl rather than libargon2, making it more likely to be available, so perhaps we should expect argon2 to become the default when 8.4 is the oldest supported version.

At this point I'm also reminded of this hashing-related discussion I started over 4 years ago, but still hasn't moved!

[valorin]

As of PHP 8.4, the argon2 implementation is provided by openssl rather than libargon2, making it more likely to be available, so perhaps we should expect argon2 to become the default when 8.4 is the oldest supported version.

That would probably make the most sense, and be the easiest to sell. Bumping major versions and changing defaults.

At this point I'm also reminded of this hashing-related discussion I started over 4 years ago, but still hasn't moved!

I don't think those discussions are monitored by the team.

I agree getting rid of MD5 and SHA1 would be good. I think it can go hand-in-hand with adding other types of hashing to the Hash facade. So you can do passwords, HMACs, and raw hashes all through the facade. This way the default algos can be defined for the app, and easily used. I think that's why md5() and sha1() are still used everywhere. They are nice aliases while the better hashes are all hash('xyz', ...), which is much harder to remember.