Skip to content

Commit

Permalink
Simple protection against brute force attacks (#1152)
Browse files Browse the repository at this point in the history
  • Loading branch information
p-weston authored Nov 15, 2024
1 parent 7811f53 commit 046a0df
Show file tree
Hide file tree
Showing 4 changed files with 175 additions and 0 deletions.
8 changes: 8 additions & 0 deletions server/http_server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import {
parsePageRef,
} from "@silverbulletmd/silverbullet/lib/page_ref";
import { base64Encode } from "$lib/crypto.ts";
import { LockoutTimer } from "./lockout.ts";

const authenticationExpirySeconds = 60 * 60 * 24 * 7; // 1 week

Expand Down Expand Up @@ -314,6 +315,7 @@ export class HttpServer {
"/logo.png",
"/.auth",
];
const lockoutTimer = new LockoutTimer();

// TODO: This should probably be a POST request
this.app.get("/.logout", (c) => {
Expand Down Expand Up @@ -354,6 +356,11 @@ export class HttpServer {
pass: expectedPassword,
} = this.spaceServer.auth!;

if (lockoutTimer.isLocked()) {
console.error("Authentication locked out, redirecting to auth page.");
return c.redirect("/.auth?error=2");
}

if (username === expectedUser && password === expectedPassword) {
// Generate a JWT and set it as a cookie
const jwt = rememberMe
Expand All @@ -379,6 +386,7 @@ export class HttpServer {
return c.redirect(typeof from === "string" ? from : "/");
} else {
console.error("Authentication failed, redirecting to auth page.");
lockoutTimer.addCount();
return c.redirect("/.auth?error=1");
}
},
Expand Down
119 changes: 119 additions & 0 deletions server/lockout.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,119 @@
import { assert, assertEquals } from "@std/assert";
import { LockoutTimer } from "./lockout.ts";
import { FakeTime } from "@std/testing/time";

const lockoutTime = 60000;
const lockoutLimit = 10;

Deno.test("Lockout - failed login rate limiter", async () => {
const envLockoutTime = Deno.env.get("SB_LOCKOUT_TIME_MS") || "";
const envLockoutLimit = Deno.env.get("SB_LOCKOUT_LIMIT") || "";

using time = new FakeTime();

testDisabled(new LockoutTimer(NaN, lockoutLimit), "by period");
testDisabled(new LockoutTimer(lockoutTime, NaN), "by limit");

Deno.env.set("SB_LOCKOUT_TIME_MS", String(lockoutTime));
Deno.env.set("SB_LOCKOUT_LIMIT", "");
testDisabled(new LockoutTimer(lockoutTime, NaN), "by env period");

Deno.env.set("SB_LOCKOUT_TIME_MS", "");
Deno.env.set("SB_LOCKOUT_LIMIT", String(lockoutLimit));
testDisabled(new LockoutTimer(lockoutTime, NaN), "by env limit");

Deno.env.set("SB_LOCKOUT_TIME_MS", "");
Deno.env.set("SB_LOCKOUT_LIMIT", "");
await testLockout(new LockoutTimer(lockoutTime, 10), "explicit params");
await testLockoutPerMS(new LockoutTimer(lockoutTime, 10), "explicit params");

Deno.env.set("SB_LOCKOUT_TIME_MS", String(lockoutTime));
Deno.env.set("SB_LOCKOUT_LIMIT", String(lockoutLimit));
await testLockout(new LockoutTimer(), "params from env");
await testLockoutPerMS(new LockoutTimer(), "params from env");

Deno.env.set("SB_LOCKOUT_TIME_MS", envLockoutTime);
Deno.env.set("SB_LOCKOUT_LIMIT", envLockoutLimit);

function testDisabled(timer: LockoutTimer, txt: string) {
for (let i = 0; i < 100; i++) {
assertEquals(
timer.isLocked(),
false,
`Should be unlocked - disabled ${txt} loop ${i}`,
);
timer.addCount();
}
}

function testLockoutPerMS(timer: LockoutTimer, txt: string) {
assert(
lockoutTime > lockoutLimit,
`testLockoutPerMS assumes lockoutTime(${lockoutTime}) > lockoutLimit(${lockoutLimit}), so it can fill a bucket in 1ms jumps`,
);

const bucketPasses = 2;

let countLocked = 0;
let countUnLocked = 0;

const totalTests = lockoutTime * bucketPasses;

for (let i = 0; i < totalTests; i++) {
if (timer.isLocked()) {
countLocked++;
} else {
countUnLocked++;
}
timer.addCount();
time.tick(1);
}

// usually this will be bucketPasses+1 buckets
// but if time aligns could be just bucketPasses buckets
// and could be in between if it doesn't have time to completely fill the extra bucket

const expectedUnlocked = lockoutLimit * (bucketPasses + 1);
const expectedLocked = totalTests - expectedUnlocked;

const expectedMinUnlocked = lockoutLimit * bucketPasses;
const expectedMaxLocked = totalTests - expectedMinUnlocked;

assert(
countUnLocked >= expectedMinUnlocked && countUnLocked <= expectedUnlocked,
`Expected between ${expectedMinUnlocked} and ${expectedUnlocked} unlocks in ${bucketPasses} passes of ${lockoutTime}ms, but got ${countUnLocked} (${txt})`,
);

assert(
countLocked >= expectedLocked && countLocked <= expectedMaxLocked,
`Expected between ${expectedLocked} and ${expectedMaxLocked} locks in ${bucketPasses} passes of ${lockoutTime}ms, but got ${countLocked} (${txt})`,
);
}

function testLockout(timer: LockoutTimer, txt: string) {
for (let pass = 0; pass < 3; pass++) {
for (let i = 0; i < lockoutLimit; i++) {
assertEquals(
timer.isLocked(),
false,
`Should be unlocked pass ${pass} loop ${i} ${txt}`,
);
timer.addCount();
}
// count = lockoutLimit

for (let i = 0; i < 10; i++) {
assertEquals(
timer.isLocked(),
true,
`Should be locked before bucket rollover. pass ${pass} loop ${i} ${txt}`,
);
timer.addCount();
}
// count = lockoutLimit + 10

time.tick(lockoutTime);
// bucket should rollover, count=0
}
}
});
45 changes: 45 additions & 0 deletions server/lockout.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
export class LockoutTimer {
// A very simple, quick, inexact lockout timer
// For each request isLocked() or updateBucketTime() must be called before addCount()
// counts in buckets of countPeriodMs
// each period starts with an empty bucket
// suggest SB_LOCKOUT_TIME_MS=60000 SB_LOCKOUT_LIMIT=10
bucketTime: number = 0;
bucketCount: number = 0;
bucketSize: number;
limit: number;
disabled: boolean;

constructor(
countPeriodMs: number = Number(Deno.env.get("SB_LOCKOUT_TIME_MS")) || NaN,
limit: number = Number(Deno.env.get("SB_LOCKOUT_LIMIT")) || NaN,
) {
this.disabled = isNaN(countPeriodMs) || isNaN(limit) || countPeriodMs < 1 ||
limit < 1;
this.bucketSize = countPeriodMs;
this.limit = limit;
}

updateBucketTime(): void {
const currentBucketTime = Math.floor(Date.now() / this.bucketSize);
if (this.bucketTime === currentBucketTime) {
return;
}
// the bucket is too old - empty it
this.bucketTime = currentBucketTime;
this.bucketCount = 0;
}

isLocked(): boolean {
if (this.disabled) {
return false;
}
this.updateBucketTime();
return this.bucketCount >= this.limit;
}

addCount(): void {
// isLocked or updateBucketTime must be called first to keep bucketTime current
this.bucketCount++;
}
}
3 changes: 3 additions & 0 deletions web/auth.html
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,9 @@ <h1>
} else if (error === "1") {
document.querySelector(".error-message").innerText =
"Invalid username or password";
} else if (error === "2") {
document.querySelector(".error-message").innerText =
"Too many invalid logins. Try again later";
}

const from = params.get("from");
Expand Down

0 comments on commit 046a0df

Please sign in to comment.