Spring Security Spring Security Bcrypt with strength/log rounds = 31 results in 'Bad number of rounds' error although 31 should be ok

Describe the bug: I implement a webapp using Spring Boot and Spring Security. Passwords of my users are stored in my DB after encoding with Bcrypt (BCryptVersion $2a). I use Bcrypt of Spring Security with strength = 31 (new BCryptPasswordEncoder(31)). When I run my webapp and try to do a login with a user, I get the error Bad number of rounds.

Analysis: As far as I have seen this is because within org.springframework.security.crypto.bcrypt.BCrypt, method crypt_raw(...) roundsForLogRounds method is called with log_rounds = 31 and returns the value 2147483648, but this calculated value is greater than Integer.MAX_VALUE (=2147483647) and thus results in the mentioned error. See the following code in crypt_raw():

rounds = roundsForLogRounds(log_rounds);
if (rounds < 16 || rounds > Integer.MAX_VALUE) {
    throw new IllegalArgumentException("Bad number of rounds");
}

Expected behaviour: As far as I have seen in the JavaDoc and in the source of Spring Security, BCryptPasswordEncoder accepts a strength (= log rounds) between 4 (inclusive) and 31 (inclusive). So 31 should be ok and the resulting Bad number of rounds error is probably a bug.

Version Spring Boot v2.7.1 Spring v5.3.21 Spring Security v5.7.2

With Spring Boot v2.6.6 and Spring Security v5.6.2 strength = 31 worked as expected. Maybe the behaviour changed in commit https://github.com/spring-projects/spring-security/commit/e6297d3bf7daa215bf551dd67fc0ce7c28d5134a ?

Comment From: jzheaux

I agree that the check probably needs an adjustment, though I wonder if you really want the rounds to be set to 31 as this will cause each hash to take 2-3 days to compute on typical hardware.

Are you able to submit a PR that corrects the logic so that 31 rounds works again?

Comment From: sabinewinklerboi

@jzheaux Thanks for the hint regarding if 31 is really a meaningful value to use. I first did not question why the code was executed so fast (with the older version Spring Security v5.6.2) although 31 should lead to a computation of 2-3 days. 🙈 I will try to change it from 31 to a faster alternative (probably 12 so that the algorithm is maximally strong and encoding a pwd takes a reasonable time).

I'm new to Spring Security and new to contributing to open source projects so I would feel better if you could do the changes. Would that be ok?

I further analysed the code a little bit and as far as I have seen, if you want to support log_rounds = 31, probably the check of rounds > Integer.MAX_VALUE has to be removed and the variable i in the for loop has to be of type long instead of int.

rounds = roundsForLogRounds(log_rounds);
if (rounds < 16 || rounds > Integer.MAX_VALUE) {  // remove "rounds > Integer.MAX_VALUE" here?
    throw new IllegalArgumentException("Bad number of rounds");
}

for (int i = 0; i < rounds; i++) {  // use "long i" here?
    key(password, sign_ext_bug, safety);
    key(salt, false, safety);
}

Alternatively, from my point of view, it would also be possible to restrict strength/log_rounds to the range 4 (inclusive) to 30 (inclusive) to avoid the int overflows. But then you have to adapt the checks where the range 4 to 30 is checked and you have to adapt the samples/unit tests, JavaDoc, etc.

Comment From: jzheaux

For help with changing your existing hashes, please see https://github.com/spring-io/cve-2022-22976-bcrypt-skips-salt.

I'm new to Spring Security and new to contributing to open source projects so I would feel better if you could do the changes. Would that be ok?

Sure no problem.

Comment From: apatelWU

@jzheaux still getting same issue with spring security 5.7.2. Recently pulled the latest version of 5.7.2 but doesnt look like changes were pushed. Spring Security Spring Security Bcrypt with strength/log rounds = 31 results in 'Bad number of rounds' error although 31 should be ok

Comment From: jzheaux

@apatelWU, it was fixed in 5.7.3. If you update to the latest 5.7, you should be good.