chore: support firebase/php-jwt v6 #217
Comments
|
It seems |
|
This feature request becomes more important with the recent discovery of CVE-2021-46743 so it may be wise to drop support for previous versions but that would be a breaking change. This is a non-trivial rework of how multiple algorithms are handled, along with changes in behaviour as it now throws more exceptions. |
|
CVE-2021-46743 is not a vulnerability per se but a footgun if user against all advice enables both |
|
I had to look up what a footgun is 😆 I have taken a stab at upgrading to the recommended way. (branch) but this isn't perfect. |
|
Would dropping support for older versions be a BC break? Composer will just prevent the consumer from upgrading when using non supported versions of How much are older versions of |
|
No, dropping support for old versions of dependencies is not a BC break. It seem currently |
|
It would be great to see some movement on this. While I understand that the bug is avoidable through configuration, the presence of this package in a composer.json file within a Docker image will cause CVE alerts if using scanning tools (e.g. docker scan/AWS ECR scanning). Are there any plans to apply the patch given above (#217 (comment)) or similar to the project? I'd be happy to have a look at providing a patch myself if the team don't have capacity to deal with it right now, but don't want to duplicate any ongoing work. Thanks. |
slim-jwt-auth currently does not support firebase/php-jwt 6. This raise critical security alerts when our containers are scanned (namely CVE-2021-46743). There is an issue on the slim-jwt-auth repo for this, which I have commented on, asking for further info: tuupola/slim-jwt-auth#217 In the meantime, I have taken a copy of the key part of that package and reworked it slightly so that it functions with php-jwt v6. I also removed a lot of the options we're not using. The existing middleware which functions with slim-jwt-auth is commented out but still present. To reinstate that package, we just need to remove this commit.
slim-jwt-auth currently does not support firebase/php-jwt 6. This raise critical security alerts when our containers are scanned (namely CVE-2021-46743). There is an issue on the slim-jwt-auth repo for this, which I have commented on, asking for further info: tuupola/slim-jwt-auth#217 In the meantime, I have taken a copy of the key part of that package and reworked it slightly so that it functions with php-jwt v6. I also removed a lot of the options we're not using. The existing middleware which functions with slim-jwt-auth is commented out but still present. To reinstate that package, we just need to remove this commit.
slim-jwt-auth currently does not support firebase/php-jwt 6. This raise critical security alerts when our containers are scanned (namely CVE-2021-46743). There is an issue on the slim-jwt-auth repo for this, which I have commented on, asking for further info: tuupola/slim-jwt-auth#217 In the meantime, I have taken a copy of the key part of that package and reworked it slightly so that it functions with php-jwt v6. I also removed a lot of the options we're not using. The existing middleware which functions with slim-jwt-auth is commented out but still present. To reinstate that package, we just need to remove this commit.
slim-jwt-auth currently does not support firebase/php-jwt 6. This raise critical security alerts when our containers are scanned (namely CVE-2021-46743). There is an issue on the slim-jwt-auth repo for this, which I have commented on, asking for further info: tuupola/slim-jwt-auth#217 In the meantime, I have taken a copy of the key part of that package and reworked it slightly so that it functions with php-jwt v6. I also removed a lot of the options we're not using. The existing middleware which functions with slim-jwt-auth is commented out but still present. To reinstate that package, we just need to remove this commit.
slim-jwt-auth currently does not support firebase/php-jwt 6. This raise critical security alerts when our containers are scanned (namely CVE-2021-46743). There is an issue on the slim-jwt-auth repo for this, which I have commented on, asking for further info: tuupola/slim-jwt-auth#217 In the meantime, I have taken a copy of the key part of that package and reworked it slightly so that it functions with php-jwt v6. I also removed a lot of the options we're not using. The existing middleware which functions with slim-jwt-auth is commented out but still present. To reinstate that package, we just need to remove this commit.
|
One of my pet peeves is CVE scanners which blindly check for version numbers but not if code is actually vulnerable. That said it is a good idea to upgrade the firebase/php-jwt and silent CVE scanners are preferred. Currently for me paid work comes first so I have not been hurrying this too much. Changes in #217 (comment) do look good, but I am not sure if it breaks BC because tests were changed. I will test that soon. |
|
Agreed on blind scanning, but unfortunately it's ingrained in our culture here. I'm not sure I can get them turned off :) I appreciate you have plenty of other pressures, and am grateful for the swift response. In the meantime, I've suggested using a stripped down forked version of your code which works in our situation. I'll see what the rest of the team think. As soon as your very useful library is upgraded, we'll switch back to that and ditch our work-around. Thanks again. |
|
I know. Just last week I had to spend considerable time explaining a corporate that CentOS 7 servers are not vulnerable even when their CVE scanner warns about "vulnerable" versions of OpenSSH. To maintain stability server oriented distros patch their software instead of upgrading. In any case I do not think it is possible to upgrade to latest $app->add(new Tuupola\Middleware\JwtAuthentication([
"secret" => "supersecretkeyyoushouldnotcommittogithub"
]))$app->add(new Tuupola\Middleware\JwtAuthentication([
"secret" => [
"acme" =>"supersecretkeyyoushouldnotcommittogithub",
"beta" =>"anothersecretkeyfornevertocommittogithub"
]
]))Or you could if you hardcode a default algrorithm like I did in a proof of concept here #223. This will however break BC for anyone who was not using the default algorithm. So I probably bump the major version number and do some other BC breaking changes that have been in my todo list. |
|
It's slightly challenging to do this upgrade, but I'm more than willing to contribute in anyway I can. |
* Remove unnecessary files from service-front docker image * node_modules and the package.* files are only used for building the JS, so remove them * Remove any test files * Remove any build-enabling files This has the benefits of making the image smaller, at the same time as improving security and reducing bogus security alerts (e.g. alerts about nodejs libraries which don't have any effect at runtime). * Remove unnecessary files from service-api docker image * Remove build-related and documentation-related files * Remove test cases * Exclude scripts which aren't used at runtime This removes potential security issues caused by unused components. We keep the composer.lock as security scans use this to decide whether there are vulnerabilities in our images. * Update to latest Notify client This upgrades the firebase/php-jwt library to a secure version. * Remove unnecessary files from service-admin * Tests must be in image for unit testing in CI * Replace tuupola/slim-jwt-auth with our fork slim-jwt-auth currently does not support firebase/php-jwt 6. This raise critical security alerts when our containers are scanned (namely CVE-2021-46743). There is an issue on the slim-jwt-auth repo for this, which I have commented on, asking for further info: tuupola/slim-jwt-auth#217 In the meantime, I have taken a copy of the key part of that package and reworked it slightly so that it functions with php-jwt v6. I also removed a lot of the options we're not using. The existing middleware which functions with slim-jwt-auth is commented out but still present. To reinstate that package, we just need to remove this commit. Co-authored-by: William Falconer <williamfalconeruk@users.noreply.github.com>
Once the issue with slim-jwt-auth is fixed (see tuupola/slim-jwt-auth#217), we can reinstate that library as the JWT solution as follows: * Add tuupola/slim-jwt-auth back into composer.json * Remove tuupola/http-factory and tuupola/callable-handler (these are dependencies of tuupola/slim-jwt-auth and will be installed by that library; we no longer need to reference them directly once slim-jwt-auth is back in place) * Uncomment body of App\Middleware\Session\JwtAuthenticationFactory class * Remove App\Middleware\Session\JwtMiddleware file * Remove App\Middleware\Session\JwtMiddlewareFactory file * Uncomment lines 67-68 and remove lines 69-70 of ConfigProvider.php For more context, see commit 3efaa3b in this repo.
Once the issue with slim-jwt-auth is fixed (see tuupola/slim-jwt-auth#217), we can reinstate that library as the JWT solution as follows: * Add tuupola/slim-jwt-auth back into composer.json * Remove tuupola/http-factory and tuupola/callable-handler (these are dependencies of tuupola/slim-jwt-auth and will be installed by that library; we no longer need to reference them directly once slim-jwt-auth is back in place) * Uncomment body of App\Middleware\Session\JwtAuthenticationFactory class * Remove App\Middleware\Session\JwtMiddleware file * Remove App\Middleware\Session\JwtMiddlewareFactory file * Uncomment lines 67-68 and remove lines 69-70 of ConfigProvider.php For more context, see commit 3efaa3b in this repo.
Once the issue with slim-jwt-auth is fixed (see tuupola/slim-jwt-auth#217), we can reinstate that library as the JWT solution as follows: * Add tuupola/slim-jwt-auth back into composer.json * Remove tuupola/http-factory and tuupola/callable-handler (these are dependencies of tuupola/slim-jwt-auth and will be installed by that library; we no longer need to reference them directly once slim-jwt-auth is back in place) * Uncomment body of App\Middleware\Session\JwtAuthenticationFactory class * Remove App\Middleware\Session\JwtMiddleware file * Remove App\Middleware\Session\JwtMiddlewareFactory file * Uncomment lines 67-68 and remove lines 69-70 of ConfigProvider.php For more context, see commit 3efaa3b in this repo.
…965) * Add psalm as service-admin dev dependency * Update service-admin psalm config to match other components * First pass of level 4 psalm check * Implement simplified LoggerTrait The MakeLogger LoggerTrait is designed to work with laminas-mvc apps and will not work correctly in service-admin (which is not a laminas-mvc app). Implement a simple alternative logger which can be used within service-admin as a drop-in replacement for MakeLogger. As we are not so concerned about logging trace IDs here, we can resort to logging errors directly and not as JSON. If this is unacceptable, we will have to consider rewriting the MakeLogger code so we can output JSON in non-laminas-mvc apps. The code in this commit improves things, as at the moment I would expect logging to fail completely for service-admin due to missing laminas-mvc dependencies. * Second pass of pass of level 4 psalm check * Comment out currently-unused class Once the issue with slim-jwt-auth is fixed (see tuupola/slim-jwt-auth#217), we can reinstate that library as the JWT solution as follows: * Add tuupola/slim-jwt-auth back into composer.json * Remove tuupola/http-factory and tuupola/callable-handler (these are dependencies of tuupola/slim-jwt-auth and will be installed by that library; we no longer need to reference them directly once slim-jwt-auth is back in place) * Uncomment body of App\Middleware\Session\JwtAuthenticationFactory class * Remove App\Middleware\Session\JwtMiddleware file * Remove App\Middleware\Session\JwtMiddlewareFactory file * Uncomment lines 67-68 and remove lines 69-70 of ConfigProvider.php For more context, see commit 3efaa3b in this repo. * Update to PHP 8.1 in composer config * Set a default form element value of '' if it is null * Upgrade to PHP 8.1 in service-admin docker container * Update to version 14 of opg-lpa-datamodels * psalm scan service-admin with PHP 8.1 * Update composer dependencies * Upgrade front-app docker image to PHP 8.1 * Passing null to DateTime is deprecated When we get the last login time for a user, we set this to null if they have not logged in before. This value is then passed to new DateTime($lastLogin). However, PHP 8.1 is not happy with passing a null to the DateTime constructor and throws a deprecation message. Default this value to 'now' if the user has never logged in before to fix. * Move simplified LoggerTrait into MakeLogger shared directory
> This was a fork of [tuupola/slim-jwt-auth](https://github.com/tuupola/slim-jwt-auth) by [Mika Tuupola](https://github.com/tuupola). The fork was taken from `3.x` branch (at [this state](https://github.com/tuupola/slim-jwt-auth/tree/a4d6b3857daccb393f885473a08b2ea25874ae6b)). > > Thanks to Mika Tuupola & the package's contributors for their hard work. > > We forked it because we wanted to use [firebase/php-jwt](https://github.com/firebase/php-jwt) version 6 which had not supported at that time. Related [issue](tuupola/slim-jwt-auth#217).
|
I just want to give a +1 on bumping the major version number to break BC there:
I would gladly help to do this. |
|
More and more of my dependencies require Thank you! |
|
@josefsabl I think #246 needs to be merged and a new major release issued. |
|
Hello everybody, Is there a way to fix this vulnerability issue : new release or new library? Thank you |
We should support the latest version of firebase/php-jwt which is v6
The text was updated successfully, but these errors were encountered: