Skip to content

IBX-10124: Add support for Argon2 password hashes #581

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 our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 28 commits into
base: main
Choose a base branch
from

Conversation

glye
Copy link
Contributor

@glye glye commented Jun 5, 2025

🎫 Issue IBX-10124

Related PRs:

Description:

Add support for PASSWORD_ARGON2I and PASSWORD_ARGON2ID password hash types, which increase security over the standard bcrypt. Long enough bcrypt hashes are still considered secure, but there are some recommendations to upgrade to Argon2.

Before this PR, UserService::updateUserPassword() always goes with the current hashtype the user has. It seems that the code we had for updating hash type from md5 to bcrypt long ago was removed when we removed md5 support.

PR goals

  • Add PASSWORD_ARGON2I and PASSWORD_ARGON2ID support.
  • Add a default hash type to config. Previously there was only a default value in a constructor parameter.
  • Add two booleans to config: 1) for whether existing users should be upgraded to the default hash type on password change. 2) for whether existing users should be upgraded to the default hash type on login.
  • Whichever hash type set as default will be used for new users, same as before.
  • If configured (default: no) the hash type will be changed when the user changes their password.
  • If configured (default: no) the hash type will be changed when the user logs in, and changes in PHP's default hash options will take effect.
  • Changes in PHP's default hash options always take effect on password change, same as before.

Manual tests

Enabled by adding this config to ibexa.yaml ibexa:system:default: in v5.0.0:

            password_hash:
                default_type: 9
                update_type_on_change: true

Convenient query for verifying that hash types are updated:
select login, password_hash_type, substring(password_hash,1,62), from_unixtime(password_updated_at) as updated_at from ibexa_user;

  • New users: Works, they get the configured default hash type
  • Changing password: Works, hash type is upgraded when enabled
  • Logging in: Works, hash type is upgraded when enabled

The current test failures hint at a deeper problem with prioritizing Ibexa's CheckPassportEvent subscriber before the Symfony one. This is however required in order to upgrade the password hash on login, because the Symfony subscriber "eats" the password.

Performance

  • Upgrade on login adds a db write to login when the upgrade happens (once per user), but after that there are no extra db calls, just the password_needs_rehash() call.

Followup, not needed in this PR:

  • Consider if we should also expose the parameters for memory_cost, time_cost and threads, or rely on defaults. The current PHP defaults are well above the OWASP recommended minimum settings.
  • There is also the general "cost" parameter for bcrypt. The default is occasionally increased in new PHP releases. This only takes effect for new users, unless we implement updates for existing users as in the first point above.
  • Upgrade hash type on login, requires bigger changes like using the Symfony PasswordHasher features.

Read more

Documentation:

Docs that need an update:
https://doc.ibexa.co/en/latest/content_management/field_types/field_type_reference/userfield/#available-password-hash-types
https://doc.ibexa.co/en/latest/infrastructure_and_maintenance/security/security_checklist/#use-secure-password-hashing

@glye glye changed the title IBX-10124: Added support for Argon2I(D) password hashes IBX-10124: Add support for Argon2 password hashes Jun 6, 2025
@glye glye force-pushed the ibx10124_argon2_password_hashes branch from 29547bd to 5319d16 Compare June 27, 2025 14:34
@glye glye force-pushed the ibx10124_argon2_password_hashes branch from 7f82ac9 to 683231f Compare July 18, 2025 11:25
@glye glye requested review from konradoboza and Copilot July 24, 2025 12:25
Copilot

This comment was marked as outdated.

@glye glye force-pushed the ibx10124_argon2_password_hashes branch 9 times, most recently from f38f8ac to c3a42ad Compare July 30, 2025 17:12
@glye
Copy link
Contributor Author

glye commented Jul 31, 2025

@konradoboza How do you think it looks now? I removed the on-login code, and fixed other test failures that came up.

@konradoboza konradoboza requested a review from a team August 1, 2025 06:29
@ezrobot ezrobot removed the request for review from a team August 1, 2025 06:29
@glye glye force-pushed the ibx10124_argon2_password_hashes branch from 1e23d25 to 049ef92 Compare August 6, 2025 12:48
Copy link

@glye
Copy link
Contributor Author

glye commented Aug 13, 2025

2f60dd3 Now passing just the settings I need to the password hash service, not the whole $repositoryConfigurationProvider. Also doing this after construction to avoid circular dependency with the RepositoryFactory.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature New feature request Ready for review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants