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 <carl@carlschwan.eu>
nextcloud · Jul 29, 2022 · f4795f6 · f4795f6
1 parent 6a3cd32
commit f4795f6
Show file tree

Hide file tree

Showing 4 changed files with 115 additions and 8 deletions.
diff --git 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'
 				]);
@@ -155,6 +155,16 @@ 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/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
@@ -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
@@ -368,7 +368,10 @@ 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)) {
+			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 @@ public function updatePasswords(string $uid, string $password) {
 		$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
@@ -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,48 @@ public function testGenerateToken() {
 		$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 @@ 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);
 
@@ -120,6 +171,10 @@ public function testUpdateToken() {
 			->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 @@ 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 +244,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 +260,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,14 +368,18 @@ public function testRenewSessionTokenWithoutPassword() {
 		$this->tokenProvider->renewSessionToken('oldId', 'newId');
 	}
 
-	public function testRenewSessionTokenWithPassword() {
+	public function testRenewSessionTokenWithPassword(): void {
 		$token = 'oldId';
 		$uid = 'user';
 		$user = 'User';
 		$password = 'password';
 		$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 @@ 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 +402,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 +512,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');
@@ -507,6 +582,10 @@ public function testUpdatePasswords() {
 			'random2',
 			IToken::PERMANENT_TOKEN,
 			IToken::REMEMBER);
+		$this->config->method('getSystemValueBool')
+			->willReturnMap([
+				['auth.storeCryptedPassword', true, true],
+			]);
 
 		$this->mapper->method('hasExpiredTokens')
 			->with($uid)