diff --git a/core/js/setupchecks.js b/core/js/setupchecks.js index 628606a9e5..62f0fb10c1 100644 --- a/core/js/setupchecks.js +++ b/core/js/setupchecks.js @@ -183,10 +183,10 @@ type: OC.SetupChecks.MESSAGE_TYPE_INFO }); } - if(!data.isUrandomAvailable) { + if(!data.isRandomnessSecure) { messages.push({ - msg: t('core', '/dev/urandom is not readable by PHP which is highly discouraged for security reasons. Further information can be found in the documentation.', {docLink: data.securityDocs}), - type: OC.SetupChecks.MESSAGE_TYPE_WARNING + msg: t('core', 'No suitable source for randomness found by PHP which is highly discouraged for security reasons. Further information can be found in the documentation.', {docLink: data.securityDocs}), + type: OC.SetupChecks.MESSAGE_TYPE_ERROR }); } if(data.isUsedTlsLibOutdated) { diff --git a/core/js/tests/specs/setupchecksSpec.js b/core/js/tests/specs/setupchecksSpec.js index 4c0545b7b4..41bed276b6 100644 --- a/core/js/tests/specs/setupchecksSpec.js +++ b/core/js/tests/specs/setupchecksSpec.js @@ -155,7 +155,7 @@ describe('OC.SetupChecks tests', function() { hasWorkingFileLocking: true, hasValidTransactionIsolationLevel: true, suggestedOverwriteCliURL: '', - isUrandomAvailable: true, + isRandomnessSecure: true, serverHasInternetConnection: false, memcacheDocs: 'https://docs.nextcloud.com/server/go.php?to=admin-performance', forwardedForHeadersWorking: true, @@ -204,7 +204,7 @@ describe('OC.SetupChecks tests', function() { hasWorkingFileLocking: true, hasValidTransactionIsolationLevel: true, suggestedOverwriteCliURL: '', - isUrandomAvailable: true, + isRandomnessSecure: true, serverHasInternetConnection: false, memcacheDocs: 'https://docs.nextcloud.com/server/go.php?to=admin-performance', forwardedForHeadersWorking: true, @@ -254,7 +254,7 @@ describe('OC.SetupChecks tests', function() { hasWorkingFileLocking: true, hasValidTransactionIsolationLevel: true, suggestedOverwriteCliURL: '', - isUrandomAvailable: true, + isRandomnessSecure: true, serverHasInternetConnection: false, isMemcacheConfigured: true, forwardedForHeadersWorking: true, @@ -301,7 +301,7 @@ describe('OC.SetupChecks tests', function() { hasWorkingFileLocking: true, hasValidTransactionIsolationLevel: true, suggestedOverwriteCliURL: '', - isUrandomAvailable: false, + isRandomnessSecure: false, securityDocs: 'https://docs.owncloud.org/myDocs.html', serverHasInternetConnection: true, isMemcacheConfigured: true, @@ -325,8 +325,8 @@ describe('OC.SetupChecks tests', function() { async.done(function( data, s, x ){ expect(data).toEqual([{ - msg: '/dev/urandom is not readable by PHP which is highly discouraged for security reasons. Further information can be found in the documentation.', - type: OC.SetupChecks.MESSAGE_TYPE_WARNING + msg: 'No suitable source for randomness found by PHP which is highly discouraged for security reasons. Further information can be found in the documentation.', + type: OC.SetupChecks.MESSAGE_TYPE_ERROR }]); done(); }); @@ -347,7 +347,7 @@ describe('OC.SetupChecks tests', function() { hasWorkingFileLocking: true, hasValidTransactionIsolationLevel: true, suggestedOverwriteCliURL: '', - isUrandomAvailable: true, + isRandomnessSecure: true, securityDocs: 'https://docs.owncloud.org/myDocs.html', serverHasInternetConnection: true, isMemcacheConfigured: true, @@ -393,7 +393,7 @@ describe('OC.SetupChecks tests', function() { hasWorkingFileLocking: true, hasValidTransactionIsolationLevel: true, suggestedOverwriteCliURL: '', - isUrandomAvailable: true, + isRandomnessSecure: true, securityDocs: 'https://docs.owncloud.org/myDocs.html', serverHasInternetConnection: true, isMemcacheConfigured: true, @@ -441,7 +441,7 @@ describe('OC.SetupChecks tests', function() { hasWorkingFileLocking: true, hasValidTransactionIsolationLevel: true, suggestedOverwriteCliURL: '', - isUrandomAvailable: true, + isRandomnessSecure: true, serverHasInternetConnection: true, isMemcacheConfigured: true, forwardedForHeadersWorking: false, @@ -487,7 +487,7 @@ describe('OC.SetupChecks tests', function() { hasWorkingFileLocking: true, hasValidTransactionIsolationLevel: true, suggestedOverwriteCliURL: '', - isUrandomAvailable: true, + isRandomnessSecure: true, serverHasInternetConnection: true, isMemcacheConfigured: true, forwardedForHeadersWorking: true, @@ -533,7 +533,7 @@ describe('OC.SetupChecks tests', function() { hasWorkingFileLocking: true, hasValidTransactionIsolationLevel: true, suggestedOverwriteCliURL: '', - isUrandomAvailable: true, + isRandomnessSecure: true, serverHasInternetConnection: true, isMemcacheConfigured: true, forwardedForHeadersWorking: true, @@ -599,7 +599,7 @@ describe('OC.SetupChecks tests', function() { hasWorkingFileLocking: true, hasValidTransactionIsolationLevel: true, suggestedOverwriteCliURL: '', - isUrandomAvailable: true, + isRandomnessSecure: true, securityDocs: 'https://docs.owncloud.org/myDocs.html', serverHasInternetConnection: true, isMemcacheConfigured: true, @@ -646,7 +646,7 @@ describe('OC.SetupChecks tests', function() { hasWorkingFileLocking: true, hasValidTransactionIsolationLevel: true, suggestedOverwriteCliURL: '', - isUrandomAvailable: true, + isRandomnessSecure: true, securityDocs: 'https://docs.owncloud.org/myDocs.html', serverHasInternetConnection: true, isMemcacheConfigured: true, @@ -693,7 +693,7 @@ describe('OC.SetupChecks tests', function() { hasWorkingFileLocking: true, hasValidTransactionIsolationLevel: true, suggestedOverwriteCliURL: '', - isUrandomAvailable: true, + isRandomnessSecure: true, securityDocs: 'https://docs.owncloud.org/myDocs.html', serverHasInternetConnection: true, isMemcacheConfigured: true, @@ -740,7 +740,7 @@ describe('OC.SetupChecks tests', function() { hasWorkingFileLocking: true, hasValidTransactionIsolationLevel: true, suggestedOverwriteCliURL: '', - isUrandomAvailable: true, + isRandomnessSecure: true, securityDocs: 'https://docs.owncloud.org/myDocs.html', serverHasInternetConnection: true, isMemcacheConfigured: true, diff --git a/settings/Controller/CheckSetupController.php b/settings/Controller/CheckSetupController.php index 747e60c7cb..9219ca1350 100644 --- a/settings/Controller/CheckSetupController.php +++ b/settings/Controller/CheckSetupController.php @@ -42,6 +42,7 @@ use OC\DB\MissingIndexInformation; use OC\IntegrityCheck\Checker; use OC\Lock\NoopLockingProvider; use OC\MemoryInfo; +use OC\Security\SecureRandom; use OCP\AppFramework\Controller; use OCP\AppFramework\Http\DataDisplayResponse; use OCP\AppFramework\Http\DataResponse; @@ -86,6 +87,8 @@ class CheckSetupController extends Controller { private $dateTimeFormatter; /** @var MemoryInfo */ private $memoryInfo; + /** @var SecureRandom */ + private $secureRandom; public function __construct($AppName, IRequest $request, @@ -100,7 +103,8 @@ class CheckSetupController extends Controller { IDBConnection $db, ILockingProvider $lockingProvider, IDateTimeFormatter $dateTimeFormatter, - MemoryInfo $memoryInfo) { + MemoryInfo $memoryInfo, + SecureRandom $secureRandom) { parent::__construct($AppName, $request); $this->config = $config; $this->clientService = $clientService; @@ -114,6 +118,7 @@ class CheckSetupController extends Controller { $this->lockingProvider = $lockingProvider; $this->dateTimeFormatter = $dateTimeFormatter; $this->memoryInfo = $memoryInfo; + $this->secureRandom = $secureRandom; } /** @@ -167,20 +172,17 @@ class CheckSetupController extends Controller { } /** - * Whether /dev/urandom is available to the PHP controller + * Whether PHP can generate "secure" pseudorandom integers * * @return bool */ - private function isUrandomAvailable() { - if(@file_exists('/dev/urandom')) { - $file = fopen('/dev/urandom', 'rb'); - if($file) { - fclose($file); - return true; - } + private function isRandomnessSecure() { + try { + $this->secureRandom->generate(1); + } catch (\Exception $ex) { + return false; } - - return false; + return true; } /** @@ -602,7 +604,7 @@ Raw output 'serverHasInternetConnection' => $this->isInternetConnectionWorking(), 'isMemcacheConfigured' => $this->isMemcacheConfigured(), 'memcacheDocs' => $this->urlGenerator->linkToDocs('admin-performance'), - 'isUrandomAvailable' => $this->isUrandomAvailable(), + 'isRandomnessSecure' => $this->isRandomnessSecure(), 'securityDocs' => $this->urlGenerator->linkToDocs('admin-security'), 'isUsedTlsLibOutdated' => $this->isUsedTlsLibOutdated(), 'phpSupported' => $this->isPhpSupported(), diff --git a/tests/Settings/Controller/CheckSetupControllerTest.php b/tests/Settings/Controller/CheckSetupControllerTest.php index cc1ed6555c..6706573a1a 100644 --- a/tests/Settings/Controller/CheckSetupControllerTest.php +++ b/tests/Settings/Controller/CheckSetupControllerTest.php @@ -24,6 +24,7 @@ namespace Tests\Settings\Controller; use OC; use OC\DB\Connection; use OC\MemoryInfo; +use OC\Security\SecureRandom; use OC\Settings\Controller\CheckSetupController; use OCP\AppFramework\Http; use OCP\AppFramework\Http\DataDisplayResponse; @@ -79,6 +80,8 @@ class CheckSetupControllerTest extends TestCase { private $dateTimeFormatter; /** @var MemoryInfo|MockObject */ private $memoryInfo; + /** @var SecureRandom|\PHPUnit_Framework_MockObject_MockObject */ + private $secureRandom; /** * Holds a list of directories created during tests. @@ -119,6 +122,7 @@ class CheckSetupControllerTest extends TestCase { $this->memoryInfo = $this->getMockBuilder(MemoryInfo::class) ->setMethods(['isMemoryLimitSufficient',]) ->getMock(); + $this->secureRandom = $this->getMockBuilder(SecureRandom::class)->getMock(); $this->checkSetupController = $this->getMockBuilder('\OC\Settings\Controller\CheckSetupController') ->setConstructorArgs([ 'settings', @@ -135,6 +139,7 @@ class CheckSetupControllerTest extends TestCase { $this->lockingProvider, $this->dateTimeFormatter, $this->memoryInfo, + $this->secureRandom, ]) ->setMethods([ 'isReadOnlyConfig', @@ -482,7 +487,7 @@ class CheckSetupControllerTest extends TestCase { 'serverHasInternetConnection' => false, 'isMemcacheConfigured' => true, 'memcacheDocs' => 'http://docs.example.org/server/go.php?to=admin-performance', - 'isUrandomAvailable' => self::invokePrivate($this->checkSetupController, 'isUrandomAvailable'), + 'isRandomnessSecure' => self::invokePrivate($this->checkSetupController, 'isRandomnessSecure'), 'securityDocs' => 'https://docs.example.org/server/8.1/admin_manual/configuration_server/hardening.html', 'isUsedTlsLibOutdated' => '', 'phpSupported' => [ @@ -528,6 +533,7 @@ class CheckSetupControllerTest extends TestCase { $this->lockingProvider, $this->dateTimeFormatter, $this->memoryInfo, + $this->secureRandom, ]) ->setMethods(null)->getMock();