fix(agent): serve only agent paths on the agent mTLS port

api/hooks/agent-tls/index.js

@@ -3,9 +3,11 @@
  *
  * After Sails lifts, it starts an HTTPS server on custom.agentTlsPort (default
  * 8443; 0 turns it off). The server runs the same Express app as the main
- * port, so every route answers on both ports and an agent uses one base URL,
- * https://<host>:<port>, for ca.cert.pem, enroll, poll and renew. It closes the
- * server when Sails lowers.
+ * port, but it passes a request to the app only for the agent's own paths:
+ * /agent/v1/* and the two CA files (agentPath). Every other path gets a 404
+ * from the listener, so this port never serves the UI, login or admin API (it
+ * bypasses the reverse proxy). An agent uses one base URL, https://<host>:<port>,
+ * for ca.cert.pem, enroll, poll and renew. It closes the server when Sails lowers.
  *
  * TLS: the server certificate is pki/server.pem (made by api/lib/agent-pki-files.js,
  * signed by the root). The server REQUESTS a client certificate and never
@@ -34,6 +36,26 @@ const path = require('path');
 // ensureServerCert on it to hold the lift at the await.
 const pkiFiles = require('../../lib/agent-pki-files');
 
+// The CA files the agent's CA probe fetches from the same base URL. These
+// repeat two routes in config/routes.js on purpose: a route reaches the agent
+// port only when it is added here too.
+const CA_FILES = ['/management/other/ca.cert.pem', '/management/other/ca.cert.der'];
+
+/**
+ * True when the agent listener may pass `url` (a request target) to the app:
+ * a path under /agent/v1/, or exactly one of the two CA files. The query string
+ * is ignored. The path is not decoded, because Express routes on the raw path
+ * and the agent never sends a percent sign. The path must use only letters,
+ * digits and `/ - _ .`, and must equal its own normalized form, so `%`, `\`,
+ * a NUL, `..`, `.` and `//` are all refused.
+ */
+const agentPath = (url) => {
+  if (typeof url !== 'string') { return false; }
+  const raw = url.split(/[?#]/)[0];
+  if (!/^[A-Za-z0-9/_.-]+$/.test(raw) || !raw.startsWith('/') || path.posix.normalize(raw) !== raw) { return false; }
+  return raw.startsWith('/agent/v1/') || CA_FILES.includes(raw);
+};
+
 /**
  * Start the agent listener on `app`. Returns the server, or null when `port`
  * is 0 or the PKI files are missing (one warning in that case). A port that is
@@ -42,6 +64,13 @@ const pkiFiles = require('../../lib/agent-pki-files');
  */
 const startAgentListener = (app, {port, pkiDir}) => {
   if (!port) { return null; }
+  // Any other path gets a 404 here, so no session, CSRF or page code runs.
+  const handler = (req, res) => {
+    if (agentPath(req.url)) { return app(req, res); }
+    const body = JSON.stringify({error: 'not found'});
+    res.writeHead(404, {'Content-Type': 'application/json', 'Content-Length': Buffer.byteLength(body)});
+    res.end(body);
+  };
   const notStarted = (err) => sails.log.warn(`Agent listener not started: cannot listen on port ${port} (${err.code || err.message}). fog-node runs without it.`);
   let server;
   try {
@@ -53,7 +82,7 @@ const startAgentListener = (app, {port, pkiDir}) => {
       requestCert: true,
       rejectUnauthorized: false,
       minVersion: 'TLSv1.2'
-    }, app);
+    }, handler);
   } catch (err) {
     sails.log.warn(`Agent listener not started: the PKI files in ${pkiDir} are missing or unreadable (${err.code || err.message}).`);
     return null;
@@ -121,3 +150,4 @@ module.exports = function defineAgentTlsHook(sails) {
 };
 
 module.exports.startAgentListener = startAgentListener;
+module.exports.agentPath = agentPath;

docs/install.md

@@ -138,9 +138,11 @@ fingerprint. You must then re-enroll every fog-agent by hand with the new
 fog-node runs a second, HTTPS listener for fog-agent. It uses the port in
 `custom.agentTlsPort` in `config/local.js`. The default is 8443. The agent
 proves itself with its client certificate on this port (mTLS), so it works on
-any OS with no proxy setup. The listener serves the same routes as the main
-port. An agent uses one base URL for everything: `https://<host>:<port>`, for
-example `https://fogserver:8443`.
+any OS with no proxy setup. The listener serves only the agent routes
+(`/agent/v1/...`) and the two CA files (`/management/other/ca.cert.pem` and
+`.der`). Every other path, such as the web UI, the login, and `/api/v1`, answers
+404 on this port. An agent uses one base URL for everything:
+`https://<host>:<port>`, for example `https://fogserver:8443`.
 
 To change the port, run `npm run setup` again, or edit `custom.agentTlsPort`
 and restart fog-node. The value `0` turns the listener off. The answers file

test/hooks/agent-tls.test.js

@@ -9,7 +9,7 @@ const path = require('path');
 const tls = require('tls');
 const defineAgentTlsHook = require('../../api/hooks/agent-tls');
 const pkiFiles = require('../../api/lib/agent-pki-files');
-const {startAgentListener} = defineAgentTlsHook;
+const {startAgentListener, agentPath} = defineAgentTlsHook;
 const agentCsr = require('../support/agent-csr');
 const support = require('../support/pki');
 const {freePort} = require('../support/port');
@@ -52,15 +52,43 @@ describe('Agent listener::', function () {
     }
   };
 
-  const get = (port, options = {}) => new Promise((resolve, reject) => {
-    const req = https.request(Object.assign({host: '127.0.0.1', port, path: '/', ca: rootPem(), agent: false}, options), (res) => {
+  // One HTTPS request to the listener. Resolves {status, body}, the body parsed as JSON.
+  const request = (port, options = {}) => new Promise((resolve, reject) => {
+    const req = https.request(Object.assign({host: '127.0.0.1', port, path: '/agent/v1/x', ca: rootPem(), agent: false}, options), (res) => {
       let data = '';
       res.on('data', (chunk) => { data += chunk; });
-      res.on('end', () => resolve(JSON.parse(data)));
+      res.on('end', () => resolve({status: res.statusCode, body: JSON.parse(data)}));
     });
     req.on('error', reject);
     req.end();
   });
+  const get = async (port, options) => (await request(port, options)).body;
+
+  describe('agentPath::', () => {
+    it('allows the agent API and the two CA files, with or without a query string', () => {
+      for (const url of [
+        '/agent/v1/poll', '/agent/v1/enroll', '/agent/v1/', '/agent/v1/payload/7', '/agent/v1/poll?x=1',
+        '/agent/v1/renew', '/agent/v1/result', '/agent/v1/payload/update/12', '/agent/v1/identity/check', '/agent/v1/directory/rejoin/start',
+        '/management/other/ca.cert.pem', '/management/other/ca.cert.der', '/management/other/ca.cert.pem?v=1'
+      ]) {
+        assert.strictEqual(agentPath(url), true, url);
+      }
+    });
+
+    it('refuses every other path, traversal and encoded traversal', () => {
+      for (const url of [
+        '/', '/login', '/api/v1/host', '/agent/v1', '/agent/v1x', '/agent/v10/', '/agent/v1/../login',
+        '/%2e%2e/login', '/agent/v1/%2e%2e/%2e%2e/login', '/agent/v1/%2E%2E/login', '/agent/v1//x', '/agent/v1/./x',
+        '/agent/v1/..%2flogin', '/agent/v1/%5c..%5clogin', '/agent/v1/%zz', '/agent/v1/%00',
+        '/%61gent/v1/poll', '/agent%2Fv1/poll', '/management/other/ca%2Ecert.pem', '/management%2Fother%2Fca.cert.pem',
+        '/agent/v1/%252e%252e/login', '/agent/v1/a b', '/agent/v1/a\\b',
+        '/management/other/ca.cert.pem.bak', '/management/other/ca.cert.der/x', '/management/other/ca.cert.pem/',
+        '/management/other/agent-ca-bundle.pem', '//agent/v1/poll', 'agent/v1/poll', 'http://x/login', '*', '', undefined
+      ]) {
+        assert.strictEqual(agentPath(url), false, String(url));
+      }
+    });
+  });
 
   it('returns null for port 0, and logs nothing', async () => {
     const {result, lines} = await warnings(() => startAgentListener(app, {port: 0, pkiDir: fixtureDir()}));
@@ -261,6 +289,13 @@ describe('Agent listener::', function () {
       }
     });
 
+    it('answers 404 with a JSON body for a path that is not the agent\'s, without calling the app', async () => {
+      // The stub app answers 200 JSON, so a 404 here comes from the listener's filter.
+      const res = await request(port, {path: '/login'});
+      assert.strictEqual(res.status, 404);
+      assert.deepStrictEqual(res.body, {error: 'not found'});
+    });
+
     it('requests a client certificate and never requires one', async () => {
       // A browser sends none: the request is answered, with no peer certificate.
       assert.deepStrictEqual(await get(port), {authorized: false, peer: false});

test/integration/agent-mtls.test.js

@@ -166,10 +166,23 @@ describe('Agent mTLS::', function () {
   after(cleanup);
 
   describe('the listener::', () => {
-    it('gives a browser-style request with no client certificate the login page', async () => {
-      const res = await mtls('GET', '/login', {headers: {Accept: 'text/html'}});
-      assert.strictEqual(res.status, 200);
-      assert.match(res.text, /name="password"/);
+    it('answers 404 for the UI, the login and the admin API, and runs no session code', async () => {
+      const before = await sessionCount();
+      for (const [method, urlPath] of [['GET', '/'], ['GET', '/login'], ['POST', '/api/v1/host'], ['GET', '/csrfToken'], ['GET', '/agent/v1/../login']]) {
+        const res = await mtls(method, urlPath, {headers: {Accept: 'text/html'}, body: method === 'POST' ? {} : undefined});
+        assert.strictEqual(res.status, 404, `${method} ${urlPath}`);
+        assert.match(res.headers['content-type'], /^application\/json/);
+        assert.deepStrictEqual(res.json, {error: 'not found'});
+        assert.strictEqual(res.headers['set-cookie'], undefined, 'no cookie');
+      }
+      assert.strictEqual(await sessionCount(), before, 'no new sess: key');
+    });
+
+    it('still serves the CA file and the agent routes', async () => {
+      const ca = await mtls('GET', '/management/other/ca.cert.pem');
+      assert.strictEqual(ca.status, 200);
+      assert.match(ca.text, /-----BEGIN CERTIFICATE-----/);
+      assert.strictEqual((await mtls('POST', '/agent/v1/poll', {body: {}})).status, 401);
     });
 
     it('serves a certificate that verifies against the root, with the right SAN entry types', async () => {