Skip to content

Commit ea507c7

Browse files
committed
ZMS-45: Implement totp nonce system for safer totp checks
1 parent df2f69d commit ea507c7

5 files changed

Lines changed: 189 additions & 6 deletions

File tree

lib/api/2fa/totp.js

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -223,6 +223,7 @@ module.exports = (db, server, userHandler) => {
223223
validationObjs: {
224224
requestBody: {
225225
token: Joi.string().length(6).required().description('6-digit number'),
226+
totpNonce: Joi.string().hex().length(40).required().description('Short-lived nonce returned by /authenticate'),
226227
sess: sessSchema,
227228
ip: sessIPSchema
228229
},
@@ -264,6 +265,7 @@ module.exports = (db, server, userHandler) => {
264265
}
265266

266267
let user = new ObjectId(result.value.user);
268+
result.value.accessTokenHash = req.accessToken?.hash;
267269
let totp = await userHandler.checkTotp(user, result.value);
268270

269271
if (!totp) {

lib/api/auth.js

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -175,6 +175,7 @@ module.exports = (db, server, userHandler) => {
175175
token: Joi.string().description(
176176
'If access token was requested then this is the value to use as access token when making API requests on behalf of logged in user.'
177177
),
178+
totpNonce: Joi.string().hex().length(40).description('Short-lived nonce for completing TOTP authentication'),
178179
passwordPwned: booleanSchema.description(
179180
'Indicates whether account password has been found in the list of Pwned passwords and should be replaced'
180181
)
@@ -273,6 +274,9 @@ module.exports = (db, server, userHandler) => {
273274
if (result.value.token) {
274275
try {
275276
authResponse.token = await userHandler.generateAuthToken(authData.user);
277+
if (Array.isArray(authData.require2fa) && authData.require2fa.includes('totp')) {
278+
authResponse.totpNonce = await userHandler.generateTotpNonce(authData.user, authResponse.token);
279+
}
276280
} catch (err) {
277281
let response = {
278282
error: err.message,
@@ -380,9 +384,7 @@ module.exports = (db, server, userHandler) => {
380384
filter: Joi.string().description('Filter ID associated with the event'),
381385
credential: Joi.string().description('WebAuthn credential ID'),
382386
appId: Joi.string().description('Optional appId which is the URL of the app'),
383-
require2fa: Joi.alternatives()
384-
.try(Joi.string(), booleanSchema.allow(false))
385-
.description('2FA requirement detail'),
387+
require2fa: Joi.alternatives().try(Joi.string(), booleanSchema.allow(false)).description('2FA requirement detail'),
386388
last: Joi.date().required().description('Date of the last update of data'),
387389
events: Joi.number().required().description('Number of times same auth log has occurred'),
388390
source: Joi.string().description('Source of auth. Example: `master` if password auth was used'),
@@ -564,9 +566,7 @@ module.exports = (db, server, userHandler) => {
564566
filter: Joi.string().description('Filter ID associated with the event'),
565567
credential: Joi.string().description('WebAuthn credential ID'),
566568
appId: Joi.string().description('Optional appId which is the URL of the app'),
567-
require2fa: Joi.alternatives()
568-
.try(Joi.string(), booleanSchema.allow(false))
569-
.description('2FA requirement detail'),
569+
require2fa: Joi.alternatives().try(Joi.string(), booleanSchema.allow(false)).description('2FA requirement detail'),
570570
last: Joi.date().required().description('Date of the last update of Event'),
571571
events: Joi.number().required().description('Number of times same auth Event has occurred'),
572572
source: Joi.string().description('Source of auth. Example: `master` if password auth was used'),

lib/consts.js

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,7 @@ module.exports = {
6262
TOTP_FAILURES: 6,
6363
// TOTP authentication window in seconds, starts counting from first invalid authentication
6464
TOTP_WINDOW: 180,
65+
TOTP_NONCE_TTL: 10 * 60,
6566

6667
SCOPES: ['imap', 'pop3', 'smtp'],
6768

lib/user-handler.js

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2291,9 +2291,52 @@ class UserHandler {
22912291
return true;
22922292
}
22932293

2294+
async generateTotpNonce(user, accessToken) {
2295+
const tokenHash = crypto.createHash('sha256').update(accessToken).digest('hex');
2296+
const totpNonce = crypto.randomBytes(20).toString('hex');
2297+
2298+
try {
2299+
await this.redis.set(getTotpNonceKey(user, totpNonce), tokenHash, 'EX', consts.TOTP_NONCE_TTL);
2300+
} catch {
2301+
// ignore
2302+
}
2303+
2304+
return totpNonce;
2305+
}
2306+
2307+
async validateTotpNonce(user, totpNonce, accessTokenHash) {
2308+
if (!accessTokenHash) {
2309+
const err = new Error('Authentication token is required');
2310+
err.response = 'NO';
2311+
err.responseCode = 403;
2312+
err.code = 'AuthenticationRequired';
2313+
throw err;
2314+
}
2315+
2316+
let storedAccessTokenHash;
2317+
const totpNonceKey = getTotpNonceKey(user, totpNonce);
2318+
2319+
try {
2320+
storedAccessTokenHash = await this.redis.get(totpNonceKey);
2321+
} catch {
2322+
// ignore
2323+
}
2324+
2325+
if (!storedAccessTokenHash || storedAccessTokenHash !== accessTokenHash) {
2326+
const err = new Error('Invalid or expired TOTP nonce');
2327+
err.response = 'NO';
2328+
err.responseCode = 403;
2329+
err.code = 'InvalidTotpNonce';
2330+
throw err;
2331+
}
2332+
2333+
return totpNonceKey;
2334+
}
2335+
22942336
async checkTotp(user, data) {
22952337
let userRlKey = `totp:${user}`;
22962338
let totpSuccessKey = `totp:${user}:${data.token}`;
2339+
let totpNonceKey = await this.validateTotpNonce(user, data.totpNonce, data.accessTokenHash);
22972340

22982341
try {
22992342
let rateLimitRes = await this.rateLimit(userRlKey, data, 0);
@@ -2401,6 +2444,24 @@ class UserHandler {
24012444
// ignore
24022445
}
24032446

2447+
if (verified) {
2448+
let nonceConsumed;
2449+
try {
2450+
nonceConsumed = await this.redis.del(totpNonceKey);
2451+
} catch {
2452+
// ignore
2453+
}
2454+
2455+
if (!nonceConsumed) {
2456+
verified = false;
2457+
const nonceError = new Error('Invalid or expired TOTP nonce');
2458+
nonceError.response = 'NO';
2459+
nonceError.responseCode = 403;
2460+
nonceError.code = 'InvalidTotpNonce';
2461+
throw nonceError;
2462+
}
2463+
}
2464+
24042465
if (verified) {
24052466
try {
24062467
await this.redis
@@ -4030,6 +4091,10 @@ function rateLimitResponse(res) {
40304091
return err;
40314092
}
40324093

4094+
function getTotpNonceKey(user, totpNonce) {
4095+
return `totpnonce:${user}:${crypto.createHash('sha256').update(totpNonce).digest('hex')}`;
4096+
}
4097+
40334098
// high collision hash function
40344099
function getStringSelector(str) {
40354100
let hash = crypto.createHash('sha1').update(str).digest();

test/api/totp-test.js

Lines changed: 115 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,115 @@
1+
/*eslint no-unused-expressions: 0, prefer-arrow-callback: 0 */
2+
3+
'use strict';
4+
5+
const supertest = require('supertest');
6+
const chai = require('chai');
7+
const speakeasy = require('speakeasy');
8+
9+
const expect = chai.expect;
10+
chai.config.includeStack = true;
11+
const config = require('@zone-eu/wild-config');
12+
13+
const server = supertest.agent(`http://127.0.0.1:${config.api.port}`);
14+
15+
describe('API TOTP', function () {
16+
this.timeout(10000); // eslint-disable-line no-invalid-this
17+
18+
let user;
19+
let seed;
20+
let accessToken;
21+
let totpNonce;
22+
23+
it('should POST /users expect success / create a user with TOTP', async () => {
24+
const response = await server
25+
.post('/users')
26+
.send({
27+
username: 'totpnonceuser',
28+
name: 'Totp Nonce User',
29+
address: 'totpnonce@example.com',
30+
password: 'totpsecretvalue'
31+
})
32+
.expect(200);
33+
34+
expect(response.body.success).to.be.true;
35+
user = response.body.id;
36+
});
37+
38+
it('should POST /users/{user}/2fa/totp/setup expect success', async () => {
39+
const response = await server
40+
.post(`/users/${user}/2fa/totp/setup`)
41+
.send({
42+
issuer: 'WildDuck Test'
43+
})
44+
.expect(200);
45+
46+
expect(response.body.success).to.be.true;
47+
expect(response.body.seed).to.exist;
48+
49+
seed = response.body.seed;
50+
});
51+
52+
it('should POST /users/{user}/2fa/totp/enable expect success', async () => {
53+
const response = await server
54+
.post(`/users/${user}/2fa/totp/enable`)
55+
.send({
56+
token: speakeasy.totp({
57+
secret: seed,
58+
encoding: 'base32'
59+
})
60+
})
61+
.expect(200);
62+
63+
expect(response.body.success).to.be.true;
64+
});
65+
66+
it('should POST /authenticate expect success / return auth token and totpNonce', async () => {
67+
const response = await server
68+
.post('/authenticate')
69+
.send({
70+
username: 'totpnonceuser',
71+
password: 'totpsecretvalue',
72+
token: true
73+
})
74+
.expect(200);
75+
76+
expect(response.body.success).to.be.true;
77+
expect(response.body.id).to.equal(user);
78+
expect(response.body.require2fa).to.deep.equal(['totp']);
79+
expect(response.body.token).to.match(/^[0-9a-f]{40}$/);
80+
expect(response.body.totpNonce).to.match(/^[0-9a-f]{40}$/);
81+
82+
accessToken = response.body.token;
83+
totpNonce = response.body.totpNonce;
84+
});
85+
86+
it('should POST /users/{user}/2fa/totp/check expect success', async () => {
87+
const response = await server
88+
.post(`/users/${user}/2fa/totp/check?accessToken=${accessToken}`)
89+
.send({
90+
token: speakeasy.totp({
91+
secret: seed,
92+
encoding: 'base32'
93+
}),
94+
totpNonce
95+
})
96+
.expect(200);
97+
98+
expect(response.body.success).to.be.true;
99+
});
100+
101+
it('should POST /users/{user}/2fa/totp/check expect failure / reject reused totpNonce', async () => {
102+
const response = await server
103+
.post(`/users/${user}/2fa/totp/check?accessToken=${accessToken}`)
104+
.send({
105+
token: speakeasy.totp({
106+
secret: seed,
107+
encoding: 'base32'
108+
}),
109+
totpNonce
110+
})
111+
.expect(403);
112+
113+
expect(response.body.code).to.equal('InvalidTotpNonce');
114+
});
115+
});

0 commit comments

Comments
 (0)