Skip to content
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

fix(argon2): respect max value for hashingThreads #45027

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

remicollet
Copy link

@remicollet remicollet commented Apr 25, 2024

Summary

When argon2 password hashing is provided bu sodium extension, threads is not supported, so value > 1 raise an exception

See https://github.com/php/php-src/blob/PHP-8.1.28/ext/sodium/sodium_pwhash.c#L65

Threads are only supported by standard extension (libargon2) and soon by openssl extension (with OpenSSL 3.2 and PHP 8.4)

Checklist

Signed-off-by: Remi Collet <remi@php.net>
@solracsf solracsf added the 3. to review Waiting for reviews label Apr 25, 2024
@solracsf solracsf added this to the Nextcloud 30 milestone Apr 25, 2024
@solracsf solracsf changed the title also respect max value for hashingThreads fix(argon2): respect max value for hashingThreads Apr 25, 2024
if (\defined('PASSWORD_ARGON2_PROVIDER')) {
// password_hash fails, when the minimum values are undershot or maximum overshot
// In this case, apply minimum/maximum.
if (PASSWORD_ARGON2_PROVIDER === 'sodium') {

Check failure

Code scanning / Psalm

TypeDoesNotContainType Error

'sodium' cannot be identical to 'standard'
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

obviously a false positive

$ php -r 'var_dump(PASSWORD_ARGON2_PROVIDER);'
string(6) "sodium"

This was referenced Jul 30, 2024
@Altahrim Altahrim mentioned this pull request Aug 5, 2024
@skjnldsv skjnldsv added 2. developing Work in progress stale Ticket or PR with no recent activity and removed 3. to review Waiting for reviews labels Aug 6, 2024
@Altahrim Altahrim mentioned this pull request Aug 7, 2024
@skjnldsv skjnldsv mentioned this pull request Aug 13, 2024
@skjnldsv skjnldsv modified the milestones: Nextcloud 30, Nextcloud 31 Aug 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2. developing Work in progress bug stale Ticket or PR with no recent activity
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants