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

Only recommand for php-sodium on >= PHP 7.4 #28332

Merged
merged 1 commit into from
Aug 31, 2021
Merged

Conversation

CarlSchwan
Copy link
Member

@CarlSchwan CarlSchwan commented Aug 6, 2021

This is because php-sodium will solve the missing PASSWORD_ARGON2I
constant problem only on >= php 7.4, previously argon2 wasn't part of
the standard extension and was disabled on Centos/RHEL.

So installing php-sodium on php 7.2 for example wouldn't hide the
message. Tested on Fedora with php 7.4, Centos 7 with php 7.2,
Centos 8 with php 7.2 and openSUSE with php 7.4.

Link for more info: https://lists.fedoraproject.org/archives/list/[email protected]/message/DPUS5BTCJD6IXREHJKMRK7JEFNSHKFSR/

Signed-off-by: Carl Schwan [email protected]

This is because php-sodium will solve the missing PASSWORD_ARGON2I
constant problem only on >= php 7.4, previously argon2 wasn't part of
the standard extension and was disabled on Centos/RHEL.

So installing php-sodium on php 7.2 for example wouldn't hide the
message. Tested on Fedora with php 7.4, Centos 7 with php 7.2,
Centos 8 with php 7.2 and openSUSE with php 7.4.

Signed-off-by: Carl Schwan <[email protected]>
@CarlSchwan CarlSchwan requested a review from juliusknorr August 6, 2021 12:27
@CarlSchwan CarlSchwan added the 3. to review Waiting for reviews label Aug 6, 2021
@CarlSchwan
Copy link
Member Author

/backport to stable20

@CarlSchwan
Copy link
Member Author

/backport to stable21

@CarlSchwan
Copy link
Member Author

/backport to stable22

@CarlSchwan
Copy link
Member Author

Fun @Conan-Kudo just told me that contrary to that 1 says, not even epel will provide php-sodium with PHP 7.4 :( It's a bit annoying and I added a comment to https://bugzilla.redhat.com/show_bug.cgi?id=1990844

@MichaIng
Copy link
Member

MichaIng commented Aug 7, 2021

Is the same true for PASSWORD_ARGON2ID or may it be available as successor when PASSWORD_ARGON2I is not?

Generally, before giving Argon2 too much weight, remember the discussion with its RFC author where it turned out to be less secure than bcrypt in the <1 seconds hashing time we usually want here. So an alternative would be to revert to bcrypt as default and drop all Argon2 related checks and warnings.

@szaimen szaimen added this to the Nextcloud 23 milestone Aug 31, 2021
Copy link
Contributor

@szaimen szaimen left a comment

Choose a reason for hiding this comment

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

LGTM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3. to review Waiting for reviews
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants