From f2a2b34e4639e88f8d948a388a51f010212b42a3 Mon Sep 17 00:00:00 2001 From: Fabrizio Steiner Date: Sun, 7 May 2017 23:10:02 +0200 Subject: [PATCH] Increase device password entropy. Use lower- and upper-case characters and digits, but exclude ambiguous characters. The number of digits has also been increased to 25. Signed-off-by: Fabrizio Steiner --- lib/public/Security/ISecureRandom.php | 7 +++++++ settings/Controller/AuthSettingsController.php | 8 ++++---- settings/css/settings.css | 2 +- tests/Settings/Controller/AuthSettingsControllerTest.php | 6 +++--- 4 files changed, 15 insertions(+), 8 deletions(-) diff --git a/lib/public/Security/ISecureRandom.php b/lib/public/Security/ISecureRandom.php index c60529ef80..14190639f4 100644 --- a/lib/public/Security/ISecureRandom.php +++ b/lib/public/Security/ISecureRandom.php @@ -44,6 +44,13 @@ interface ISecureRandom { const CHAR_DIGITS = '0123456789'; const CHAR_SYMBOLS = '!\"#$%&\\\'()* +,-./:;<=>?@[\]^_`{|}~'; + /** + * Characters that can be used for generate($length, $characters), to + * generate human readable random strings. Lower- and upper-case characters and digits + * are included. Characters which are ambiguous are excluded, such as I, l, and 1 and so on. + */ + const CHAR_HUMAN_READABLE = "abcdefgijkmnopqrstwxyzABCDEFGHJKLMNPQRSTWXYZ23456789"; + /** * Convenience method to get a low strength random number generator. * diff --git a/settings/Controller/AuthSettingsController.php b/settings/Controller/AuthSettingsController.php index 57192e119a..7bb8a6654e 100644 --- a/settings/Controller/AuthSettingsController.php +++ b/settings/Controller/AuthSettingsController.php @@ -154,16 +154,16 @@ class AuthSettingsController extends Controller { } /** - * Return a 20 digit device password + * Return a 25 digit device password * - * Example: ABCDE-FGHIJ-KLMNO-PQRST + * Example: AbCdE-fGhIj-KlMnO-pQrSt-12345 * * @return string */ private function generateRandomDeviceToken() { $groups = []; - for ($i = 0; $i < 4; $i++) { - $groups[] = $this->random->generate(5, implode('', range('A', 'Z'))); + for ($i = 0; $i < 5; $i++) { + $groups[] = $this->random->generate(5, ISecureRandom::CHAR_HUMAN_READABLE); } return implode('-', $groups); } diff --git a/settings/css/settings.css b/settings/css/settings.css index 0777f7e4cf..acd8ce8e47 100644 --- a/settings/css/settings.css +++ b/settings/css/settings.css @@ -343,7 +343,7 @@ table.nostyle td { padding: 0.2em 0; } #new-app-login-name, #new-app-password { - width: 186px; + width: 245px; font-family: monospace; background-color: lightyellow; } diff --git a/tests/Settings/Controller/AuthSettingsControllerTest.php b/tests/Settings/Controller/AuthSettingsControllerTest.php index 7f4277acd7..5c1280ff4b 100644 --- a/tests/Settings/Controller/AuthSettingsControllerTest.php +++ b/tests/Settings/Controller/AuthSettingsControllerTest.php @@ -133,11 +133,11 @@ class AuthSettingsControllerTest extends TestCase { ->method('getLoginName') ->will($this->returnValue('User13')); - $this->secureRandom->expects($this->exactly(4)) + $this->secureRandom->expects($this->exactly(5)) ->method('generate') - ->with(5, implode('', range('A', 'Z'))) + ->with(5, ISecureRandom::CHAR_HUMAN_READABLE) ->will($this->returnValue('XXXXX')); - $newToken = 'XXXXX-XXXXX-XXXXX-XXXXX'; + $newToken = 'XXXXX-XXXXX-XXXXX-XXXXX-XXXXX'; $this->tokenProvider->expects($this->once()) ->method('generateToken')