)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":14250,"name":"Grzegorz Grasza","email":"xek@redhat.com","username":"xek"},"change_message_id":"1d2bc2dae0e305209e364a287251b495363e45b1","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"dbaaa11f_a55df668","updated":"2026-07-08 13:05:34.000000000","message":"There\u0027s a TOCTOU race condition (it\u0027s not the first one in the codebase, but let\u0027s start doing it properly). The auth reads the user in a read session (line 64), checks the password, closes the session, then the rehash opens a new write session to overwrite the hash.\nBetween those two sessions, an admin could reset the user\u0027s password:\n\n  1. Alice logs in with OldPass → read session loads hash(\"OldPass\") ✓\n  2. Admin resets Alice\u0027s password to NewPass → DB now has hash(\"NewPass\")\n  3. Alice\u0027s _check_password(\"OldPass\", user_ref) passes (using the stale read)\n  4. _rehash_password(alice_id, \"OldPass\") opens a new write session, re-fetches Alice, writes hash(\"OldPass\") unconditionally\n  5. DB now has hash(\"OldPass\") again — admin\u0027s reset is silently undone\n\nAlice can now log in with her old password despite the admin reset.\n\nThe fix is a compare-and-swap — only write the rehash if the stored hash hasn\u0027t changed since the auth check:\n\n  def _rehash_password(self, user_id, password, old_hash):\n      new_hash \u003d password_hashing.hash_password(password)\n      with sql.session_for_write() as session:\n          user_ref \u003d session.get(model.User, user_id)\n          if (user_ref is not None and user_ref.password_ref is not None\n                  and user_ref.password_ref.password_hash \u003d\u003d old_hash):\n              user_ref.password_ref.password_hash \u003d new_hash\n\nPass user_ref.password as old_hash from authenticate. The extra comparison is a cheap string equality check and eliminates the race entirely.\n\nThe rest of the patch looks correct — needs_rehash properly checks algorithm identity and PBKDF2 iteration count, and bypassing User.password setter to preserve history/expiry metadata is the right approach.","commit_id":"a6286af89a608455e10e3fb1036e1fc44a1cd8d9"},{"author":{"_account_id":35125,"name":"Mauricio Harley","email":"mharley@redhat.com","username":"mharley-rh"},"change_message_id":"296724ea545840107e1b70938683229eca05f3bd","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"df851509_c9ef732d","in_reply_to":"dbaaa11f_a55df668","updated":"2026-07-21 12:44:22.000000000","message":"Implemented the compare-and-swap you suggested: `_rehash_password()` now receives the `old_hash` from `authenticate()` and only writes the new hash if the stored hash hasn\u0027t changed. Added a test that simulates a concurrent admin password reset and verifies the rehash is skipped.","commit_id":"a6286af89a608455e10e3fb1036e1fc44a1cd8d9"},{"author":{"_account_id":34120,"name":"Andre Aranha","display_name":"afariasa","email":"afariasa@redhat.com","username":"afariasa"},"change_message_id":"f2287da9605f90b114f535918b2da8bd950e4453","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"63797278_c285f6f4","updated":"2026-07-22 12:38:09.000000000","message":"It looks good, I just have a few notes.","commit_id":"d5e25d8a890f4fc1239f388f1887cb679a063f50"}],"keystone/common/password_hashers/pbkdf2.py":[{"author":{"_account_id":34120,"name":"Andre Aranha","display_name":"afariasa","email":"afariasa@redhat.com","username":"afariasa"},"change_message_id":"f2287da9605f90b114f535918b2da8bd950e4453","unresolved":true,"context_lines":[{"line_number":63,"context_line":"            binascii.b2a_base64(salt).rstrip(b\"\u003d\\n\").decode(\"ascii\")"},{"line_number":64,"context_line":"        )"},{"line_number":65,"context_line":""},{"line_number":66,"context_line":"        return f\"$pbkdf2-sha512${rounds}${salt_str}${digest_str}\""},{"line_number":67,"context_line":""},{"line_number":68,"context_line":"    @staticmethod"},{"line_number":69,"context_line":"    def needs_rehash("}],"source_content_type":"text/x-python","patch_set":2,"id":"1b50f267_b04431c9","line":66,"updated":"2026-07-22 12:38:09.000000000","message":"That\u0027s not this patch change, so you may ignore this comment.\nMaybe would be better to reuse the ident (line 30) here instead of rewriting it again.","commit_id":"d5e25d8a890f4fc1239f388f1887cb679a063f50"},{"author":{"_account_id":35125,"name":"Mauricio Harley","email":"mharley@redhat.com","username":"mharley-rh"},"change_message_id":"d8a1fb7e966c4d5e2776a74ee41e917ee8f62616","unresolved":false,"context_lines":[{"line_number":63,"context_line":"            binascii.b2a_base64(salt).rstrip(b\"\u003d\\n\").decode(\"ascii\")"},{"line_number":64,"context_line":"        )"},{"line_number":65,"context_line":""},{"line_number":66,"context_line":"        return f\"$pbkdf2-sha512${rounds}${salt_str}${digest_str}\""},{"line_number":67,"context_line":""},{"line_number":68,"context_line":"    @staticmethod"},{"line_number":69,"context_line":"    def needs_rehash("}],"source_content_type":"text/x-python","patch_set":2,"id":"6c91885a_3e44ebae","line":66,"in_reply_to":"1b50f267_b04431c9","updated":"2026-07-29 10:14:30.000000000","message":"Good catch. Patchset 3 now uses `Sha512.ident` instead of the hardcoded string.","commit_id":"d5e25d8a890f4fc1239f388f1887cb679a063f50"},{"author":{"_account_id":34120,"name":"Andre Aranha","display_name":"afariasa","email":"afariasa@redhat.com","username":"afariasa"},"change_message_id":"f2287da9605f90b114f535918b2da8bd950e4453","unresolved":true,"context_lines":[{"line_number":74,"context_line":"        :param str hashed: Stored password hash."},{"line_number":75,"context_line":"        :param int current_rounds: Target iteration count (defaults to"},{"line_number":76,"context_line":"            DEFAULT_ROUNDS; callers should pass the operator-configured value)."},{"line_number":77,"context_line":"        :returns: True when the hash should be upgraded on next login."},{"line_number":78,"context_line":"        \"\"\""},{"line_number":79,"context_line":"        parts \u003d hashed[1:].split(\u0027$\u0027)"},{"line_number":80,"context_line":"        if len(parts) \u003d\u003d 4:"}],"source_content_type":"text/x-python","patch_set":2,"id":"2ea4ebcd_c511ec7e","line":77,"updated":"2026-07-22 12:38:09.000000000","message":"I believe at this point this method is more generic and not specific to login scenarios. What this method does is to check if the number of rounds is less them the current_rounds, so we should reflect also the method name to that, something like: does_hash_has_less_rounds_than()","commit_id":"d5e25d8a890f4fc1239f388f1887cb679a063f50"},{"author":{"_account_id":35125,"name":"Mauricio Harley","email":"mharley@redhat.com","username":"mharley-rh"},"change_message_id":"d8a1fb7e966c4d5e2776a74ee41e917ee8f62616","unresolved":false,"context_lines":[{"line_number":74,"context_line":"        :param str hashed: Stored password hash."},{"line_number":75,"context_line":"        :param int current_rounds: Target iteration count (defaults to"},{"line_number":76,"context_line":"            DEFAULT_ROUNDS; callers should pass the operator-configured value)."},{"line_number":77,"context_line":"        :returns: True when the hash should be upgraded on next login."},{"line_number":78,"context_line":"        \"\"\""},{"line_number":79,"context_line":"        parts \u003d hashed[1:].split(\u0027$\u0027)"},{"line_number":80,"context_line":"        if len(parts) \u003d\u003d 4:"}],"source_content_type":"text/x-python","patch_set":2,"id":"c9848892_971017c9","line":77,"in_reply_to":"2ea4ebcd_c511ec7e","updated":"2026-07-29 10:14:30.000000000","message":"I\u0027d prefer to keep `needs_rehash`. It\u0027s a well-established convention in password hashing libraries (passlib, bcrypt, argon2-cffi all use this name). The method also returns `True` when the hash is malformed, so it\u0027s not strictly about comparing round counts. Renaming it would break the convention without adding clarity.","commit_id":"d5e25d8a890f4fc1239f388f1887cb679a063f50"}],"keystone/common/password_hashing.py":[{"author":{"_account_id":34120,"name":"Andre Aranha","display_name":"afariasa","email":"afariasa@redhat.com","username":"afariasa"},"change_message_id":"f2287da9605f90b114f535918b2da8bd950e4453","unresolved":true,"context_lines":[{"line_number":128,"context_line":"    hash was computed with fewer iterations than the currently configured"},{"line_number":129,"context_line":"    (or default) value."},{"line_number":130,"context_line":""},{"line_number":131,"context_line":"    Callers should rehash transparently on the next successful"},{"line_number":132,"context_line":"    authentication."},{"line_number":133,"context_line":"    \"\"\""},{"line_number":134,"context_line":"    if not hashed:"}],"source_content_type":"text/x-python","patch_set":2,"id":"146dfafb_43ea3faf","line":131,"updated":"2026-07-22 12:38:09.000000000","message":"Same as in pbkdf2.py comment, except here it additionally checks if the hasher is the same or not.","commit_id":"d5e25d8a890f4fc1239f388f1887cb679a063f50"},{"author":{"_account_id":35125,"name":"Mauricio Harley","email":"mharley@redhat.com","username":"mharley-rh"},"change_message_id":"d8a1fb7e966c4d5e2776a74ee41e917ee8f62616","unresolved":false,"context_lines":[{"line_number":128,"context_line":"    hash was computed with fewer iterations than the currently configured"},{"line_number":129,"context_line":"    (or default) value."},{"line_number":130,"context_line":""},{"line_number":131,"context_line":"    Callers should rehash transparently on the next successful"},{"line_number":132,"context_line":"    authentication."},{"line_number":133,"context_line":"    \"\"\""},{"line_number":134,"context_line":"    if not hashed:"}],"source_content_type":"text/x-python","patch_set":2,"id":"02d479ff_1fc492cf","line":131,"in_reply_to":"146dfafb_43ea3faf","updated":"2026-07-29 10:14:30.000000000","message":"Same reasoning as above. The top-level `needs_rehash` checks algorithm identity and delegates iteration comparison to the hasher. Both levels follow the same naming convention.","commit_id":"d5e25d8a890f4fc1239f388f1887cb679a063f50"}],"keystone/tests/unit/common/test_password_hashing.py":[{"author":{"_account_id":34120,"name":"Andre Aranha","display_name":"afariasa","email":"afariasa@redhat.com","username":"afariasa"},"change_message_id":"f2287da9605f90b114f535918b2da8bd950e4453","unresolved":true,"context_lines":[{"line_number":120,"context_line":""},{"line_number":121,"context_line":"    def test_needs_rehash_algorithm_mismatch(self):"},{"line_number":122,"context_line":"        self.config_fixture.config("},{"line_number":123,"context_line":"            group\u003d\"identity\", password_hash_algorithm\u003d\"pbkdf2_sha512\""},{"line_number":124,"context_line":"        )"},{"line_number":125,"context_line":"        self.config_fixture.config(group\u003d\"identity\", max_password_length\u003d\"72\")"},{"line_number":126,"context_line":"        self.config_fixture.config("}],"source_content_type":"text/x-python","patch_set":2,"id":"9412c56b_b9c5026b","line":123,"updated":"2026-07-22 12:38:09.000000000","message":"I think we don\u0027t need this statement here, since it\u0027ll be be changed on the next lines without being used before that.","commit_id":"d5e25d8a890f4fc1239f388f1887cb679a063f50"},{"author":{"_account_id":35125,"name":"Mauricio Harley","email":"mharley@redhat.com","username":"mharley-rh"},"change_message_id":"d8a1fb7e966c4d5e2776a74ee41e917ee8f62616","unresolved":false,"context_lines":[{"line_number":120,"context_line":""},{"line_number":121,"context_line":"    def test_needs_rehash_algorithm_mismatch(self):"},{"line_number":122,"context_line":"        self.config_fixture.config("},{"line_number":123,"context_line":"            group\u003d\"identity\", password_hash_algorithm\u003d\"pbkdf2_sha512\""},{"line_number":124,"context_line":"        )"},{"line_number":125,"context_line":"        self.config_fixture.config(group\u003d\"identity\", max_password_length\u003d\"72\")"},{"line_number":126,"context_line":"        self.config_fixture.config("}],"source_content_type":"text/x-python","patch_set":2,"id":"6fa68e95_afff8899","line":123,"in_reply_to":"9412c56b_b9c5026b","updated":"2026-07-29 10:14:30.000000000","message":"You\u0027re right, removed in patchset 3. The first `pbkdf2_sha512` config was immediately overwritten by the bcrypt config two lines below.","commit_id":"d5e25d8a890f4fc1239f388f1887cb679a063f50"}]}