Browse Source

Small code quality improvements

- Fix misleading JSDoc comment in cache.ts
- Mitigate timing-based username enumeration in Basic auth
- Extract duplicated TOTP configuration into private method
- Replace manual peer counter with clients.length in Prometheus metrics
- Simplify isValidPasswordHash return expression
pull/2553/head
Anghios 3 months ago
committed by Bernd Storath
parent
commit
6deb2bacbd
  1. 39
      src/server/database/repositories/user/service.ts
  2. 4
      src/server/routes/metrics/prometheus.get.ts
  3. 2
      src/server/utils/cache.ts
  4. 6
      src/server/utils/password.ts
  5. 17
      src/server/utils/session.ts

39
src/server/database/repositories/user/service.ts

@ -58,6 +58,17 @@ export class UserService {
this.#statements = createPreparedStatement(db); this.#statements = createPreparedStatement(db);
} }
#createTotp(user: Pick<UserType, 'username' | 'totpKey'>) {
return new TOTP({
issuer: 'wg-easy',
label: user.username,
algorithm: 'SHA1',
digits: 6,
period: 30,
secret: user.totpKey!,
});
}
async getAll() { async getAll() {
return this.#statements.findAll.execute(); return this.#statements.findAll.execute();
} }
@ -160,18 +171,8 @@ export class UserService {
return { success: false, error: 'UNEXPECTED_ERROR' }; return { success: false, error: 'UNEXPECTED_ERROR' };
} }
const totp = new TOTP({ const totp = this.#createTotp(txUser);
issuer: 'wg-easy', if (totp.validate({ token: code, window: 1 }) === null) {
label: txUser.username,
algorithm: 'SHA1',
digits: 6,
period: 30,
secret: txUser.totpKey,
});
const valid = totp.validate({ token: code, window: 1 });
if (valid === null) {
return { success: false, error: 'INVALID_TOTP_CODE' }; return { success: false, error: 'INVALID_TOTP_CODE' };
} }
} }
@ -199,18 +200,8 @@ export class UserService {
throw new Error('TOTP key is not set'); throw new Error('TOTP key is not set');
} }
const totp = new TOTP({ const totp = this.#createTotp(txUser);
issuer: 'wg-easy', if (totp.validate({ token: code, window: 1 }) === null) {
label: txUser.username,
algorithm: 'SHA1',
digits: 6,
period: 30,
secret: txUser.totpKey,
});
const valid = totp.validate({ token: code, window: 1 });
if (valid === null) {
throw new Error('Invalid TOTP code'); throw new Error('Invalid TOTP code');
} }

4
src/server/routes/metrics/prometheus.get.ts

@ -6,14 +6,12 @@ export default defineMetricsHandler('prometheus', async ({ event }) => {
async function getPrometheusResponse() { async function getPrometheusResponse() {
const wgInterface = await Database.interfaces.get(); const wgInterface = await Database.interfaces.get();
const clients = await WireGuard.getAllClients(); const clients = await WireGuard.getAllClients();
let wireguardPeerCount = 0;
let wireguardEnabledPeersCount = 0; let wireguardEnabledPeersCount = 0;
let wireguardConnectedPeersCount = 0; let wireguardConnectedPeersCount = 0;
const wireguardSentBytes = []; const wireguardSentBytes = [];
const wireguardReceivedBytes = []; const wireguardReceivedBytes = [];
const wireguardLatestHandshakeSeconds = []; const wireguardLatestHandshakeSeconds = [];
for (const client of clients) { for (const client of clients) {
wireguardPeerCount++;
if (client.enabled === true) { if (client.enabled === true) {
wireguardEnabledPeersCount++; wireguardEnabledPeersCount++;
} }
@ -41,7 +39,7 @@ async function getPrometheusResponse() {
const returnText = [ const returnText = [
'# HELP wireguard_configured_peers', '# HELP wireguard_configured_peers',
'# TYPE wireguard_configured_peers gauge', '# TYPE wireguard_configured_peers gauge',
`wireguard_configured_peers{${id}} ${wireguardPeerCount}`, `wireguard_configured_peers{${id}} ${clients.length}`,
'', '',
'# HELP wireguard_enabled_peers', '# HELP wireguard_enabled_peers',
'# TYPE wireguard_enabled_peers gauge', '# TYPE wireguard_enabled_peers gauge',

2
src/server/utils/cache.ts

@ -6,7 +6,7 @@ type Opts = {
}; };
/** /**
* Cache function for 1 hour * Cache the result of a function for the given expiry time
*/ */
export function cacheFunction<T>(fn: () => T, { expiry }: Opts): () => T { export function cacheFunction<T>(fn: () => T, { expiry }: Opts): () => T {
let cache: { value: T; expiry: number } | null = null; let cache: { value: T; expiry: number } | null = null;

6
src/server/utils/password.ts

@ -28,11 +28,7 @@ export function isValidPasswordHash(hash: string): boolean {
try { try {
const obj = deserialize(hash); const obj = deserialize(hash);
if (obj.id !== 'argon2i' && obj.id !== 'argon2d' && obj.id !== 'argon2id') { return obj.id === 'argon2i' || obj.id === 'argon2d' || obj.id === 'argon2id';
return false;
}
return true;
} catch { } catch {
return false; return false;
} }

17
src/server/utils/session.ts

@ -70,26 +70,21 @@ export async function getCurrentUser(event: H3Event) {
}); });
} }
// TODO: timing can be used to enumerate usernames
const foundUser = await Database.users.getByUsername(username); const foundUser = await Database.users.getByUsername(username);
if (!foundUser) { // Always verify password to prevent timing-based username enumeration
throw createError({ const userHashPassword =
statusCode: 401, foundUser?.password ??
statusMessage: 'Session failed', '$argon2id$v=19$m=65536,t=3,p=4$aaaaaaaaaaaaaaaa$bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb';
});
}
const userHashPassword = foundUser.password;
const passwordValid = await isPasswordValid(password, userHashPassword); const passwordValid = await isPasswordValid(password, userHashPassword);
if (!passwordValid) { if (!foundUser || !passwordValid) {
throw createError({ throw createError({
statusCode: 401, statusCode: 401,
statusMessage: 'Session failed', statusMessage: 'Session failed',
}); });
} }
user = foundUser; user = foundUser;
} else { } else {
throw createError({ throw createError({

Loading…
Cancel
Save