diff options
author | Carl Schwan <carl@carlschwan.eu> | 2022-07-05 12:25:44 +0300 |
---|---|---|
committer | Carl Schwan <carl@carlschwan.eu> | 2022-07-29 17:15:38 +0300 |
commit | f4795f6dacb5bf8f568e99ced155ea6389190137 (patch) | |
tree | 340c03d2ef5875fd75d909ce38a3106a86bd90fd | |
parent | 6a3cd32a2dc1e5451ca762e171b62493be11dbca (diff) |
Handle one time and large passwordsbackport/stable24/one-time-password
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 <carl@carlschwan.eu>
4 files changed, 115 insertions, 8 deletions
diff --git a/apps/settings/lib/Controller/ChangePasswordController.php b/apps/settings/lib/Controller/ChangePasswordController.php index 8dd1e6ba028..f595368563f 100644 --- a/apps/settings/lib/Controller/ChangePasswordController.php +++ b/apps/settings/lib/Controller/ChangePasswordController.php @@ -107,7 +107,7 @@ class ChangePasswordController extends Controller { } try { - if ($newpassword === null || $user->setPassword($newpassword) === false) { + if ($newpassword === null || strlen($newpassword) > 469 || $user->setPassword($newpassword) === false) { return new JSONResponse([ 'status' => 'error' ]); @@ -155,6 +155,16 @@ class ChangePasswordController extends Controller { ]); } + 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/config/config.sample.php b/config/config.sample.php index f7b63b1491f..d770aeb903d 100644 --- a/config/config.sample.php +++ b/config/config.sample.php @@ -309,6 +309,21 @@ $CONFIG = [ '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 * app is unavailable. diff --git a/lib/private/Authentication/Token/PublicKeyTokenProvider.php b/lib/private/Authentication/Token/PublicKeyTokenProvider.php index a1d75828e27..26928025b23 100644 --- a/lib/private/Authentication/Token/PublicKeyTokenProvider.php +++ b/lib/private/Authentication/Token/PublicKeyTokenProvider.php @@ -346,7 +346,7 @@ class PublicKeyTokenProvider implements IProvider { $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 @@ -368,7 +368,10 @@ class PublicKeyTokenProvider implements IProvider { $dbToken->setPublicKey($publicKey); $dbToken->setPrivateKey($this->encrypt($privateKey, $token)); - if (!is_null($password)) { + 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)); } @@ -398,7 +401,7 @@ class PublicKeyTokenProvider implements IProvider { $this->cache->clear(); // prevent setting an empty pw as result of pw-less-login - if ($password === '') { + if ($password === '' || !$this->config->getSystemValueBool('auth.storeCryptedPassword', true)) { return; } diff --git a/tests/lib/Authentication/Token/PublicKeyTokenProviderTest.php b/tests/lib/Authentication/Token/PublicKeyTokenProviderTest.php index 6ad57515c16..1ef0aa80817 100644 --- a/tests/lib/Authentication/Token/PublicKeyTokenProviderTest.php +++ b/tests/lib/Authentication/Token/PublicKeyTokenProviderTest.php @@ -25,6 +25,7 @@ namespace Test\Authentication\Token; 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 @@ class PublicKeyTokenProviderTest extends TestCase { $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,48 @@ class PublicKeyTokenProviderTest extends TestCase { $this->assertSame($password, $this->tokenProvider->getPassword($actual, $token)); } + public function testGenerateTokenNoPassword(): void { + $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 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'; @@ -103,6 +150,10 @@ class PublicKeyTokenProviderTest extends TestCase { . '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); @@ -120,6 +171,10 @@ class PublicKeyTokenProviderTest extends TestCase { ->method('updateActivity') ->with($tk, $this->time); $tk->setLastActivity($this->time - 200); + $this->config->method('getSystemValueBool') + ->willReturnMap([ + ['auth.storeCryptedPassword', true, true], + ]); $this->tokenProvider->updateTokenActivity($tk); @@ -157,6 +212,10 @@ class PublicKeyTokenProviderTest extends TestCase { $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 +244,10 @@ class PublicKeyTokenProviderTest extends TestCase { $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 +260,10 @@ class PublicKeyTokenProviderTest extends TestCase { $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 +368,7 @@ class PublicKeyTokenProviderTest extends TestCase { $this->tokenProvider->renewSessionToken('oldId', 'newId'); } - public function testRenewSessionTokenWithPassword() { + public function testRenewSessionTokenWithPassword(): void { $token = 'oldId'; $uid = 'user'; $user = 'User'; @@ -309,6 +376,10 @@ class PublicKeyTokenProviderTest extends TestCase { $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 +390,7 @@ class PublicKeyTokenProviderTest extends TestCase { $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 +402,14 @@ class PublicKeyTokenProviderTest extends TestCase { $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 +512,10 @@ class PublicKeyTokenProviderTest extends TestCase { $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'); @@ -507,6 +582,10 @@ class PublicKeyTokenProviderTest extends TestCase { 'random2', IToken::PERMANENT_TOKEN, IToken::REMEMBER); + $this->config->method('getSystemValueBool') + ->willReturnMap([ + ['auth.storeCryptedPassword', true, true], + ]); $this->mapper->method('hasExpiredTokens') ->with($uid) |