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!)
This commit is contained in:
ShittyKopper 2024-02-25 19:55:49 +03:00
parent 2fa0e238b7
commit b31a59a297
15 changed files with 54 additions and 64 deletions

View file

@ -5,8 +5,7 @@
import { randomUUID } from 'node:crypto'; import { randomUUID } from 'node:crypto';
import { Inject, Injectable } from '@nestjs/common'; 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 { IsNull, DataSource } from 'typeorm';
import { genRsaKeyPair } from '@/misc/gen-key-pair.js'; import { genRsaKeyPair } from '@/misc/gen-key-pair.js';
import { MiUser } from '@/models/User.js'; import { MiUser } from '@/models/User.js';
@ -33,8 +32,8 @@ export class CreateSystemUserService {
const password = randomUUID(); const password = randomUUID();
// Generate hash of password // Generate hash of password
//const salt = await bcrypt.genSalt(8); const salt = await bcrypt.genSalt(8);
const hash = await argon2.hash(password); const hash = await bcrypt.hash(password, salt);
// Generate secret // Generate secret
const secret = generateNativeUserToken(); const secret = generateNativeUserToken();

View file

@ -5,8 +5,7 @@
import { generateKeyPair } from 'node:crypto'; import { generateKeyPair } from 'node:crypto';
import { Inject, Injectable } from '@nestjs/common'; import { Inject, Injectable } from '@nestjs/common';
//import bcrypt from 'bcryptjs'; import bcrypt from 'bcryptjs';
import * as argon2 from 'argon2';
import { DataSource, IsNull } from 'typeorm'; import { DataSource, IsNull } from 'typeorm';
import { DI } from '@/di-symbols.js'; import { DI } from '@/di-symbols.js';
import type { UsedUsernamesRepository, UsersRepository } from '@/models/_.js'; import type { UsedUsernamesRepository, UsersRepository } from '@/models/_.js';
@ -69,8 +68,8 @@ export class SignupService {
} }
// Generate hash of password // Generate hash of password
//const salt = await bcrypt.genSalt(8); const salt = await bcrypt.genSalt(8);
hash = await argon2.hash(password); hash = await bcrypt.hash(password, salt);
} }
// Generate secret // Generate secret

View file

@ -139,7 +139,22 @@ export class SigninApiService {
} }
// Compare password // 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 }) => { const fail = async (status?: number, failure?: { id: string }) => {
// Append signin history // Append signin history
@ -156,12 +171,6 @@ export class SigninApiService {
if (!profile.twoFactorEnabled) { if (!profile.twoFactorEnabled) {
if (same) { 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 }); if (!instance.approvalRequiredForSignup && !user.approved) this.usersRepository.update(user.id, { approved: true });
return this.signinService.signin(request, reply, user); return this.signinService.signin(request, reply, user);
@ -180,12 +189,6 @@ export class SigninApiService {
} }
try { 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); await this.userAuthService.twoFactorAuthenticate(profile, token);
} catch (e) { } catch (e) {
return await fail(403, { return await fail(403, {

View file

@ -4,8 +4,7 @@
*/ */
import { Inject, Injectable } from '@nestjs/common'; import { Inject, Injectable } from '@nestjs/common';
//import bcrypt from 'bcryptjs'; import bcrypt from 'bcryptjs';
import * as argon2 from 'argon2';
import { IsNull } from 'typeorm'; import { IsNull } from 'typeorm';
import { DI } from '@/di-symbols.js'; import { DI } from '@/di-symbols.js';
import type { RegistrationTicketsRepository, UsedUsernamesRepository, UserPendingsRepository, UserProfilesRepository, UsersRepository, MiRegistrationTicket } from '@/models/_.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 { FastifyReplyError } from '@/misc/fastify-reply-error.js';
import { bindThis } from '@/decorators.js'; import { bindThis } from '@/decorators.js';
import { L_CHARS, secureRndstr } from '@/misc/secure-rndstr.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 { RoleService } from '@/core/RoleService.js';
import { SigninService } from './SigninService.js';
import instance from './endpoints/charts/instance.js';
import type { FastifyRequest, FastifyReply } from 'fastify';
@Injectable() @Injectable()
export class SignupApiService { export class SignupApiService {
@ -193,8 +192,8 @@ export class SignupApiService {
const code = secureRndstr(16, { chars: L_CHARS }); const code = secureRndstr(16, { chars: L_CHARS });
// Generate hash of password // Generate hash of password
//const salt = await bcrypt.genSalt(8); const salt = await bcrypt.genSalt(8);
const hash = await argon2.hash(password); const hash = await bcrypt.hash(password, salt);
const pendingUser = await this.userPendingsRepository.insert({ const pendingUser = await this.userPendingsRepository.insert({
id: this.idService.gen(), id: this.idService.gen(),

View file

@ -4,8 +4,7 @@
*/ */
import { Inject, Injectable } from '@nestjs/common'; import { Inject, Injectable } from '@nestjs/common';
//import bcrypt from 'bcryptjs'; import bcrypt from 'bcryptjs';
import * as argon2 from 'argon2';
import { Endpoint } from '@/server/api/endpoint-base.js'; import { Endpoint } from '@/server/api/endpoint-base.js';
import type { UsersRepository, UserProfilesRepository } from '@/models/_.js'; import type { UsersRepository, UserProfilesRepository } from '@/models/_.js';
import { DI } from '@/di-symbols.js'; import { DI } from '@/di-symbols.js';
@ -66,7 +65,8 @@ export default class extends Endpoint<typeof meta, typeof paramDef> { // eslint-
const passwd = secureRndstr(8); const passwd = secureRndstr(8);
// Generate hash of password // 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({ await this.userProfilesRepository.update({
userId: user.id, userId: user.id,

View file

@ -3,8 +3,7 @@
* SPDX-License-Identifier: AGPL-3.0-only * SPDX-License-Identifier: AGPL-3.0-only
*/ */
//import bcrypt from 'bcryptjs'; import bcrypt from 'bcryptjs';
import * as argon2 from 'argon2';
import { Inject, Injectable } from '@nestjs/common'; import { Inject, Injectable } from '@nestjs/common';
import { Endpoint } from '@/server/api/endpoint-base.js'; import { Endpoint } from '@/server/api/endpoint-base.js';
import { UserEntityService } from '@/core/entities/UserEntityService.js'; import { UserEntityService } from '@/core/entities/UserEntityService.js';
@ -87,7 +86,7 @@ export default class extends Endpoint<typeof meta, typeof paramDef> {
} }
} }
const passwordMatched = await argon2.verify(profile.password ?? '', ps.password); const passwordMatched = await bcrypt.compare(ps.password, profile.password ?? '');
if (!passwordMatched) { if (!passwordMatched) {
throw new ApiError(meta.errors.incorrectPassword); throw new ApiError(meta.errors.incorrectPassword);
} }

View file

@ -3,8 +3,7 @@
* SPDX-License-Identifier: AGPL-3.0-only * SPDX-License-Identifier: AGPL-3.0-only
*/ */
//import bcrypt from 'bcryptjs'; import bcrypt from 'bcryptjs';
import * as argon2 from 'argon2';
import { Inject, Injectable } from '@nestjs/common'; import { Inject, Injectable } from '@nestjs/common';
import { Endpoint } from '@/server/api/endpoint-base.js'; import { Endpoint } from '@/server/api/endpoint-base.js';
import type { UserProfilesRepository } from '@/models/_.js'; import type { UserProfilesRepository } from '@/models/_.js';
@ -219,7 +218,7 @@ export default class extends Endpoint<typeof meta, typeof paramDef> {
} }
} }
const passwordMatched = await argon2.verify(profile.password ?? '', ps.password); const passwordMatched = await bcrypt.compare(ps.password, profile.password ?? '');
if (!passwordMatched) { if (!passwordMatched) {
throw new ApiError(meta.errors.incorrectPassword); throw new ApiError(meta.errors.incorrectPassword);
} }

View file

@ -3,8 +3,7 @@
* SPDX-License-Identifier: AGPL-3.0-only * SPDX-License-Identifier: AGPL-3.0-only
*/ */
//import bcrypt from 'bcryptjs'; import bcrypt from 'bcryptjs';
import * as argon2 from 'argon2';
import * as OTPAuth from 'otpauth'; import * as OTPAuth from 'otpauth';
import * as QRCode from 'qrcode'; import * as QRCode from 'qrcode';
import { Inject, Injectable } from '@nestjs/common'; import { Inject, Injectable } from '@nestjs/common';
@ -78,7 +77,7 @@ export default class extends Endpoint<typeof meta, typeof paramDef> { // eslint-
} }
} }
const passwordMatched = await argon2.verify(profile.password ?? '', ps.password); const passwordMatched = await bcrypt.compare(ps.password, profile.password ?? '');
if (!passwordMatched) { if (!passwordMatched) {
throw new ApiError(meta.errors.incorrectPassword); throw new ApiError(meta.errors.incorrectPassword);
} }

View file

@ -3,8 +3,7 @@
* SPDX-License-Identifier: AGPL-3.0-only * SPDX-License-Identifier: AGPL-3.0-only
*/ */
//import bcrypt from 'bcryptjs'; import bcrypt from 'bcryptjs';
import * as argon2 from 'argon2';
import { Inject, Injectable } from '@nestjs/common'; import { Inject, Injectable } from '@nestjs/common';
import { Endpoint } from '@/server/api/endpoint-base.js'; import { Endpoint } from '@/server/api/endpoint-base.js';
import type { UserProfilesRepository, UserSecurityKeysRepository } from '@/models/_.js'; import type { UserProfilesRepository, UserSecurityKeysRepository } from '@/models/_.js';
@ -68,7 +67,7 @@ export default class extends Endpoint<typeof meta, typeof paramDef> { // eslint-
} }
} }
const passwordMatched = await argon2.verify(profile.password ?? '', ps.password); const passwordMatched = await bcrypt.compare(ps.password, profile.password ?? '');
if (!passwordMatched) { if (!passwordMatched) {
throw new ApiError(meta.errors.incorrectPassword); throw new ApiError(meta.errors.incorrectPassword);
} }

View file

@ -3,8 +3,7 @@
* SPDX-License-Identifier: AGPL-3.0-only * SPDX-License-Identifier: AGPL-3.0-only
*/ */
//import bcrypt from 'bcryptjs'; import bcrypt from 'bcryptjs';
import * as argon2 from 'argon2';
import { Inject, Injectable } from '@nestjs/common'; import { Inject, Injectable } from '@nestjs/common';
import { Endpoint } from '@/server/api/endpoint-base.js'; import { Endpoint } from '@/server/api/endpoint-base.js';
import { UserEntityService } from '@/core/entities/UserEntityService.js'; import { UserEntityService } from '@/core/entities/UserEntityService.js';
@ -63,7 +62,7 @@ export default class extends Endpoint<typeof meta, typeof paramDef> { // eslint-
} }
} }
const passwordMatched = await argon2.verify(profile.password ?? '', ps.password); const passwordMatched = await bcrypt.compare(ps.password, profile.password ?? '');
if (!passwordMatched) { if (!passwordMatched) {
throw new ApiError(meta.errors.incorrectPassword); throw new ApiError(meta.errors.incorrectPassword);
} }

