[!!!][TASK] Always include pageId in cHash calculation 83/51883/5
authorBenni Mack <benni@typo3.org>
Tue, 28 Feb 2017 06:17:43 +0000 (07:17 +0100)
committerChristian Kuhn <lolli@schwarzbu.ch>
Wed, 1 Mar 2017 12:57:08 +0000 (13:57 +0100)
The option $TYPO3_CONF_VARS[FE][cHashIncludePageId]
was included as part of a security fix to stay backwards-compatible
with existing installations.

The option was only there as a intermediate step, however
the cHash calculation should always contain the pageID,
so the option is removed and should be enabled by default.

The change removes the option via a silent configuration
updater in the Install Tool, removes the status report and
changes the cHash calculation to always include a pageId,
and if the page ID is not given, an exception is now
thrown all the time.

Resolves: #80050
Releases: master
Change-Id: Iac8eef1273848309da62deb24160f1c14ef338f4
Reviewed-on: https://review.typo3.org/51883
Reviewed-by: Helmut Hummel <typo3@helhum.io>
Tested-by: Helmut Hummel <typo3@helhum.io>
Tested-by: TYPO3com <no-reply@typo3.com>
Reviewed-by: Christian Kuhn <lolli@schwarzbu.ch>
Tested-by: Christian Kuhn <lolli@schwarzbu.ch>
typo3/sysext/core/Classes/Core/Bootstrap.php
typo3/sysext/core/Configuration/DefaultConfiguration.php
typo3/sysext/core/Configuration/DefaultConfigurationDescription.php
typo3/sysext/core/Configuration/FactoryConfiguration.php
typo3/sysext/core/Documentation/Changelog/master/Breaking-80050-RemovedOptionCHashIncludePageIdFromCHashCalculation.rst [new file with mode: 0644]
typo3/sysext/frontend/Classes/Page/CacheHashCalculator.php
typo3/sysext/frontend/Tests/Unit/Page/CacheHashCalculatorTest.php
typo3/sysext/install/Classes/Service/SilentConfigurationUpgradeService.php
typo3/sysext/lang/Resources/Private/Language/locallang_core.xlf
typo3/sysext/reports/Classes/Report/Status/SecurityStatus.php
typo3/sysext/reports/Resources/Private/Language/locallang_reports.xlf

index fee17d8..8ebea8d 100644 (file)
@@ -572,7 +572,6 @@ class Bootstrap
             'cachedParametersWhiteList' => GeneralUtility::trimExplode(',', $GLOBALS['TYPO3_CONF_VARS']['FE']['cHashOnlyForParameters'], true),
             'excludedParameters' => GeneralUtility::trimExplode(',', $GLOBALS['TYPO3_CONF_VARS']['FE']['cHashExcludedParameters'], true),
             'requireCacheHashPresenceParameters' => GeneralUtility::trimExplode(',', $GLOBALS['TYPO3_CONF_VARS']['FE']['cHashRequiredParameters'], true),
