diff --git a/api/src/host-check.js b/api/src/host-check.js index ec84a57f..1af4f555 100644 --- a/api/src/host-check.js +++ b/api/src/host-check.js @@ -7,15 +7,20 @@ const { hostnameOf, isLocalAddress } = require('../../ui/js/host-names.js'); cannot point at this server (DNS rebinding). Never add a name to the list from a request: a rebound page can send that request first. */ +/** @returns {boolean} whether the request names a local address or a listed one */ +function knownHost(req, cfg) { + const host = hostnameOf(req.headers.host); + if (!host) return false; + const list = cfg.settings?.server?.allowedHosts; + return isLocalAddress(host) || (Array.isArray(list) && list.includes(host)); +} + /** @returns {string | null} the refused host name, or null when allowed */ function refusedHost(req) { if (IS_DEMO) return null; const cfg = loadConfig(); - if (authActive(cfg)) return null; - const host = hostnameOf(req.headers.host); - if (!host) return String(req.headers.host || ''); - const list = cfg.settings?.server?.allowedHosts; - return isLocalAddress(host) || (Array.isArray(list) && list.includes(host)) ? null : host; + if (authActive(cfg) || knownHost(req, cfg)) return null; + return hostnameOf(req.headers.host) || String(req.headers.host || ''); } -module.exports = { refusedHost }; +module.exports = { refusedHost, knownHost }; diff --git a/api/src/routes/auth.js b/api/src/routes/auth.js index 448b4063..b9f10126 100644 --- a/api/src/routes/auth.js +++ b/api/src/routes/auth.js @@ -4,6 +4,7 @@ const { loadConfig, loadConfigForUpdate, saveConfig } = require('../config'); const log = require('../log'); const { fail, KIND } = require('../api-error'); const { normalizeHostList } = require('../../../ui/js/host-names.js'); +const { knownHost } = require('../host-check'); const { getOrCreateSecret, rotateSessionSecret, @@ -60,7 +61,8 @@ on('POST', '/api/auth/login', async (req, res) => { const cfg = loadConfig(); if (!authActive(cfg)) return json(res, 200, { ok: true }); const hash = cfg.settings.auth.passwordHash; - const limitErr = registerLoginAttempt(ip); + const key = attemptKey(req, cfg); + const limitErr = registerLoginAttempt(key); if (limitErr) { log.audit('login blocked', { ip, reason: 'rate_limit' }); return json(res, 429, { error: limitErr, kind: KIND.BLOCKED, code: 'blocked.rate-limit' }); @@ -70,7 +72,7 @@ on('POST', '/api/auth/login', async (req, res) => { log.audit('login failed', { ip }); return json(res, 401, { error: 'Incorrect password.', kind: KIND.AUTH }); } - clearAttempts(ip); + clearAttempts(key); log.audit('login success', { ip }); /* A failure here must not fail the login. The password is correct either way and the old hash still verifies. */ @@ -108,10 +110,18 @@ on('POST', '/api/auth/logout', (req, res) => { json(res, 200, { ok: true }); }); +/* Keep unlisted names on a count of their own. A page on another site that + points its name at this server could otherwise lock the owner out. */ +function attemptKey(req, cfg) { + const ip = getIp(req); + return knownHost(req, cfg) ? ip : `${ip} unlisted`; +} + /* Keep the lockout count. Without it a session can brute-force the password. */ async function refuseWrongCurrentPassword(req, res, hash, currentPassword) { const ip = getIp(req); - const limitErr = registerLoginAttempt(ip); + const key = attemptKey(req, loadConfig()); + const limitErr = registerLoginAttempt(key); if (limitErr) { log.audit('password check blocked', { ip, reason: 'rate_limit' }); json(res, 429, { error: limitErr, kind: KIND.BLOCKED, code: 'blocked.rate-limit' }); @@ -122,7 +132,7 @@ async function refuseWrongCurrentPassword(req, res, hash, currentPassword) { json(res, 403, { error: 'Current password is incorrect.', kind: KIND.INVALID, code: CURRENT_PASSWORD_CODE }); return true; } - clearAttempts(ip); + clearAttempts(key); return false; } diff --git a/api/test/login-limit-hosts.test.js b/api/test/login-limit-hosts.test.js new file mode 100644 index 00000000..6da2d968 --- /dev/null +++ b/api/test/login-limit-hosts.test.js @@ -0,0 +1,81 @@ +const path = require('node:path'); +const { tmpDir } = require('../test-support/tmp'); +const DIR = tmpDir('login-limit-hosts'); +process.env.CONFIG_PATH = path.join(DIR, 'apps.json'); + +const { test, before, after, beforeEach, mock } = require('node:test'); +const assert = require('node:assert/strict'); +const http = require('node:http'); + +require('../src/routes'); +const { dispatch } = require('../src/router'); +const { saveConfig } = require('../src/config'); +const { hashPassword } = require('../src/auth'); + +let server, port; +let clock = Date.now(); + +/* Host and Origin name the same site, as a browser sends them after DNS + rebinding, so the Origin check passes. */ +function login(host, password) { + return new Promise((resolve, reject) => { + const payload = JSON.stringify({ password }); + const req = http.request( + { + host: '127.0.0.1', + port, + path: '/api/auth/login', + method: 'POST', + headers: { + Host: host, + Origin: `http://${host}`, + 'Content-Type': 'application/json', + 'Content-Length': Buffer.byteLength(payload), + }, + }, + res => { + res.resume(); + res.on('end', () => resolve(res.statusCode)); + }, + ); + req.on('error', reject); + req.end(payload); + }); +} + +before(async () => { + mock.method(Date, 'now', () => clock); + server = http.createServer(dispatch); + await new Promise(r => server.listen(0, '127.0.0.1', r)); + port = server.address().port; +}); + +after(() => new Promise(r => server.close(r))); + +beforeEach(async () => { + /* Past the 15-minute window, so each test starts with no attempts counted. */ + clock += 16 * 60 * 1000; + const passwordHash = await hashPassword('correct-horse'); + saveConfig({ + items: [], + settings: { auth: { enabled: true, passwordHash }, server: { allowedHosts: ['dash.example.com'] } }, + }); +}); + +test('wrong passwords sent under an unlisted name do not lock out the owner', async () => { + for (let i = 0; i < 5; i++) assert.equal(await login('rebind.evil.example', 'wrong'), 401); + assert.equal(await login('rebind.evil.example', 'wrong'), 429); + assert.equal(await login('rebind.evil.example', 'correct-horse'), 429); + assert.equal(await login('192.168.1.10', 'correct-horse'), 200); + assert.equal(await login('dash.example.com', 'correct-horse'), 200); +}); + +test('every unlisted name shares one count, so changing names gains nothing', async () => { + for (let i = 0; i < 5; i++) assert.equal(await login(`n${i}.evil.example`, 'wrong'), 401); + assert.equal(await login('another.evil.example', 'wrong'), 429); +}); + +test('the owner is still limited on a known address', async () => { + for (let i = 0; i < 5; i++) assert.equal(await login('192.168.1.10', 'wrong'), 401); + assert.equal(await login('dash.example.com', 'correct-horse'), 429); +});