Skip to content

Commit 3c43bef

Browse files
CarlSchwanbackportbot-nextcloud[bot]
CarlSchwan
authored and
backportbot-nextcloud[bot]
committed
Handle one time and large passwords
For passwords bigger than 250 characters, use a bigger key since the performance impact is minor (around one second to encrypt the password). For passwords bigger than 470 characters, give up earlier and throw exeception recommanding admin to either enable the previously enabled configuration or use smaller passwords. This adds an option to disable storing passwords in the database. This might be desirable when using single use token as passwords or very large passwords. Signed-off-by: Carl Schwan <[email protected]>
1 parent 4f02d95 commit 3c43bef

File tree

4 files changed

+114
-8
lines changed

4 files changed

+114
-8
lines changed

‎apps/settings/lib/Controller/ChangePasswordController.php‎

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -107,7 +107,7 @@ public function changePersonalPassword(string $oldpassword = '', string $newpass
107107
}
108108

109109
try {
110-
if ($newpassword === null || $user->setPassword($newpassword) === false) {
110+
if ($newpassword === null || strlen($newpassword) > 469 || $user->setPassword($newpassword) === false) {
111111
return new JSONResponse([
112112
'status' => 'error'
113113
]);
@@ -155,6 +155,16 @@ public function changeUserPassword(string $username = null, string $password = n
155155
]);
156156
}
157157

158+
if (strlen($password) > 469) {
159+
return new JSONResponse([
160+
'status' => 'error',
161+
'data' => [
162+
'message' => $this->l->t('Unable to change password. Password too long.'),
163+
],
164+
]);
165+
}
166+
167+
158168
$currentUser = $this->userSession->getUser();
159169
$targetUser = $this->userManager->get($username);
160170
if ($currentUser === null || $targetUser === null ||

‎config/config.sample.php‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -318,6 +318,21 @@
318318
*/
319319
'auth.webauthn.enabled' => true,
320320

321+
/**
322+
* Whether encrypted password should be stored in the database
323+
*
324+
* The passwords are only decrypted using the login token stored uniquely in the
325+
* clients and allow to connect to external storages, autoconfigure mail account in
326+
* the mail app and periodically check if the password it still valid.
327+
*
328+
* This might be desirable to disable this functionality when using one time
329+
* passwords or when having a password policy enforcing long passwords (> 300
330+
* characters).
331+
*
332+
* By default the passwords are stored encrypted in the database.
333+
*/
334+
'auth.storeCryptedPassword' => true,
335+
321336
/**
322337
* By default the login form is always available. There are cases (SSO) where an
323338
* admin wants to avoid users entering their credentials to the system if the SSO

‎lib/private/Authentication/Token/PublicKeyTokenProvider.php‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -370,7 +370,7 @@ private function newToken(string $token,
370370

371371
$config = array_merge([
372372
'digest_alg' => 'sha512',
373-
'private_key_bits' => 2048,
373+
'private_key_bits' => $password !== null && strlen($password) > 250 ? 4096 : 2048,
374374
], $this->config->getSystemValue('openssl', []));
375375

376376
// Generate new key
@@ -392,7 +392,10 @@ private function newToken(string $token,
392392
$dbToken->setPublicKey($publicKey);
393393
$dbToken->setPrivateKey($this->encrypt($privateKey, $token));
394394

395-
if (!is_null($password)) {
395+
if (!is_null($password) && $this->config->getSystemValueBool('auth.storeCryptedPassword', true)) {
396+
if (strlen($password) > 469) {
397+
throw new \RuntimeException('Trying to save a password with more than 469 characters is not supported. If you want to use big passwords, disable the auth.storeCryptedPassword option in config.php');
398+
}
396399
$dbToken->setPassword($this->encryptPassword($password, $publicKey));
397400
}
398401

@@ -422,7 +425,7 @@ public function updatePasswords(string $uid, string $password) {
422425
$this->cache->clear();
423426

424427
// prevent setting an empty pw as result of pw-less-login
425-
if ($password === '') {
428+
if ($password === '' || !$this->config->getSystemValueBool('auth.storeCryptedPassword', true)) {
426429
return;
427430
}
428431

‎tests/lib/Authentication/Token/PublicKeyTokenProviderTest.php‎

Lines changed: 82 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -85,6 +85,10 @@ public function testGenerateToken() {
8585
$name = 'User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.2.12) Gecko/20101026 Firefox/3.6.12';
8686
$type = IToken::PERMANENT_TOKEN;
8787

88+
$this->config->method('getSystemValueBool')
89+
->willReturnMap([
90+
['auth.storeCryptedPassword', true, true],
91+
]);
8892
$actual = $this->tokenProvider->generateToken($token, $uid, $user, $password, $name, $type, IToken::DO_NOT_REMEMBER);
8993

9094
$this->assertInstanceOf(PublicKeyToken::class, $actual);
@@ -95,6 +99,48 @@ public function testGenerateToken() {
9599
$this->assertSame($password, $this->tokenProvider->getPassword($actual, $token));
96100
}
97101

102+
public function testGenerateTokenNoPassword(): void {
103+
$token = 'token';
104+
$uid = 'user';
105+
$user = 'User';
106+
$password = 'passme';
107+
$name = 'User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.2.12) Gecko/20101026 Firefox/3.6.12';
108+
$type = IToken::PERMANENT_TOKEN;
109+
$this->config->method('getSystemValueBool')
110+
->willReturnMap([
111+
['auth.storeCryptedPassword', true, false],
112+
]);
113+
$this->expectException(PasswordlessTokenException::class);
114+
115+
$actual = $this->tokenProvider->generateToken($token, $uid, $user, $password, $name, $type, IToken::DO_NOT_REMEMBER);
116+
117+
$this->assertInstanceOf(PublicKeyToken::class, $actual);
118+
$this->assertSame($uid, $actual->getUID());
119+
$this->assertSame($user, $actual->getLoginName());
120+
$this->assertSame($name, $actual->getName());
121+
$this->assertSame(IToken::DO_NOT_REMEMBER, $actual->getRemember());
122+
$this->tokenProvider->getPassword($actual, $token);
123+
}
124+
125+
public function testGenerateTokenLongPassword() {
126+
$token = 'token';
127+
$uid = 'user';
128+
$user = 'User';
129+
$password = '';
130+
for ($i = 0; $i < 500; $i++) {
131+
$password .= 'e';
132+
}
133+
$name = 'User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.2.12) Gecko/20101026 Firefox/3.6.12';
134+
$type = IToken::PERMANENT_TOKEN;
135+
$this->config->method('getSystemValueBool')
136+
->willReturnMap([
137+
['auth.storeCryptedPassword', true, true],
138+
]);
139+
$this->expectException(\RuntimeException::class);
140+
141+
$actual = $this->tokenProvider->generateToken($token, $uid, $user, $password, $name, $type, IToken::DO_NOT_REMEMBER);
142+
}
143+
98144
public function testGenerateTokenInvalidName() {
99145
$token = 'token';
100146
$uid = 'user';
@@ -105,6 +151,10 @@ public function testGenerateTokenInvalidName() {
105151
. 'User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.2.12) Gecko/20101026 Firefox/3.6.12'
106152
. 'User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.2.12) Gecko/20101026 Firefox/3.6.12';
107153
$type = IToken::PERMANENT_TOKEN;
154+
$this->config->method('getSystemValueBool')
155+
->willReturnMap([
156+
['auth.storeCryptedPassword', true, true],
157+
]);
108158

109159
$actual = $this->tokenProvider->generateToken($token, $uid, $user, $password, $name, $type, IToken::DO_NOT_REMEMBER);
110160

@@ -122,6 +172,10 @@ public function testUpdateToken() {
122172
->method('updateActivity')
123173
->with($tk, $this->time);
124174
$tk->setLastActivity($this->time - 200);
175+
$this->config->method('getSystemValueBool')
176+
->willReturnMap([
177+
['auth.storeCryptedPassword', true, true],
178+
]);
125179

126180
$this->tokenProvider->updateTokenActivity($tk);
127181

@@ -159,6 +213,10 @@ public function testGetPassword() {
159213
$password = 'passme';
160214
$name = 'User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.2.12) Gecko/20101026 Firefox/3.6.12';
161215
$type = IToken::PERMANENT_TOKEN;
216+
$this->config->method('getSystemValueBool')
217+
->willReturnMap([
218+
['auth.storeCryptedPassword', true, true],
219+
]);
162220

163221
$actual = $this->tokenProvider->generateToken($token, $uid, $user, $password, $name, $type, IToken::DO_NOT_REMEMBER);
164222

@@ -187,6 +245,10 @@ public function testGetPasswordInvalidToken() {
187245
$name = 'User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.2.12) Gecko/20101026 Firefox/3.6.12';
188246
$type = IToken::PERMANENT_TOKEN;
189247

248+
$this->config->method('getSystemValueBool')
249+
->willReturnMap([
250+
['auth.storeCryptedPassword', true, true],
251+
]);
190252
$actual = $this->tokenProvider->generateToken($token, $uid, $user, $password, $name, $type, IToken::DO_NOT_REMEMBER);
191253

192254
$this->tokenProvider->getPassword($actual, 'wrongtoken');
@@ -199,6 +261,10 @@ public function testSetPassword() {
199261
$password = 'passme';
200262
$name = 'User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.2.12) Gecko/20101026 Firefox/3.6.12';
201263
$type = IToken::PERMANENT_TOKEN;
264+
$this->config->method('getSystemValueBool')
265+
->willReturnMap([
266+
['auth.storeCryptedPassword', true, true],
267+
]);
202268

203269
$actual = $this->tokenProvider->generateToken($token, $uid, $user, $password, $name, $type, IToken::DO_NOT_REMEMBER);
204270

@@ -303,14 +369,18 @@ public function testRenewSessionTokenWithoutPassword() {
303369
$this->tokenProvider->renewSessionToken('oldId', 'newId');
304370
}
305371

306-
public function testRenewSessionTokenWithPassword() {
372+
public function testRenewSessionTokenWithPassword(): void {
307373
$token = 'oldId';
308374
$uid = 'user';
309375
$user = 'User';
310376
$password = 'password';
311377
$name = 'User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.2.12) Gecko/20101026 Firefox/3.6.12';
312378
$type = IToken::PERMANENT_TOKEN;
313379

380+
$this->config->method('getSystemValueBool')
381+
->willReturnMap([
382+
['auth.storeCryptedPassword', true, true],
383+
]);
314384
$oldToken = $this->tokenProvider->generateToken($token, $uid, $user, $password, $name, $type, IToken::DO_NOT_REMEMBER);
315385

316386
$this->mapper
@@ -321,7 +391,7 @@ public function testRenewSessionTokenWithPassword() {
321391
$this->mapper
322392
->expects($this->once())
323393
->method('insert')
324-
->with($this->callback(function (PublicKeyToken $token) use ($user, $uid, $name) {
394+
->with($this->callback(function (PublicKeyToken $token) use ($user, $uid, $name): bool {
325395
return $token->getUID() === $uid &&
326396
$token->getLoginName() === $user &&
327397
$token->getName() === $name &&
@@ -333,14 +403,14 @@ public function testRenewSessionTokenWithPassword() {
333403
$this->mapper
334404
->expects($this->once())
335405
->method('delete')
336-
->with($this->callback(function ($token) use ($oldToken) {
406+
->with($this->callback(function ($token) use ($oldToken): bool {
337407
return $token === $oldToken;
338408
}));
339409

340410
$this->tokenProvider->renewSessionToken('oldId', 'newId');
341411
}
342412

343-
public function testGetToken() {
413+
public function testGetToken(): void {
344414
$token = new PublicKeyToken();
345415

346416
$this->config->method('getSystemValue')
@@ -443,6 +513,10 @@ public function testRotate() {
443513
$name = 'User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.2.12) Gecko/20101026 Firefox/3.6.12';
444514
$type = IToken::PERMANENT_TOKEN;
445515

516+
$this->config->method('getSystemValueBool')
517+
->willReturnMap([
518+
['auth.storeCryptedPassword', true, true],
519+
]);
446520
$actual = $this->tokenProvider->generateToken($token, $uid, $user, $password, $name, $type, IToken::DO_NOT_REMEMBER);
447521

448522
$new = $this->tokenProvider->rotate($actual, 'oldtoken', 'newtoken');
@@ -540,6 +614,10 @@ public function testUpdatePasswords() {
540614
'random2',
541615
IToken::PERMANENT_TOKEN,
542616
IToken::REMEMBER);
617+
$this->config->method('getSystemValueBool')
618+
->willReturnMap([
619+
['auth.storeCryptedPassword', true, true],
620+
]);
543621

544622
$this->mapper->method('hasExpiredTokens')
545623
->with($uid)

0 commit comments

Comments
 (0)