mirror of
https://git.joinsharkey.org/Sharkey/Sharkey.git
synced 2024-11-29 19:23:09 +02:00
Compare commits
2 commits
9d6c24fb6d
...
68d8b11be7
Author | SHA1 | Date | |
---|---|---|---|
|
68d8b11be7 | ||
|
b6c59fa98e |
5 changed files with 209 additions and 150 deletions
|
@ -11,7 +11,7 @@ import type { MiUserProfile, UserProfilesRepository, UsersRepository } from '@/m
|
||||||
import { bindThis } from '@/decorators.js';
|
import { bindThis } from '@/decorators.js';
|
||||||
import { isDuplicateKeyValueError } from '@/misc/is-duplicate-key-value-error.js';
|
import { isDuplicateKeyValueError } from '@/misc/is-duplicate-key-value-error.js';
|
||||||
import type { MiLocalUser } from '@/models/User.js';
|
import type { MiLocalUser } from '@/models/User.js';
|
||||||
import * as crypto from 'node:crypto';
|
import { secureishCompare } from '@/misc/secure-ish-compare.js';
|
||||||
|
|
||||||
@Injectable()
|
@Injectable()
|
||||||
export class UserAuthService {
|
export class UserAuthService {
|
||||||
|
@ -29,7 +29,7 @@ export class UserAuthService {
|
||||||
if (profile.twoFactorBackupSecret?.includes(token)) {
|
if (profile.twoFactorBackupSecret?.includes(token)) {
|
||||||
await this.userProfilesRepository.update({ userId: profile.userId }, {
|
await this.userProfilesRepository.update({ userId: profile.userId }, {
|
||||||
twoFactorBackupSecret: profile.twoFactorBackupSecret.filter(
|
twoFactorBackupSecret: profile.twoFactorBackupSecret.filter(
|
||||||
(secret) => !crypto.timingSafeEqual(secret, token)
|
(secret) => !secureishCompare(secret, token)
|
||||||
),
|
),
|
||||||
});
|
});
|
||||||
} else {
|
} else {
|
||||||
|
|
33
packages/backend/src/misc/secure-ish-compare.ts
Normal file
33
packages/backend/src/misc/secure-ish-compare.ts
Normal file
|
@ -0,0 +1,33 @@
|
||||||
|
/*
|
||||||
|
* SPDX-FileCopyrightText: dakkar and other sharkey contributors
|
||||||
|
* SPDX-License-Identifier: AGPL-3.0-only
|
||||||
|
*/
|
||||||
|
|
||||||
|
import * as crypto from 'node:crypto';
|
||||||
|
|
||||||
|
export function secureishCompare(expected: string, given:string) {
|
||||||
|
const expectedLen = expected.length;
|
||||||
|
const givenLen = given.length;
|
||||||
|
|
||||||
|
/*
|
||||||
|
this ensures that the two strings are the same length, so
|
||||||
|
`timingSafeEqual` will not throw an error
|
||||||
|
|
||||||
|
notice that we perform the same operations regardless of the
|
||||||
|
length of the strings, we're trying not to leak timing
|
||||||
|
information!
|
||||||
|
*/
|
||||||
|
const paddedExpected = Buffer.from(expected + given);
|
||||||
|
const paddedGiven = Buffer.from(given + expected);
|
||||||
|
|
||||||
|
/*
|
||||||
|
if the two strings were equal, `sortOfEqual` will be true
|
||||||
|
|
||||||
|
if will be true in other cases, too! like `abc` and `abcabc`; but
|
||||||
|
then, `expectedLen` and `givenLen` would be different, and we'd
|
||||||
|
return false
|
||||||
|
*/
|
||||||
|
const sortOfEqual = crypto.timingSafeEqual(paddedExpected, paddedGiven);
|
||||||
|
|
||||||
|
return sortOfEqual && (givenLen === expectedLen);
|
||||||
|
}
|
|
@ -38,6 +38,7 @@ import type { FastifyInstance, FastifyRequest, FastifyReply, FastifyPluginOption
|
||||||
import type { FindOptionsWhere } from 'typeorm';
|
import type { FindOptionsWhere } from 'typeorm';
|
||||||
import type Logger from '@/logger.js';
|
import type Logger from '@/logger.js';
|
||||||
import { LoggerService } from '@/core/LoggerService.js';
|
import { LoggerService } from '@/core/LoggerService.js';
|
||||||
|
import { secureishCompare } from '@/misc/secure-ish-compare.js';
|
||||||
|
|
||||||
const ACTIVITY_JSON = 'application/activity+json; charset=utf-8';
|
const ACTIVITY_JSON = 'application/activity+json; charset=utf-8';
|
||||||
const LD_JSON = 'application/ld+json; profile="https://www.w3.org/ns/activitystreams"; charset=utf-8';
|
const LD_JSON = 'application/ld+json; profile="https://www.w3.org/ns/activitystreams"; charset=utf-8';
|
||||||
|
@ -287,7 +288,7 @@ export class ActivityPubServerService {
|
||||||
|
|
||||||
const hash = crypto.createHash('sha256').update(request.rawBody).digest('base64');
|
const hash = crypto.createHash('sha256').update(request.rawBody).digest('base64');
|
||||||
|
|
||||||
if (! crypto.timingSafeEqual(hash, digestValue)) {
|
if (!secureishCompare(hash, digestValue)) {
|
||||||
// Invalid digest
|
// Invalid digest
|
||||||
reply.code(401);
|
reply.code(401);
|
||||||
return;
|
return;
|
||||||
|
|
25
packages/backend/test/unit/misc/secure-ish-compare.ts
Normal file
25
packages/backend/test/unit/misc/secure-ish-compare.ts
Normal file
|
@ -0,0 +1,25 @@
|
||||||
|
/*
|
||||||
|
* SPDX-FileCopyrightText: dakkar and other sharkey contributors
|
||||||
|
* SPDX-License-Identifier: AGPL-3.0-only
|
||||||
|
*/
|
||||||
|
|
||||||
|
import { secureishCompare } from '@/misc/secure-ish-compare.js';
|
||||||
|
|
||||||
|
describe(secureishCompare, () => {
|
||||||
|
it('should return true if strings are equal', () => {
|
||||||
|
expect(secureishCompare('abc','abc')).toBe(true);
|
||||||
|
expect(secureishCompare('aaa','aaa')).toBe(true);
|
||||||
|
});
|
||||||
|
it('should return false if strings are different', () => {
|
||||||
|
expect(secureishCompare('abc','def')).toBe(false);
|
||||||
|
});
|
||||||
|
it('should return false if one is prefix of the other', () => {
|
||||||
|
expect(secureishCompare('abc','abcabc')).toBe(false);
|
||||||
|
expect(secureishCompare('abcabc','abc')).toBe(false);
|
||||||
|
expect(secureishCompare('aaa','aa')).toBe(false);
|
||||||
|
});
|
||||||
|
it('should return false if strings are very different', () => {
|
||||||
|
expect(secureishCompare('abc','defghi')).toBe(false);
|
||||||
|
expect(secureishCompare('defghi','abc')).toBe(false);
|
||||||
|
});
|
||||||
|
});
|
Loading…
Reference in a new issue