Skip to content

Commit 5680743

Browse files
committed
Harden updater authentication
- Reset tokens after 2 hours as discussed at owncloud/updater#220 (comment) - Used BCrypt for storing the password in the config.php. This makes it substantially harder in case of a leakage of the token to bruteforce it. In the future we can evaluate also an HMAC including the IP. That's a bit tricker though at the moment considering that we support reverse proxies. Didn't feel brave enough to touch that dragon now as well ;)
1 parent 5c89cf9 commit 5680743

3 files changed

Lines changed: 6 additions & 5 deletions

File tree

apps/updatenotification/controller/admincontroller.php

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -77,8 +77,8 @@ public function createCredentials() {
7777
$this->config->setAppValue('core', 'updater.secret.created', $this->timeFactory->getTime());
7878

7979
// Create a new token
80-
$newToken = $this->secureRandom->generate(32);
81-
$this->config->setSystemValue('updater.secret', $newToken);
80+
$newToken = $this->secureRandom->generate(64);
81+
$this->config->setSystemValue('updater.secret', password_hash($newToken, PASSWORD_DEFAULT));
8282

8383
return new DataResponse($newToken);
8484
}

apps/updatenotification/lib/resettokenbackgroundjob.php

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,8 @@ private function fixDIForJobs() {
6767
* @param $argument
6868
*/
6969
protected function run($argument) {
70-
if($this->timeFactory->getTime() - $this->config->getAppValue('core', 'updater.secret.created', $this->timeFactory->getTime()) >= 86400) {
70+
// Delete old tokens after 2 days
71+
if($this->timeFactory->getTime() - $this->config->getAppValue('core', 'updater.secret.created', $this->timeFactory->getTime()) >= 172800) {
7172
$this->config->deleteSystemValue('updater.secret');
7273
}
7374
}

apps/updatenotification/tests/controller/AdminControllerTest.php

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -77,12 +77,12 @@ public function testCreateCredentials() {
7777
$this->secureRandom
7878
->expects($this->once())
7979
->method('generate')
80-
->with(32)
80+
->with(64)
8181
->willReturn('MyGeneratedToken');
8282
$this->config
8383
->expects($this->once())
8484
->method('setSystemValue')
85-
->with('updater.secret', 'MyGeneratedToken');
85+
->with('updater.secret');
8686
$this->timeFactory
8787
->expects($this->once())
8888
->method('getTime')

0 commit comments

Comments
 (0)