Security: stop trusting the first host name while no password is set

api/src/host-check.js

@@ -1,39 +1,21 @@
-const { loadConfig, loadConfigForUpdate, saveConfig } = require('./config');
+const { loadConfig } = require('./config');
 const { authActive } = require('./auth');
 const { IS_DEMO } = require('./demo');
-const log = require('./log');
 const { hostnameOf, isLocalAddress } = require('../../ui/js/host-names.js');
 
 /* While no password is set, answer only addresses a page on another site
-   cannot point at this server (DNS rebinding). The first page load sets the
-   list when none is stored. An IP address sets an empty one. */
+   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 {string | null} the refused host name, or null when allowed */
-function refusedHost(req, pathname) {
+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;
-  const local = isLocalAddress(host);
-  if (!Array.isArray(list) && req.method === 'GET' && pathname === '/api/auth/check') {
-    trustFirst(local ? [] : [host], host);
-    return null;
-  }
-  return local || (Array.isArray(list) && list.includes(host)) ? null : host;
-}
-
-function trustFirst(list, host) {
-  try {
-    const cfg = loadConfigForUpdate();
-    if (Array.isArray(cfg.settings?.server?.allowedHosts)) return;
-    cfg.settings.server = { ...cfg.settings.server, allowedHosts: list };
-    saveConfig(cfg);
-    log.audit('allowed addresses set from the first visit', { host, allowed: list.join(',') || 'local only' });
-  } catch (e) {
-    log.warn('could not store the allowed addresses', { error: e.message });
-  }
+  return isLocalAddress(host) || (Array.isArray(list) && list.includes(host)) ? null : host;
 }
 
 module.exports = { refusedHost };

api/src/router.js

