Skip to content

Commit

Permalink
enhance(backend): Replace bcrypt with Argon2
Browse files Browse the repository at this point in the history
Squashed commit of the following:

commit 4862fd8467d529c54d0aa3e6abff15a574459a8b
Author: Shun Sakai <sorairolake@protonmail.ch>
Date:   Wed Oct 23 20:44:30 2024 +0900

    chore(backend): Update `argon2` package

commit a52eff5deaee39c0a70c83da49ca58d0eab8d513
Author: NoriDev <m1nthing2322@gmail.com>
Date:   Mon Oct 7 18:11:34 2024 +0900

    Revert "tweak 0dc322b6 (1673beta/cherrypick#88)"

    This reverts commit ab6a5d0c3dbe7146de19d72d08658b1c011fe30a.

commit be51daec8a916a2668ea5794e067bde06499e1a4
Author: Mar0xy <marie@kaifa.ch>
Date:   Wed Sep 27 21:46:56 2023 +0200

    upd: rehash misskey passwords with argon2 on login

commit 67b124b7e6e8f1b1d1738ea9a123ab0500876d58
Author: Mar0xy <marie@kaifa.ch>
Date:   Fri Sep 22 00:21:57 2023 +0200

    upd: swap bcrypt to argon2
  • Loading branch information
sorairolake committed Nov 17, 2024
1 parent 9aebf0c commit 3cae483
Show file tree
Hide file tree
Showing 19 changed files with 106 additions and 92 deletions.
1 change: 1 addition & 0 deletions packages/backend/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,7 @@
"accepts": "1.3.8",
"ajv": "8.17.1",
"archiver": "7.0.1",
"argon2": "^0.41.1",
"async-mutex": "0.5.0",
"bcryptjs": "2.4.3",
"blurhash": "2.0.5",
Expand Down
5 changes: 2 additions & 3 deletions packages/backend/src/core/CreateSystemUserService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@

import { randomUUID } from 'node:crypto';
import { Inject, Injectable } from '@nestjs/common';
import bcrypt from 'bcryptjs';
import * as argon2 from 'argon2';
import { IsNull, DataSource } from 'typeorm';
import { genRsaKeyPair } from '@/misc/gen-key-pair.js';
import { MiUser } from '@/models/User.js';
Expand All @@ -32,8 +32,7 @@ export class CreateSystemUserService {
const password = randomUUID();

// Generate hash of password
const salt = await bcrypt.genSalt(8);
const hash = await bcrypt.hash(password, salt);
const hash = await argon2.hash(password);

// Generate secret
const secret = generateNativeUserToken();
Expand Down
5 changes: 2 additions & 3 deletions packages/backend/src/core/SignupService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@

import { generateKeyPair } from 'node:crypto';
import { Inject, Injectable } from '@nestjs/common';
import bcrypt from 'bcryptjs';
import * as argon2 from 'argon2';
import { DataSource, IsNull } from 'typeorm';
import { DI } from '@/di-symbols.js';
import type { MiMeta, UsedUsernamesRepository, UsersRepository } from '@/models/_.js';
Expand Down Expand Up @@ -69,8 +69,7 @@ export class SignupService {
}

// Generate hash of password
const salt = await bcrypt.genSalt(8);
hash = await bcrypt.hash(password, salt);
hash = await argon2.hash(password);
}

// Generate secret
Expand Down
19 changes: 18 additions & 1 deletion packages/backend/src/server/api/SigninApiService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@

import { Inject, Injectable } from '@nestjs/common';
import bcrypt from 'bcryptjs';
import * as argon2 from 'argon2';
import { IsNull } from 'typeorm';
import * as Misskey from 'misskey-js';
import { DI } from '@/di-symbols.js';
Expand Down Expand Up @@ -155,7 +156,7 @@ export class SigninApiService {
}

// Compare password
const same = await bcrypt.compare(password, profile.password!);
const same = await argon2.verify(profile.password!, password) || bcrypt.compareSync(password, profile.password!);

