Skip to content

GitLab

Commit d3908160 authored by catch's avatar catch
Browse files
Options

Issue #1444620 by dawehner, sun, heyrocker: Remove file signing from configuration system.

parent 263c6802
Hide whitespace changes
Inline Side-by-side
Showing with 124 additions and 111 deletions
core/includes/bootstrap.inc
View file @ d3908160
......@@ -596,7 +596,7 @@ function drupal_settings_initialize() {
global $base_url, $base_path, $base_root, $script_path;
// Export the following settings.php variables to the global namespace
global $databases, $cookie_domain, $conf, $installed_profile, $update_free_access, $db_url, $db_prefix, $drupal_hash_salt, $is_https, $base_secure_url, $base_insecure_url, $config_directory_name, $config_signature_key;
global $databases, $cookie_domain, $conf, $installed_profile, $update_free_access, $db_url, $db_prefix, $drupal_hash_salt, $is_https, $base_secure_url, $base_insecure_url, $config_directory_name;
$conf = array();
if (file_exists(DRUPAL_ROOT . '/' . conf_path() . '/settings.php')) {
......
core/includes/config.inc
View file @ d3908160
......@@ -71,7 +71,7 @@ function config_install_default_config($module) {
* @return
* An array of file names under a branch.
*/
function config_get_signed_file_storage_names_with_prefix($prefix = '') {
function config_get_files_with_prefix($prefix = '') {
$files = glob(config_get_config_directory() . '/' . $prefix . '*.xml');
$clean_name = function ($value) {
return basename($value, '.xml');
......@@ -79,23 +79,6 @@ function config_get_signed_file_storage_names_with_prefix($prefix = '') {
return array_map($clean_name, $files);
}
/**
* Generates a hash of a config file's contents using our encryption key.
*
* @param $data
* The contents of a configuration file.
*
* @return
* A hash of the data.
*/
function config_sign_data($data) {
// The configuration key is loaded from settings.php and imported into the global namespace
global $config_signature_key;
// SHA-512 is both secure and very fast on 64 bit CPUs.
return hash_hmac('sha512', $data, $config_signature_key);
}
/**
* @todo
*
......
core/includes/install.core.inc
View file @ d3908160
......@@ -1002,11 +1002,6 @@ function install_settings_form_submit($form, &$form_state) {
'required' => TRUE,
);
$settings['config_signature_key'] = array(
'value' => drupal_hash_base64(drupal_random_bytes(55)),
'required' => TRUE,
);
// This duplicates drupal_get_token() because that function can't work yet.
// Wondering if it makes sense to move this later in the process, but its
// nice having all the settings stuff here.
......@@ -1016,7 +1011,7 @@ function install_settings_form_submit($form, &$form_state) {
// already has the db stuff in it, and right now in that case your
// config directory never gets created. So this needs to be moved elsewhere.
$settings['config_directory_name'] = array(
'value' => 'config_' . drupal_hmac_base64('', session_id() . $settings['config_signature_key']['value'] . $settings['drupal_hash_salt']['value']),
'value' => 'config_' . drupal_hmac_base64('', session_id() . $settings['drupal_hash_salt']['value']),
'required' => TRUE,
);
......
core/includes/install.inc
View file @ d3908160
<?php
use Drupal\Core\Database\Database;
use Drupal\Core\Config\SignedFileStorage;
use Drupal\Core\Config\FileStorage;
/**
* Indicates that a module has not been installed yet.
......@@ -403,9 +403,9 @@ function drupal_uninstall_modules($module_list = array(), $uninstall_dependents
$parts = explode('/', $file);
$file = array_pop($parts);
$config_name = str_replace('.xml', '', $file);
$signed_storage = new SignedFileStorage($config_name);
$file_storage = new FileStorage($config_name);
// Delete the configuration from storage.
$signed_storage->delete();
$file_storage->delete();
}
}
......
core/lib/Drupal/Core/Config/ConfigFileStorageSignatureException.php deleted 100644 → 0
View file @ 263c6802
<?php
namespace Drupal\Core\Config;
use Drupal\Core\Config\ConfigFileStorageException;
/**
* @todo
*/
class ConfigFileStorageSignatureException extends ConfigFileStorageException {}
core/lib/Drupal/Core/Config/DrupalConfigVerifiedStorage.php
View file @ d3908160
......@@ -3,7 +3,7 @@
namespace Drupal\Core\Config;
use Drupal\Core\Config\DrupalConfigVerifiedStorageInterface;
use Drupal\Core\Config\SignedFileStorage;
use Drupal\Core\Config\FileStorage;
/**
* @todo
......@@ -13,11 +13,11 @@ abstract class DrupalConfigVerifiedStorage implements DrupalConfigVerifiedStorag
protected $name;
/**
* The local signed file object to read from and write to.
* The local file object to read from and write to.
*
* @var SignedFileStorage
* @var Drupal\Core\Config\FileStorage
*/
protected $signedFile;
protected $fileStorage;
/**
* Implements DrupalConfigVerifiedStorageInterface::__construct().
......@@ -27,16 +27,16 @@ function __construct($name) {
}
/**
* Instantiates a new signed file object or returns the existing one.
* Instantiates a new file storage object or returns the existing one.
*
* @return SignedFileStorage
* The signed file object for this configuration object.
* @return Drupal\Core\Config\FileStorage
* The file object for this configuration object.
*/
protected function signedFileStorage() {
if (!isset($this->signedFile)) {
$this->signedFile = new SignedFileStorage($this->name);
protected function fileStorage() {
if (!isset($this->fileStorage)) {
$this->fileStorage = new FileStorage($this->name);
}
return $this->signedFile;
return $this->fileStorage;
}
/**
......@@ -50,7 +50,7 @@ public function copyToFile() {
* Implements DrupalConfigVerifiedStorageInterface::deleteFile().
*/
public function deleteFile() {
return $this->signedFileStorage()->delete();
return $this->fileStorage()->delete();
}
/**
......@@ -67,7 +67,7 @@ public function copyFromFile() {
* @todo
*/
public function readFromFile() {
return $this->signedFileStorage()->read($this->name);
return $this->fileStorage()->read($this->name);
}
/**
......@@ -89,7 +89,7 @@ public function write($data) {
* Implements DrupalConfigVerifiedStorageInterface::writeToFile().
*/
public function writeToFile($data) {
return $this->signedFileStorage()->write($data);
return $this->fileStorage()->write($data);
}
/**
......
core/lib/Drupal/Core/Config/DrupalConfigVerifiedStorageInterface.php
View file @ d3908160
......@@ -6,7 +6,7 @@
* Defines an interface for verified storage manipulation.
*
* This class allows reading and writing configuration data from/to the
* verified storage and copying to/from the signed file storing the same data.
* verified storage and copying to/from the file storing the same data.
*/
interface DrupalConfigVerifiedStorageInterface {
......
core/lib/Drupal/Core/Config/SignedFileStorage.php → core/lib/Drupal/Core/Config/FileStorage.php
View file @ d3908160
......@@ -3,16 +3,15 @@
namespace Drupal\Core\Config;
/**
* Represents the signed file storage interface.
* Represents the file storage interface.
*
* Classes implementing this interface allow reading and writing configuration
* data to and from disk, while automatically managing and verifying
* cryptographic signatures.
* data to and from disk.
*/
class SignedFileStorage {
class FileStorage {
/**
* Constructs a SignedFileStorage object.
* Constructs a FileStorage object.
*
* @param string $name
* The name for the configuration data. Should be lowercase.
......@@ -22,24 +21,20 @@ public function __construct($name) {
}
/**
* Reads and returns a signed file and its signature.
* Reads and returns a file.
*
* @return
* An array with "signature" and "data" keys.
* The data of the file.
*
* @throws
* Exception
*/
protected function readWithSignature() {
$content = file_get_contents($this->getFilePath());
if ($content === FALSE) {
protected function readData() {
$data = file_get_contents($this->getFilePath());
if ($data === FALSE) {
throw new \Exception('Read file is invalid.');
}
$signature = file_get_contents($this->getFilePath() . '.sig');
if ($signature === FALSE) {
throw new \Exception('Signature file is invalid.');
}
return array('data' => $content, 'signature' => $signature);
return $data;
}
/**
......@@ -62,41 +57,6 @@ public function getFilePath() {
return config_get_config_directory() . '/' . $this->name . '.xml';
}
/**
* Recreates the signature for the file.
*/
public function resign() {
if ($this->exists()) {
$parts = $this->readWithSignature();
$this->write($parts['data']);
}
}
/**
* Cryptographically verifies the integrity of the configuration file.
*
* @param $contentOnSuccess
* Whether or not to return the contents of the verified configuration file.
*
* @return mixed
* If $contentOnSuccess was TRUE, returns the contents of the verified
* configuration file; otherwise returns TRUE on success. Always returns
* FALSE if the configuration file was not successfully verified.
*/
public function verify($contentOnSuccess = FALSE) {
if ($this->exists()) {
$split = $this->readWithSignature();
$expected_signature = config_sign_data($split['data']);
if ($expected_signature === $split['signature']) {
if ($contentOnSuccess) {
return $split['data'];
}
return TRUE;
}
}
return FALSE;
}
/**
* Writes the contents of the configuration file to disk.
*
......@@ -107,13 +67,9 @@ public function verify($contentOnSuccess = FALSE) {
* Exception
*/
public function write($data) {
$signature = config_sign_data($data);
if (!file_put_contents($this->getFilePath(), $data)) {
throw new \Exception('Failed to write configuration file: ' . $this->getFilePath());
}
if (!file_put_contents($this->getFilePath() . '.sig', $signature)) {
throw new \Exception('Failed to write signature file: ' . $this->getFilePath());
}
}
/**
......@@ -124,12 +80,10 @@ public function write($data) {
*/
public function read() {
if ($this->exists()) {
$verification = $this->verify(TRUE);
if ($verification === FALSE) {
throw new \Exception('Invalid signature in file header.');
}
return $verification;
$data = $this->readData();
return $data;
}
return FALSE;
}
/**
......@@ -138,6 +92,5 @@ public function read() {
public function delete() {
// Needs error handling and etc.
@drupal_unlink($this->getFilePath());
@drupal_unlink($this->getFilePath() . '.sig');
}
}
core/modules/config/config.test
View file @ d3908160
......@@ -5,14 +5,12 @@
* Tests for the Configuration module.
*/
use Drupal\Core\Config\SignedFileStorage;
use Drupal\Core\Config\FileStorage;
/**
* Tests the secure file writer.
*/
class ConfigFileSecurityTestCase extends DrupalWebTestCase {
protected $profile = 'testing';
protected $filename = 'foo.bar';
protected $testContent = 'Good morning, Denver!';
......@@ -25,27 +23,11 @@ class ConfigFileSecurityTestCase extends DrupalWebTestCase {
);
}
/**
* Tests that a file written by this system has a valid signature.
*/
function testFileVerify() {
$file = new SignedFileStorage($this->filename);
$file->write($this->testContent);
$this->assertTrue($file->verify(), 'A file verifies after being written.');
unset($file);
// Load the file again, so that there is no stale data from the old object.
$file = new SignedFileStorage($this->filename);
$this->assertTrue($file->verify(), 'A file verifies after being written and reloaded.');
}
/**
* Tests that a file written by this system can be successfully read back.
*/
function testFilePersist() {
$file = new SignedFileStorage($this->filename);
$file = new FileStorage($this->filename);
$file->write($this->testContent);
unset($file);
......@@ -54,7 +36,7 @@ class ConfigFileSecurityTestCase extends DrupalWebTestCase {
// Note that if any other exception is thrown, we let the test system
// handle catching and reporting it.
try {
$file = new SignedFileStorage($this->filename);
$file = new FileStorage($this->filename);
$saved_content = $file->read();
$this->assertEqual($saved_content, $this->testContent, 'A file can be read back successfully.');
......@@ -63,35 +45,12 @@ class ConfigFileSecurityTestCase extends DrupalWebTestCase {
$this->fail('File failed verification when being read.');
}
}
/**
* Tests that a file fails validation if it's been monkeyed with.
*/
function testFileNotVerify() {
$file = new SignedFileStorage($this->filename);
$file->write($this->testContent);
// Manually overwrite the body of the secure file. Note that we skip the
// first line, which is reserved for the signature and such, to overwrite
// just the payload.
$raw_file = new SplFileObject($file->getFilePath(), 'a+');
$raw_file->fwrite('Good morning, Detroit!');
$raw_file->fflush();
unset($raw_file);
unset($file);
$file = new SignedFileStorage($this->filename);
$this->assertFalse($file->verify(), 'Corrupted file does not verify.');
}
}
/**
* Tests reading and writing file contents.
*/
class ConfigFileContentTestCase extends DrupalWebTestCase {
protected $profile = 'testing';
protected $fileExtension = 'xml';
public static function getInfo() {
......@@ -238,22 +197,22 @@ class ConfigFileContentTestCase extends DrupalWebTestCase {
// Get file listing for all files starting with 'foo'. Should return
// two elements.
$files = config_get_signed_file_storage_names_with_prefix('foo');
$files = config_get_files_with_prefix('foo');
$this->assertEqual(count($files), 2, 'Two files listed with the prefix \'foo\'.');
// Get file listing for all files starting with 'biff'. Should return
// one element.
$files = config_get_signed_file_storage_names_with_prefix('biff');
$files = config_get_files_with_prefix('biff');
$this->assertEqual(count($files), 1, 'One file listed with the prefix \'biff\'.');
// Get file listing for all files starting with 'foo.bar'. Should return
// one element.
$files = config_get_signed_file_storage_names_with_prefix('foo.bar');
$files = config_get_files_with_prefix('foo.bar');
$this->assertEqual(count($files), 1, 'One file listed with the prefix \'foo.bar\'.');
// Get file listing for all files starting with 'bar'. Should return
// an empty array.
$files = config_get_signed_file_storage_names_with_prefix('bar');
$files = config_get_files_with_prefix('bar');
$this->assertEqual($files, array(), 'No files listed with the prefix \'bar\'.');
// Delete the configuration.
......
core/modules/simpletest/drupal_web_test_case.php
View file @ d3908160
......@@ -1344,7 +1344,6 @@ protected function prepareEnvironment() {
$this->originalLanguage = $language_interface;
$this->originalLanguageDefault = variable_get('language_default');
$this->originalConfigDirectory = $GLOBALS['config_directory_name'];
$this->originalConfigSignatureKey = $GLOBALS['config_signature_key'];
$this->originalFileDirectory = variable_get('file_public_path', conf_path() . '/files');
$this->originalProfile = drupal_get_profile();
$this->originalUser = $user;
......@@ -1377,7 +1376,6 @@ protected function prepareEnvironment() {
$GLOBALS['config_directory_name'] = 'simpletest/' . substr($this->databasePrefix, 10) . '/config';
$this->configFileDirectory = $this->originalFileDirectory . '/' . $GLOBALS['config_directory_name'];
file_prepare_directory($this->configFileDirectory, FILE_CREATE_DIRECTORY | FILE_MODIFY_PERMISSIONS);
$GLOBALS['config_signature_key'] = drupal_hash_base64(drupal_random_bytes(55));
// Log fatal errors.
ini_set('log_errors', 1);
......@@ -1647,7 +1645,6 @@ protected function tearDown() {
// Reset configuration globals.
$GLOBALS['config_directory_name'] = $this->originalConfigDirectory;
$GLOBALS['config_signature_key'] = $this->originalConfigSignatureKey;
// Reset language.
$language_interface = $this->originalLanguage;
......
sites/default/default.settings.php
View file @ d3908160
......@@ -251,13 +251,6 @@
*/
$config_directory_name = '';
/**
* Configuration signature key.
*
* Drupal configuration files are signed using this key.
*/
$config_signature_key = '';
/**
* Base URL (optional).
*
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment