argon2 support #308

Merged
toast merged 5 commits from argon2 into main 2022-12-29 20:13:48 +00:00
4 changed files with 106 additions and 16 deletions

View file

@ -35,6 +35,7 @@
"lodash": "^4.17.21"
},
"dependencies": {
"argon2": "^0.30.2",
"execa": "5.1.1",
"gulp": "4.0.2",
"gulp-cssnano": "2.1.3",

View file

@ -1,10 +1,17 @@
import bcrypt from 'bcryptjs';
import * as argon2 from 'argon2';
export async function hashPassword(password: string): Promise<string> {
const salt = await bcrypt.genSalt(8);
return await bcrypt.hash(password, salt);
return argon2.hash(password);
}
export async function comparePassword(password: string, hash: string): Promise<boolean> {
return await bcrypt.compare(password, hash);
if (isOldAlgorithm(hash)) return bcrypt.compare(password, hash);
return argon2.verify(hash, password);
}
export function isOldAlgorithm(hash: string): boolean {
// bcrypt hashes start with $2[ab]$
return hash.startsWith('$2');
}

View file

@ -8,7 +8,7 @@ import { Users, Signins, UserProfiles, UserSecurityKeys, AttestationChallenges }
import { ILocalUser } from '@/models/entities/user.js';
import { genId } from '@/misc/gen-id.js';
import { getIpHash } from '@/misc/get-ip-hash.js';
import { comparePassword } from '@/misc/password.js';
import { comparePassword, hashPassword, isOldAlgorithm } from '@/misc/password.js';
import signin from '../common/signin.js';
import { verifyLogin, hash } from '../2fa.js';
import { limiter } from '../limiter.js';
@ -69,6 +69,11 @@ export default async (ctx: Koa.Context) => {
// Compare password
const same = await comparePassword(password, profile.password!);
if (same && isOldAlgorithm(profile.password!)) {
toast marked this conversation as resolved
Review

I'm not sure if putting this here before potential other checks doesn't make it susceptible to timing attacks?

I'm not sure if putting this here before potential other checks doesn't make it susceptible to timing attacks?
Review

same is a static check and short circuits.
If same is true, the login would already be successful.
The only difference in timing would be in case of successful validation, in which case it's also static except for ONE time when the hash is updated (and it's already updated by the time you can figure that out).

`same` is a static check and short circuits. If `same` is true, the login would already be successful. The only difference in timing would be in case of successful validation, in which case it's also static except for ONE time when the hash is updated (and it's already updated by the time you can figure that out).
Review

same is a static check and short circuits.
If same is true, the login would already be successful.
The only difference in timing would be in case of successful validation, in which case it's also static except for ONE time when the hash is updated (and it's already updated by the time you can figure that out).