@@ -52,7 +52,7 @@ function route(req, res) {
   }
 
   if (u.pathname !== '/health') {
-    const host = refusedHost(req, u.pathname);
+    const host = refusedHost(req);
     if (host !== null) {
       return json(res, 403, {
         error: 'Forbidden: this address is not allowed while no password is set',

api/test/host-check.test.js

@@ -86,20 +86,28 @@ test('a rebound page cannot read or replace the config', async () => {
   assert.equal(loadConfig().items.length, 1);
 });
 
-test('the first page load by name trusts that name and no other', async () => {
-  const first = await request('GET', '/api/auth/check', 'Dash.Example.com:8700');
-  assert.equal(first.status, 200);
-  assert.deepEqual(stored(), ['dash.example.com']);
+test('a rebound page that loads first is refused and stores nothing', async () => {
+  const first = await request('GET', '/api/auth/check', 'evil.example:8700');
+  assert.equal(first.status, 403);
+  assert.equal(first.body.code, 'blocked.host');
+  assert.equal(fs.existsSync(CFG), false);
   expireCache();
-  assert.equal((await request('GET', '/api/config', 'dash.example.com')).status, 200);
   assert.equal((await request('GET', '/api/auth/check', 'evil.example')).status, 403);
 });
 
-test('the first page load by IP address trusts no name', async () => {
+test('a page load by IP address trusts no name, and a name added there is answered', async () => {
   assert.equal((await request('GET', '/api/auth/check', '192.168.1.10:8700')).status, 200);
-  assert.deepEqual(stored(), []);
+  assert.equal(fs.existsSync(CFG), false);
   expireCache();
   assert.equal((await request('GET', '/api/auth/check', 'dash.example.com')).status, 403);
+  const add = await request('POST', '/api/config', '192.168.1.10', {
+    items: [],
+    settings: { server: { allowedHosts: ['dash.example.com'] } },
+  });
+  assert.equal(add.status, 200);
+  expireCache();
+  assert.equal((await request('GET', '/api/auth/check', 'dash.example.com')).status, 200);
+  assert.equal((await request('GET', '/api/auth/check', 'evil.example')).status, 403);
 });
 
 test('local addresses are always answered', async () => {

ui/index.html

@@ -56,5 +56,5 @@
     </div>
   </div>
 </dialog>
-<script type="module" src="/js/dashboard.js?v=550f6505"></script>
+<script type="module" src="/js/dashboard.js?v=25d81400"></script>
 </body></html>

ui/js/dashboard.js

@@ -746,18 +746,16 @@ async function boot() {
       v => ({ v }),
       e => ({ e }),
     );
-  const loadConfig = () =>
-    settled(fetch('/api/config', { cache: 'no-store', signal: AbortSignal.timeout(BOOT_TIMEOUT_MS) }));
-  let configReq = loadConfig();
+  const configReq = settled(fetch('/api/config', { cache: 'no-store', signal: AbortSignal.timeout(BOOT_TIMEOUT_MS) }));
   const loadWidgets = () =>
     settled(
       fetch('/api/widgets', { cache: 'no-store' }).then(r => {
-        if (!r.ok) throw Object.assign(new Error(`HTTP ${r.status}`), { status: r.status });
+        if (!r.ok) throw new Error(`HTTP ${r.status}`);
         return r.json();
       }),
     );
   let widgetsReq = loadWidgets();
-  let iconsReq = loadLocalIcons();
+  const iconsReq = loadLocalIcons();
 
   let authData = null;
   try {
@@ -779,11 +777,6 @@ async function boot() {
       window.location.href = '/admin/';
       return;
     }
-    /* Only the sign-in check stores the first address. Requests sent before it
-       on a first visit by host name are refused. */
-    if ((await configReq).v?.status === 403) configReq = loadConfig();
-    if ((await widgetsReq).e?.status === 403) widgetsReq = loadWidgets();
-    if ((await iconsReq) === 403) iconsReq = loadLocalIcons();
   } catch {
     /* API down, handled below */
   }

ui/test/boot-requests.test.mjs

@@ -59,69 +59,6 @@ test('a widget list refused while the first-run password was set is requested ag
   assert.match(prompt.slice(0, 200), /if \(\(await widgetsReq\)\.e\) widgetsReq = loadWidgets\(\);/);
 });
 
-const firstVisitOrders = [
-  ['/api/config', '/api/widgets', '/api/icons/local'],
-  ['/api/widgets'],
-  ['/api/icons/local'],
-  ['/api/config'],
-];
-
-for (const early of firstVisitOrders) {
-  test(`a first visit by host name asks again for what was refused: ${early.join(', ')}`, async () => {
-    const start = dashboard.indexOf('async function boot()');
-    const src = dashboard.slice(start, dashboard.indexOf('\n}\n', start) + 2);
-    let trusted = false;
-    const sent = [];
-    const reply = (status, body) => ({ ok: status < 300, status, json: async () => body });
-    const fetch = async url => {
-      sent.push(url);
-      const wait = url === '/api/auth/check' ? 5 : early.includes(url) ? 0 : 10;
-      await new Promise(r => setTimeout(r, wait));
-      if (url === '/api/auth/check') {
-        trusted = true;
-        return reply(200, { enabled: false, passwordSet: false, setupPrompted: false });
-      }
-      if (!trusted) return reply(403, { kind: 'blocked', code: 'blocked.host' });
-      return reply(200, url === '/api/config' ? { items: [], settings: {}, _rev: 1 } : { widgets: [], files: [] });
-    };
-    const shown = [];
-    const stop = new Error('reached the first-run prompt');
-    const boot = new Function(
-      'fetch, loadLocalIcons, document, initI18n, sanitizeItemLinks, showSetupPrompt, setHtml, html, t, BOOT_TIMEOUT_MS, blockingScreenFor, showBlockingScreen, console',
-      `let items, S, _rev, widgetReg; ${src} return boot;`,
-    )(
-      fetch,
-      async () => (await fetch('/api/icons/local')).status,
-      {
-        body: { appendChild: el => shown.push(el.className), classList: { add() {} } },
-        createElement: () => ({ querySelector: () => null }),
-      },
-      async () => {},
-      x => x,
-      async () => {
-        throw stop;
-      },
-      () => {},
-      () => '',
-      k => k,
-      1000,
-      () => null,
-      async () => {},
-      { error() {} },
-    );
-    await assert.rejects(boot(), stop);
-    assert.deepEqual(shown, [], 'the API-down screen was shown');
-    for (const url of ['/api/config', '/api/widgets', '/api/icons/local']) {
-      const times = early.includes(url) ? 2 : 1;
-      assert.equal(
-        sent.filter(u => u === url).length,
-        times,
-        times === 2 ? `${url} not sent again after a refusal` : `${url} sent again without a refusal`,
-      );
-    }
-  });
-}
-
 for (const failures of [1, 2]) {
   test(`a widget list that fails ${failures === 1 ? 'once is asked for again' : 'twice shows the API-down screen'}`, async () => {
     const start = dashboard.indexOf('async function boot()');