-            'includePageId' => $GLOBALS['TYPO3_CONF_VARS']['FE']['cHashIncludePageId']
         ];
         if (trim($GLOBALS['TYPO3_CONF_VARS']['FE']['cHashExcludedParametersIfEmpty']) === '*') {
             $GLOBALS['TYPO3_CONF_VARS']['FE']['cacheHash']['excludeAllEmptyParameters'] = true;
index 94eeeac..880e248 100644 (file)
@@ -924,7 +924,6 @@ return [
         'eID_include' => [], // Array of key/value pairs where key is "tx_[ext]_[optional suffix]" and value is relative filename of class to include. Key is used as "?eID=" for \TYPO3\CMS\Frontend\Http\RequestHandlerRequestHandler to include the code file which renders the page from that point. (Useful for functionality that requires a low initialization footprint, eg. frontend ajax applications)
         'disableNoCacheParameter' => false,
         'cacheHash' => [], // Array: Processed values of the cHash* parameters, handled by core bootstrap internally
-        'cHashIncludePageId' => false,
         'cHashExcludedParameters' => 'L, pk_campaign, pk_kwd, utm_source, utm_medium, utm_campaign, utm_term, utm_content',
         'cHashOnlyForParameters' => '',
         'cHashRequiredParameters' => '',
index e87097b..2fb9e41 100644 (file)
@@ -159,7 +159,6 @@ return [
         'pageOverlayFields' => 'List of fields from the table "pages_language_overlay" which should be overlaid on page records. See \\TYPO3\\CMS\\Frontend\\Page\\PageRepository::getPageOverlay()',
         'hidePagesIfNotTranslatedByDefault' => 'Boolean: If TRUE, pages that has no translation will be hidden by default. Basically this will inverse the effect of the page localization setting "Hide page if no translation for current language exists" to "Show page even if no translation exists"',
         'disableNoCacheParameter' => 'Boolean: If set, the no_cache request parameter will become ineffective. This is currently still an experimental feature and will require a website only with plugins that don\'t use this parameter. However, using "&amp;no_cache=1" should be avoided anyway because there are better ways to disable caching for a certain part of the website (see COA_INT/USER_INT documentation in TSref).',
-        'cHashIncludePageId' => 'Boolean: If enabled the cHash calculation is bound to the current page ID. This is recommended to avoid generation of not required page cache entries. Changing this value will void all links that have a cHash argument, as the cHash will change.',
         'cHashExcludedParameters' => 'String: The the given parameters will be ignored in the cHash calculation. Example: L,tx_search_pi1[query]',
         'cHashOnlyForParameters' => 'String: Only the given parameters will be evaluated in the cHash calculation. Example: tx_news_pi1[uid]',
         'cHashRequiredParameters' => 'Optional: Configure Parameters that require a cHash. If no cHash is given but one of the parameters are set, then TYPO3 triggers the configured cHash Error behaviour',
index 98f4c38..4c90eea 100644 (file)
@@ -32,7 +32,6 @@ return [
     ],
     'FE' => [
         'loginSecurityLevel' => 'rsa',
-        'cHashIncludePageId' => true,
     ],
     'GFX' => [
         'jpg_quality' => '80',
diff --git a/typo3/sysext/core/Documentation/Changelog/master/Breaking-80050-RemovedOptionCHashIncludePageIdFromCHashCalculation.rst b/typo3/sysext/core/Documentation/Changelog/master/Breaking-80050-RemovedOptionCHashIncludePageIdFromCHashCalculation.rst
new file mode 100644 (file)
index 0000000..5660a88
--- /dev/null
@@ -0,0 +1,35 @@
+.. include:: ../../Includes.txt
+
+==========================================================================
+Breaking: #80050 - Remove option cHashIncludePageId from cHash calculation
+==========================================================================
+
+See :issue:`80050`
+
+Description
+===========
+
+The global configuration option `$GLOBALS['TYPO3_CONF_VARS']['FE']['cHashIncludePageId']`
+has been removed, as the functionality is now always activated.
+
+The option was introduced in 2016 as part of a security bugfix for existing releases to
+allow the inclusion the cHash calculation (= caching identifier for pages with different GET variables)
+and was active for new installations.
+
+
+Impact
+======
+
+Setting the option has no effect anymore.
+
+If the option was disabled before, all existing cached contents and existing cHash calculations for URL
+rewrites (e.g. RealURL) of existing pages are invalidated and will throw a "page not found" exception
+if called directly.
+
+
+Affected Installations
+======================
+
+Any existing TYPO3 installation that did not have the option activated before.
+
+.. index:: Frontend, LocalConfiguration
\ No newline at end of file
index c139ad8..f3bc6da 100644 (file)
@@ -47,11 +47,6 @@ class CacheHashCalculator implements SingletonInterface
     protected $excludeAllEmptyParameters = false;
 
     /**
-     * @var bool
-     */
-    protected $includePageId = false;
-
-    /**
      * Initialise class properties by using the relevant TYPO3 configuration
      */
     public function __construct()
@@ -131,12 +126,10 @@ class CacheHashCalculator implements SingletonInterface
             $relevantParameters[$parameterName] = $parameterValue;
         }
         if (!empty($relevantParameters)) {
-            if ($this->includePageId) {
-                if (empty($parameters['id'])) {
-                    throw new \RuntimeException('ID parameter needs to be passed for the cHash calculation! As a temporary not recommended workaround, you can set $GLOBALS[\'TYPO3_CONF_VARS\'][\'FE\'][\'cHashIncludePageId\'] to false to avoid this error.', 1467983513);
-                }
-                $relevantParameters['id'] = $parameters['id'];
+            if (empty($parameters['id'])) {
+                throw new \RuntimeException('ID parameter needs to be passed for the cHash calculation!', 1467983513);
             }
+            $relevantParameters['id'] = $parameters['id'];
             // Finish and sort parameters array by keys:
             $relevantParameters['encryptionKey'] = $GLOBALS['TYPO3_CONF_VARS']['SYS']['encryptionKey'];
             ksort($relevantParameters);
@@ -261,14 +254,6 @@ class CacheHashCalculator implements SingletonInterface
     }
 
     /**
-     * @param bool $includePageId
-     */
-    protected function setIncludePageId($includePageId)
-    {
-        $this->includePageId = $includePageId;
-    }
-
-    /**
      * @param bool $excludeAllEmptyParameters
      */
     protected function setExcludeAllEmptyParameters($excludeAllEmptyParameters)
index c8e25e2..fba751c 100644 (file)
@@ -35,7 +35,6 @@ class CacheHashCalculatorTest extends \TYPO3\TestingFramework\Core\Unit\UnitTest
             'cachedParametersWhiteList' => [],
             'requireCacheHashPresenceParameters' => ['req1', 'req2'],
             'excludedParametersIfEmpty' => [],
-            'includePageId' => true,
             'excludeAllEmptyParameters' => false
         ]);
     }
index d73bcbd..89783ca 100644 (file)
@@ -91,7 +91,9 @@ class SilentConfigurationUpgradeService
         // #78835
         'SYS/cookieHttpOnly',
         // #71095
-        'BE/lang'
+        'BE/lang',
+        // #80050
+        'FE/cHashIncludePageId',
     ];
 
     public function __construct(ConfigurationManager $configurationManager = null)
