From 4a7ed9086a8e4ace6f8787ebdfd3849a5e99ab77 Mon Sep 17 00:00:00 2001 From: lucashedding <lucashedding@1463982.no-reply.drupal.org> Date: Fri, 11 Oct 2019 13:27:13 -0600 Subject: [PATCH] Issue #3071193 by heddn, catch: Use live SHA256SUMS --- artifacts/keys/root.pub | 2 + composer.json | 1 + src/Services/ModifiedFiles.php | 53 ++--- .../src/Controller/HashesController.php | 45 ----- .../Controller/ModifiedFilesController.php | 78 ++++++++ .../test_automatic_updates.routing.yml | 8 +- tests/src/Build/ModifiedFilesTest.php | 185 ++++++++++++++++++ .../Build/QuickStart/QuickStartTestBase.php | 2 - tests/src/Functional/ModifiedFilesTest.php | 112 ----------- 9 files changed, 297 insertions(+), 189 deletions(-) create mode 100644 artifacts/keys/root.pub delete mode 100644 tests/modules/test_automatic_updates/src/Controller/HashesController.php create mode 100644 tests/modules/test_automatic_updates/src/Controller/ModifiedFilesController.php create mode 100644 tests/src/Build/ModifiedFilesTest.php delete mode 100644 tests/src/Functional/ModifiedFilesTest.php diff --git a/artifacts/keys/root.pub b/artifacts/keys/root.pub new file mode 100644 index 0000000000..78824ffd7c --- /dev/null +++ b/artifacts/keys/root.pub @@ -0,0 +1,2 @@ +untrusted comment: TESTING root keypair - not for production use public key +RWTtDh2Yu5YZWcv9ZD6oAOLUn+rw9uvlkiz9w51OFs+Kl5rBYfD01Ows diff --git a/composer.json b/composer.json index 96dd1871ac..43efface79 100644 --- a/composer.json +++ b/composer.json @@ -14,6 +14,7 @@ "ext-json": "*", "ext-zip": "*", "composer/semver": "^1.0", + "drupal/php-signify": "^1.0@dev", "ocramius/package-versions": "^1.4", "webflo/drupal-finder": "^1.2" }, diff --git a/src/Services/ModifiedFiles.php b/src/Services/ModifiedFiles.php index 2937862ef5..955d754e21 100644 --- a/src/Services/ModifiedFiles.php +++ b/src/Services/ModifiedFiles.php @@ -6,6 +6,9 @@ use Drupal\automatic_updates\IgnoredPathsTrait; use Drupal\automatic_updates\ProjectInfoTrait; use Drupal\Core\Config\ConfigFactoryInterface; use Drupal\Core\Url; +use Drupal\Signify\ChecksumList; +use Drupal\Signify\FailedCheckumFilter; +use Drupal\Signify\Verifier; use DrupalFinder\DrupalFinder; use GuzzleHttp\ClientInterface; use GuzzleHttp\Promise\EachPromise; @@ -88,41 +91,39 @@ class ModifiedFiles implements ModifiedFilesInterface { * Process checking hashes of files from external URL. * * @param array $resource - * An array of response resource and project info. + * An array of http response and project info. * @param array $modified_files * The list of modified files. */ protected function processHashes(array $resource, array &$modified_files) { - $response = $resource['response']; + $contents = $resource['contents']; $info = $resource['info']; - $file_root = $info['install path']; - while (($line = fgets($response)) !== FALSE) { - list($hash, $file) = preg_split('/\s+/', $line, 2); - $file = trim($file); - // If the line is empty, proceed to the next line. - if (empty($hash) && empty($file)) { - continue; - } - // If one of the values is invalid, log and continue. - if (!$hash || !$file) { - $this->logger->error('@hash or @file is empty; the hash file is malformed for this line.', ['@hash' => $hash, '@file' => $file]); + $directory_root = $info['install path']; + if ($info['project'] === 'drupal') { + $directory_root = ''; + } + $module_path = drupal_get_path('module', 'automatic_updates'); + $key = file_get_contents($module_path . '/artifacts/keys/root.pub'); + $verifier = new Verifier($key); + $files = $verifier->verifyCsigMessage($contents, new \DateTime('2020-01-01', new \DateTimeZone('UTC'))); + $checksums = new ChecksumList($files, TRUE); + foreach (new FailedCheckumFilter($checksums, $directory_root) as $failed_checksum) { + $file_path = implode(DIRECTORY_SEPARATOR, array_filter([ + $directory_root, + $failed_checksum->filename, + ])); + if ($this->isIgnoredPath($file_path)) { continue; } - if ($this->isIgnoredPath($file)) { + if (!file_exists($file_path)) { + $modified_files[] = $file_path; continue; } - if ($info['project'] === 'drupal') { - $file_root = $this->drupalFinder->getDrupalRoot(); - } - $file_path = $file_root . DIRECTORY_SEPARATOR . $file; - if (!file_exists($file_path) || hash_file('sha512', $file_path) !== $hash) { + $actual_hash = @hash_file(strtolower($failed_checksum->algorithm), $file_path); + if ($actual_hash === FALSE || empty($actual_hash) || strlen($actual_hash) < 64 || strcmp($actual_hash, $failed_checksum->hex_hash) !== 0) { $modified_files[] = $file_path; } } - if (!feof($response)) { - $this->logger->error('Stream for resource closed prematurely.'); - } - fclose($response); } /** @@ -162,7 +163,7 @@ class ModifiedFiles implements ModifiedFilesInterface { ]) ->then(function (ResponseInterface $response) use ($info) { return [ - 'response' => $response->getBody()->detach(), + 'contents' => $response->getBody()->getContents(), 'info' => $info, ]; }); @@ -195,11 +196,11 @@ class ModifiedFiles implements ModifiedFilesInterface { * The hash name. */ protected function getHashName(array $info) { - $hash_name = 'contents-sha512sums'; + $hash_name = 'contents-sha256sums'; if ($info['packaged']) { $hash_name .= '-packaged'; } - return $hash_name . '.txt'; + return $hash_name . '.csig'; } } diff --git a/tests/modules/test_automatic_updates/src/Controller/HashesController.php b/tests/modules/test_automatic_updates/src/Controller/HashesController.php deleted file mode 100644 index fb9622c67c..0000000000 --- a/tests/modules/test_automatic_updates/src/Controller/HashesController.php +++ /dev/null @@ -1,45 +0,0 @@ -<?php - -namespace Drupal\test_automatic_updates\Controller; - -use Drupal\Core\Controller\ControllerBase; -use Symfony\Component\HttpFoundation\Response; - -/** - * Class HashesController. - */ -class HashesController extends ControllerBase { - - /** - * Test hashes controller. - * - * @param string $extension - * The extension name. - * @param string $version - * The version string. - * - * @return \Symfony\Component\HttpFoundation\Response - * A file with hashes. - */ - public function hashes($extension, $version) { - $response = Response::create(); - $response->headers->set('Content-Type', 'text/plain'); - if ($extension === 'core' && $version === '8.7.0') { - $response->setContent("2cedbfcde76961b1f65536e3c69e13d8ad850619235f4aa2752ae66fe5e5a2d928578279338f099b5318d92c410040e995cb62ba1cc4512ec17cf21715c760a2 core/LICENSE.txt\n"); - } - elseif ($extension === 'core' && $version === '8.0.0') { - // Fake out a change in the LICENSE.txt. - $response->setContent("2d4ce6b272311ca4159056fb75138eba1814b65323c35ae5e0978233918e45e62bb32fdd2e0e8f657954fd5823c045762b3b59645daf83246d88d8797726e02c core/LICENSE.txt\n"); - } - elseif ($extension === 'ctools' && $version === '3.2') { - // Fake out a change in the LICENSE.txt. - $response->setContent("aee80b1f9f7f4a8a00dcf6e6ce6c41988dcaedc4de19d9d04460cbfb05d99829ffe8f9d038468eabbfba4d65b38e8dbef5ecf5eb8a1b891d9839cda6c48ee957 LICENSE.txt\n"); - } - elseif ($extension === 'ctools' && $version === '3.1') { - // Fake out a change in the LICENSE.txt. - $response->setContent("c82147962109321f8fb9c802735d31aab659a1cc3cd13d36dc5371c8b682ff60f23d41c794f2d9dc970ef9634b7fc8bcf35e3b95132644fe2ec97a341658a3f6 LICENSE.txt\n"); - } - return $response; - } - -} diff --git a/tests/modules/test_automatic_updates/src/Controller/ModifiedFilesController.php b/tests/modules/test_automatic_updates/src/Controller/ModifiedFilesController.php new file mode 100644 index 0000000000..ef80d1c243 --- /dev/null +++ b/tests/modules/test_automatic_updates/src/Controller/ModifiedFilesController.php @@ -0,0 +1,78 @@ +<?php + +namespace Drupal\test_automatic_updates\Controller; + +use Drupal\automatic_updates\ProjectInfoTrait; +use Drupal\automatic_updates\Services\ModifiedFilesInterface; +use Drupal\Core\Controller\ControllerBase; +use Symfony\Component\DependencyInjection\ContainerInterface; +use Symfony\Component\HttpFoundation\Response; + +/** + * Class ModifiedFilesController. + */ +class ModifiedFilesController extends ControllerBase { + use ProjectInfoTrait; + + /** + * The modified files service. + * + * @var \Drupal\automatic_updates\Services\ModifiedFilesInterface + */ + protected $modifiedFiles; + + /** + * ModifiedFilesController constructor. + * + * @param \Drupal\automatic_updates\Services\ModifiedFilesInterface $modified_files + * The modified files service. + */ + public function __construct(ModifiedFilesInterface $modified_files) { + $this->modifiedFiles = $modified_files; + } + + /** + * {@inheritdoc} + */ + public static function create(ContainerInterface $container) { + return new static( + $container->get('automatic_updates.modified_files') + ); + } + + /** + * Test modified files service. + * + * @param string $project_type + * The project type. + * @param string $extension + * The extension name. + * + * @return \Symfony\Component\HttpFoundation\Response + * A status message of modified files . + */ + public function modified($project_type, $extension) { + // Special edge case for core. + if ($project_type === 'core') { + $infos = $this->getInfos('module'); + $extensions = array_filter($infos, function (array $info) { + return $info['project'] === 'drupal'; + }); + } + // Filter for the main project. + else { + $infos = $this->getInfos($project_type); + $extensions = array_filter($infos, function (array $info) use ($extension, $project_type) { + return $info['install path'] === "{$project_type}s/contrib/$extension"; + }); + } + + $response = Response::create('No modified files!'); + $messages = $this->modifiedFiles->getModifiedFiles($extensions); + if (!empty($messages)) { + $response->setContent('Modified files include: ' . implode(', ', $messages)); + } + return $response; + } + +} diff --git a/tests/modules/test_automatic_updates/test_automatic_updates.routing.yml b/tests/modules/test_automatic_updates/test_automatic_updates.routing.yml index 5099a4c811..e502219e0d 100644 --- a/tests/modules/test_automatic_updates/test_automatic_updates.routing.yml +++ b/tests/modules/test_automatic_updates/test_automatic_updates.routing.yml @@ -12,11 +12,11 @@ test_automatic_updates.json_test_denied_controller: _title: 'JSON' requirements: _access: 'FALSE' -test_automatic_updates.hashes_endpoint: - path: '/automatic_updates/{extension}/{version}/SHA512SUMS' +test_automatic_updates.modified_files: + path: '/automatic_updates/modified-files/{project_type}/{extension}' defaults: - _controller: '\Drupal\test_automatic_updates\Controller\HashesController::hashes' - _title: 'SHA512SUMS' + _controller: '\Drupal\test_automatic_updates\Controller\ModifiedFilesController::modified' + _title: 'Modified Files' requirements: _access: 'TRUE' test_automatic_updates.inplace-update: diff --git a/tests/src/Build/ModifiedFilesTest.php b/tests/src/Build/ModifiedFilesTest.php new file mode 100644 index 0000000000..b8d3ae2271 --- /dev/null +++ b/tests/src/Build/ModifiedFilesTest.php @@ -0,0 +1,185 @@ +<?php + +namespace Drupal\Tests\automatic_updates\Build; + +use Drupal\Component\Utility\Html; +use Drupal\Tests\automatic_updates\Build\QuickStart\QuickStartTestBase; +use Symfony\Component\Filesystem\Filesystem as SymfonyFilesystem; + +/** + * @coversDefaultClass \Drupal\automatic_updates\Services\ModifiedFiles + * + * @group Update + * + * @requires externalCommand composer + * @requires externalCommand curl + * @requires externalCommand git + * @requires externalCommand tar + */ +class ModifiedFilesTest extends QuickStartTestBase { + + /** + * Symfony file system. + * + * @var \Symfony\Component\Filesystem\Filesystem + */ + protected $symfonyFileSystem; + + /** + * {@inheritdoc} + */ + protected function setUp() { + parent::setUp(); + $this->symfonyFileSystem = new SymfonyFilesystem(); + } + + /** + * @covers ::getModifiedFiles + * @dataProvider coreProjectProvider + */ + public function testCoreModified($version, array $modifications = []) { + $this->copyCodebase(); + + // We have to fetch the tags for this shallow repo. It might not be a + // shallow clone, therefore we use executeCommand instead of assertCommand. + $this->executeCommand('git fetch --unshallow --tags'); + $this->symfonyFileSystem->chmod($this->getWorkspaceDirectory() . '/sites/default', 0700, 0000); + $this->executeCommand("git checkout $version -f"); + $this->assertCommandSuccessful(); + + // Assert modifications. + $this->assertModifications('core', 'drupal', $modifications); + } + + /** + * @covers ::getModifiedFiles + * @dataProvider contribProjectsProvider + */ + public function testContribModified($project, $project_type, $version, array $modifications = []) { + $this->copyCodebase(); + + // Download the project. + $this->symfonyFileSystem->mkdir($this->getWorkspaceDirectory() . "/{$project_type}s/contrib/$project"); + $this->executeCommand("curl -fsSL https://ftp.drupal.org/files/projects/$project-$version.tar.gz | tar xvz -C {$project_type}s/contrib/$project --strip 1"); + $this->assertCommandSuccessful(); + + // Assert modifications. + $this->assertModifications($project_type, $project, $modifications); + } + + /** + * Core project data provider. + */ + public function coreProjectProvider() { + $datum[] = [ + 'version' => '8.7.3', + 'modifications' => [ + 'core/LICENSE.txt', + ], + ]; + return $datum; + } + + /** + * Contrib project data provider. + */ + public function contribProjectsProvider() { + $datum[] = [ + 'project' => 'bootstrap', + 'project_type' => 'theme', + 'version' => '8.x-3.20', + 'modifications' => [ + 'themes/contrib/bootstrap/LICENSE.txt', + ], + ]; + $datum[] = [ + 'project' => 'token', + 'project_type' => 'module', + 'version' => '8.x-1.5', + 'modifications' => [ + 'modules/contrib/token/LICENSE.txt', + ], + ]; + return $datum; + } + + /** + * Assert modified files. + * + * @param string $project_type + * The project type. + * @param string $project + * The project to assert. + * @param array $modifications + * The modified files to assert. + */ + protected function assertModifications($project_type, $project, array $modifications) { + $this->destroyBuild = FALSE; + $this->symfonyFileSystem->chmod($this->getWorkspaceDirectory() . '/sites/default', 0700, 0000); + $this->executeCommand('COMPOSER_DISCARD_CHANGES=true composer install --no-dev --no-interaction'); + $this->assertErrorOutputContains('Generating autoload files'); + $this->executeCommand('COMPOSER_DISCARD_CHANGES=true composer require ocramius/package-versions:^1.4 webflo/drupal-finder:^1.1 composer/semver:^1.0 drupal/php-signify:^1.0@dev --no-interaction'); + $this->assertErrorOutputContains('Generating autoload files'); + $this->installQuickStart('minimal'); + + // Currently, this test has to use extension_discovery_scan_tests so we can + // enable test modules. + $this->symfonyFileSystem->chmod($this->getWorkspaceDirectory() . '/sites/default', 0750, 0000); + $this->symfonyFileSystem->chmod($this->getWorkspaceDirectory() . '/sites/default/settings.php', 0640, 0000); + $settings_php = $this->getWorkspaceDirectory() . '/sites/default/settings.php'; + $this->symfonyFileSystem->appendToFile($settings_php, PHP_EOL . '$settings[\'extension_discovery_scan_tests\'] = TRUE;' . PHP_EOL); + // Intentionally mark composer.json and composer.lock as ignored. + $this->symfonyFileSystem->appendToFile($settings_php, PHP_EOL . '$config[\'automatic_updates.settings\'][\'ignored_paths\'] = "composer.json\ncomposer.lock\nmodules/custom/*\nthemes/custom/*\nprofiles/custom/*";' . PHP_EOL); + + // Restart server for config override to apply. + $this->stopServer(); + $this->standUpServer(); + + // Log in so that we can install modules. + $this->formLogin($this->adminUsername, $this->adminPassword); + $this->moduleEnable('update'); + $this->moduleEnable('automatic_updates'); + $this->moduleEnable('test_automatic_updates'); + + // Assert that the site is functional. + $this->visit(); + $this->assertDrupalVisit(); + + // Validate project is not modified. + $this->visit("/automatic_updates/modified-files/$project_type/$project"); + $assert = $this->getMink()->assertSession(); + $assert->statusCodeEquals(200); + $assert->pageTextContains('No modified files!'); + + // Assert modifications. + $this->assertNotEmpty($modifications); + foreach ($modifications as $modification) { + $file = $this->getWorkspaceDirectory() . DIRECTORY_SEPARATOR . $modification; + $this->fileExists($file); + $this->symfonyFileSystem->appendToFile($file, PHP_EOL . '// file is modified' . PHP_EOL); + } + $this->visit("/automatic_updates/modified-files/$project_type/$project"); + $assert->pageTextContains('Modified files include:'); + foreach ($modifications as $modification) { + $assert->pageTextContains($modification); + } + } + + /** + * Helper method that uses Drupal's module page to enable a module. + */ + protected function moduleEnable($module_name) { + $this->visit('/admin/modules'); + $field = Html::getClass("edit-modules $module_name enable"); + // No need to enable a module if it is already enabled. + if ($this->getMink()->getSession()->getPage()->findField($field)->isChecked()) { + return; + } + $assert = $this->getMink()->assertSession(); + $assert->fieldExists($field)->check(); + $session = $this->getMink()->getSession(); + $session->getPage()->findButton('Install')->submit(); + $assert->fieldExists($field)->isChecked(); + } + +} diff --git a/tests/src/Build/QuickStart/QuickStartTestBase.php b/tests/src/Build/QuickStart/QuickStartTestBase.php index 6ca9930638..5acaa96960 100644 --- a/tests/src/Build/QuickStart/QuickStartTestBase.php +++ b/tests/src/Build/QuickStart/QuickStartTestBase.php @@ -7,8 +7,6 @@ use Symfony\Component\Process\PhpExecutableFinder; /** * Helper methods for using the quickstart feature of Drupal. - * - * @TODO: remove after https://www.drupal.org/project/drupal/issues/3082230. */ abstract class QuickStartTestBase extends BuildTestBase { diff --git a/tests/src/Functional/ModifiedFilesTest.php b/tests/src/Functional/ModifiedFilesTest.php deleted file mode 100644 index 51b88850d1..0000000000 --- a/tests/src/Functional/ModifiedFilesTest.php +++ /dev/null @@ -1,112 +0,0 @@ -<?php - -namespace Drupal\Tests\automatic_updates\Functional; - -use Drupal\automatic_updates\Services\ModifiedFiles; -use Drupal\Core\Url; -use Drupal\Tests\BrowserTestBase; - -/** - * Tests of automatic updates. - * - * @group automatic_updates - */ -class ModifiedFilesTest extends BrowserTestBase { - - /** - * {@inheritdoc} - */ - protected static $modules = [ - 'automatic_updates', - 'test_automatic_updates', - ]; - - /** - * Tests modified files service. - */ - public function testModifiedFiles() { - // No modified code. - $modified_files = new TestModifiedFiles( - $this->container->get('logger.channel.automatic_updates'), - $this->container->get('automatic_updates.drupal_finder'), - $this->container->get('http_client'), - $this->container->get('config.factory') - ); - $this->initHashesEndpoint($modified_files, 'core', '8.7.0'); - $extensions = $modified_files->getInfos('module'); - $extensions = array_filter($extensions, function (array $extension) { - return $extension['project'] === 'drupal'; - }); - $files = $modified_files->getModifiedFiles($extensions); - $this->assertEmpty($files); - - // Hash doesn't match i.e. modified code, including contrib logic. - $this->initHashesEndpoint($modified_files, 'core', '8.0.0'); - $files = $modified_files->getModifiedFiles($extensions); - $this->assertCount(1, $files); - $this->assertStringEndsWith('core/LICENSE.txt', $files[0]); - - // Test contrib hash matches. - $extensions = $modified_files->getInfos('module'); - $extensions = array_filter($extensions, function (array $extension) { - return $extension['name'] === 'Chaos Tools'; - }); - $this->initHashesEndpoint($modified_files, 'ctools', '3.2'); - $files = $modified_files->getModifiedFiles($extensions); - $this->assertEmpty($files); - - // Test contrib doesn't match. - $this->initHashesEndpoint($modified_files, 'ctools', '3.1'); - $files = $modified_files->getModifiedFiles($extensions); - $this->assertCount(1, $files); - $this->assertStringEndsWith('contrib/ctools/LICENSE.txt', $files[0]); - } - - /** - * Set the hashes endpoint. - * - * @param TestModifiedFiles $modified_code - * The modified code object. - * @param string $extension - * The extension name. - * @param string $version - * The version. - */ - protected function initHashesEndpoint(TestModifiedFiles $modified_code, $extension, $version) { - $modified_code->endpoint = $this->buildUrl(Url::fromRoute('test_automatic_updates.hashes_endpoint', [ - 'extension' => $extension, - 'version' => $version, - ])); - } - -} - -/** - * Class TestModifiedCode. - */ -class TestModifiedFiles extends ModifiedFiles { - - /** - * The endpoint url. - * - * @var string - */ - public $endpoint; - - /** - * {@inheritdoc} - */ - protected function buildUrl(array $info) { - return $this->endpoint; - } - - // @codingStandardsIgnoreStart - /** - * {@inheritdoc} - */ - public function getInfos($extension_type) { - return parent::getInfos($extension_type); - } - // codingStandardsIgnoreEnd - -} -- GitLab