Skip to content

Extend Argon2i support check to account for sodium_compat #25412

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

Merged
merged 1 commit into from
Dec 10, 2017
Merged

Extend Argon2i support check to account for sodium_compat #25412

merged 1 commit into from
Dec 10, 2017

Conversation

mbabker
Copy link
Contributor

@mbabker mbabker commented Dec 9, 2017

Q A
Branch? 3.4
Bug fix? yes
New feature? no
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets N/A
License MIT
Doc PR N/A

In the Argon2i password encoder, if in an environment where sodium_compat is installed without either natively running PHP 7.2 or the (lib)sodium extension, the isSupported check can return true because the library exposes the sodium_crypto_pwhash_str() function however a pure PHP implementation of the method is not implemented, so the library does not actually support the hashes.

paragonie/sodium_compat#55 requested a way to check support through the polyfill to avoid this condition and the 1.4 release added it. This PR extends the encoder's isSupported check to be aware of the sodium_compat library and use its support check if able to avoid misreporting that sodium_crypto_pwhash_str() is available for use when it isn't.

return true;
}

if (\class_exists('\\ParagonIE_Sodium_Compat') && \method_exists('\\ParagonIE_Sodium_Compat', 'crypto_pwhash_is_available')) {
Copy link
Member

Choose a reason for hiding this comment

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

you can remove the \\ in the strings

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

@chalasr
Copy link
Member

chalasr commented Dec 10, 2017

Thanks for fixing this bug @mbabker.

@chalasr chalasr merged commit 95c1fc8 into symfony:3.4 Dec 10, 2017
chalasr pushed a commit that referenced this pull request Dec 10, 2017
…(mbabker)

This PR was merged into the 3.4 branch.

Discussion
----------

Extend Argon2i support check to account for sodium_compat

| Q             | A
| ------------- | ---
| Branch?       | 3.4
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | N/A
| License       | MIT
| Doc PR        | N/A

In the Argon2i password encoder, if in an environment where `sodium_compat` is installed without either natively running PHP 7.2 or the (lib)sodium extension, the `isSupported` check can return true because the library exposes the `sodium_crypto_pwhash_str()` function however a pure PHP implementation of the method is not implemented, so the library does not actually support the hashes.

paragonie/sodium_compat#55 requested a way to check support through the polyfill to avoid this condition and the 1.4 release added it.  This PR extends the encoder's `isSupported` check to be aware of the `sodium_compat` library and use its support check if able to avoid misreporting that `sodium_crypto_pwhash_str()` is available for use when it isn't.

Commits
-------

95c1fc8 Extend Argon2i support check to account for sodium_compat
@mbabker mbabker deleted the 3.4-argon-check branch December 10, 2017 20:44
This was referenced Dec 15, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants