diff --git a/apps/api/src/adminAuth.js b/apps/api/src/adminAuth.js index 299723d..9f1e710 100644 --- a/apps/api/src/adminAuth.js +++ b/apps/api/src/adminAuth.js @@ -4,7 +4,20 @@ import jwt from "jsonwebtoken"; export const ADMIN_COOKIE_NAME = "admin_session"; const SESSION_TTL_S = 12 * 3600; // 12h const LOCKOUT_WINDOW_MIN = 15; -const LOCKOUT_MAX_ATTEMPTS = 5; +// Deux compteurs distincts, volontairement asymetriques. +// +// L'ancienne requete comptait `username = $2 OR ip = $3` dans un seul total : +// cinq echecs avec le nom d'un admin, depuis n'importe quelle adresse, +// verrouillaient ce compte un quart d'heure. N'importe qui connaissant le nom +// d'utilisateur pouvait donc mettre l'admin dehors, indefiniment et sans cout. +// +// L'IP reste stricte : c'est elle qui freine une attaque par force brute, et +// s'auto-verrouiller n'a aucun interet pour un attaquant. Le compteur par +// username subsiste contre une attaque repartie sur plusieurs adresses, mais +// avec un seuil bien plus haut : un attaquant devra bruler 4 IP avant de +// commencer a genner le titulaire du compte. +const LOCKOUT_MAX_PER_IP = 5; +const LOCKOUT_MAX_PER_USERNAME = 20; // Hash bcrypt valide mais sans correspondance, utilisé pour égaliser le temps // de réponse quand le username n'existe pas (évite l'énumération de comptes). const DUMMY_HASH = "$2b$12$CwTycUXWue0Thq9StjUM0uJ8vKR1dlT0LYzGsv8ZE8nFI8q9Z5T9."; @@ -45,14 +58,17 @@ export function verifyAdminSession(token) { export async function isLockedOut(pool, username, ip) { const r = await pool.query( - `SELECT count(*)::int AS n + `SELECT + count(*) FILTER (WHERE ip = $3)::int AS par_ip, + count(*) FILTER (WHERE username = $2)::int AS par_username FROM admin_login_attempts WHERE success = false AND created_at > now() - make_interval(mins => $1) AND (username = $2 OR ip = $3)`, [LOCKOUT_WINDOW_MIN, username, ip] ); - return r.rows[0].n >= LOCKOUT_MAX_ATTEMPTS; + const { par_ip, par_username } = r.rows[0]; + return par_ip >= LOCKOUT_MAX_PER_IP || par_username >= LOCKOUT_MAX_PER_USERNAME; } export async function recordLoginAttempt(pool, username, ip, success) { diff --git a/apps/api/src/app.js b/apps/api/src/app.js index c7fb20c..c5dd399 100644 --- a/apps/api/src/app.js +++ b/apps/api/src/app.js @@ -73,6 +73,14 @@ export async function buildServer(opts = {}) { const origin = request.headers.origin; const allowedOrigins = corsOrigins; + // `Vary: Origin` est indispensable des lors que l'en-tete de reponse DEPEND + // de l'origine demandee : sans lui, un cache intermediaire (CDN, proxy) + // peut servir a une origine la reponse mise en cache pour une autre, ce qui + // revient a autoriser une origine qui ne l'est pas. Pose systematiquement, + // y compris quand l'origine est refusee -- sinon le refus lui-meme se + // retrouve mis en cache pour une origine legitime. + reply.header('Vary', 'Origin'); + if (allowedOrigins.includes(origin)) { reply.header('Access-Control-Allow-Origin', origin); } diff --git a/apps/api/test/adminAuth.test.js b/apps/api/test/adminAuth.test.js index 260a1a0..92d1fe5 100644 --- a/apps/api/test/adminAuth.test.js +++ b/apps/api/test/adminAuth.test.js @@ -115,18 +115,32 @@ describe("verifyPassword", () => { }); describe("isLockedOut", () => { - it("verrouille à partir de 5 tentatives échouées", async () => { - const under = makeFakePool([{ match: "admin_login_attempts", result: rows({ n: 4 }) }]); - const at = makeFakePool([{ match: "admin_login_attempts", result: rows({ n: 5 }) }]); - expect(await isLockedOut(under, "nico", "1.2.3.4")).toBe(false); - expect(await isLockedOut(at, "nico", "1.2.3.4")).toBe(true); + const compteurs = (par_ip, par_username) => + makeFakePool([{ match: "admin_login_attempts", result: rows({ par_ip, par_username }) }]); + + it("verrouille à partir de 5 échecs venant de la même IP", async () => { + expect(await isLockedOut(compteurs(4, 4), "nico", "1.2.3.4")).toBe(false); + expect(await isLockedOut(compteurs(5, 5), "nico", "1.2.3.4")).toBe(true); }); - it("compte les tentatives par username OU par IP, sur 15 minutes", async () => { - const pool = makeFakePool([{ match: "admin_login_attempts", result: rows({ n: 0 }) }]); + it("un tiers ne peut pas verrouiller un compte en échouant depuis ailleurs", async () => { + // Le coeur du correctif : les deux compteurs etaient additionnes dans un + // seul total. Cinq echecs avec le nom d'un admin, depuis n'importe quelle + // adresse, le mettaient dehors un quart d'heure — a repeter indefiniment. + // L'admin legitime, lui, n'a aucun echec a son IP : il doit passer. + expect(await isLockedOut(compteurs(0, 19), "nico", "1.2.3.4")).toBe(false); + }); + + it("un seuil par username subsiste contre une attaque répartie", async () => { + expect(await isLockedOut(compteurs(0, 20), "nico", "1.2.3.4")).toBe(true); + }); + + it("compte sur une fenêtre de 15 minutes", async () => { + const pool = compteurs(0, 0); await isLockedOut(pool, "nico", "1.2.3.4"); const call = pool.find("admin_login_attempts"); - expect(call.sql).toMatch(/username = \$2 OR ip = \$3/); + expect(call.sql).toMatch(/FILTER \(WHERE ip = \$3\)/); + expect(call.sql).toMatch(/FILTER \(WHERE username = \$2\)/); expect(call.values).toEqual([15, "nico", "1.2.3.4"]); }); }); diff --git a/apps/api/test/publicRoutes.test.js b/apps/api/test/publicRoutes.test.js index 2f85deb..b03ca03 100644 --- a/apps/api/test/publicRoutes.test.js +++ b/apps/api/test/publicRoutes.test.js @@ -73,6 +73,19 @@ describe("CORS", () => { expect(res.headers["access-control-allow-origin"]).toBeUndefined(); }); + it("annonce Vary: Origin, y compris quand l'origine est refusée", async () => { + // L'en-tete Allow-Origin DEPEND de l'origine demandee. Sans Vary, un cache + // intermediaire peut servir a une origine la reponse mise en cache pour une + // autre — ce qui revient a autoriser une origine qui ne l'est pas, ou a + // faire refuser une origine legitime. + for (const origin of ["https://metalfrom.eu", "https://evil.example", undefined]) { + const res = await build().inject({ + method: "GET", url: "/api/health", headers: origin ? { origin } : {}, + }); + expect(res.headers["vary"]).toContain("Origin"); + } + }); + it("ne fait pas de match par préfixe sur l'origine", async () => { const res = await build().inject({ method: "GET", url: "/api/health", headers: { origin: "https://metalfrom.eu.evil.example" }, @@ -337,7 +350,7 @@ describe("/admin/auth/login", () => { it("répond 429 quand le compte est verrouillé, sans vérifier le mot de passe", async () => { const res = await build([ - { match: "admin_login_attempts", result: rows({ n: 99 }) }, + { match: "FROM admin_login_attempts", result: rows({ par_ip: 99, par_username: 0 }) }, { match: "FROM admin_users", result: rows({ password_hash: "x" }) }, ]).inject({ method: "POST", url: "/admin/auth/login", @@ -352,7 +365,7 @@ describe("/admin/auth/login", () => { // Hash à coût 4 : le DUMMY_HASH de production est à coût 12 (~330 ms). const hash = bcrypt.hashSync("autre-chose", 4); const res = await build([ - { match: "SELECT count(*)::int AS n", result: rows({ n: 0 }) }, + { match: "FROM admin_login_attempts", result: rows({ par_ip: 0, par_username: 0 }) }, { match: "FROM admin_users", result: rows({ password_hash: hash }) }, { match: "INSERT INTO admin_login_attempts", result: rows() }, ]).inject({ @@ -368,7 +381,7 @@ describe("/admin/auth/login", () => { const bcrypt = (await import("bcryptjs")).default; const hash = bcrypt.hashSync("bon", 4); const res = await build([ - { match: "SELECT count(*)::int AS n", result: rows({ n: 0 }) }, + { match: "FROM admin_login_attempts", result: rows({ par_ip: 0, par_username: 0 }) }, { match: "FROM admin_users", result: rows({ password_hash: hash }) }, { match: "INSERT INTO admin_login_attempts", result: rows() }, { match: "UPDATE admin_users", result: rows() }, @@ -389,7 +402,7 @@ describe("/admin/auth/login", () => { const bcrypt = (await import("bcryptjs")).default; const hash = bcrypt.hashSync("x", 4); await build([ - { match: "SELECT count(*)::int AS n", result: rows({ n: 0 }) }, + { match: "FROM admin_login_attempts", result: rows({ par_ip: 0, par_username: 0 }) }, { match: "FROM admin_users", result: rows({ password_hash: hash }) }, { match: "INSERT INTO admin_login_attempts", result: rows() }, ]).inject({