From b31a59a297a03157b183e28852893affd537aa42 Mon Sep 17 00:00:00 2001 From: ShittyKopper Date: Sun, 25 Feb 2024 19:55:49 +0300 Subject: [PATCH] upd: flip rehash behavior, convert argon2 into bcrypt argon2 is only really used to allow migrations from firefish-like instances. using argon2 for everything prevents seamless migrations to upstream misskey in exchange for a debatable[1][2] increase in security. so, let's keep accepting existing argon2 hashes, but rehash them to bcrypt on login. [1]: https://infosec.exchange/@epixoip/110912922574721750, https://github.com/epixoip/hmac-bcrypt/?tab=readme-ov-file#justification [2]: the bcrypt implementation used in misskey doesn't support passwords > 72 bytes, but we cannot do anything about *that* without breaking compatibility, bringing us back to where we started (upstream; if you're reading this, please consider hmac-bcrypt!) --- .../src/core/CreateSystemUserService.ts | 7 ++--- packages/backend/src/core/SignupService.ts | 7 ++--- .../src/server/api/SigninApiService.ts | 29 ++++++++++--------- .../src/server/api/SignupApiService.ts | 13 ++++----- .../api/endpoints/admin/reset-password.ts | 6 ++-- .../server/api/endpoints/i/2fa/key-done.ts | 5 ++-- .../api/endpoints/i/2fa/register-key.ts | 5 ++-- .../server/api/endpoints/i/2fa/register.ts | 5 ++-- .../server/api/endpoints/i/2fa/remove-key.ts | 5 ++-- .../server/api/endpoints/i/2fa/unregister.ts | 5 ++-- .../server/api/endpoints/i/change-password.ts | 9 +++--- .../server/api/endpoints/i/delete-account.ts | 5 ++-- .../api/endpoints/i/regenerate-token.ts | 5 ++-- .../server/api/endpoints/i/update-email.ts | 5 ++-- .../server/api/endpoints/reset-password.ts | 7 ++--- 15 files changed, 54 insertions(+), 64 deletions(-) diff --git a/packages/backend/src/core/CreateSystemUserService.ts b/packages/backend/src/core/CreateSystemUserService.ts index 14d814b0e..71c2db77a 100644 --- a/packages/backend/src/core/CreateSystemUserService.ts +++ b/packages/backend/src/core/CreateSystemUserService.ts @@ -5,8 +5,7 @@ import { randomUUID } from 'node:crypto'; import { Inject, Injectable } from '@nestjs/common'; -import * as argon2 from 'argon2'; -//import bcrypt from 'bcryptjs'; +import bcrypt from 'bcryptjs'; import { IsNull, DataSource } from 'typeorm'; import { genRsaKeyPair } from '@/misc/gen-key-pair.js'; import { MiUser } from '@/models/User.js'; @@ -33,8 +32,8 @@ export class CreateSystemUserService { const password = randomUUID(); // Generate hash of password - //const salt = await bcrypt.genSalt(8); - const hash = await argon2.hash(password); + const salt = await bcrypt.genSalt(8); + const hash = await bcrypt.hash(password, salt); // Generate secret const secret = generateNativeUserToken(); diff --git a/packages/backend/src/core/SignupService.ts b/packages/backend/src/core/SignupService.ts index e3d69e5e9..e2da26e6e 100644 --- a/packages/backend/src/core/SignupService.ts +++ b/packages/backend/src/core/SignupService.ts @@ -5,8 +5,7 @@ import { generateKeyPair } from 'node:crypto'; import { Inject, Injectable } from '@nestjs/common'; -//import bcrypt from 'bcryptjs'; -import * as argon2 from 'argon2'; +import bcrypt from 'bcryptjs'; import { DataSource, IsNull } from 'typeorm'; import { DI } from '@/di-symbols.js'; import type { UsedUsernamesRepository, UsersRepository } from '@/models/_.js'; @@ -69,8 +68,8 @@ export class SignupService { } // Generate hash of password - //const salt = await bcrypt.genSalt(8); - hash = await argon2.hash(password); + const salt = await bcrypt.genSalt(8); + hash = await bcrypt.hash(password, salt); } // Generate secret diff --git a/packages/backend/src/server/api/SigninApiService.ts b/packages/backend/src/server/api/SigninApiService.ts index 6fbcacbc1..0fcaabc08 100644 --- a/packages/backend/src/server/api/SigninApiService.ts +++ b/packages/backend/src/server/api/SigninApiService.ts @@ -139,7 +139,22 @@ export class SigninApiService { } // Compare password - const same = await argon2.verify(profile.password!, password) || bcrypt.compareSync(password, profile.password!); + let same; + + if (profile.password?.startsWith('$argon2')) { + same = await argon2.verify(profile.password, password); + + if (same) { + // rehash + const salt = await bcrypt.genSalt(8); + const newHash = await bcrypt.hash(password, salt); + await this.userProfilesRepository.update(user.id, { + password: newHash, + }); + } + } else { + same = await bcrypt.compare(password, profile.password!); + } const fail = async (status?: number, failure?: { id: string }) => { // Append signin history @@ -156,12 +171,6 @@ export class SigninApiService { if (!profile.twoFactorEnabled) { if (same) { - if (profile.password!.startsWith('$2')) { - const newHash = await argon2.hash(password); - this.userProfilesRepository.update(user.id, { - password: newHash - }); - } if (!instance.approvalRequiredForSignup && !user.approved) this.usersRepository.update(user.id, { approved: true }); return this.signinService.signin(request, reply, user); @@ -180,12 +189,6 @@ export class SigninApiService { } try { - if (profile.password!.startsWith('$2')) { - const newHash = await argon2.hash(password); - this.userProfilesRepository.update(user.id, { - password: newHash - }); - } await this.userAuthService.twoFactorAuthenticate(profile, token); } catch (e) { return await fail(403, { diff --git a/packages/backend/src/server/api/SignupApiService.ts b/packages/backend/src/server/api/SignupApiService.ts index 9c221314a..cc5701bd3 100644 --- a/packages/backend/src/server/api/SignupApiService.ts +++ b/packages/backend/src/server/api/SignupApiService.ts @@ -4,8 +4,7 @@ */ import { Inject, Injectable } from '@nestjs/common'; -//import bcrypt from 'bcryptjs'; -import * as argon2 from 'argon2'; +import bcrypt from 'bcryptjs'; import { IsNull } from 'typeorm'; import { DI } from '@/di-symbols.js'; import type { RegistrationTicketsRepository, UsedUsernamesRepository, UserPendingsRepository, UserProfilesRepository, UsersRepository, MiRegistrationTicket } from '@/models/_.js'; @@ -20,10 +19,10 @@ import { MiLocalUser } from '@/models/User.js'; import { FastifyReplyError } from '@/misc/fastify-reply-error.js'; import { bindThis } from '@/decorators.js'; import { L_CHARS, secureRndstr } from '@/misc/secure-rndstr.js'; -import { SigninService } from './SigninService.js'; -import type { FastifyRequest, FastifyReply } from 'fastify'; -import instance from './endpoints/charts/instance.js'; import { RoleService } from '@/core/RoleService.js'; +import { SigninService } from './SigninService.js'; +import instance from './endpoints/charts/instance.js'; +import type { FastifyRequest, FastifyReply } from 'fastify'; @Injectable() export class SignupApiService { @@ -193,8 +192,8 @@ export class SignupApiService { const code = secureRndstr(16, { chars: L_CHARS }); // Generate hash of password - //const salt = await bcrypt.genSalt(8); - const hash = await argon2.hash(password); + const salt = await bcrypt.genSalt(8); + const hash = await bcrypt.hash(password, salt); const pendingUser = await this.userPendingsRepository.insert({ id: this.idService.gen(), diff --git a/packages/backend/src/server/api/endpoints/admin/reset-password.ts b/packages/backend/src/server/api/endpoints/admin/reset-password.ts index 828dbae71..6450da48a 100644 --- a/packages/backend/src/server/api/endpoints/admin/reset-password.ts +++ b/packages/backend/src/server/api/endpoints/admin/reset-password.ts @@ -4,8 +4,7 @@ */ import { Inject, Injectable } from '@nestjs/common'; -//import bcrypt from 'bcryptjs'; -import * as argon2 from 'argon2'; +import bcrypt from 'bcryptjs'; import { Endpoint } from '@/server/api/endpoint-base.js'; import type { UsersRepository, UserProfilesRepository } from '@/models/_.js'; import { DI } from '@/di-symbols.js'; @@ -66,7 +65,8 @@ export default class extends Endpoint { // eslint- const passwd = secureRndstr(8); // Generate hash of password - const hash = await argon2.hash(passwd); + const salt = await bcrypt.genSalt(8); + const hash = await bcrypt.hash(passwd, salt); await this.userProfilesRepository.update({ userId: user.id, diff --git a/packages/backend/src/server/api/endpoints/i/2fa/key-done.ts b/packages/backend/src/server/api/endpoints/i/2fa/key-done.ts index 74ee90b3d..5f738420f 100644 --- a/packages/backend/src/server/api/endpoints/i/2fa/key-done.ts +++ b/packages/backend/src/server/api/endpoints/i/2fa/key-done.ts @@ -3,8 +3,7 @@ * SPDX-License-Identifier: AGPL-3.0-only */ -//import bcrypt from 'bcryptjs'; -import * as argon2 from 'argon2'; +import bcrypt from 'bcryptjs'; import { Inject, Injectable } from '@nestjs/common'; import { Endpoint } from '@/server/api/endpoint-base.js'; import { UserEntityService } from '@/core/entities/UserEntityService.js'; @@ -87,7 +86,7 @@ export default class extends Endpoint { } } - const passwordMatched = await argon2.verify(profile.password ?? '', ps.password); + const passwordMatched = await bcrypt.compare(ps.password, profile.password ?? ''); if (!passwordMatched) { throw new ApiError(meta.errors.incorrectPassword); } diff --git a/packages/backend/src/server/api/endpoints/i/2fa/register-key.ts b/packages/backend/src/server/api/endpoints/i/2fa/register-key.ts index cc6e9ee42..f290d77e8 100644 --- a/packages/backend/src/server/api/endpoints/i/2fa/register-key.ts +++ b/packages/backend/src/server/api/endpoints/i/2fa/register-key.ts @@ -3,8 +3,7 @@ * SPDX-License-Identifier: AGPL-3.0-only */ -//import bcrypt from 'bcryptjs'; -import * as argon2 from 'argon2'; +import bcrypt from 'bcryptjs'; import { Inject, Injectable } from '@nestjs/common'; import { Endpoint } from '@/server/api/endpoint-base.js'; import type { UserProfilesRepository } from '@/models/_.js'; @@ -219,7 +218,7 @@ export default class extends Endpoint { } } - const passwordMatched = await argon2.verify(profile.password ?? '', ps.password); + const passwordMatched = await bcrypt.compare(ps.password, profile.password ?? ''); if (!passwordMatched) { throw new ApiError(meta.errors.incorrectPassword); } diff --git a/packages/backend/src/server/api/endpoints/i/2fa/register.ts b/packages/backend/src/server/api/endpoints/i/2fa/register.ts index 7283159f8..a54c59821 100644 --- a/packages/backend/src/server/api/endpoints/i/2fa/register.ts +++ b/packages/backend/src/server/api/endpoints/i/2fa/register.ts @@ -3,8 +3,7 @@ * SPDX-License-Identifier: AGPL-3.0-only */ -//import bcrypt from 'bcryptjs'; -import * as argon2 from 'argon2'; +import bcrypt from 'bcryptjs'; import * as OTPAuth from 'otpauth'; import * as QRCode from 'qrcode'; import { Inject, Injectable } from '@nestjs/common'; @@ -78,7 +77,7 @@ export default class extends Endpoint { // eslint- } } - const passwordMatched = await argon2.verify(profile.password ?? '', ps.password); + const passwordMatched = await bcrypt.compare(ps.password, profile.password ?? ''); if (!passwordMatched) { throw new ApiError(meta.errors.incorrectPassword); } diff --git a/packages/backend/src/server/api/endpoints/i/2fa/remove-key.ts b/packages/backend/src/server/api/endpoints/i/2fa/remove-key.ts index 098fd5930..4f645e932 100644 --- a/packages/backend/src/server/api/endpoints/i/2fa/remove-key.ts +++ b/packages/backend/src/server/api/endpoints/i/2fa/remove-key.ts @@ -3,8 +3,7 @@ * SPDX-License-Identifier: AGPL-3.0-only */ -//import bcrypt from 'bcryptjs'; -import * as argon2 from 'argon2'; +import bcrypt from 'bcryptjs'; import { Inject, Injectable } from '@nestjs/common'; import { Endpoint } from '@/server/api/endpoint-base.js'; import type { UserProfilesRepository, UserSecurityKeysRepository } from '@/models/_.js'; @@ -68,7 +67,7 @@ export default class extends Endpoint { // eslint- } } - const passwordMatched = await argon2.verify(profile.password ?? '', ps.password); + const passwordMatched = await bcrypt.compare(ps.password, profile.password ?? ''); if (!passwordMatched) { throw new ApiError(meta.errors.incorrectPassword); } diff --git a/packages/backend/src/server/api/endpoints/i/2fa/unregister.ts b/packages/backend/src/server/api/endpoints/i/2fa/unregister.ts index 8da331505..b5a53cc88 100644 --- a/packages/backend/src/server/api/endpoints/i/2fa/unregister.ts +++ b/packages/backend/src/server/api/endpoints/i/2fa/unregister.ts @@ -3,8 +3,7 @@ * SPDX-License-Identifier: AGPL-3.0-only */ -//import bcrypt from 'bcryptjs'; -import * as argon2 from 'argon2'; +import bcrypt from 'bcryptjs'; import { Inject, Injectable } from '@nestjs/common'; import { Endpoint } from '@/server/api/endpoint-base.js'; import { UserEntityService } from '@/core/entities/UserEntityService.js'; @@ -63,7 +62,7 @@ export default class extends Endpoint { // eslint- } } - const passwordMatched = await argon2.verify(profile.password ?? '', ps.password); + const passwordMatched = await bcrypt.compare(ps.password, profile.password ?? ''); if (!passwordMatched) { throw new ApiError(meta.errors.incorrectPassword); } diff --git a/packages/backend/src/server/api/endpoints/i/change-password.ts b/packages/backend/src/server/api/endpoints/i/change-password.ts index 6aedde717..bb78d4714 100644 --- a/packages/backend/src/server/api/endpoints/i/change-password.ts +++ b/packages/backend/src/server/api/endpoints/i/change-password.ts @@ -3,8 +3,7 @@ * SPDX-License-Identifier: AGPL-3.0-only */ -//import bcrypt from 'bcryptjs'; -import * as argon2 from 'argon2'; +import bcrypt from 'bcryptjs'; import { Inject, Injectable } from '@nestjs/common'; import { Endpoint } from '@/server/api/endpoint-base.js'; import type { UserProfilesRepository } from '@/models/_.js'; @@ -51,15 +50,15 @@ export default class extends Endpoint { // eslint- } } - const passwordMatched = await argon2.verify(profile.password!, ps.currentPassword); + const passwordMatched = await bcrypt.compare(ps.currentPassword, profile.password!); if (!passwordMatched) { throw new Error('incorrect password'); } // Generate hash of password - //const salt = await bcrypt.genSalt(8); - const hash = await argon2.hash(ps.newPassword); + const salt = await bcrypt.genSalt(8); + const hash = await bcrypt.hash(ps.newPassword, salt); await this.userProfilesRepository.update(me.id, { password: hash, diff --git a/packages/backend/src/server/api/endpoints/i/delete-account.ts b/packages/backend/src/server/api/endpoints/i/delete-account.ts index af4d601ad..bfa0b4605 100644 --- a/packages/backend/src/server/api/endpoints/i/delete-account.ts +++ b/packages/backend/src/server/api/endpoints/i/delete-account.ts @@ -3,8 +3,7 @@ * SPDX-License-Identifier: AGPL-3.0-only */ -//import bcrypt from 'bcryptjs'; -import * as argon2 from 'argon2'; +import bcrypt from 'bcryptjs'; import { Inject, Injectable } from '@nestjs/common'; import type { UsersRepository, UserProfilesRepository } from '@/models/_.js'; import { Endpoint } from '@/server/api/endpoint-base.js'; @@ -60,7 +59,7 @@ export default class extends Endpoint { // eslint- return; } - const passwordMatched = await argon2.verify(profile.password!, ps.password); + const passwordMatched = await bcrypt.compare(ps.password, profile.password!); if (!passwordMatched) { throw new Error('incorrect password'); } diff --git a/packages/backend/src/server/api/endpoints/i/regenerate-token.ts b/packages/backend/src/server/api/endpoints/i/regenerate-token.ts index e1cdfdc18..78f3cce9a 100644 --- a/packages/backend/src/server/api/endpoints/i/regenerate-token.ts +++ b/packages/backend/src/server/api/endpoints/i/regenerate-token.ts @@ -3,8 +3,7 @@ * SPDX-License-Identifier: AGPL-3.0-only */ -//import bcrypt from 'bcryptjs'; -import * as argon2 from 'argon2'; +import bcrypt from 'bcryptjs'; import { Inject, Injectable } from '@nestjs/common'; import { Endpoint } from '@/server/api/endpoint-base.js'; import type { UsersRepository, UserProfilesRepository } from '@/models/_.js'; @@ -44,7 +43,7 @@ export default class extends Endpoint { // eslint- const profile = await this.userProfilesRepository.findOneByOrFail({ userId: me.id }); // Compare password - const same = await argon2.verify(profile.password!, ps.password); + const same = await bcrypt.compare(ps.password, profile.password!); if (!same) { throw new Error('incorrect password'); diff --git a/packages/backend/src/server/api/endpoints/i/update-email.ts b/packages/backend/src/server/api/endpoints/i/update-email.ts index 08a8301bd..386827869 100644 --- a/packages/backend/src/server/api/endpoints/i/update-email.ts +++ b/packages/backend/src/server/api/endpoints/i/update-email.ts @@ -5,8 +5,7 @@ import { Inject, Injectable } from '@nestjs/common'; import ms from 'ms'; -//import bcrypt from 'bcryptjs'; -import * as argon2 from 'argon2'; +import bcrypt from 'bcryptjs'; import { Endpoint } from '@/server/api/endpoint-base.js'; import type { UserProfilesRepository } from '@/models/_.js'; import { UserEntityService } from '@/core/entities/UserEntityService.js'; @@ -88,7 +87,7 @@ export default class extends Endpoint { // eslint- } } - const passwordMatched = await argon2.verify(profile.password!, ps.password); + const passwordMatched = await bcrypt.compare(ps.password, profile.password!); if (!passwordMatched) { throw new ApiError(meta.errors.incorrectPassword); } diff --git a/packages/backend/src/server/api/endpoints/reset-password.ts b/packages/backend/src/server/api/endpoints/reset-password.ts index 1639b57bc..969389263 100644 --- a/packages/backend/src/server/api/endpoints/reset-password.ts +++ b/packages/backend/src/server/api/endpoints/reset-password.ts @@ -3,8 +3,7 @@ * SPDX-License-Identifier: AGPL-3.0-only */ -//import bcrypt from 'bcryptjs'; -import * as argon2 from 'argon2'; +import bcrypt from 'bcryptjs'; import { Inject, Injectable } from '@nestjs/common'; import type { UserProfilesRepository, PasswordResetRequestsRepository } from '@/models/_.js'; import { Endpoint } from '@/server/api/endpoint-base.js'; @@ -54,8 +53,8 @@ export default class extends Endpoint { // eslint- } // Generate hash of password - //const salt = await bcrypt.genSalt(8); - const hash = await argon2.hash(ps.password); + const salt = await bcrypt.genSalt(8); + const hash = await bcrypt.hash(ps.password, salt); await this.userProfilesRepository.update(req.userId, { password: hash,