fix(passwords): use argon2id instead of bcrypt
Some checks failed
Docker / build-and-push (backend) (push) Has been cancelled
Docker / build-and-push (frontend) (push) Has been cancelled
E2E Tests / backend-sqlite (push) Has been cancelled
E2E Tests / backend-mariadb (push) Has been cancelled
E2E Tests / backend-postgres (push) Has been cancelled
E2E Tests / Build test build of frontend (push) Has been cancelled
Lint and check format / Lint files and check formatting (push) Has been cancelled
REUSE Compliance Check / reuse (push) Has been cancelled
Scorecard supply-chain security / Scorecard analysis (push) Has been cancelled
Static Analysis / Njsscan code scanning (push) Has been cancelled
Static Analysis / CodeQL analysis (javascript) (push) Has been cancelled
Run tests & build / Test and build with NodeJS 20 (push) Has been cancelled
E2E Tests / frontend-cypress (1) (push) Has been cancelled
E2E Tests / frontend-cypress (2) (push) Has been cancelled
E2E Tests / frontend-cypress (3) (push) Has been cancelled

OWASP [1] recommends for password hashing the following algorithms in
descending order: argon2id, scrypt, bcrypt. They state that bcrypt may
be used in legacy systems or when required due to legal regulations.
We're however not building any legacy application. Even HedgeDoc 1.x
utilizes a more modern algorithm by using scrypt.

While bcrypt is not insecure per se, our implementation had a major
security flaw, leading to invalid passwords being accepted in certain
cases. The bcrypt nodejs package - and the OWASP cheatsheet as well -
point out, that the maximum input length of passwords is limited to 72
bytes with bcrypt. When some user has a password longer than 72 bytes in
use, only the first 72 bytes are required to log in successfully.
Depending on the encoding (which could be UTF-8 or UTF-16 depending on
different circumstances) this could in worst-case be at 36 characters,
which is not very unusual for a password. See also [2].

This commit changes the used algorithm to argon2id. Argon2id has been in
use for several years now and seems to be a well-designed password
hashing function that even won the 2015 Password Hashing Competition.
Argon2 does not have any real-world max input length for passwords (it
is at 4 GiB).

The node-rs/argon2 implementation seems to be well maintained, widely
used (more than 150k downloads per week) and is published with
provenance, proving that the npm package was built on GitHub actions
using the source code in the repository. The implementation is written
in Rust, so it should be safe against memory leakages etc.

[1]: https://cheatsheetseries.owasp.org/cheatsheets/Password_Storage_Che
     at_Sheet.html#password-hashing-algorithms
[2]: https://security.stackexchange.com/a/39851

Signed-off-by: Erik Michelson <github@erik.michelson.eu>
This commit is contained in:
Erik Michelson 2024-08-08 12:05:20 +02:00
parent 6684b0f886
commit f30f0d8e51
5 changed files with 271 additions and 125 deletions

View file