View file

@ -3,8 +3,7 @@
* SPDX-License-Identifier: AGPL-3.0-only * SPDX-License-Identifier: AGPL-3.0-only
*/ */
//import bcrypt from 'bcryptjs'; import bcrypt from 'bcryptjs';
import * as argon2 from 'argon2';
import { Inject, Injectable } from '@nestjs/common'; import { Inject, Injectable } from '@nestjs/common';
import { Endpoint } from '@/server/api/endpoint-base.js'; import { Endpoint } from '@/server/api/endpoint-base.js';
import type { UserProfilesRepository } from '@/models/_.js'; import type { UserProfilesRepository } from '@/models/_.js';
@ -51,15 +50,15 @@ export default class extends Endpoint<typeof meta, typeof paramDef> { // eslint-
} }
} }
const passwordMatched = await argon2.verify(profile.password!, ps.currentPassword); const passwordMatched = await bcrypt.compare(ps.currentPassword, profile.password!);
if (!passwordMatched) { if (!passwordMatched) {
throw new Error('incorrect password'); throw new Error('incorrect password');
} }
// Generate hash of password // Generate hash of password
//const salt = await bcrypt.genSalt(8); const salt = await bcrypt.genSalt(8);
const hash = await argon2.hash(ps.newPassword); const hash = await bcrypt.hash(ps.newPassword, salt);
await this.userProfilesRepository.update(me.id, { await this.userProfilesRepository.update(me.id, {
password: hash, password: hash,

View file

@ -3,8 +3,7 @@
* SPDX-License-Identifier: AGPL-3.0-only * SPDX-License-Identifier: AGPL-3.0-only
*/ */
//import bcrypt from 'bcryptjs'; import bcrypt from 'bcryptjs';
import * as argon2 from 'argon2';
import { Inject, Injectable } from '@nestjs/common'; import { Inject, Injectable } from '@nestjs/common';
import type { UsersRepository, UserProfilesRepository } from '@/models/_.js'; import type { UsersRepository, UserProfilesRepository } from '@/models/_.js';
import { Endpoint } from '@/server/api/endpoint-base.js'; import { Endpoint } from '@/server/api/endpoint-base.js';
@ -60,7 +59,7 @@ export default class extends Endpoint<typeof meta, typeof paramDef> { // eslint-
return; return;
} }
const passwordMatched = await argon2.verify(profile.password!, ps.password); const passwordMatched = await bcrypt.compare(ps.password, profile.password!);
if (!passwordMatched) { if (!passwordMatched) {
throw new Error('incorrect password'); throw new Error('incorrect password');
} }

View file

@ -3,8 +3,7 @@
* SPDX-License-Identifier: AGPL-3.0-only * SPDX-License-Identifier: AGPL-3.0-only
*/ */
//import bcrypt from 'bcryptjs'; import bcrypt from 'bcryptjs';
import * as argon2 from 'argon2';
import { Inject, Injectable } from '@nestjs/common'; import { Inject, Injectable } from '@nestjs/common';
import { Endpoint } from '@/server/api/endpoint-base.js'; import { Endpoint } from '@/server/api/endpoint-base.js';
import type { UsersRepository, UserProfilesRepository } from '@/models/_.js'; import type { UsersRepository, UserProfilesRepository } from '@/models/_.js';
@ -44,7 +43,7 @@ export default class extends Endpoint<typeof meta, typeof paramDef> { // eslint-
const profile = await this.userProfilesRepository.findOneByOrFail({ userId: me.id }); const profile = await this.userProfilesRepository.findOneByOrFail({ userId: me.id });
// Compare password // Compare password
const same = await argon2.verify(profile.password!, ps.password); const same = await bcrypt.compare(ps.password, profile.password!);
if (!same) { if (!same) {
throw new Error('incorrect password'); throw new Error('incorrect password');

View file

@ -5,8 +5,7 @@
import { Inject, Injectable } from '@nestjs/common'; import { Inject, Injectable } from '@nestjs/common';
import ms from 'ms'; import ms from 'ms';
//import bcrypt from 'bcryptjs'; import bcrypt from 'bcryptjs';
import * as argon2 from 'argon2';
import { Endpoint } from '@/server/api/endpoint-base.js'; import { Endpoint } from '@/server/api/endpoint-base.js';
import type { UserProfilesRepository } from '@/models/_.js'; import type { UserProfilesRepository } from '@/models/_.js';
import { UserEntityService } from '@/core/entities/UserEntityService.js'; import { UserEntityService } from '@/core/entities/UserEntityService.js';
@ -88,7 +87,7 @@ export default class extends Endpoint<typeof meta, typeof paramDef> { // eslint-
} }
} }
const passwordMatched = await argon2.verify(profile.password!, ps.password); const passwordMatched = await bcrypt.compare(ps.password, profile.password!);
if (!passwordMatched) { if (!passwordMatched) {
throw new ApiError(meta.errors.incorrectPassword); throw new ApiError(meta.errors.incorrectPassword);
} }

View file

@ -3,8 +3,7 @@
* SPDX-License-Identifier: AGPL-3.0-only * SPDX-License-Identifier: AGPL-3.0-only
*/ */
//import bcrypt from 'bcryptjs'; import bcrypt from 'bcryptjs';
import * as argon2 from 'argon2';
import { Inject, Injectable } from '@nestjs/common'; import { Inject, Injectable } from '@nestjs/common';
import type { UserProfilesRepository, PasswordResetRequestsRepository } from '@/models/_.js'; import type { UserProfilesRepository, PasswordResetRequestsRepository } from '@/models/_.js';
import { Endpoint } from '@/server/api/endpoint-base.js'; import { Endpoint } from '@/server/api/endpoint-base.js';
@ -54,8 +53,8 @@ export default class extends Endpoint<typeof meta, typeof paramDef> { // eslint-
} }
// Generate hash of password // Generate hash of password
//const salt = await bcrypt.genSalt(8); const salt = await bcrypt.genSalt(8);
const hash = await argon2.hash(ps.password); const hash = await bcrypt.hash(ps.password, salt);
await this.userProfilesRepository.update(req.userId, { await this.userProfilesRepository.update(req.userId, {
password: hash, password: hash,