Add diagnostic that checks if server directories that should be private are accessible (#17490)

* add initial diagnostic

* unfinished

* Complete diagnostic and get to work.

* Update plugins/Diagnostics/Diagnostic/RequiredPrivateDirectories.php

Co-authored-by: Stefan Giehl <stefan@matomo.org>

* apply review feedback

* fixing some UI tests

Co-authored-by: Stefan Giehl <stefan@matomo.org>

4 files changed, 152 insertions, 3 deletions

diff --git a/plugins/Diagnostics/Diagnostic/RequiredPrivateDirectories.php b/plugins/Diagnostics/Diagnostic/RequiredPrivateDirectories.php
new file mode 100644
index 0000000000..d41cb12b43
--- /dev/null
+++ b/plugins/Diagnostics/Diagnostic/RequiredPrivateDirectories.php
@@ -0,0 +1,141 @@
+<?php
+/**
+ * Matomo - free/libre analytics platform
+ *
+ * @link https://matomo.org
+ * @license http://www.gnu.org/licenses/gpl-3.0.html GPL v3 or later
+ */
+
+namespace Piwik\Plugins\Diagnostics\Diagnostic;
+
+use Piwik\Common;
+use Piwik\Filesystem;
+use Piwik\Http;
+use Piwik\SettingsPiwik;
+use Piwik\Translation\Translator;
+
+/**
+ * Checks whether certain directories in Matomo that should be private are accessible through the internet.
+ */
+class RequiredPrivateDirectories implements Diagnostic
+{
+    /**
+     * @var Translator
+     */
+    private $translator;
+
+    public function __construct(Translator $translator)
+    {
+        $this->translator = $translator;
+    }
+
+    public function execute()
+    {
+        if (!SettingsPiwik::isMatomoInstalled()) {
+            return [];
+        }
+
+        $label = $this->translator->translate('Diagnostics_RequiredPrivateDirectories');
+
+        $privatePaths = [
+            ['tmp/', 'tmp/empty', 'tmp/cache/tracker/matomocache_general.php'], // tmp/empty is created by this diagnostic
+            ['.git/', '.git/config'],
+            ['lang/en.json'],
+        ];
+
+        // create test file to check if tmp/empty exists
+        Filesystem::mkdir(PIWIK_INCLUDE_PATH . '/tmp');
+        file_put_contents(PIWIK_INCLUDE_PATH . '/tmp/empty', 'test');
+
+        $baseUrl = SettingsPiwik::getPiwikUrl();
+        if (!Common::stringEndsWith($baseUrl, '/')) {
+            $baseUrl .= '/';
+        }
+
+        $manualCheck = $this->translator->translate('Diagnostics_PrivateDirectoryManualCheck');
+
+        $testUrls = [];
+        foreach ($privatePaths as $checks) {
+            foreach ($checks as $path) {
+                if (!file_exists($path)) {
+                    continue;
+                }
+
+                $testUrls[] = $baseUrl . $path;
+            }
+        }
+
+        $isInternetEnabled = SettingsPiwik::isInternetEnabled();
+        if (!$isInternetEnabled) {
+            $testUrlsList = $this->getUrlList($testUrls);
+
+            $unknown = $this->translator->translate('Diagnostics_PrivateDirectoryInternetDisabled') . ' ' . $manualCheck
+                . $testUrlsList;
+            $results[] = DiagnosticResult::singleResult($label, DiagnosticResult::STATUS_WARNING, $unknown);
+            return $results;
+        }
+
+        $result = new DiagnosticResult($label);
+
+        $isConfigIniAccessible = $this->checkConfigIni($result, $baseUrl);
+
+        $atLeastOneIsAccessible = $isConfigIniAccessible;
+        foreach ($testUrls as $testUrl) {
+            try {
+                $response = Http::sendHttpRequest($testUrl, $timeout = 2, null, null, null, false, false, true);
+                $status = $response['status'];
+
+                $isAccessible = !($status >= 400 && $status < 500);
+                if ($isAccessible) {
+                    $result->addItem(new DiagnosticResultItem(DiagnosticResult::STATUS_ERROR, $testUrl));
+                    $atLeastOneIsAccessible = true;
+                }
+            } catch (\Exception $e) {
+                $error = $e->getMessage();
+                $result->addItem(new DiagnosticResultItem(DiagnosticResult::STATUS_WARNING, 'Unable to execute check for ' .
+                    Common::sanitizeInputValue($testUrl) . ': ' . Common::sanitizeInputValue($error)));
+            }
+        }
+
+        if ($atLeastOneIsAccessible) {
+            $pathIsAccessible = $this->translator->translate('Diagnostics_PrivateDirectoryIsAccessible');
+            if ($isConfigIniAccessible) {
+                $pathIsAccessible .= '<br/><br/>' . $this->translator->translate('Diagnostics_ConfigIniAccessible');
+            }
+            $result->setLongErrorMessage($pathIsAccessible);
+        } else {
+            $result->addItem(new DiagnosticResultItem(DiagnosticResult::STATUS_OK, $this->translator->translate('Diagnostics_AllPrivateDirectoriesAreInaccessible')));
+        }
+
+        return [$result];
+    }
+
+    private function getUrlList(array $testUrls)
+    {
+        $testUrlsList = '';
+        foreach ($testUrls as $testUrl) {
+            $testUrlsList .= '<br/>' . Common::sanitizeInputValue($testUrl);
+        }
+        return $testUrlsList;
+    }
+
+    private function checkConfigIni(DiagnosticResult $result, $baseUrl)
+    {
+        $testUrl = $baseUrl . 'config/config.ini.php';
+        try {
+            $response = Http::sendHttpRequest($testUrl, $timeout = 2, null, null, null, false, false, true);
+            $data = $response['data'];
+
+            $isAccessible = strpos($data, ';') !== false;
+            if ($isAccessible) {
+                $result->addItem(new DiagnosticResultItem(DiagnosticResult::STATUS_ERROR, $testUrl));
+                return true;
+            }
+        } catch (\Exception $e) {
+            $error = $e->getMessage();
+            $result->addItem(new DiagnosticResultItem(DiagnosticResult::STATUS_WARNING, 'Unable to execute check for '
+                . Common::sanitizeInputValue($testUrl) . ': ' . Common::sanitizeInputValue($error)));
+        }
+        return false;
+    }
+}
diff --git a/plugins/Diagnostics/config/config.php b/plugins/Diagnostics/config/config.php
index e5be0cf756..121388c119 100644
--- a/plugins/Diagnostics/config/config.php
+++ b/plugins/Diagnostics/config/config.php
@@ -1,6 +1,7 @@
 <?php
 
 use Piwik\Plugins\Diagnostics\Diagnostic\CronArchivingLastRunCheck;
+use Piwik\Plugins\Diagnostics\Diagnostic\RequiredPrivateDirectories;
 
 return array(
     // Diagnostics for everything that is required for Piwik to run
@@ -15,6 +16,7 @@ return array(
     ),
     // Diagnostics for recommended features
     'diagnostics.optional' => array(
+        DI\get(RequiredPrivateDirectories::class),
         DI\get('Piwik\Plugins\Diagnostics\Diagnostic\FileIntegrityCheck'),
         DI\get('Piwik\Plugins\Diagnostics\Diagnostic\TrackerCheck'),
         DI\get('Piwik\Plugins\Diagnostics\Diagnostic\MemoryLimitCheck'),
diff --git a/plugins/Diagnostics/lang/en.json b/plugins/Diagnostics/lang/en.json
index ba8b14ed45..d46aef60c4 100644
--- a/plugins/Diagnostics/lang/en.json
+++ b/plugins/Diagnostics/lang/en.json
@@ -20,6 +20,12 @@
         "CronArchivingRunDetails": "Please check that you have setup a crontab calling the %1$s console command, and that you have configured a %2$s to receive errors by email if archiving fails. You can also try to run the console command to archive your reports manually: %3$s. %4$sLearn more.%5$s",
         "CronArchivingRanSuccessfullyXAgo": "The archiving process completed successfully %1$s ago.",
         "BrowserTriggeredArchivingEnabled": "For optimal performance and a speedy Matomo, it is highly recommended to set up a crontab to automatically archive your reports, and to disable browser triggering in the Matomo settings. %1$sLearn more.%2$s",
-        "NoDataForReportArchivingNotRun": "The archiving of your reports hasn't been executed recently, %1$slearn more about how to generate your reports.%2$s"
+        "NoDataForReportArchivingNotRun": "The archiving of your reports hasn't been executed recently, %1$slearn more about how to generate your reports.%2$s",
+        "RequiredPrivateDirectories": "Required Private Directories",
+        "PrivateDirectoryManualCheck": "Please open the URLs manually in a browser to see if you can access it. If you can, you might need to modify your server configuration as these files/directories should not be accessible via a browser from the Internet or Intranet.",
+        "PrivateDirectoryInternetDisabled": "We couldn't check if the following URLs are accessible because internet features are disabled on this Matomo.",
+        "PrivateDirectoryIsAccessible": "We found that the above URLs are accessible via the browser, but they should NOT be. Allowing them to be accessed can pose a potential security risk since the contents can provide information about your server and potentially your users. Please restrict access to them.",
+        "ConfigIniAccessible": "We also found that Matomo's config directory is publicly accessible. While attackers can't read the config now, if your webserver stops executing PHP files for some reason, your MySQL credentials and other information will be available to anyone. Please check your webserver config and deny access to this directory.",
+        "AllPrivateDirectoriesAreInaccessible": "All private directories are inaccessible from the internet."
     }
 }
\ No newline at end of file
diff --git a/plugins/Diagnostics/tests/UI/expected-screenshots/Diagnostics_page.png b/plugins/Diagnostics/tests/UI/expected-screenshots/Diagnostics_page.png
index 67fdc97a03..26cd5b633c 100644
--- a/plugins/Diagnostics/tests/UI/expected-screenshots/Diagnostics_page.png
+++ b/plugins/Diagnostics/tests/UI/expected-screenshots/Diagnostics_page.png
@@ -1,3 +1,3 @@
 version https://git-lfs.github.com/spec/v1
-oid sha256:c586102da6a34417c4868ca3d4762f813962dcd2001c6ce283ae373972510aa6
-size 444934
+oid sha256:8809c4c3142064eb43329a7c9dddaeaaf8edf97c78c8e55b4615a0324562be29
+size 452174