Skip to content

Commit

Permalink
[FEATURE] Add HTTPS security check to reports module
Browse files Browse the repository at this point in the history
Two new interfaces are added which can be implemented
by reports and status providers to access the current
PSR-7 server request.

Change-Id: I280bee3a71d425861af197ef1e907c4f60ff003f
Resolves: #84466
Releases: master
Reviewed-on: https://review.typo3.org/56349
Reviewed-by: Benjamin Franzke <bfr@qbus.de>
Tested-by: Benjamin Franzke <bfr@qbus.de>
Reviewed-by: Tobi Kretschmann <tobi@tobishome.de>
Tested-by: TYPO3com <no-reply@typo3.com>
Reviewed-by: Mathias Schreiber <mathias.schreiber@typo3.com>
Tested-by: Mathias Schreiber <mathias.schreiber@typo3.com>
Reviewed-by: Jan Helke <typo3@helke.de>
Tested-by: Jan Helke <typo3@helke.de>
Reviewed-by: Georg Ringer <georg.ringer@gmail.com>
Tested-by: Georg Ringer <georg.ringer@gmail.com>
  • Loading branch information
mbrodala authored and georgringer committed Mar 26, 2018
1 parent 8c59bff commit 2ee2312
Show file tree
Hide file tree
Showing 7 changed files with 189 additions and 23 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
.. include:: ../../Includes.txt

===========================================================
Feature: #84466 - Request aware interfaces added to reports
===========================================================

See :issue:`84466`

Description
===========

Two new interfaces where added to mark reports and status providers as request aware:

* :php:`TYPO3\CMS\Reports\RequestAwareReportInterface` (extends :php:`TYPO3\CMS\Reports\ReportInterface`)
* :php:`TYPO3\CMS\Reports\RequestAwareStatusProviderInterface` (extends :php:`TYPO3\CMS\Reports\StatusProviderInterface`)

Both interfaces allow reports or status providers to receive an optional PSR-7 server
request argument for their respective interface methods:

* :php:`getReport()`
* :php:`getStatus()`


Impact
======

Reports and status providers can now cleanly access information from the current server request.
They only need to implement one of the interfaces to get the current server request injected.

.. index:: Backend, PHP-API, ext:reports
9 changes: 8 additions & 1 deletion typo3/sysext/reports/Classes/Controller/ReportController.php
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
use TYPO3\CMS\Core\Utility\GeneralUtility;
use TYPO3\CMS\Fluid\View\StandaloneView;
use TYPO3\CMS\Reports\ReportInterface;
use TYPO3\CMS\Reports\RequestAwareReportInterface;
use TYPO3Fluid\Fluid\View\ViewInterface;

/**
Expand Down Expand Up @@ -136,12 +137,18 @@ public function detailAction(ServerRequestInterface $request)
$reportClass = $GLOBALS['TYPO3_CONF_VARS']['SC_OPTIONS']['reports'][$extension][$report]['report'] ?? null;

$reportInstance = GeneralUtility::makeInstance($reportClass, $this);

if ($reportInstance instanceof ReportInterface) {
$content = $reportInstance->getReport();
if ($reportInstance instanceof RequestAwareReportInterface) {
$content = $reportInstance->getReport($request);
} else {
$content = $reportInstance->getReport();
}
$this->saveState($extension, $report);
} else {
$error = $reportClass . ' does not implement the Report Interface which is necessary to be displayed here.';
}

$this->view->assignMultiple([
'content' => $content,
'error' => $error,
Expand Down
80 changes: 65 additions & 15 deletions typo3/sysext/reports/Classes/Report/Status/SecurityStatus.php
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
<?php
declare(strict_types = 1);
namespace TYPO3\CMS\Reports\Report\Status;

/*
Expand All @@ -14,28 +15,35 @@
* The TYPO3 project - inspiring people to share!
*/

