From 357d9b51084e4978f2c05dafd41c6d7412b33006 Mon Sep 17 00:00:00 2001 From: Philipp Holzer Date: Mon, 10 Jun 2019 14:43:25 +0200 Subject: [PATCH 1/3] Introduce HiddenString for Config-Values --- composer.json | 1 + composer.lock | 84 +++++++++++++++---- src/Core/Config/Cache/ConfigCache.php | 21 ++++- src/Core/Config/Configuration.php | 2 +- src/Factory/DBFactory.php | 3 +- .../src/Core/Config/Cache/ConfigCacheTest.php | 34 ++++++++ 6 files changed, 121 insertions(+), 24 deletions(-) diff --git a/composer.json b/composer.json index a8b4a20d2..aac5c10bc 100644 --- a/composer.json +++ b/composer.json @@ -37,6 +37,7 @@ "mobiledetect/mobiledetectlib": "2.8.*", "monolog/monolog": "^1.24", "nikic/fast-route": "^1.3", + "paragonie/hidden-string": "^1.0", "pear/text_languagedetect": "1.*", "pragmarx/google2fa": "^5.0", "pragmarx/recovery": "^0.1.0", diff --git a/composer.lock b/composer.lock index 75c10b78f..af51b6dfe 100644 --- a/composer.lock +++ b/composer.lock @@ -4,7 +4,7 @@ "Read more about it at https://getcomposer.org/doc/01-basic-usage.md#installing-dependencies", "This file is @generated automatically" ], - "content-hash": "67821d2270bdf8cdd24e7a047b9544e7", + "content-hash": "eb985236d64ed0b0fe1fc2e4ac6616e2", "packages": [ { "name": "asika/simple-console", @@ -1723,25 +1723,24 @@ }, { "name": "paragonie/constant_time_encoding", - "version": "v1.0.4", + "version": "v2.2.3", "source": { "type": "git", "url": "https://github.com/paragonie/constant_time_encoding.git", - "reference": "2132f0f293d856026d7d11bd81b9f4a23a1dc1f6" + "reference": "55af0dc01992b4d0da7f6372e2eac097bbbaffdb" }, "dist": { "type": "zip", - "url": "https://api.github.com/repos/paragonie/constant_time_encoding/zipball/2132f0f293d856026d7d11bd81b9f4a23a1dc1f6", - "reference": "2132f0f293d856026d7d11bd81b9f4a23a1dc1f6", + "url": "https://api.github.com/repos/paragonie/constant_time_encoding/zipball/55af0dc01992b4d0da7f6372e2eac097bbbaffdb", + "reference": "55af0dc01992b4d0da7f6372e2eac097bbbaffdb", "shasum": "" }, "require": { - "php": "^5.3|^7" + "php": "^7" }, "require-dev": { - "paragonie/random_compat": "^1.4|^2", - "phpunit/phpunit": "4.*|5.*", - "vimeo/psalm": "^0.3|^1" + "phpunit/phpunit": "^6|^7", + "vimeo/psalm": "^1|^2" }, "type": "library", "autoload": { @@ -1782,7 +1781,56 @@ "hex2bin", "rfc4648" ], - "time": "2018-04-30T17:57:16+00:00" + "time": "2019-01-03T20:26:31+00:00" + }, + { + "name": "paragonie/hidden-string", + "version": "v1.0.0", + "source": { + "type": "git", + "url": "https://github.com/paragonie/hidden-string.git", + "reference": "0bbb00be0e33b8e1d48fa79ea35cd42d3091a936" + }, + "dist": { + "type": "zip", + "url": "https://api.github.com/repos/paragonie/hidden-string/zipball/0bbb00be0e33b8e1d48fa79ea35cd42d3091a936", + "reference": "0bbb00be0e33b8e1d48fa79ea35cd42d3091a936", + "shasum": "" + }, + "require": { + "paragonie/constant_time_encoding": "^2", + "paragonie/sodium_compat": "^1.6", + "php": "^7" + }, + "require-dev": { + "phpunit/phpunit": "^6|^7", + "vimeo/psalm": "^1" + }, + "type": "library", + "autoload": { + "psr-4": { + "ParagonIE\\HiddenString\\": "./src" + } + }, + "notification-url": "https://packagist.org/downloads/", + "license": [ + "MPL-2.0" + ], + "authors": [ + { + "name": "Paragon Initiative Enterprises", + "email": "info@paragonie.com", + "homepage": "https://paragonie.com" + } + ], + "description": "Encapsulate strings in an object to hide them from stack traces", + "homepage": "https://github.com/paragonie/hidden-string", + "keywords": [ + "hidden", + "stack trace", + "string" + ], + "time": "2018-05-07T20:28:06+00:00" }, { "name": "paragonie/random_compat", @@ -2793,12 +2841,12 @@ "version": "v1.6.5", "source": { "type": "git", - "url": "https://github.com/mikey179/vfsStream.git", + "url": "https://github.com/bovigo/vfsStream.git", "reference": "d5fec95f541d4d71c4823bb5e30cf9b9e5b96145" }, "dist": { "type": "zip", - "url": "https://api.github.com/repos/mikey179/vfsStream/zipball/d5fec95f541d4d71c4823bb5e30cf9b9e5b96145", + "url": "https://api.github.com/repos/bovigo/vfsStream/zipball/d5fec95f541d4d71c4823bb5e30cf9b9e5b96145", "reference": "d5fec95f541d4d71c4823bb5e30cf9b9e5b96145", "shasum": "" }, @@ -3701,7 +3749,7 @@ } ], "description": "Provides the functionality to compare PHP values for equality", - "homepage": "http://www.github.com/sebastianbergmann/comparator", + "homepage": "https://github.com/sebastianbergmann/comparator", "keywords": [ "comparator", "compare", @@ -3803,7 +3851,7 @@ } ], "description": "Provides functionality to handle HHVM/PHP environments", - "homepage": "http://www.github.com/sebastianbergmann/environment", + "homepage": "https://github.com/sebastianbergmann/environment", "keywords": [ "Xdebug", "environment", @@ -3871,7 +3919,7 @@ } ], "description": "Provides the functionality to export PHP variables for visualization", - "homepage": "http://www.github.com/sebastianbergmann/exporter", + "homepage": "https://github.com/sebastianbergmann/exporter", "keywords": [ "export", "exporter" @@ -3923,7 +3971,7 @@ } ], "description": "Snapshotting of global state", - "homepage": "http://www.github.com/sebastianbergmann/global-state", + "homepage": "https://github.com/sebastianbergmann/global-state", "keywords": [ "global state" ], @@ -4025,7 +4073,7 @@ } ], "description": "Provides functionality to recursively process PHP variables", - "homepage": "http://www.github.com/sebastianbergmann/recursion-context", + "homepage": "https://github.com/sebastianbergmann/recursion-context", "time": "2016-11-19T07:33:16+00:00" }, { @@ -4158,7 +4206,7 @@ }, { "name": "Gert de Pagter", - "email": "backendtea@gmail.com" + "email": "BackEndTea@gmail.com" } ], "description": "Symfony polyfill for ctype functions", diff --git a/src/Core/Config/Cache/ConfigCache.php b/src/Core/Config/Cache/ConfigCache.php index 3314e184f..9aea367d9 100644 --- a/src/Core/Config/Cache/ConfigCache.php +++ b/src/Core/Config/Cache/ConfigCache.php @@ -2,6 +2,8 @@ namespace Friendica\Core\Config\Cache; +use ParagonIE\HiddenString\HiddenString; + /** * The Friendica config cache for the application * Initial, all *.config.php files are loaded into this cache with the @@ -15,10 +17,17 @@ class ConfigCache implements IConfigCache, IPConfigCache private $config; /** - * @param array $config A initial config array + * @var bool */ - public function __construct(array $config = []) + private $hidePasswordOutput; + + /** + * @param array $config A initial config array + * @param bool $hidePasswordOutput True, if cache variables should take extra care of password values + */ + public function __construct(array $config = [], $hidePasswordOutput = true) { + $this->hidePasswordOutput = $hidePasswordOutput; $this->load($config); } @@ -84,8 +93,12 @@ class ConfigCache implements IConfigCache, IPConfigCache $this->config[$cat] = []; } - $this->config[$cat][$key] = $value; - + if ($this->hidePasswordOutput && + $key == 'password') { + $this->config[$cat][$key] = new HiddenString($value); + } else { + $this->config[$cat][$key] = $value; + } return true; } diff --git a/src/Core/Config/Configuration.php b/src/Core/Config/Configuration.php index 532ed982a..18191d042 100644 --- a/src/Core/Config/Configuration.php +++ b/src/Core/Config/Configuration.php @@ -88,7 +88,7 @@ class Configuration if (isset($dbvalue)) { $this->configCache->set($cat, $key, $dbvalue); - return $dbvalue; + unset($dbvalue); } } diff --git a/src/Factory/DBFactory.php b/src/Factory/DBFactory.php index 1c01f7331..7caa63ec4 100644 --- a/src/Factory/DBFactory.php +++ b/src/Factory/DBFactory.php @@ -6,6 +6,7 @@ use Friendica\Core\Config\Cache; use Friendica\Database; use Friendica\Util\Logger\VoidLogger; use Friendica\Util\Profiler; +use ParagonIE\HiddenString\HiddenString; class DBFactory { @@ -45,7 +46,7 @@ class DBFactory } else { $db_user = $server['MYSQL_USER']; } - $db_pass = (string) $server['MYSQL_PASSWORD']; + $db_pass = new HiddenString((string) $server['MYSQL_PASSWORD']); $db_data = $server['MYSQL_DATABASE']; } diff --git a/tests/src/Core/Config/Cache/ConfigCacheTest.php b/tests/src/Core/Config/Cache/ConfigCacheTest.php index e6ac8255e..76ee26438 100644 --- a/tests/src/Core/Config/Cache/ConfigCacheTest.php +++ b/tests/src/Core/Config/Cache/ConfigCacheTest.php @@ -275,4 +275,38 @@ class ConfigCacheTest extends MockedTest $this->assertEmpty($configCache->keyDiff($diffConfig)); } + + /** + * Test the default hiding of passwords inside the cache + */ + public function testPasswordHide() + { + $configCache = new ConfigCache([ + 'database' => [ + 'password' => 'supersecure', + 'username' => 'notsecured', + ], + ]); + + $this->assertEquals('supersecure', $configCache->get('database', 'password')); + $this->assertNotEquals('supersecure', print_r($configCache->get('database', 'password'), true)); + $this->assertEquals('notsecured', print_r($configCache->get('database', 'username'), true)); + } + + /** + * Test disabling the hiding of passwords inside the cache + */ + public function testPasswordShow() + { + $configCache = new ConfigCache([ + 'database' => [ + 'password' => 'supersecure', + 'username' => 'notsecured', + ], + ], false); + + $this->assertEquals('supersecure', $configCache->get('database', 'password')); + $this->assertEquals('supersecure', print_r($configCache->get('database', 'password'), true)); + $this->assertEquals('notsecured', print_r($configCache->get('database', 'username'), true)); + } } From 4666b18e5b23608905c3946c904364276c23b6ca Mon Sep 17 00:00:00 2001 From: Philipp Holzer Date: Mon, 10 Jun 2019 15:39:21 +0200 Subject: [PATCH 2/3] Bugfixing empty password setting --- src/Core/Config/Cache/ConfigCache.php | 3 ++- tests/src/Core/Config/Cache/ConfigCacheTest.php | 16 ++++++++++++++++ 2 files changed, 18 insertions(+), 1 deletion(-) diff --git a/src/Core/Config/Cache/ConfigCache.php b/src/Core/Config/Cache/ConfigCache.php index 9aea367d9..c3cec19e2 100644 --- a/src/Core/Config/Cache/ConfigCache.php +++ b/src/Core/Config/Cache/ConfigCache.php @@ -94,7 +94,8 @@ class ConfigCache implements IConfigCache, IPConfigCache } if ($this->hidePasswordOutput && - $key == 'password') { + $key == 'password' && + !empty($value)) { $this->config[$cat][$key] = new HiddenString($value); } else { $this->config[$cat][$key] = $value; diff --git a/tests/src/Core/Config/Cache/ConfigCacheTest.php b/tests/src/Core/Config/Cache/ConfigCacheTest.php index 76ee26438..11845379d 100644 --- a/tests/src/Core/Config/Cache/ConfigCacheTest.php +++ b/tests/src/Core/Config/Cache/ConfigCacheTest.php @@ -309,4 +309,20 @@ class ConfigCacheTest extends MockedTest $this->assertEquals('supersecure', print_r($configCache->get('database', 'password'), true)); $this->assertEquals('notsecured', print_r($configCache->get('database', 'username'), true)); } + + /** + * Test a empty password + */ + public function testEmptyPassword() + { + $confiCache = new ConfigCache([ + 'database' => [ + 'password' => '', + 'username' => '', + ] + ]); + + $this->assertEmpty($confiCache->get('database', 'password')); + $this->assertEmpty($confiCache->get('database', 'username')); + } } From 50d8dbb12388dbb16af5f31aea420ab94c03cc57 Mon Sep 17 00:00:00 2001 From: Philipp Holzer Date: Mon, 10 Jun 2019 15:46:34 +0200 Subject: [PATCH 3/3] Bugfixing wrong typed password setting --- src/Core/Config/Cache/ConfigCache.php | 4 +-- .../src/Core/Config/Cache/ConfigCacheTest.php | 29 +++++++++++++++++-- 2 files changed, 28 insertions(+), 5 deletions(-) diff --git a/src/Core/Config/Cache/ConfigCache.php b/src/Core/Config/Cache/ConfigCache.php index c3cec19e2..441cdee81 100644 --- a/src/Core/Config/Cache/ConfigCache.php +++ b/src/Core/Config/Cache/ConfigCache.php @@ -95,8 +95,8 @@ class ConfigCache implements IConfigCache, IPConfigCache if ($this->hidePasswordOutput && $key == 'password' && - !empty($value)) { - $this->config[$cat][$key] = new HiddenString($value); + !empty($value) && is_string($value)) { + $this->config[$cat][$key] = new HiddenString((string) $value); } else { $this->config[$cat][$key] = $value; } diff --git a/tests/src/Core/Config/Cache/ConfigCacheTest.php b/tests/src/Core/Config/Cache/ConfigCacheTest.php index 11845379d..f8f81f9ee 100644 --- a/tests/src/Core/Config/Cache/ConfigCacheTest.php +++ b/tests/src/Core/Config/Cache/ConfigCacheTest.php @@ -315,14 +315,37 @@ class ConfigCacheTest extends MockedTest */ public function testEmptyPassword() { - $confiCache = new ConfigCache([ + $configCache = new ConfigCache([ 'database' => [ 'password' => '', 'username' => '', ] ]); - $this->assertEmpty($confiCache->get('database', 'password')); - $this->assertEmpty($confiCache->get('database', 'username')); + $this->assertEmpty($configCache->get('database', 'password')); + $this->assertEmpty($configCache->get('database', 'username')); + } + + public function testWrongTypePassword() + { + $configCache = new ConfigCache([ + 'database' => [ + 'password' => new \stdClass(), + 'username' => '', + ] + ]); + + $this->assertNotEmpty($configCache->get('database', 'password')); + $this->assertEmpty($configCache->get('database', 'username')); + + $configCache = new ConfigCache([ + 'database' => [ + 'password' => 23, + 'username' => '', + ] + ]); + + $this->assertEquals(23, $configCache->get('database', 'password')); + $this->assertEmpty($configCache->get('database', 'username')); } }