@ -1,51 +1,67 @@
/*
* SPDX-FileCopyrightText: 2021 The HedgeDoc developers (see AUTHORS file)
* SPDX-FileCopyrightText: 2024 The HedgeDoc developers (see AUTHORS file)
*
* SPDX-License-Identifier: AGPL-3.0-only
*/
import bcrypt from 'bcrypt';
import { randomBytes } from 'crypto';
import argon2 from '@node-rs/argon2';
import { bufferToBase64Url, checkPassword, hashPassword } from './password';
const testPassword = 'thisIsATestPassword';
const hashOfTestPassword =
'$argon2id$v=19$m=19456,t=2,p=1$40fR6RcTofpngCk4xXhY8w$wAkstPrKkMgrb26TyNqrUzT78jZ+EIjwcJYZHcjrL+Q';
describe('hashPassword', () => {
it('output looks like a bcrypt hash with 2^12 rounds of hashing', async () => {
it('output looks like a argon2 hash', async () => {
/*
* a bcrypt hash example with the different parts highlighted:
* $2a$10$N9qo8uLOickgx2ZMRZoMyeIjZAgcfl7p92ldGxad68LJZdL17lhWy
* \__/\/ \____________________/\_____________________________/
* Alg Cost Salt Hash
* from https://en.wikipedia.org/wiki/Bcrypt#Description
* a argon2 hash example with the different parts highlighted:
* $argon2id$v=19$m=19456,t=2,p=1$40fR6RcTofpngCk4xXhY8w$wAkstPrKkMgrb26TyNqrUzT78jZ+EIjwcJYZHcjrL+Q
* \________/\___/\______________/\_____________________/\_________________________________________/
* Alg Ver Parameters Salt Hash
*/
const regexBcrypt = /^\$2[abxy]\$12\$[A-Za-z0-9/.]{53}$/;
const regexArgon2 =
/^\$argon2id\$v=19\$m=19456,t=2,p=1\$[\w+./]{22}\$[\w+./]{43}$/;
const hash = await hashPassword(testPassword);
expect(regexBcrypt.test(hash)).toBeTruthy();
expect(regexArgon2.test(hash)).toBeTruthy();
});
it('calls bcrypt.hash with the correct parameters', async () => {
const spy = jest.spyOn(bcrypt, 'hash');
it('calls argon2.hash with the correct parameters', async () => {
const spy = jest.spyOn(argon2, 'hash');
await hashPassword(testPassword);
expect(spy).toHaveBeenCalledWith(testPassword, 12);
expect(spy).toHaveBeenCalledWith(testPassword, {
memoryCost: 19456,
timeCost: 2,
parallelism: 1,
});
});
});
describe('checkPassword', () => {
it("is returning true if the inputs are a plaintext password and it's bcrypt-hashed version", async () => {
const hashOfTestPassword =
'$2a$12$WHKCq4c0rg19zyx5WgX0p.or0rjSKYpIBcHhQQGLrxrr6FfMPylIW';
it("is returning true if the inputs are a plaintext password and it's hashed version", async () => {
await checkPassword(testPassword, hashOfTestPassword).then((result) =>
expect(result).toBeTruthy(),
);
});
it('fails, if secret is too short', async () => {
const secret = bufferToBase64Url(randomBytes(54));
const hash = await hashPassword(secret);
await checkPassword(secret, hash).then((result) =>
it('fails, if password is non-matching', async () => {
const password = 'anotherTestPassword';
await checkPassword(password, hashOfTestPassword).then((result) =>
expect(result).toBeFalsy(),
);
});
it('calls argon2.verify with the correct parameters', async () => {
const spy = jest.spyOn(argon2, 'verify');
await checkPassword(testPassword, hashOfTestPassword);
expect(spy).toHaveBeenCalledWith(hashOfTestPassword, testPassword);
});
it('verifies even passwords longer than 72 bytes', async () => {
const password = 'a'.repeat(70);
const hash =
'$argon2id$v=19$m=19456,t=2,p=1$4aBLKxd7MqYQqf/th835yQ$iUMe+HHphn8B8q6gQ3IPL2k1+Bdbb505r7LuqZIMTjg';
await checkPassword(password, hash).then((result) =>
expect(result).toBeTruthy(),
);
await checkPassword(secret.substr(0, secret.length - 1), hash).then(
(result) => expect(result).toBeFalsy(),
const password2 = 'a'.repeat(73);
await checkPassword(password2, hash).then((result) =>
expect(result).toBeFalsy(),
);
});
});

View file

@ -1,21 +1,38 @@
/*
* SPDX-FileCopyrightText: 2021 The HedgeDoc developers (see AUTHORS file)
* SPDX-FileCopyrightText: 2024 The HedgeDoc developers (see AUTHORS file)
*
* SPDX-License-Identifier: AGPL-3.0-only
*/
import { compare, hash } from 'bcrypt';
import { hash, verify } from '@node-rs/argon2';
/**
* Hashes a password using argon2id
*
* @param cleartext The password to hash
* @returns The hashed password
*/
export async function hashPassword(cleartext: string): Promise<string> {
// hash the password with bcrypt and 2^12 iterations
// this was decided on the basis of https://cheatsheetseries.owasp.org/cheatsheets/Password_Storage_Cheat_Sheet.html#bcrypt
return await hash(cleartext, 12);
// options recommended by OWASP
// https://cheatsheetseries.owasp.org/cheatsheets/Password_Storage_Cheat_Sheet.html#argon2id
return await hash(cleartext, {
memoryCost: 19456,
timeCost: 2,
parallelism: 1,
});
}
/**
* Checks if a cleartext password matches a password hash
*
* @param cleartext The cleartext password
* @param passwordHash The password hash
* @returns Whether the password matches the hash
*/
export async function checkPassword(
cleartext: string,
password: string,
passwordHash: string,
): Promise<boolean> {
return await compare(cleartext, password);
return await verify(passwordHash, cleartext);
}
export function bufferToBase64Url(text: Buffer): string {