fix(sécurité): plafond de /admin/import contournable en variant le jeton
Le limiteur était keyé sur le Bearer fourni par l'appelant : keyGenerator: (req) => authBearer(req) || req.ip La clé était donc une valeur non authentifiée, entièrement choisie par l'attaquant. Envoyer un jeton différent à chaque requête ouvrait un seau neuf à chaque fois : sur 30 tentatives avec un jeton variable, 0 étaient bloquées, là où 20 l'étaient avec un jeton fixe. Le plafond ne freinait donc rien — alors qu'il est précisément là pour ralentir la recherche du jeton par force brute. Chaque jeton inédit consommait en prime une entrée du cache LRU du limiteur, évinçant les compteurs légitimes. La clé redevient l'IP. Isoler par identité réelle est impossible à cette couche : le limiteur s'exécute avant l'authentification. Le test « limite par jeton, pas globalement » verrouillait explicitement le comportement vulnérable. Il est remplacé, avec la raison de l'abandon. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
parent
ebdcd74973
commit
cded10cb3b
2 changed files with 39 additions and 9 deletions
|
|
@ -664,9 +664,14 @@ export async function buildServer(opts = {}) {
|
|||
await adminRoutes.register(rateLimit, {
|
||||
max: adminRateLimitMax,
|
||||
timeWindow: '1 minute',
|
||||
keyGenerator: (req) => {
|
||||
return authBearer(req) || req.ip;
|
||||
}
|
||||
// La cle DOIT etre l'IP, jamais le jeton fourni. Keyer sur le Bearer
|
||||
// donnait a l'appelant le controle de son propre compteur : il suffisait
|
||||
// d'envoyer un jeton different a chaque requete pour obtenir un seau
|
||||
// neuf a chaque fois et contourner entierement le plafond -- or ce
|
||||
// plafond existe precisement pour ralentir la recherche du jeton par
|
||||
// force brute. En prime, chaque jeton inedit consommait une entree du
|
||||
// cache LRU du limiteur, evincant les compteurs legitimes.
|
||||
keyGenerator: (req) => req.ip,
|
||||
});
|
||||
|
||||
adminRoutes.post("/admin/import", async (req, reply) => {
|
||||
|
|
|
|||
|
|
@ -64,21 +64,24 @@ describe("plafond de /admin/import", () => {
|
|||
await app.close();
|
||||
});
|
||||
|
||||
it("limite par jeton, pas globalement", async () => {
|
||||
it("limite par IP, y compris pour un appelant authentifié", async () => {
|
||||
// Ce test verrouillait auparavant l'inverse — un compteur par jeton, pour
|
||||
// qu'un client bruyant n'affame pas les autres. L'intention se défend, mais
|
||||
// le limiteur s'exécute AVANT l'authentification : la cle etait donc une
|
||||
// valeur non verifiee, fournie par l'appelant. Isoler par identite reelle
|
||||
// est impossible a cette couche ; on retombe sur l'IP, seule cle que
|
||||
// l'appelant ne choisit pas.
|
||||
const app = await appWithRealLimits({ adminRateLimitMax: 2 });
|
||||
const call = (token) =>
|
||||
app.inject({
|
||||
method: "POST", url: "/admin/import",
|
||||
headers: { authorization: `Bearer ${token}` }, payload: { bands: [] },
|
||||
headers: { "x-forwarded-for": "4.4.4.4", authorization: `Bearer ${token}` },
|
||||
payload: { bands: [] },
|
||||
});
|
||||
|
||||
await call(TOKEN);
|
||||
await call(TOKEN);
|
||||
// Le 3e appel avec CE jeton est bloqué…
|
||||
expect((await call(TOKEN)).statusCode).toBe(429);
|
||||
// …mais un autre jeton dispose de son propre compteur (il sera rejeté en
|
||||
// 401, pas en 429 : c'est bien l'authentification qui tranche, pas le débit).
|
||||
expect((await call("un-autre-jeton-de-la-meme-taille")).statusCode).toBe(401);
|
||||
await app.close();
|
||||
});
|
||||
});
|
||||
|
|
@ -107,6 +110,28 @@ describe("plafond de /admin/auth/login", () => {
|
|||
await app.close();
|
||||
});
|
||||
|
||||
it("le plafond d'import ne se contourne pas en changeant de jeton", async () => {
|
||||
// Le limiteur etait keye sur le Bearer fourni, c'est-a-dire sur une valeur
|
||||
// entierement controlee par l'appelant : envoyer un jeton different a
|
||||
// chaque requete donnait un seau neuf a chaque fois, et le plafond ne
|
||||
// freinait plus rien -- alors qu'il est justement la pour ralentir la
|
||||
// recherche du jeton par force brute.
|
||||
const app = await appWithRealLimits({ adminRateLimitMax: 2 });
|
||||
const call = (token) =>
|
||||
app.inject({
|
||||
method: "POST",
|
||||
url: "/admin/import",
|
||||
headers: { "x-forwarded-for": "9.9.9.9", authorization: `Bearer ${token}` },
|
||||
payload: { bands: [] },
|
||||
});
|
||||
|
||||
expect((await call("faux-a")).statusCode).toBe(401);
|
||||
expect((await call("faux-b")).statusCode).toBe(401);
|
||||
// 3e requete depuis la meme IP : bloquee, quel que soit le jeton presente.
|
||||
expect((await call("faux-c")).statusCode).toBe(429);
|
||||
await app.close();
|
||||
});
|
||||
|
||||
it("limite par IP", async () => {
|
||||
const app = await appWithRealLimits({ authRateLimitMax: 1 });
|
||||
const call = (ip) =>
|
||||
|
|
|
|||
Loading…
Reference in a new issue