Security: count sign-in attempts from unlisted host names separately

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 };

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;
 }
 

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);
+});