Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork9.6k
[Security] Support hashing the hashed password using crc32c when putting the user in the session#59562
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
e45fa9e
tob8f77ae
Comparesrc/Symfony/Component/Security/Core/Tests/Authentication/Token/Fixtures/CustomUser.php OutdatedShow resolvedHide resolved
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
src/Symfony/Component/Security/Http/Tests/Firewall/AccessListenerTest.php OutdatedShow resolvedHide resolved
Uh oh!
There was an error while loading.Please reload this page.
src/Symfony/Component/Security/Http/Tests/Firewall/ContextListenerTest.php OutdatedShow resolvedHide resolved
Uh oh!
There was an error while loading.Please reload this page.
46faf11
tod1ee5c4
Compared1ee5c4
to53a1aab
Compare53a1aab
to2dcb3d9
ComparePR ready /cc @symfony/mergers |
9cab9e1
to7f7247b
Compare(PR rebased) |
Uh oh!
There was an error while loading.Please reload this page.
7f7247b
to05f606b
Compare8243e9a
tof6742c6
CompareThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
LGTM. Just a question in tests.
Uh oh!
There was an error while loading.Please reload this page.
@@ -303,7 +307,7 @@ private static function hasUserChanged(UserInterface $originalUser, TokenInterfa | |||
return true; | |||
} | |||
if ($originalUser instanceof LegacyPasswordAuthenticatedUserInterface && $refreshedUser instanceof LegacyPasswordAuthenticatedUserInterface && $originalUser->getSalt() !== $refreshedUser->getSalt()) { | |||
if ($originalUser instanceof LegacyPasswordAuthenticatedUserInterface && $originalUser->getSalt() !== $refreshedUser->getSalt()) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
The psalm false-positive is reportedvimeo/psalm#9956
Uh oh!
There was an error while loading.Please reload this page.
…ing the user in the session
f6742c6
toa4f8a76
CompareThank you@nicolas-grekas. |
802940e
intosymfony:7.3Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
This PR builds on#59539 following the discussion that happened there.
Here is what I added to PasswordAuthenticatedUserInterface to explain what this PR enables:
crc32c is selected because its probability to change when the password hash changes is high, so that the invalidation of sessions is effective. But it's also selected because there are many possible valid password hashes that generate the same crc32c. This protects against brute-forcing the password hash: let's say one is able to find a password hash that has the same crc32c as the real password hash: one would still be unable to confirm that this password hash is the correct one. To do so, they would have to brute-force the password hash itself, and this would require brute-forcing bcrypt et al. The cost of doing so on one bcrypted-password is already prohibitive. Doing so with a very high number of possible candidates (as collisions are generated) would be even more prohibitive.
Note that to generate a collision, one just needs to generate a random string that's formatted as a real hash, like this line for a bcrypted-password:
(one could likely create a crc32c-aware collision generator for this purpose, but that wouldn't reduce the difficulty of validating the generated hashes).
On the contrary, using a more collision resistant hashing algorithm would make it too easy to validate that a generated hash is the real hash.