`same` is a static check and short circuits. If `same` is true, the login would already be successful. The only difference in timing would be in case of successful validation, in which case it's also static except for ONE time when the hash is updated (and it's already updated by the time you can figure that out).
profile.password = await hashPassword(password);
await UserProfiles.save(profile);
}
async function fail(): void {
// Append signin history
await Signins.insert({

101
yarn.lock
View file

@ -1051,6 +1051,25 @@ __metadata:
languageName: node
linkType: hard
"@mapbox/node-pre-gyp@npm:^1.0.10":
version: 1.0.10
resolution: "@mapbox/node-pre-gyp@npm:1.0.10"
dependencies:
detect-libc: ^2.0.0
https-proxy-agent: ^5.0.0
make-dir: ^3.1.0
node-fetch: ^2.6.7
nopt: ^5.0.0
npmlog: ^5.0.1
rimraf: ^3.0.2
semver: ^7.3.5
tar: ^6.1.11
bin:
node-pre-gyp: bin/node-pre-gyp
checksum: 1a98db05d955b74dad3814679593df293b9194853698f3f5f1ed00ecd93128cdd4b14fb8767fe44ac6981ef05c23effcfdc88710e7c1de99ccb6f647890597c8
languageName: node
linkType: hard
"@microsoft/api-extractor-model@npm:7.23.3":
version: 7.23.3
resolution: "@microsoft/api-extractor-model@npm:7.23.3"
@ -1203,6 +1222,13 @@ __metadata:
languageName: node
linkType: hard
"@phc/format@npm:^1.0.0":
version: 1.0.0
resolution: "@phc/format@npm:1.0.0"
checksum: 15ee02504fbc16590923d89b1f1c2f5892df27cf2bf19180e5678511413e87b6e5355815a092749cd01698855ee5a0fc5d2393951c727acd650934eed290e26e
languageName: node
linkType: hard
"@redis/bloom@npm:1.0.2":
version: 1.0.2
resolution: "@redis/bloom@npm:1.0.2"
@ -3177,6 +3203,16 @@ __metadata:
languageName: node
linkType: hard
"are-we-there-yet@npm:^2.0.0":
version: 2.0.0
resolution: "are-we-there-yet@npm:2.0.0"
dependencies:
delegates: ^1.0.0
readable-stream: ^3.6.0
checksum: 6c80b4fd04ecee6ba6e737e0b72a4b41bdc64b7d279edfc998678567ff583c8df27e27523bc789f2c99be603ffa9eaa612803da1d886962d2086e7ff6fa90c7c
languageName: node
linkType: hard
"are-we-there-yet@npm:^3.0.0":
version: 3.0.1
resolution: "are-we-there-yet@npm:3.0.1"
@ -3194,6 +3230,17 @@ __metadata:
languageName: node
linkType: hard
"argon2@npm:^0.30.2":
version: 0.30.2
resolution: "argon2@npm:0.30.2"
dependencies:
"@mapbox/node-pre-gyp": ^1.0.10
"@phc/format": ^1.0.0
node-addon-api: ^5.0.0
checksum: 5b0d680d2bb482ed5f46ae2933ff2dc5c1d5d2a984a5c81c63cb311b55a5f67393e2b6da1adf4a1342e146580dd3f888a695d1c56834df710a141c62e9f73ef7
languageName: node
linkType: hard
"argparse@npm:^1.0.7, argparse@npm:~1.0.9":
version: 1.0.10
resolution: "argparse@npm:1.0.10"
@ -4994,7 +5041,7 @@ __metadata:
languageName: node
linkType: hard
"color-support@npm:^1.1.3":
"color-support@npm:^1.1.2, color-support@npm:^1.1.3":
version: 1.1.3
resolution: "color-support@npm:1.1.3"
bin:
@ -5190,7 +5237,7 @@ __metadata:
languageName: node
linkType: hard
"console-control-strings@npm:^1.1.0":
"console-control-strings@npm:^1.0.0, console-control-strings@npm:^1.1.0":
version: 1.1.0
resolution: "console-control-strings@npm:1.1.0"
checksum: 8755d76787f94e6cf79ce4666f0c5519906d7f5b02d4b884cf41e11dcd759ed69c57da0670afd9236d229a46e0f9cf519db0cd829c6dca820bb5a5c3def584ed
@ -7841,6 +7888,7 @@ __metadata:
"@types/gulp": 4.0.9
"@types/gulp-rename": 2.0.1
"@typescript-eslint/parser": ^5.46.1
argon2: ^0.30.2
cross-env: 7.0.3
cypress: 10.3.0
execa: 5.1.1
@ -8021,6 +8069,23 @@ __metadata:
languageName: node
linkType: hard
"gauge@npm:^3.0.0":
version: 3.0.2
resolution: "gauge@npm:3.0.2"
dependencies:
aproba: ^1.0.3 || ^2.0.0
color-support: ^1.1.2
console-control-strings: ^1.0.0
has-unicode: ^2.0.1
object-assign: ^4.1.1
signal-exit: ^3.0.0
string-width: ^4.2.3
strip-ansi: ^6.0.1
wide-align: ^1.1.2
checksum: 81296c00c7410cdd48f997800155fbead4f32e4f82109be0719c63edc8560e6579946cc8abd04205297640691ec26d21b578837fd13a4e96288ab4b40b1dc3e9
languageName: node
linkType: hard
"gauge@npm:^4.0.3":
version: 4.0.4
resolution: "gauge@npm:4.0.4"
@ -11514,7 +11579,7 @@ __metadata:
languageName: node
linkType: hard
"make-dir@npm:^3.0.0":
"make-dir@npm:^3.0.0, make-dir@npm:^3.1.0":
version: 3.1.0
resolution: "make-dir@npm:3.1.0"
dependencies:
@ -12325,7 +12390,7 @@ __metadata:
languageName: node
linkType: hard
"node-fetch@npm:2.6.7, node-fetch@npm:^2.6.1":
"node-fetch@npm:2.6.7, node-fetch@npm:^2.6.1, node-fetch@npm:^2.6.7":
version: 2.6.7
resolution: "node-fetch@npm:2.6.7"
dependencies:
@ -12566,6 +12631,18 @@ __metadata:
languageName: node
linkType: hard
"npmlog@npm:^5.0.1":
version: 5.0.1
resolution: "npmlog@npm:5.0.1"
dependencies:
are-we-there-yet: ^2.0.0
console-control-strings: ^1.1.0
gauge: ^3.0.0
set-blocking: ^2.0.0
checksum: 516b2663028761f062d13e8beb3f00069c5664925871a9b57989642ebe09f23ab02145bf3ab88da7866c4e112cafff72401f61a672c7c8a20edc585a7016ef5f
languageName: node
linkType: hard
"npmlog@npm:^6.0.0":
version: 6.0.2
resolution: "npmlog@npm:6.0.2"
@ -15328,6 +15405,13 @@ __metadata:
languageName: node
linkType: hard
"signal-exit@npm:^3.0.0, signal-exit@npm:^3.0.7":
version: 3.0.7
resolution: "signal-exit@npm:3.0.7"
checksum: a2f098f247adc367dffc27845853e9959b9e88b01cb301658cfe4194352d8d2bb32e18467c786a7fe15f1d44b233ea35633d076d5e737870b7139949d1ab6318
languageName: node
linkType: hard
"signal-exit@npm:^3.0.2, signal-exit@npm:^3.0.3":
version: 3.0.3
resolution: "signal-exit@npm:3.0.3"
@ -15335,13 +15419,6 @@ __metadata:
languageName: node
linkType: hard
"signal-exit@npm:^3.0.7":
version: 3.0.7
resolution: "signal-exit@npm:3.0.7"
checksum: a2f098f247adc367dffc27845853e9959b9e88b01cb301658cfe4194352d8d2bb32e18467c786a7fe15f1d44b233ea35633d076d5e737870b7139949d1ab6318
languageName: node
linkType: hard
"simple-concat@npm:^1.0.0":
version: 1.0.1
resolution: "simple-concat@npm:1.0.1"
@ -17763,7 +17840,7 @@ __metadata:
languageName: node
linkType: hard
"wide-align@npm:^1.1.5":
"wide-align@npm:^1.1.2, wide-align@npm:^1.1.5":
version: 1.1.5
resolution: "wide-align@npm:1.1.5"
dependencies: