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

PHP 7.2 and libsodium #8863

Closed
Adrien-H opened this issue Dec 6, 2017 · 51 comments
Closed

PHP 7.2 and libsodium #8863

Adrien-H opened this issue Dec 6, 2017 · 51 comments
Labels

Comments

@Adrien-H
Copy link

Adrien-H commented Dec 6, 2017

I am currently deploying a Symfony 4 application, which uses the Argon2i algorithm for password hashing. PHP 7.2 natively implements the libsodium library. Of course, PHP 7.2 is mandatory in this build and everything is fine on my locale machine with 7.2.0. Application running, tests OK, etc.

But the build fails while deploying.

I have this in my Travis file:

# .travis.yml
language: php
php:
  - '7.2'

Here is the part of the build logs where PHP is being successfuly installed:

$ phpenv global 7.2 2>/dev/null
7.2 is not pre-installed; installing
Downloading archive: https://s3.amazonaws.com/travis-php-archives/binaries/ubuntu/14.04/x86_64/php-7.2.tar.bz2
12.26s$ curl -s -o archive.tar.bz2 $archive_url && tar xjf archive.tar.bz2 --directory /
0.01s
0.02s$ phpenv global 7.2
0.60s$ composer self-update
You are already using composer version 1.5.5 (stable channel).
$ php --version
PHP 7.2.0 (cli) (built: Dec  2 2017 17:12:55) ( ZTS )
Copyright (c) 1997-2017 The PHP Group
Zend Engine v3.2.0, Copyright (c) 1998-2017 Zend Technologies
    with Zend OPcache v7.2.0, Copyright (c) 1999-2017, by Zend Technologies
    with Xdebug v2.6.0-dev, Copyright (c) 2002-2017, by Derick Rethans

But after the composer install, the build fails with this message:

Executing script cache:clear [KO]
 [KO]
Script cache:clear returned with error code 1
!!  
!!  In SecurityExtension.php line 509:
!!                                                                                 
!!    Argon2i algorithm is not supported. Please install the libsodium extension   
!!    or upgrade to PHP 7.2+.                                                      
!!                                                                                 
!!  
!!  
The command "composer install" failed and exited with 1 during .
Your build has been stopped.

The message from Symfony is pretty clear, I already saw it while testing with PHP 7.1. But it should work with 7.2, I cannot come to understand what is happening.

Any idea?

@BanzaiMan
Copy link
Contributor

Is it a compile-time configuration? Or is it supposed to be enabled by default?

@BanzaiMan BanzaiMan added the php label Dec 6, 2017
@Adrien-H
Copy link
Author

Adrien-H commented Dec 6, 2017

I was locally using a pre-configured PHP, so to be sure I just compiled PHP 7.2 from sources.

$ ./php -v
PHP 7.2.0 (cli) (built: Dec  6 2017 15:26:29) ( NTS )
Copyright (c) 1997-2017 The PHP Group
Zend Engine v3.2.0, Copyright (c) 1998-2017 Zend Technologies

Then I tryed to use the ARGON2I algorithm such as described in official docs:

$ ./php -r 'echo password_hash("test", PASSWORD_ARGON2I) . "\n";'
Warning: Use of undefined constant PASSWORD_ARGON2I - assumed 'PASSWORD_ARGON2I' (this will throw an Error in a future version of PHP) in Command line code on line 1

Warning: password_hash() expects parameter 2 to be integer, string given in Command line code on line 1

While not having any problem with BCRYPT:

$ ./php -r 'echo password_hash("test", PASSWORD_BCRYPT) . "\n";'
$2y$10$wsWe3BhyzenVqDs6JV/fPOB0XKh0oTuGdrgLp61MnUPzOUdw4jZey

This is strange. I would have expected this algorithm to be part of the default PHP 7.2 build, just as other hash algorithms. And nothing seems to indicate the opposite in the docs. I'll investigate. Maybe I understood something wrong... but this looks like a bug to me, since they say here that PASSWORD_ARGON2I is part of PHP core.

@BanzaiMan
Copy link
Contributor