use Psr\Http\Message\ServerRequestInterface;
use TYPO3\CMS\Core\Database\ConnectionPool;
use TYPO3\CMS\Core\Database\Query\Restriction\DeletedRestriction;
use TYPO3\CMS\Core\Localization\LanguageService;
use TYPO3\CMS\Core\Messaging\FlashMessage;
use TYPO3\CMS\Core\Utility\GeneralUtility;
use TYPO3\CMS\Reports\RequestAwareStatusProviderInterface;
use TYPO3\CMS\Reports\Status as ReportStatus;
use TYPO3\CMS\Reports\StatusProviderInterface;
use TYPO3\CMS\Saltedpasswords\Salt\SaltFactory;
use TYPO3\CMS\Saltedpasswords\Utility\ExtensionManagerConfigurationUtility;
use TYPO3\CMS\Saltedpasswords\Utility\SaltedPasswordsUtility;

/**
* Performs several checks about the system's health
*/
class SecurityStatus implements StatusProviderInterface
class SecurityStatus implements RequestAwareStatusProviderInterface
{
/**
* @var ServerRequestInterface
*/
protected $request;

/**
* Determines the security of this TYPO3 installation
*
* @return \TYPO3\CMS\Reports\Status[] List of statuses
* @param ServerRequestInterface|null $request
* @return ReportStatus[] List of statuses
*/
public function getStatus()
public function getStatus(ServerRequestInterface $request = null)
{
$statuses = [
'trustedHostsPattern' => $this->getTrustedHostsPatternStatus(),
Expand All @@ -44,33 +52,64 @@ public function getStatus()
'htaccessUpload' => $this->getHtaccessUploadStatus(),
'saltedpasswords' => $this->getSaltedPasswordsStatus(),
];

if ($request !== null) {
$statuses['encryptedConnectionStatus'] = $this->getEncryptedConnectionStatus($request);
}

return $statuses;
}

/**
* Checks if the current connection is encrypted (HTTPS)
*
* @param ServerRequestInterface $request
* @return ReportStatus
*/
protected function getEncryptedConnectionStatus(ServerRequestInterface $request): ReportStatus
{
$value = $this->getLanguageService()->getLL('status_ok');
$message = '';
$severity = ReportStatus::OK;

/** @var \TYPO3\CMS\Core\Http\NormalizedParams $normalizedParams */
$normalizedParams = $request->getAttribute('normalizedParams');

if (!$normalizedParams->isHttps()) {
$value = $this->getLanguageService()->getLL('status_insecure');
$severity = ReportStatus::WARNING;
$message = $this->getLanguageService()->sL('LLL:EXT:reports/Resources/Private/Language/locallang_reports.xlf:status_encryptedConnectionStatus_insecure');
}

return GeneralUtility::makeInstance(ReportStatus::class, $this->getLanguageService()->getLL('status_encryptedConnectionStatus'), $value, $message, $severity);
}

/**
* Checks if the trusted hosts pattern check is disabled.
*
* @return \TYPO3\CMS\Reports\Status An object representing whether the check is disabled
* @return ReportStatus An object representing whether the check is disabled
*/
protected function getTrustedHostsPatternStatus()
protected function getTrustedHostsPatternStatus(): ReportStatus
{
$value = $this->getLanguageService()->getLL('status_ok');
$message = '';
$severity = ReportStatus::OK;

if ($GLOBALS['TYPO3_CONF_VARS']['SYS']['trustedHostsPattern'] === GeneralUtility::ENV_TRUSTED_HOSTS_PATTERN_ALLOW_ALL) {
$value = $this->getLanguageService()->getLL('status_insecure');
$severity = ReportStatus::ERROR;
$message = $this->getLanguageService()->sL('LLL:EXT:lang/Resources/Private/Language/locallang_core.xlf:warning.install_trustedhosts');
}

return GeneralUtility::makeInstance(ReportStatus::class, $this->getLanguageService()->getLL('status_trustedHostsPattern'), $value, $message, $severity);
}

/**
* Checks whether a BE user account named admin with default password exists.
*
* @return \TYPO3\CMS\Reports\Status An object representing whether a default admin account exists
* @return ReportStatus An object representing whether a default admin account exists
*/
protected function getAdminAccountStatus()
protected function getAdminAccountStatus(): ReportStatus
{
$value = $this->getLanguageService()->getLL('status_ok');
$message = '';
Expand Down Expand Up @@ -125,22 +164,24 @@ protected function getAdminAccountStatus()
);
}
}

