From cdf3b60555eb559ea5f9b141903054afbc273062 Mon Sep 17 00:00:00 2001 From: Carl Schwan Date: Tue, 5 Jul 2022 11:25:44 +0200 Subject: [PATCH 1/3] Handle one time 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 --- config/config.sample.php | 15 +++++ .../Token/PublicKeyTokenProvider.php | 2 +- .../Token/PublicKeyTokenProviderTest.php | 60 +++++++++++++++++-- 3 files changed, 72 insertions(+), 5 deletions(-) diff --git a/config/config.sample.php b/config/config.sample.php index 9856aeba4d725..025cf1105a01a 100644 --- a/config/config.sample.php +++ b/config/config.sample.php @@ -308,6 +308,21 @@ */ 'auth.webauthn.enabled' => true, +/** + * Whether encrypted password should be stored in the database + * + * The passwords are only decrypted using the login token stored uniquely in the + * clients and allow to connect to external storages, autoconfigure mail account in + * the mail app and periodically check if the password it still valid. + * + * This might be desirable to disable this functionality when using one time + * passwords or when having a password policy enforcing long passwords (> 300 + * characters). + * + * By default the passwords are stored encrypted in the database. + */ +'auth.storeCryptedPassword' => true, + /** * By default the login form is always available. There are cases (SSO) where an * admin wants to avoid users entering their credentials to the system if the SSO diff --git a/lib/private/Authentication/Token/PublicKeyTokenProvider.php b/lib/private/Authentication/Token/PublicKeyTokenProvider.php index a1d75828e2758..16425a2e1c337 100644 --- a/lib/private/Authentication/Token/PublicKeyTokenProvider.php +++ b/lib/private/Authentication/Token/PublicKeyTokenProvider.php @@ -368,7 +368,7 @@ private function newToken(string $token, $dbToken->setPublicKey($publicKey); $dbToken->setPrivateKey($this->encrypt($privateKey, $token)); - if (!is_null($password)) { + if (!is_null($password) && $this->config->getSystemValueBool('auth.storeCryptedPassword', true)) { $dbToken->setPassword($this->encryptPassword($password, $publicKey)); } diff --git a/tests/lib/Authentication/Token/PublicKeyTokenProviderTest.php b/tests/lib/Authentication/Token/PublicKeyTokenProviderTest.php index 6ad57515c1627..8e6f699f0b85b 100644 --- a/tests/lib/Authentication/Token/PublicKeyTokenProviderTest.php +++ b/tests/lib/Authentication/Token/PublicKeyTokenProviderTest.php @@ -25,6 +25,7 @@ use OC\Authentication\Exceptions\ExpiredTokenException; use OC\Authentication\Exceptions\InvalidTokenException; +use OC\Authentication\Exceptions\PasswordlessTokenException; use OC\Authentication\Token\IToken; use OC\Authentication\Token\PublicKeyToken; use OC\Authentication\Token\PublicKeyTokenMapper; @@ -83,6 +84,10 @@ public function testGenerateToken() { $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'; $type = IToken::PERMANENT_TOKEN; + $this->config->method('getSystemValueBool') + ->willReturnMap([ + ['auth.storeCryptedPassword', true, true], + ]); $actual = $this->tokenProvider->generateToken($token, $uid, $user, $password, $name, $type, IToken::DO_NOT_REMEMBER); $this->assertInstanceOf(PublicKeyToken::class, $actual); @@ -93,6 +98,29 @@ public function testGenerateToken() { $this->assertSame($password, $this->tokenProvider->getPassword($actual, $token)); } + public function testGenerateTokenNoPassword() { + $token = 'token'; + $uid = 'user'; + $user = 'User'; + $password = 'passme'; + $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'; + $type = IToken::PERMANENT_TOKEN; + $this->config->method('getSystemValueBool') + ->willReturnMap([ + ['auth.storeCryptedPassword', true, false], + ]); + $this->expectException(PasswordlessTokenException::class); + + $actual = $this->tokenProvider->generateToken($token, $uid, $user, $password, $name, $type, IToken::DO_NOT_REMEMBER); + + $this->assertInstanceOf(PublicKeyToken::class, $actual); + $this->assertSame($uid, $actual->getUID()); + $this->assertSame($user, $actual->getLoginName()); + $this->assertSame($name, $actual->getName()); + $this->assertSame(IToken::DO_NOT_REMEMBER, $actual->getRemember()); + $this->tokenProvider->getPassword($actual, $token); + } + public function testGenerateTokenInvalidName() { $token = 'token'; $uid = 'user'; @@ -103,6 +131,10 @@ public function testGenerateTokenInvalidName() { . 'User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.2.12) Gecko/20101026 Firefox/3.6.12' . 'User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.2.12) Gecko/20101026 Firefox/3.6.12'; $type = IToken::PERMANENT_TOKEN; + $this->config->method('getSystemValueBool') + ->willReturnMap([ + ['auth.storeCryptedPassword', true, true], + ]); $actual = $this->tokenProvider->generateToken($token, $uid, $user, $password, $name, $type, IToken::DO_NOT_REMEMBER); @@ -157,6 +189,10 @@ public function testGetPassword() { $password = 'passme'; $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'; $type = IToken::PERMANENT_TOKEN; + $this->config->method('getSystemValueBool') + ->willReturnMap([ + ['auth.storeCryptedPassword', true, true], + ]); $actual = $this->tokenProvider->generateToken($token, $uid, $user, $password, $name, $type, IToken::DO_NOT_REMEMBER); @@ -185,6 +221,10 @@ public function testGetPasswordInvalidToken() { $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'; $type = IToken::PERMANENT_TOKEN; + $this->config->method('getSystemValueBool') + ->willReturnMap([ + ['auth.storeCryptedPassword', true, true], + ]); $actual = $this->tokenProvider->generateToken($token, $uid, $user, $password, $name, $type, IToken::DO_NOT_REMEMBER); $this->tokenProvider->getPassword($actual, 'wrongtoken'); @@ -197,6 +237,10 @@ public function testSetPassword() { $password = 'passme'; $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'; $type = IToken::PERMANENT_TOKEN; + $this->config->method('getSystemValueBool') + ->willReturnMap([ + ['auth.storeCryptedPassword', true, true], + ]); $actual = $this->tokenProvider->generateToken($token, $uid, $user, $password, $name, $type, IToken::DO_NOT_REMEMBER); @@ -301,7 +345,7 @@ public function testRenewSessionTokenWithoutPassword() { $this->tokenProvider->renewSessionToken('oldId', 'newId'); } - public function testRenewSessionTokenWithPassword() { + public function testRenewSessionTokenWithPassword(): void { $token = 'oldId'; $uid = 'user'; $user = 'User'; @@ -309,6 +353,10 @@ public function testRenewSessionTokenWithPassword() { $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'; $type = IToken::PERMANENT_TOKEN; + $this->config->method('getSystemValueBool') + ->willReturnMap([ + ['auth.storeCryptedPassword', true, true], + ]); $oldToken = $this->tokenProvider->generateToken($token, $uid, $user, $password, $name, $type, IToken::DO_NOT_REMEMBER); $this->mapper @@ -319,7 +367,7 @@ public function testRenewSessionTokenWithPassword() { $this->mapper ->expects($this->once()) ->method('insert') - ->with($this->callback(function (PublicKeyToken $token) use ($user, $uid, $name) { + ->with($this->callback(function (PublicKeyToken $token) use ($user, $uid, $name): bool { return $token->getUID() === $uid && $token->getLoginName() === $user && $token->getName() === $name && @@ -331,14 +379,14 @@ public function testRenewSessionTokenWithPassword() { $this->mapper ->expects($this->once()) ->method('delete') - ->with($this->callback(function ($token) use ($oldToken) { + ->with($this->callback(function ($token) use ($oldToken): bool { return $token === $oldToken; })); $this->tokenProvider->renewSessionToken('oldId', 'newId'); } - public function testGetToken() { + public function testGetToken(): void { $token = new PublicKeyToken(); $this->config->method('getSystemValue') @@ -441,6 +489,10 @@ public function testRotate() { $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'; $type = IToken::PERMANENT_TOKEN; + $this->config->method('getSystemValueBool') + ->willReturnMap([ + ['auth.storeCryptedPassword', true, true], + ]); $actual = $this->tokenProvider->generateToken($token, $uid, $user, $password, $name, $type, IToken::DO_NOT_REMEMBER); $new = $this->tokenProvider->rotate($actual, 'oldtoken', 'newtoken'); From 1c23c029af1ef83935badb8b63cb4dffac59b1e4 Mon Sep 17 00:00:00 2001 From: Carl Schwan Date: Tue, 5 Jul 2022 11:37:14 +0200 Subject: [PATCH 2/3] Handler 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. Signed-off-by: Carl Schwan --- .../Token/PublicKeyTokenProvider.php | 5 ++++- .../Token/PublicKeyTokenProviderTest.php | 19 +++++++++++++++++++ 2 files changed, 23 insertions(+), 1 deletion(-) diff --git a/lib/private/Authentication/Token/PublicKeyTokenProvider.php b/lib/private/Authentication/Token/PublicKeyTokenProvider.php index 16425a2e1c337..96bf9a8608798 100644 --- a/lib/private/Authentication/Token/PublicKeyTokenProvider.php +++ b/lib/private/Authentication/Token/PublicKeyTokenProvider.php @@ -346,7 +346,7 @@ private function newToken(string $token, $config = array_merge([ 'digest_alg' => 'sha512', - 'private_key_bits' => 2048, + 'private_key_bits' => $password !== null && strlen($password) > 250 ? 4096 : 2048, ], $this->config->getSystemValue('openssl', [])); // Generate new key @@ -369,6 +369,9 @@ private function newToken(string $token, $dbToken->setPrivateKey($this->encrypt($privateKey, $token)); if (!is_null($password) && $this->config->getSystemValueBool('auth.storeCryptedPassword', true)) { + if (strlen($password) > 469) { + 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'); + } $dbToken->setPassword($this->encryptPassword($password, $publicKey)); } diff --git a/tests/lib/Authentication/Token/PublicKeyTokenProviderTest.php b/tests/lib/Authentication/Token/PublicKeyTokenProviderTest.php index 8e6f699f0b85b..db61244db5b65 100644 --- a/tests/lib/Authentication/Token/PublicKeyTokenProviderTest.php +++ b/tests/lib/Authentication/Token/PublicKeyTokenProviderTest.php @@ -121,6 +121,25 @@ public function testGenerateTokenNoPassword() { $this->tokenProvider->getPassword($actual, $token); } + public function testGenerateTokenLongPassword() { + $token = 'token'; + $uid = 'user'; + $user = 'User'; + $password = ''; + for ($i = 0; $i < 500; $i++) { + $password .= 'e'; + } + $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'; + $type = IToken::PERMANENT_TOKEN; + $this->config->method('getSystemValueBool') + ->willReturnMap([ + ['auth.storeCryptedPassword', true, true], + ]); + $this->expectException(\RuntimeException::class); + + $actual = $this->tokenProvider->generateToken($token, $uid, $user, $password, $name, $type, IToken::DO_NOT_REMEMBER); + } + public function testGenerateTokenInvalidName() { $token = 'token'; $uid = 'user'; From f99a06c89a116cbc447b5fb5d2ec27462b9fba51 Mon Sep 17 00:00:00 2001 From: Carl Schwan Date: Tue, 5 Jul 2022 11:47:25 +0200 Subject: [PATCH 3/3] Don't allow setting password bigger than 469 characters Signed-off-by: Carl Schwan --- .../lib/Controller/ChangePasswordController.php | 11 ++++++++++- apps/settings/src/components/UserList/UserRow.vue | 1 + .../templates/settings/personal/security/password.php | 1 + 3 files changed, 12 insertions(+), 1 deletion(-) diff --git a/apps/settings/lib/Controller/ChangePasswordController.php b/apps/settings/lib/Controller/ChangePasswordController.php index 7c3ab9546bc59..41f2584721cd8 100644 --- a/apps/settings/lib/Controller/ChangePasswordController.php +++ b/apps/settings/lib/Controller/ChangePasswordController.php @@ -107,7 +107,7 @@ public function changePersonalPassword(string $oldpassword = '', string $newpass } try { - if ($newpassword === null || $user->setPassword($newpassword) === false) { + if ($newpassword === null || strlen($newpassword) > 469 || $user->setPassword($newpassword) === false) { return new JSONResponse([ 'status' => 'error', 'data' => [ @@ -158,6 +158,15 @@ public function changeUserPassword(string $username = null, string $password = n ]); } + if (strlen($password) > 469) { + return new JSONResponse([ + 'status' => 'error', + 'data' => [ + 'message' => $this->l->t('Unable to change password. Password too long.'), + ], + ]); + } + $currentUser = $this->userSession->getUser(); $targetUser = $this->userManager->get($username); if ($currentUser === null || $targetUser === null || diff --git a/apps/settings/src/components/UserList/UserRow.vue b/apps/settings/src/components/UserList/UserRow.vue index de0a09f2221de..f2947019f40bf 100644 --- a/apps/settings/src/components/UserList/UserRow.vue +++ b/apps/settings/src/components/UserList/UserRow.vue @@ -107,6 +107,7 @@ ref="password" :disabled="loading.password || loading.all" :minlength="minPasswordLength" + maxlength="469" :placeholder="t('settings', 'Add new password')" autocapitalize="off" autocomplete="new-password" diff --git a/apps/settings/templates/settings/personal/security/password.php b/apps/settings/templates/settings/personal/security/password.php index 88536ab6b238a..85959e252ccdf 100644 --- a/apps/settings/templates/settings/personal/security/password.php +++ b/apps/settings/templates/settings/personal/security/password.php @@ -46,6 +46,7 @@