const fail = async (status?: number, failure?: { id: string; }) => {
// Append signin history
Expand Down Expand Up @@ -204,6 +205,14 @@ export class SigninApiService {
}

if (same) {
// Check if the password is still hashed using bcrypt
if (profile.password!.startsWith('$2')) {
// Rehash the password using Argon2
const newHash = await argon2.hash(password);
this.userProfilesRepository.update(user.id, {
password: newHash,
});
}
return this.signinService.signin(request, reply, user);
} else {
return await fail(403, {
Expand All @@ -220,6 +229,14 @@ export class SigninApiService {
}

try {
// Check if the password is still hashed using bcrypt
if (profile.password!.startsWith('$2')) {
// Rehash the password using Argon2
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, {
Expand Down
5 changes: 2 additions & 3 deletions packages/backend/src/server/api/SignupApiService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
*/

import { Inject, Injectable } from '@nestjs/common';
import bcrypt from 'bcryptjs';
import * as argon2 from 'argon2';
import { IsNull } from 'typeorm';
import { DI } from '@/di-symbols.js';
import type { RegistrationTicketsRepository, UsedUsernamesRepository, UserPendingsRepository, UserProfilesRepository, UsersRepository, MiRegistrationTicket, MiMeta } from '@/models/_.js';
Expand Down Expand Up @@ -186,8 +186,7 @@ export class SignupApiService {
const code = secureRndstr(16, { chars: L_CHARS });

// Generate hash of password
const salt = await bcrypt.genSalt(8);
const hash = await bcrypt.hash(password, salt);
const hash = await argon2.hash(password);

const pendingUser = await this.userPendingsRepository.insertOne({
id: this.idService.gen(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
*/

import { Inject, Injectable } from '@nestjs/common';
import bcrypt from 'bcryptjs';
import * as argon2 from 'argon2';
import { Endpoint } from '@/server/api/endpoint-base.js';
import type { UsersRepository, UserProfilesRepository } from '@/models/_.js';
import { DI } from '@/di-symbols.js';
Expand Down Expand Up @@ -65,7 +65,7 @@ export default class extends Endpoint<typeof meta, typeof paramDef> { // eslint-
const passwd = secureRndstr(8);

// Generate hash of password
const hash = bcrypt.hashSync(passwd);
const hash = await argon2.hash(passwd);

await this.userProfilesRepository.update({
userId: user.id,
Expand Down
4 changes: 2 additions & 2 deletions packages/backend/src/server/api/endpoints/i/2fa/key-done.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
* SPDX-License-Identifier: AGPL-3.0-only
*/

import bcrypt from 'bcryptjs';
import * as argon2 from 'argon2';
import { Inject, Injectable } from '@nestjs/common';
import { Endpoint } from '@/server/api/endpoint-base.js';
import { UserEntityService } from '@/core/entities/UserEntityService.js';
Expand Down Expand Up @@ -86,7 +86,7 @@ export default class extends Endpoint<typeof meta, typeof paramDef> {
}
}

const passwordMatched = await bcrypt.compare(ps.password, profile.password ?? '');
const passwordMatched = await argon2.verify(profile.password ?? '', ps.password);
if (!passwordMatched) {
throw new ApiError(meta.errors.incorrectPassword);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
* SPDX-License-Identifier: AGPL-3.0-only
*/

import bcrypt from 'bcryptjs';
import * as argon2 from 'argon2';
import { Inject, Injectable } from '@nestjs/common';
import { Endpoint } from '@/server/api/endpoint-base.js';
import type { UserProfilesRepository } from '@/models/_.js';
Expand Down Expand Up @@ -217,7 +217,7 @@ export default class extends Endpoint<typeof meta, typeof paramDef> {
}
}

const passwordMatched = await bcrypt.compare(ps.password, profile.password ?? '');
const passwordMatched = await argon2.verify(profile.password ?? '', ps.password);
if (!passwordMatched) {
throw new ApiError(meta.errors.incorrectPassword);
}
Expand Down
4 changes: 2 additions & 2 deletions packages/backend/src/server/api/endpoints/i/2fa/register.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
* SPDX-License-Identifier: AGPL-3.0-only
*/

import bcrypt from 'bcryptjs';
import * as argon2 from 'argon2';
import * as OTPAuth from 'otpauth';
import * as QRCode from 'qrcode';
import { Inject, Injectable } from '@nestjs/common';
Expand Down Expand Up @@ -77,7 +77,7 @@ export default class extends Endpoint<typeof meta, typeof paramDef> { // eslint-
}
}

const passwordMatched = await bcrypt.compare(ps.password, profile.password ?? '');
const passwordMatched = await argon2.verify(profile.password ?? '', ps.password);
if (!passwordMatched) {
throw new ApiError(meta.errors.incorrectPassword);
}
Expand Down
4 changes: 2 additions & 2 deletions packages/backend/src/server/api/endpoints/i/2fa/remove-key.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
* SPDX-License-Identifier: AGPL-3.0-only
*/

import bcrypt from 'bcryptjs';
import * as argon2 from 'argon2';
import { Inject, Injectable } from '@nestjs/common';
import { Endpoint } from '@/server/api/endpoint-base.js';
import type { UserProfilesRepository, UserSecurityKeysRepository } from '@/models/_.js';
Expand Down Expand Up @@ -66,7 +66,7 @@ export default class extends Endpoint<typeof meta, typeof paramDef> { // eslint-
}
}

const passwordMatched = await bcrypt.compare(ps.password, profile.password ?? '');
const passwordMatched = await argon2.verify(profile.password ?? '', ps.password);
if (!passwordMatched) {
throw new ApiError(meta.errors.incorrectPassword);
}
Expand Down
4 changes: 2 additions & 2 deletions packages/backend/src/server/api/endpoints/i/2fa/unregister.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
* SPDX-License-Identifier: AGPL-3.0-only
*/

import bcrypt from 'bcryptjs';
import * as argon2 from 'argon2';
import { Inject, Injectable } from '@nestjs/common';
import { Endpoint } from '@/server/api/endpoint-base.js';
import { UserEntityService } from '@/core/entities/UserEntityService.js';
Expand Down Expand Up @@ -62,7 +62,7 @@ export default class extends Endpoint<typeof meta, typeof paramDef> { // eslint-
}
}

const passwordMatched = await bcrypt.compare(ps.password, profile.password ?? '');
const passwordMatched = await argon2.verify(profile.password ?? '', ps.password);
if (!passwordMatched) {
throw new ApiError(meta.errors.incorrectPassword);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
* SPDX-License-Identifier: AGPL-3.0-only
*/

import bcrypt from 'bcryptjs';
import { Inject, Injectable } from '@nestjs/common';
import { Endpoint } from '@/server/api/endpoint-base.js';
import type { UserSecurityKeysRepository } from '@/models/_.js';
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
* SPDX-License-Identifier: AGPL-3.0-only
*/

import bcrypt from 'bcryptjs';
import * as argon2 from 'argon2';
import { Inject, Injectable } from '@nestjs/common';
import { Endpoint } from '@/server/api/endpoint-base.js';
import type { UserProfilesRepository } from '@/models/_.js';
Expand Down Expand Up @@ -50,15 +50,14 @@ export default class extends Endpoint<typeof meta, typeof paramDef> { // eslint-
}
}

const passwordMatched = await bcrypt.compare(ps.currentPassword, profile.password!);
const passwordMatched = await argon2.verify(profile.password!, ps.currentPassword);

if (!passwordMatched) {
throw new Error('incorrect password');
}

// Generate hash of password
const salt = await bcrypt.genSalt(8);
const hash = await bcrypt.hash(ps.newPassword, salt);
const hash = await argon2.hash(ps.newPassword);

await this.userProfilesRepository.update(me.id, {
password: hash,
Expand Down
4 changes: 2 additions & 2 deletions packages/backend/src/server/api/endpoints/i/delete-account.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
* SPDX-License-Identifier: AGPL-3.0-only
*/

import bcrypt from 'bcryptjs';
import * as argon2 from 'argon2';
import { Inject, Injectable } from '@nestjs/common';
import type { UsersRepository, UserProfilesRepository } from '@/models/_.js';
import { Endpoint } from '@/server/api/endpoint-base.js';
Expand Down Expand Up @@ -59,7 +59,7 @@ export default class extends Endpoint<typeof meta, typeof paramDef> { // eslint-
return;
}

const passwordMatched = await bcrypt.compare(ps.password, profile.password!);
const passwordMatched = await argon2.verify(profile.password!, ps.password);
if (!passwordMatched) {
throw new Error('incorrect password');
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
* SPDX-License-Identifier: AGPL-3.0-only
*/

import bcrypt from 'bcryptjs';
import * as argon2 from 'argon2';
import { Inject, Injectable } from '@nestjs/common';
import { Endpoint } from '@/server/api/endpoint-base.js';
import type { UsersRepository, UserProfilesRepository } from '@/models/_.js';
Expand Down Expand Up @@ -43,7 +43,7 @@ export default class extends Endpoint<typeof meta, typeof paramDef> { // eslint-
const profile = await this.userProfilesRepository.findOneByOrFail({ userId: me.id });

// Compare password
const same = await bcrypt.compare(ps.password, profile.password!);
const same = await argon2.verify(profile.password!, ps.password);

if (!same) {
throw new Error('incorrect password');
Expand Down
4 changes: 2 additions & 2 deletions packages/backend/src/server/api/endpoints/i/update-email.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@

import { Inject, Injectable } from '@nestjs/common';
import ms from 'ms';
import bcrypt from 'bcryptjs';
import * as argon2 from 'argon2';
import { Endpoint } from '@/server/api/endpoint-base.js';
import type { MiMeta, UserProfilesRepository } from '@/models/_.js';
import { UserEntityService } from '@/core/entities/UserEntityService.js';
Expand Down Expand Up @@ -96,7 +96,7 @@ export default class extends Endpoint<typeof meta, typeof paramDef> { // eslint-
}
}

const passwordMatched = await bcrypt.compare(ps.password, profile.password!);
const passwordMatched = await argon2.verify(profile.password!, ps.password);
if (!passwordMatched) {
throw new ApiError(meta.errors.incorrectPassword);
}
Expand Down
5 changes: 2 additions & 3 deletions packages/backend/src/server/api/endpoints/reset-password.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
* SPDX-License-Identifier: AGPL-3.0-only
*/

import bcrypt from 'bcryptjs';
import * as argon2 from 'argon2';
import { Inject, Injectable } from '@nestjs/common';
import type { UserProfilesRepository, PasswordResetRequestsRepository } from '@/models/_.js';
import { Endpoint } from '@/server/api/endpoint-base.js';
Expand Down Expand Up @@ -53,8 +53,7 @@ export default class extends Endpoint<typeof meta, typeof paramDef> { // eslint-
}

// Generate hash of password
const salt = await bcrypt.genSalt(8);
const hash = await bcrypt.hash(ps.password, salt);
const hash = await argon2.hash(ps.password);

await this.userProfilesRepository.update(req.userId, {
password: hash,
Expand Down
10 changes: 5 additions & 5 deletions packages/backend/test/e2e/timelines.ts
Original file line number Diff line number Diff line change
Expand Up @@ -397,7 +397,7 @@ describe('Timelines', () => {
assert.strictEqual(res.body.some(note => note.id === bobNote2.id), true);
assert.strictEqual(res.body.some(note => note.id === carolNote1.id), false);
assert.strictEqual(res.body.some(note => note.id === carolNote2.id), false);
}, 1000 * 10);
}, 1000 * 100);

test.concurrent('フォローしているユーザーのチャンネル投稿が含まれない', async () => {
const [alice, bob] = await Promise.all([signup(), signup()]);
Expand Down Expand Up @@ -744,7 +744,7 @@ describe('Timelines', () => {

assert.strictEqual(res.body.some(note => note.id === bobNote1.id), false);
assert.strictEqual(res.body.some(note => note.id === bobNote2.id), true);
}, 1000 * 10);
}, 1000 * 100);
});

describe('Social TL', () => {
Expand Down Expand Up @@ -955,7 +955,7 @@ describe('Timelines', () => {

assert.strictEqual(res.body.some(note => note.id === bobNote1.id), false);
assert.strictEqual(res.body.some(note => note.id === bobNote2.id), true);
}, 1000 * 10);
}, 1000 * 100);
});

describe('User List TL', () => {
Expand Down Expand Up @@ -1168,7 +1168,7 @@ describe('Timelines', () => {

assert.strictEqual(res.body.some(note => note.id === bobNote1.id), false);
assert.strictEqual(res.body.some(note => note.id === bobNote2.id), true);
}, 1000 * 10);
}, 1000 * 100);

test.concurrent('リスインしているユーザーの自身宛ての visibility: specified なノートが含まれる', async () => {
const [alice, bob] = await Promise.all([signup(), signup()]);
Expand Down Expand Up @@ -1327,7 +1327,7 @@ describe('Timelines', () => {

assert.strictEqual(res.body.some(note => note.id === bobNote1.id), false);
assert.strictEqual(res.body.some(note => note.id === bobNote2.id), true);
}, 1000 * 10);
}, 1000 * 100);

test.concurrent('[withChannelNotes: true] チャンネル投稿が含まれる', async () => {
const [alice, bob] = await Promise.all([signup(), signup()]);
Expand Down
Loading

0 comments on commit 3cae483

Please sign in to comment.