index 0cb7dbb..149ec8b 100644 (file)
@@ -1137,9 +1137,6 @@ Do you want to refresh it now?</source>
                        <trans-unit id="warning.install_trustedhosts">
                                <source>The trusted hosts pattern check is disabled. Please define the allowed hosts in the [SYS][trustedHostsPattern] section of the Install Tool.</source>
                        </trans-unit>
-                       <trans-unit id="warning.install_cache_flooding">
-                               <source>Cache Flooding Protection is disabled. Please enable [FE][cHashIncludePageId] in the Install Tool.</source>
-                       </trans-unit>
                        <trans-unit id="warning.install_encryption">
                                <source>The encryption key is not set. Set it in the %sBasic Configuration section%s of the Install Tool.</source>
                        </trans-unit>
index c170f4e..d38112a 100644 (file)
@@ -46,28 +46,11 @@ class SecurityStatus implements StatusProviderInterface
             'fileDenyPattern' => $this->getFileDenyPatternStatus(),
             'htaccessUpload' => $this->getHtaccessUploadStatus(),
             'saltedpasswords' => $this->getSaltedPasswordsStatus(),
-            'cacheFloodingProtection' => $this->getCacheFloodingProtectionStatus()
         ];
         return $statuses;
     }
 
     /**
-     * @return \TYPO3\CMS\Reports\Status An object representing whether the check is disabled
-     */
-    protected function getCacheFloodingProtectionStatus()
-    {
-        $value = $this->getLanguageService()->getLL('status_ok');
-        $message = '';
-        $severity = ReportStatus::OK;
-        if (empty($GLOBALS['TYPO3_CONF_VARS']['FE']['cHashIncludePageId'])) {
-            $value = $this->getLanguageService()->getLL('status_insecure');
-            $severity = ReportStatus::ERROR;
-            $message = $this->getLanguageService()->sL('LLL:EXT:lang/Resources/Private/Language/locallang_core.xlf:warning.install_cache_flooding');
-        }
-        return GeneralUtility::makeInstance(ReportStatus::class, $this->getLanguageService()->getLL('status_cacheFloodingProtection'), $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
index 8e677e4..c219dea 100644 (file)
                        <trans-unit id="status_trustedHostsPattern">
                                <source>Trusted Hosts Pattern</source>
                        </trans-unit>
-                       <trans-unit id="status_cacheFloodingProtection">
-                               <source>Cache Flooding Protection</source>
-                       </trans-unit>
                        <trans-unit id="status_adminUserAccount">
                                <source>Admin User Account</source>
                        </trans-unit>