Skip to content

Commit

Permalink
LPAL-733 Fix php-jwt vulnerabilities (#936)
Browse files Browse the repository at this point in the history
* 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 <[email protected]>
  • Loading branch information
townxelliot and williamfalconeruk authored May 23, 2022
1 parent e81de20 commit 3efaa3b
Show file tree
Hide file tree
Showing 13 changed files with 503 additions and 162 deletions.
8 changes: 7 additions & 1 deletion service-admin/composer.json
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,13 @@
"ministryofjustice/opg-lpa-datamodels": "^13.3",
"php-http/guzzle6-adapter": "^2.0.2",
"slim/flash": "^0.4.0",
"tuupola/slim-jwt-auth": "^3.5.2",

"firebase/php-jwt": "^6.0",
"psr/http-message": "^1.0",
"tuupola/http-factory": "^0.4.0|^1.0.2",
"tuupola/callable-handler": "^0.3.0|^0.4.0|^1.0",
"psr/http-server-middleware": "^1.0",

"laminas/laminas-authentication": "^2.8",
"laminas/laminas-cache": "^3.1.2",
"laminas/laminas-cache-storage-adapter-memory": "2.0",
Expand Down
132 changes: 31 additions & 101 deletions service-admin/composer.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

7 changes: 5 additions & 2 deletions service-admin/config/autoload/mezzio.global.php
Original file line number Diff line number Diff line change
Expand Up @@ -25,14 +25,17 @@

'api_base_uri' => getenv('OPG_LPA_ENDPOINTS_API') ?: null,

'admin_accounts' => (getenv('OPG_LPA_COMMON_ADMIN_ACCOUNTS') ? explode(',', getenv('OPG_LPA_COMMON_ADMIN_ACCOUNTS')) : []),
'admin_accounts' => (
getenv('OPG_LPA_COMMON_ADMIN_ACCOUNTS') ?
explode(',', getenv('OPG_LPA_COMMON_ADMIN_ACCOUNTS')) : []
),

'jwt' => [
'secret' => getenv('OPG_LPA_ADMIN_JWT_SECRET') ?: null,
'path' => '/',
'header' => 'lpa-admin',
'cookie' => 'lpa-admin',
'ttl' => 60 * 15, // 15 minutes
'ttl' => 60 * 15, // 15 minutes
'algo' => 'HS256',
],

Expand Down
12 changes: 7 additions & 5 deletions service-admin/config/pipeline.php
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
<?php

/**
* Setup middleware pipeline:
*/

declare(strict_types=1);

use App\Middleware;
Expand All @@ -17,10 +21,7 @@
use Mezzio\Router\Middleware\RouteMiddleware;
use Laminas\Stratigility\Middleware\ErrorHandler;

/**
* Setup middleware pipeline:
*/
return function (Application $app, MiddlewareFactory $factory, ContainerInterface $container) : void {
return function (Application $app, MiddlewareFactory $factory, ContainerInterface $container): void {
// The error handler should be the first (most outer) middleware to catch
// all Exceptions.
$app->pipe(ErrorHandler::class);
Expand Down Expand Up @@ -60,7 +61,8 @@

// Set up the custom middleware to handle sessions and authorization
$app->pipe(Middleware\Session\SessionMiddleware::class);
$app->pipe(JwtAuthentication::class);
//$app->pipe(JwtAuthentication::class);
$app->pipe(Middleware\Session\JwtMiddleware::class);
$app->pipe(Middleware\Authorization\AuthorizationMiddleware::class);
$app->pipe(Middleware\Session\CsrfMiddleware::class);
$app->pipe(Middleware\Flash\SlimFlashMiddleware::class);
Expand Down
15 changes: 14 additions & 1 deletion service-admin/docker/app/Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -34,12 +34,25 @@ RUN if [[ $ENABLE_XDEBUG = 1 ]] ; then \

# Clean up build dependencies, but only after everything else we need has been installed
RUN apk del .build-dependencies

COPY --chown=appuser:appuser service-admin/composer.lock /app/composer.lock

COPY --chown=appuser:appuser service-admin/config /app/config
COPY --chown=appuser:appuser service-admin/public /app/public
COPY --chown=appuser:appuser service-admin/src /app/src

# Need this to run unit tests in CI
COPY --chown=appuser:appuser service-admin/phpunit.xml.dist /app/phpunit.xml.dist
COPY --chown=appuser:appuser service-admin/test /app/test

COPY --chown=appuser:appuser shared/logging /shared/logging
COPY --chown=appuser:appuser service-admin /app
COPY --chown=appuser:appuser --from=composer /app/vendor /app/vendor

COPY --chown=appuser:appuser service-admin/docker/app/app-php.ini /usr/local/etc/php/conf.d/
COPY --chown=appuser:appuser service-admin/docker/app/php-fpm-logging.conf /usr/local/etc/php-fpm.d/

WORKDIR /app

USER appuser

CMD php-fpm
8 changes: 5 additions & 3 deletions service-admin/src/App/src/ConfigProvider.php
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
namespace App;

use App\Logging\LoggingErrorListenerDelegatorFactory;
use Tuupola\Middleware\JwtAuthentication;
//use Tuupola\Middleware\JwtAuthentication;
use Laminas\Stratigility\Middleware\ErrorHandler;

/**
Expand Down Expand Up @@ -64,8 +64,10 @@ public function getDependencies(): array
Handler\UserFindHandler::class => Handler\UserFindHandlerFactory::class,

// Middleware
JwtAuthentication::class =>
Middleware\Session\JwtAuthenticationFactory::class,
//JwtAuthentication::class =>
// Middleware\Session\JwtAuthenticationFactory::class,
Middleware\Session\JwtMiddleware::class =>
Middleware\Session\JwtMiddlewareFactory::class,
Middleware\Authorization\AuthorizationMiddleware::class =>
Middleware\Authorization\AuthorizationMiddlewareFactory::class,
Middleware\Session\SessionMiddleware::class =>
Expand Down
Loading

0 comments on commit 3efaa3b

Please sign in to comment.