For the record, configuration options used for compiling 7.2.0 can be found here: https://travis-ci.org/travis-ci/php-src-builder/jobs/310545936#L631

@mpiot
Copy link

mpiot commented Dec 7, 2017

It appear it's an option, we need to compile PHP with this option: --with-password-argon2 (sources: https://wiki.php.net/rfc/argon2_password_hash)

I suppose, we can't personnalize it in our Travis configuation ?
Options are in https://github.com/travis-ci/php-src-builder default_configure_options ? (I think it's the same for all PHP versions ? Then we can't add it in this files, no ?)

@Adrien-H
Copy link
Author

Adrien-H commented Dec 8, 2017

In case of Symfony 4... I did not try yet and intend to let you know when done, but for now I guess the problem can be skirted by using a "travis" environment extending "test" and defining:

# config/packages/travis/security.yaml
security:
    encoders:
        Symfony\Component\Security\Core\User\User:
            # algorithm: 'argon2i'
            algorithm: 'bcrypt'

This would get my DataFixtures work (just using Bcrypt instead of Argon2 is no big deal for tests purposes). This is however a plaster and won't get the job done for everyone.

I agree with @mpiot about the need of being able to configure PHP build. Is there any way?

@mpiot
Copy link

mpiot commented Dec 8, 2017

An idea is to use the method mensionned in https://symfony.com/blog/new-in-symfony-3-4-argon2i-password-hasher, by using the libsodium-php library (when we haven't PHP 7.2), but just in Travis:

before_install:
    # Fix Argon2i password hasher in TravisCi PHP version
    - composer require paragonie/sodium_compat

Maybe it works well :-)

@Adrien-H
Copy link
Author

Adrien-H commented Dec 8, 2017

