Skip to content

Commit 3e6e0dc

Browse files
committed
feat: argon2id implementation with backwards compatibility for pdkbf2
1 parent adc2400 commit 3e6e0dc

File tree

15 files changed

+819
-46
lines changed

15 files changed

+819
-46
lines changed

app/controllers/web/mobile-config.js

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -430,7 +430,11 @@ async function mobileConfig(ctx, next) {
430430

431431
// validate password
432432
// ensure that the token is valid
433-
const isValid = await isValidPassword(alias.tokens, decrypt(ctx.query.p));
433+
const isValid = await isValidPassword(
434+
alias.tokens,
435+
decrypt(ctx.query.p),
436+
alias
437+
);
434438

435439
if (!isValid) return next(); // 404
436440

app/controllers/web/my-account/download-alias-backup.js

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,8 @@ async function downloadAliasBackup(ctx) {
6767
// ensure that the token is valid
6868
const isValid = await isValidPassword(
6969
alias.tokens,
70-
ctx.request.body.password
70+
ctx.request.body.password,
71+
alias
7172
);
7273

7374
if (!isValid) {

app/controllers/web/my-account/generate-alias-password.js

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -105,7 +105,8 @@ async function generateAliasPassword(ctx) {
105105
// ensure that the token is valid
106106
const isValid = await isValidPassword(
107107
alias.tokens,
108-
ctx.request.body.password
108+
ctx.request.body.password,
109+
alias
109110
);
110111

111112
if (!isValid) {

app/controllers/web/my-account/retrieve-qrcode.js

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,8 @@ async function retrieveQRCode(ctx) {
7474
// ensure that the token is valid
7575
const isValid = await isValidPassword(
7676
alias.tokens,
77-
ctx.request.body.password
77+
ctx.request.body.password,
78+
alias
7879
);
7980

8081
if (!isValid) {

app/models/aliases.js

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -57,13 +57,25 @@ const Token = new mongoose.Schema({
5757
type: String,
5858
select: false,
5959
required: true
60+
},
61+
has_pbkdf2_migration: {
62+
type: Boolean,
63+
default: false,
64+
select: false
6065
}
6166
});
6267

6368
Token.plugin(mongooseCommonPlugin, {
6469
object: 'token',
6570
omitCommonFields: false,
66-
omitExtraFields: ['_id', '__v', 'description', 'salt', 'hash'],
71+
omitExtraFields: [
72+
'_id',
73+
'__v',
74+
'description',
75+
'salt',
76+
'hash',
77+
'has_pbkdf2_migration'
78+
],
6779
uniqueId: false,
6880
locale: false
6981
});

app/models/domains.js

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -171,13 +171,18 @@ const Token = new mongoose.Schema({
171171
type: String,
172172
select: false,
173173
required: true
174+
},
175+
has_pbkdf2_migration: {
176+
type: Boolean,
177+
default: false,
178+
select: false
174179
}
175180
});
176181

177182
Token.plugin(mongooseCommonPlugin, {
178183
object: 'token',
179184
omitCommonFields: false,
180-
omitExtraFields: ['_id', '__v', 'salt', 'hash'],
185+
omitExtraFields: ['_id', '__v', 'salt', 'hash', 'has_pbkdf2_migration'],
181186
uniqueId: false,
182187
locale: false
183188
});

config/index.js

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -720,7 +720,7 @@ const config = {
720720
smtpQueueTimeout: ms('180s'),
721721
smtpLimitMessages: env.NODE_ENV === 'test' ? 10 : 300,
722722
smtpLimitAuth: env.NODE_ENV === 'test' ? Number.MAX_VALUE : 10,
723-
smtpLimitAuthDuration: ms('1h'),
723+
smtpLimitAuthDuration: ms('1d'),
724724
smtpLimitDuration: ms('1d'),
725725
smtpLimitNamespace: `smtp_auth_limit_${env.NODE_ENV.toLowerCase()}`,
726726
supportEmail: env.EMAIL_DEFAULT_FROM_EMAIL,
@@ -1194,6 +1194,21 @@ const config = {
11941194
}
11951195
},
11961196

1197+
//
1198+
// argon2 configuration for domain and alias tokens
1199+
// (separate from passportLocalMongoose which is used for user authentication)
1200+
// <https://github.com/napi-rs/node-rs>
1201+
// <https://cheatsheetseries.owasp.org/cheatsheets/Password_Storage_Cheat_Sheet.html>
1202+
// Bitwarden uses Argon2id for key derivation, with default settings of:
1203+
// 64 MiB memory, 3 iterations, and 4 parallelism
1204+
//
1205+
argon2: {
1206+
memoryCost: 19456, // 19 MiB
1207+
timeCost: 2, // iterations
1208+
parallelism: 1,
1209+
outputLen: 32 // hash length in bytes (8 bits per byte, 32 x 8 = 256 bits)
1210+
},
1211+
11971212
// passport callback options
11981213
passportCallbackOptions: {
11991214
successReturnToOrRedirect: '/my-account',
@@ -1666,6 +1681,7 @@ config.alternatives = alternatives;
16661681
config.views.locals.config = _.pick(config, [
16671682
'smtpMessageMaxSize',
16681683
'alternatives',
1684+
'argon2',
16691685
'smtpLimitMessages',
16701686
'smtpLimitDuration',
16711687
'supportEmail',

helpers/create-password.js

Lines changed: 10 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -4,15 +4,13 @@
44
*/
55

66
const crypto = require('node:crypto');
7-
const { Buffer } = require('node:buffer');
87
const { promisify } = require('node:util');
98

109
const Boom = require('@hapi/boom');
10+
const argon2 = require('@node-rs/argon2');
1111
const cryptoRandomString = require('crypto-random-string');
1212
const isSANB = require('is-string-and-not-blank');
1313

14-
const pbkdf2 = require('./pbkdf2');
15-
1614
const config = require('#config');
1715
const phrases = require('#config/phrases');
1816

@@ -65,18 +63,17 @@ async function createPassword(existingPassword, userInputs = []) {
6563
(await cryptoRandomString.async({
6664
length: 24
6765
}));
66+
67+
//
68+
// use argon2 for domain and alias tokens (quantum-resistant)
69+
// <https://github.com/ranisalt/node-argon2>
70+
//
71+
const hash = await argon2.hash(password, config.argon2);
72+
73+
// generate a random salt for backwards compatibility checking
6874
const buffer = await randomBytes(config.passportLocalMongoose.saltlen);
6975
const salt = buffer.toString(config.passportLocalMongoose.encoding);
70-
const rawHash = await pbkdf2({
71-
password,
72-
salt,
73-
iterations: config.passportLocalMongoose.iterations,
74-
keylen: config.passportLocalMongoose.keylen,
75-
digestAlgorithm: config.passportLocalMongoose.digestAlgorithm
76-
});
77-
const hash = Buffer.from(rawHash, 'binary').toString(
78-
config.passportLocalMongoose.encoding
79-
);
76+
8077
return { password, salt, hash };
8178
}
8279

helpers/get-database.js

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1066,7 +1066,8 @@ function retryGetDatabase(...args) {
10661066
if (alias && Array.isArray(alias.tokens) && alias.tokens.length > 0) {
10671067
isValid = await isValidPassword(
10681068
alias.tokens,
1069-
decrypt(session.user.password)
1069+
decrypt(session.user.password),
1070+
alias
10701071
);
10711072
}
10721073

helpers/is-valid-password.js

Lines changed: 149 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,9 @@
66
const crypto = require('node:crypto');
77
const { Buffer } = require('node:buffer');
88

9+
const argon2 = require('@node-rs/argon2');
10+
const mongoose = require('mongoose');
11+
912
//
1013
// NOTE: scmp and tsse appear to be identical
1114
// <https://github.com/freewil/scmp/issues/18>
@@ -17,6 +20,14 @@ const { Buffer } = require('node:buffer');
1720
const pbkdf2 = require('./pbkdf2');
1821

1922
const config = require('#config');
23+
const logger = require('#helpers/logger');
24+
25+
const conn = mongoose.connections.find(
26+
(conn) => conn[Symbol.for('connection.name')] === 'MONGO_URI'
27+
);
28+
if (!conn) {
29+
throw new Error('Mongoose connection does not exist');
30+
}
2031

2132
function timingSafeCompare(a, b) {
2233
if (a.length !== b.length) {
@@ -29,7 +40,7 @@ function timingSafeCompare(a, b) {
2940
return crypto.timingSafeEqual(a, b);
3041
}
3142

32-
async function isValidPassword(tokens = [], password) {
43+
async function isValidPassword(tokens = [], password, instance) {
3344
if (
3445
typeof tokens !== 'object' ||
3546
!Array.isArray(tokens) ||
@@ -40,6 +51,8 @@ async function isValidPassword(tokens = [], password) {
4051
return false;
4152

4253
let match = false;
54+
let matchedToken = null;
55+
4356
for (const token of tokens) {
4457
if (
4558
typeof token !== 'object' ||
@@ -50,22 +63,141 @@ async function isValidPassword(tokens = [], password) {
5063
)
5164
continue;
5265

53-
const rawHash = await pbkdf2({
54-
password,
55-
salt: token.salt,
56-
iterations: config.passportLocalMongoose.iterations,
57-
keylen: config.passportLocalMongoose.keylen,
58-
digestAlgorithm: config.passportLocalMongoose.digestAlgorithm
59-
});
60-
61-
// const scmpResult = scmp(
62-
const scmpResult = timingSafeCompare(
63-
rawHash,
64-
Buffer.from(token.hash, config.passportLocalMongoose.encoding)
65-
);
66-
if (scmpResult) {
67-
match = true;
68-
break;
66+
//
67+
// if the token has already been migrated to argon2, skip pbkdf2 check
68+
//
69+
if (token.has_pbkdf2_migration === true) {
70+
try {
71+
const isValid = await argon2.verify(token.hash, password);
72+
if (isValid) {
73+
match = true;
74+
matchedToken = token;
75+
break;
76+
}
77+
} catch {
78+
// argon2 verification failed, continue to next token
79+
continue;
80+
}
81+
} else {
82+
//
83+
// try argon2 first (new format)
84+
//
85+
try {
86+
const isValid = await argon2.verify(token.hash, password);
87+
if (isValid) {
88+
match = true;
89+
matchedToken = token;
90+
break;
91+
}
92+
} catch {
93+
// argon2 verification failed, try pbkdf2 (old format)
94+
}
95+
96+
//
97+
// fallback to pbkdf2 for backwards compatibility
98+
//
99+
const rawHash = await pbkdf2({
100+
password,
101+
salt: token.salt,
102+
iterations: config.passportLocalMongoose.iterations,
103+
keylen: config.passportLocalMongoose.keylen,
104+
digestAlgorithm: config.passportLocalMongoose.digestAlgorithm
105+
});
106+
107+
// const scmpResult = scmp(
108+
const scmpResult = timingSafeCompare(
109+
rawHash,
110+
Buffer.from(token.hash, config.passportLocalMongoose.encoding)
111+
);
112+
113+
if (scmpResult) {
114+
match = true;
115+
matchedToken = token;
116+
break;
117+
}
118+
}
119+
}
120+
121+
//
122+
// if match found and token needs migration, perform live migration
123+
// and automatically save if model instance is provided
124+
//
125+
if (match && matchedToken && matchedToken.has_pbkdf2_migration !== true) {
126+
try {
127+
// check if hash is argon2 format (starts with $argon2)
128+
const isArgon2Hash = matchedToken.hash.startsWith('$argon2');
129+
if (isArgon2Hash) {
130+
// already argon2, just mark as migrated
131+
matchedToken.has_pbkdf2_migration = true;
132+
} else {
133+
// this is a pbkdf2 hash, perform migration
134+
const newHash = await argon2.hash(password, config.argon2);
135+
matchedToken.hash = newHash;
136+
matchedToken.has_pbkdf2_migration = true;
137+
}
138+
139+
// if model instance provided, save the migration
140+
if (instance) {
141+
try {
142+
if (!mongoose.isObjectIdOrHexString(instance._id))
143+
throw new TypeError(
144+
'instance._id was not an ObjectId nor hex string'
145+
);
146+
if (typeof instance.object !== 'string')
147+
throw new TypeError('instance.object was undefined');
148+
if (instance.object === 'domain') {
149+
// enforce schema and require all required paths (dummyproof while we migrate)
150+
if (
151+
!tokens.every((token) => token.user && token.salt && token.hash)
152+
)
153+
throw new TypeError('token missing user, salt, or hash');
154+
// TODO: this migration needs improved/safeguarded since we don't use __v version key
155+
// (e.g. if we detect `__v` then query for equality and increase it by 1 as well)
156+
await conn.models.Domains.findOneAndUpdate(
157+
{
158+
_id: instance._id,
159+
tokens: { $size: tokens.length }
160+
},
161+
{
162+
$set: {
163+
tokens
164+
}
165+
}
166+
);
167+
} else if (instance.object === 'alias') {
168+
// enforce schema and require all required paths (dummyproof while we migrate)
169+
if (!tokens.every((token) => token.salt && token.hash))
170+
throw new TypeError('token missing user, salt, or hash');
171+
// TODO: this migration needs improved/safeguarded since we don't use __v version key
172+
// (e.g. if we detect `__v` then query for equality and increase it by 1 as well)
173+
await conn.models.Aliases.findOneAndUpdate(
174+
{
175+
_id: instance._id,
176+
tokens: { $size: tokens.length }
177+
},
178+
{
179+
$set: {
180+
tokens
181+
}
182+
}
183+
);
184+
} else {
185+
throw new TypeError(
186+
'instance.object must be equal to "domain" or "alias"'
187+
);
188+
}
189+
} catch (err) {
190+
// log error but don't fail authentication
191+
// the migration will be retried on next authentication
192+
logger.fatal(err);
193+
const _err = new TypeError('Failed to save argon2 migration');
194+
_err.err = err;
195+
console.error(_err);
196+
}
197+
}
198+
} catch {
199+
// migration failed, but authentication succeeded
200+
// caller can retry migration later
69201
}
70202
}
71203

0 commit comments

Comments
 (0)