return GeneralUtility::makeInstance(ReportStatus::class, $this->getLanguageService()->getLL('status_adminUserAccount'), $value, $message, $severity);
}

/**
* Checks if fileDenyPattern was changed which is dangerous on Apache
*
* @return \TYPO3\CMS\Reports\Status An object representing whether the file deny pattern has changed
* @return ReportStatus An object representing whether the file deny pattern has changed
*/
protected function getFileDenyPatternStatus()
protected function getFileDenyPatternStatus(): ReportStatus
{
$value = $this->getLanguageService()->getLL('status_ok');
$message = '';
$severity = ReportStatus::OK;
$defaultParts = GeneralUtility::trimExplode('|', FILE_DENY_PATTERN_DEFAULT, true);
$givenParts = GeneralUtility::trimExplode('|', $GLOBALS['TYPO3_CONF_VARS']['BE']['fileDenyPattern'], true);
$result = array_intersect($defaultParts, $givenParts);

if ($defaultParts !== $result) {
$value = $this->getLanguageService()->getLL('status_insecure');
$severity = ReportStatus::ERROR;
Expand All @@ -149,35 +190,38 @@ protected function getFileDenyPatternStatus()
'<br /><pre>' . htmlspecialchars(FILE_DENY_PATTERN_DEFAULT) . '</pre><br />'
);
}

return GeneralUtility::makeInstance(ReportStatus::class, $this->getLanguageService()->getLL('status_fileDenyPattern'), $value, $message, $severity);
}

/**
* Checks if fileDenyPattern allows to upload .htaccess files which is
* dangerous on Apache.
*
* @return \TYPO3\CMS\Reports\Status An object representing whether it's possible to upload .htaccess files
* @return ReportStatus An object representing whether it's possible to upload .htaccess files
*/
protected function getHtaccessUploadStatus()
protected function getHtaccessUploadStatus(): ReportStatus
{
$value = $this->getLanguageService()->getLL('status_ok');
$message = '';
$severity = ReportStatus::OK;

if ($GLOBALS['TYPO3_CONF_VARS']['BE']['fileDenyPattern'] != FILE_DENY_PATTERN_DEFAULT
&& GeneralUtility::verifyFilenameAgainstDenyPattern('.htaccess')) {
$value = $this->getLanguageService()->getLL('status_insecure');
$severity = ReportStatus::ERROR;
$message = $this->getLanguageService()->sL('LLL:EXT:lang/Resources/Private/Language/locallang_core.xlf:warning.file_deny_htaccess');
}

return GeneralUtility::makeInstance(ReportStatus::class, $this->getLanguageService()->getLL('status_htaccessUploadProtection'), $value, $message, $severity);
}

/**
* Checks whether salted Passwords are configured or not.
*
* @return \TYPO3\CMS\Reports\Status An object representing the security of the saltedpassswords extension
* @return ReportStatus An object representing the security of the saltedpassswords extension
*/
protected function getSaltedPasswordsStatus()
protected function getSaltedPasswordsStatus(): ReportStatus
{
$value = $this->getLanguageService()->getLL('status_ok');
$severity = ReportStatus::OK;
Expand All @@ -186,6 +230,7 @@ protected function getSaltedPasswordsStatus()
$message = '<p>' . $this->getLanguageService()->getLL('status_saltedPasswords_infoText') . '</p>';
$messageDetail = '';
$resultCheck = $configCheck->checkConfigurationBackend([]);

switch ($resultCheck['errorType']) {
case FlashMessage::INFO:
$messageDetail .= $resultCheck['html'];
Expand All @@ -201,7 +246,9 @@ protected function getSaltedPasswordsStatus()
break;
default:
}

$unsecureUserCount = SaltedPasswordsUtility::getNumberOfBackendUsersWithInsecurePassword();

if ($unsecureUserCount > 0) {
$value = $this->getLanguageService()->getLL('status_insecure');
$severity = ReportStatus::ERROR;
Expand All @@ -211,17 +258,20 @@ protected function getSaltedPasswordsStatus()
'</div>' .
'</div>';
}

$message .= $messageDetail;

if (empty($messageDetail)) {
$message = '';
}

return GeneralUtility::makeInstance(ReportStatus::class, $this->getLanguageService()->getLL('status_saltedPasswords'), $value, $message, $severity);
}