Better solution than mine, indeed. (:

@mpiot
Copy link

mpiot commented Dec 15, 2017

Because Symfony has been updated, the previous solution no longer works, then I choose to directly add the libsodium extension with PECL. This method is better I think, because we install and enable the PHP extension.

We must download sources of the libsodium library because ubuntu 14.04 haven't the library, then compile it, and compile the PHP extension with pecl, and enable it.

It works well, but it take more time than the previous solution.

before_install:
  # Manually compile the libsodium library
  - sudo apt-get update -qq
  - sudo apt-get install build-essential git -y
  - git clone -b stable https://github.com/jedisct1/libsodium.git
  - cd libsodium && sudo ./configure && sudo make check && sudo make install && cd ..

install:
  # Manually install libsodium, because the TravicCi image doesn't provide PHP7.2 with libsodium
  - pecl install libsodium
  - echo "extension=sodium.so" >> ~/.phpenv/versions/$(phpenv version-name)/etc/php.ini

Edit like said by BanzaiMan:

before_install:
  # Manually compile the libsodium library
  - git clone -b stable https://github.com/jedisct1/libsodium.git
  - cd libsodium && sudo ./configure && sudo make check && sudo make install && cd ..

install:
  # Manually install libsodium, because the TravicCi image doesn't provide PHP7.2 with libsodium
  - pecl install libsodium
  - echo "extension=sodium.so" >> ~/.phpenv/versions/$(phpenv version-name)/etc/php.ini

@BanzaiMan
Copy link
Contributor

You don't need to run apt-get update nor apt-get install, as those packages should be present on the image.

Since the run-time configuration seems possible, can we close this issue at this time?

@Adrien-H
Copy link
Author

Having PHP 7.2 compiled with the core libsodium would be a better solution than having to install the PECL extension, which takes some time. But this solution gets the job done for our needs, so I guess the issue an be closed for now. (:

@BanzaiMan
Copy link
Contributor

How long does it take to compile?

@Adrien-H
Copy link
Author

before_install:
  - cd libsodium && sudo ./configure && sudo make check && sudo make install && cd ..
  - '[[ "$TRAVIS_PHP_VERSION" == "nightly" ]] || phpenv config-rm xdebug.ini'
install:
  - pecl install libsodium
  - echo "extension=sodium.so" >> ~/.phpenv/versions/$(phpenv version-name)/etc/php.ini

I just stopwatched, this part takes about 1'30''.

@Nenglish7
Copy link

Argon2i has to be enabled during build, Add Argon2i (v1.3) support in the next PHP 7.x (7.2) via –with-password-argon2.
https://wiki.php.net/rfc/argon2_password_hash

@mpiot
Copy link

mpiot commented Jan 10, 2018

Maybe we can do a test on the PHP version in the .travis.yml, then if it's a PHP > 7.2 add commands to add the option, because we can't add the option in default_configure_options files, there are used by every PHP version.

@BanzaiMan I don't really know how it works... What is the variable tha contain the PHP version ? (in https://github.com/travis-ci/php-src-builder, Because $VERSION is always master (or do you defined PHP with $VERSION manually ?), and php always 5.6 defined in matrix)
I've prepared a script that test if version is > to 7.2, install libsodium, compile it, and edit default_configure_options by adding the line --with-password-argon2=path/where/lisodium/is, I just need a way to get the PHP version to do the test.

@BanzaiMan
Copy link
Contributor

@mpiot VERSION is overridden by the API build request payload. You can see this in, for example, https://travis-ci.org/travis-ci/php-src-builder/jobs/326899986#L482.

@BanzaiMan
Copy link
Contributor

Is it acceptable to assume that this hashing algorithm is (almost) universally available in production environments?

@Condebk717
Copy link

You mean my wallet hashing?

@BanzaiMan
Copy link
Contributor

@Condebk717 Could you elaborate? I have no idea what wallet you are talking about.

@Condebk717
Copy link

Was clear on your comment mr banzaaiMan

@Condebk717
Copy link

Sorry I misunderstood the conversation my friend

@Condebk717
Copy link

Condebk717 commented Jan 10, 2018 via email

@Adrien-H
Copy link
Author

Adrien-H commented Jan 10, 2018

@BanzaiMan >

Most "modern" applications still use BCrypt, but Argon2 is expected to become a new standard in the PHP ecosystem. Even Symfony implemented this hash as core encoder and recommands using it now.

https://symfony.com/blog/new-in-symfony-3-4-argon2i-password-hasher

In existing applications using Bcrypt with a reasonably high hashing cost, there's no immediate need to rehash all the passwords using Argon2i. However, if you are creating a new project, you could consider using this new password hashing algorithm.

I think indeed it would not be a bad move to make it universally available. This is only my opinion, though.

@mpiot
Copy link

mpiot commented Jan 10, 2018

@BanzaiMan I agree with Adrien-H, Argon2 is a new way to hash password in database with PHP. For the Symfony Framework, it's the new BestPractice.

I've do something in a fork, if it can help: https://github.com/mpiot/php-src-builder/commit/a6d5349643fd7780dd33d0f806124c68b8e0eb59
I think, we haven't to PR on travis-ci/php-src-builder.

@paragonie-scott
Copy link

This adds a lot of overhead to the CI of most of the projects I develop because libsodium has to be compiled/etc. from scratch.

Can we please get a new PHP 7.2 that includes libsodium 1.0.15 and is compiled with ext/sodium support?

@codemasher
Copy link

pretty please with sugar on top https://travis-ci.org/chillerlan/php-oauth/jobs/334467006

@Nenglish7
Copy link

When can we get this fixed?

@tangrufus
Copy link

For those who having trouble compiling libsodium, you can install php 7.2 via ppa:ondrej/php.
Example: https://github.com/TypistTech/wp-password-argon-two/pull/8/files

@rayward
Copy link

rayward commented Feb 14, 2018

This was sufficient for me to get sodium working on PHP 7.0 and 7.2:

before_install:
  - sudo add-apt-repository ppa:ondrej/php -y
  - sudo apt-get -qq update
  - sudo apt-get install -y libsodium-dev
install:
  - printf "\n" | pecl install libsodium

@codemasher
Copy link

This is what i (and probably others, too) did before, but it shouldn't be necessary for PHP 7.2 as libsodium is part of it and should be enabled by default.

@codemasher
Copy link

codemasher commented Apr 6, 2018

At this point it feels like someone is deliberately breaking the package chain and taunting all of us. Isn't there anyone who can poke the right people to fix this? It's astounding that we're already 4 releases into php 7.2 and the linux builds for the major distros are still broken. Meanwhile, the official windows php builds work perfectly fine (and libsodium for php < 7.2/win supports even the old and new sodium syntax...).

image

@Jean85
Copy link

Jean85 commented May 8, 2018

I've do something in a fork, if it can help: mpiot/php-src-builder@a6d5349

@mpiot I've opened a PR starting from your fork, thanks!

travis-ci/php-src-builder#20

@mpiot
Copy link

mpiot commented May 8, 2018

@Jean85 You're welcome, thank you too :-) If they accept it, it would be a good thing :-)

@Auvipev
Copy link

Auvipev commented May 17, 2018

This is annoying please update your images.

@tristanbes
Copy link

+1 📦

@stale
Copy link

stale bot commented Sep 11, 2018

Thanks for contributing to this issue. As it has been 90 days since the last activity, we are automatically closing the issue in 24 hours. This is often because the request was already solved in some way and it just wasn't updated or it's no longer applicable. If that's not the case, please do feel free to either reopen this issue or open a new one. We'll gladly take a look again! You can read more here: https://blog.travis-ci.com/2018-03-09-closing-old-issues

@stale stale bot added the stale label Sep 11, 2018
@paragonie-scott
Copy link

Bad bot

@stale stale bot removed the stale label Sep 11, 2018
@Jean85
Copy link

Jean85 commented Sep 17, 2018

travis-ci/php-src-builder#20 got merged! 🎉

@Majkl578
Copy link

Majkl578 commented Oct 13, 2018

This issue should be resolved by travis-ci/php-src-builder#22 (fixed travis-ci/php-src-builder#20).

@stale
Copy link

stale bot commented Jan 12, 2019

Thanks for contributing to this issue. As it has been 90 days since the last activity, we are automatically closing the issue in 7 days. This is often because the request was already solved in some way and it just wasn't updated or it's no longer applicable. If that's not the case, please respond before the issue is closed, or open a new one after. We'll gladly take a look again! You can read more here: https://blog.travis-ci.com/2018-03-09-closing-old-issues

@stale stale bot added the stale label Jan 12, 2019
@paragonie-scott
Copy link

Calling it now: @Stale is probably the least helpful contributor in this thread, present and future.

@stale stale bot removed the stale label Jan 12, 2019
@Jean85
Copy link

Jean85 commented Jan 14, 2019

Does travis-ci/php-src-builder#22 completely solve this? Are we missing something else?

@paragonie-scott
Copy link

https://travis-ci.org/paragonie/halite/builds/479489381 -- if this has no build errors, then this is completely solved.

@Jean85
Copy link

Jean85 commented Jan 14, 2019

Green!! 👍

@BanzaiMan
Copy link
Contributor

Closing

@tuupola
Copy link

tuupola commented Nov 5, 2019

Apparently you need to add dist: trusty to .travis.yml for this to work. Xenial still fails.

@BanzaiMan
Copy link
Contributor

@tuupola Can you try a newer version?

@tuupola
Copy link

tuupola commented Nov 5, 2019

@BanzaiMan If you mean newer version of PHP then yes PHP 7.3 and PHP 7.4snapshot do not have problems. However I had the builds fail with PHP 7.2 unless I use the dist: trusty.

@BanzaiMan
Copy link
Contributor

I meant the more recent teeny releases of 7.2; e.g., 7.2.21. My guess is that the pre-installed 7.2 on the current Xenial image is too old to have this, and forcing the run-time installation of the newer version might solve this problem for you.

@tuupola
Copy link

tuupola commented Nov 5, 2019

Yeah that is possible. In any case adding the dist: trusty to .travis.yml solved problem for me. Just wrote a note here for others having the same problem to save some debugging time. Took a while to figure it out.

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

No branches or pull requests