/**
* @return LanguageService
*/
protected function getLanguageService()
protected function getLanguageService(): LanguageService
{
return $GLOBALS['LANG'];
}
Expand Down
23 changes: 16 additions & 7 deletions typo3/sysext/reports/Classes/Report/Status/Status.php
Original file line number Diff line number Diff line change
Expand Up @@ -14,18 +14,20 @@
* The TYPO3 project - inspiring people to share!
*/

use Psr\Http\Message\ServerRequestInterface;
use TYPO3\CMS\Core\Localization\LanguageService;
use TYPO3\CMS\Core\Registry;
use TYPO3\CMS\Core\Utility\GeneralUtility;
use TYPO3\CMS\Reports\ExtendedStatusProviderInterface;
use TYPO3\CMS\Reports\ReportInterface;
use TYPO3\CMS\Reports\RequestAwareReportInterface;
use TYPO3\CMS\Reports\RequestAwareStatusProviderInterface;
use TYPO3\CMS\Reports\Status as ReportStatus;
use TYPO3\CMS\Reports\StatusProviderInterface;

/**
* The status report
*/
class Status implements ReportInterface
class Status implements RequestAwareReportInterface
{
/**
* @var StatusProviderInterface[][]
Expand All @@ -37,19 +39,21 @@ class Status implements ReportInterface
*/
public function __construct()
{
$this->getStatusProviders();
$this->getLanguageService()->includeLLFile('EXT:reports/Resources/Private/Language/locallang_reports.xlf');
}

/**
* Takes care of creating / rendering the status report
*
* @param ServerRequestInterface|null $request the currently handled request
* @return string The status report as HTML
*/
public function getReport()
public function getReport(ServerRequestInterface $request = null)
{
$this->getStatusProviders();

$content = '';
$status = $this->getSystemStatus();
$status = $this->getSystemStatus($request);
$highestSeverity = $this->getHighestSeverity($status);
// Updating the registry
$registry = GeneralUtility::makeInstance(Registry::class);
Expand Down Expand Up @@ -77,15 +81,20 @@ protected function getStatusProviders()
/**
* Runs through all status providers and returns all statuses collected.
*
* @param ServerRequestInterface $request
* @return \TYPO3\CMS\Reports\Status[]
*/
public function getSystemStatus()
public function getSystemStatus(ServerRequestInterface $request = null)
{
$status = [];
foreach ($this->statusProviders as $statusProviderId => $statusProviderList) {
$status[$statusProviderId] = [];
foreach ($statusProviderList as $statusProvider) {
$statuses = $statusProvider->getStatus();
if ($statusProvider instanceof RequestAwareStatusProviderInterface) {
$statuses = $statusProvider->getStatus($request);
} else {
$statuses = $statusProvider->getStatus();
}
$status[$statusProviderId] = array_merge($status[$statusProviderId], $statuses);
}
}
Expand Down
32 changes: 32 additions & 0 deletions typo3/sysext/reports/Classes/RequestAwareReportInterface.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
<?php
declare(strict_types = 1);
namespace TYPO3\CMS\Reports;

/*
* This file is part of the TYPO3 CMS project.
*
* It is free software; you can redistribute it and/or modify it under
* the terms of the GNU General Public License, either version 2
* of the License, or any later version.
*
* For the full copyright and license information, please read the
* LICENSE.txt file that was distributed with this source code.
*
* The TYPO3 project - inspiring people to share!
*/

use Psr\Http\Message\ServerRequestInterface;

/**
* Interface for classes which provide a report using information from the current request
*/
interface RequestAwareReportInterface extends ReportInterface
{
/**
* Returns the content for a report
*
* @param ServerRequestInterface|null $request the currently handled request
* @return string A reports rendered HTML
*/
public function getReport(ServerRequestInterface $request = null);
}

0 comments on commit 2ee2312

Please sign in to comment.