fix(security): throttle failed logins; add security headers to the API

app/assets/report/index.pug

@@ -277,5 +277,6 @@ html(lang="en")
       img(alt="")
       span.viewer-caption
 
-    script
+    //- The nonce matches the Content-Security-Policy the API serves the report with.
+    script(nonce=nonce)
       include report.js

app/controllers/build.controller.js

@@ -1,4 +1,5 @@
 const { validationResult } = require('express-validator');
+const crypto = require('crypto');
 const mongoose = require('mongoose');
 const debug = require('debug');
 
@@ -20,6 +21,7 @@ const {
   handleError,
 } = require('../exceptions/errors.js');
 const authMiddleware = require('../utils/auth-middleware.js');
+const { reportPolicy } = require('../utils/security-headers.js');
 
 const log = debug('build:controller');
 
@@ -280,8 +282,16 @@ exports.getReport = (req, res) => {
       const query = { build: mongoose.Types.ObjectId(build._id) };
       return Screenshot.find(query).lean();
     })
-    // eslint-disable-next-line global-require
-    .then((screenshots) => res.render('index', { build, screenshots, moment: require('moment') }))
+    .then((screenshots) => {
+      // A fresh nonce per report: the report's one inline script carries it, so nothing
+      // else injected into the page could run when it is opened from the API.
+      const nonce = crypto.randomBytes(16).toString('base64');
+      res.set('Content-Security-Policy', reportPolicy(nonce));
+      return res.render('index', {
+        // eslint-disable-next-line global-require
+        build, screenshots, nonce, moment: require('moment'),
+      });
+    })
     .catch((err) => handleError(err, res));
 };
 

app/routes/auth.routes.js

@@ -3,6 +3,7 @@ const passport = require('passport');
 const debug = require('debug');
 const authConfig = require('../../config/auth.config.js');
 const featureConfig = require('../../config/feature.config.js');
+const loginThrottle = require('../utils/login-throttle.js');
 const {
   isProviderReady,
   getReadyProvider,
@@ -38,7 +39,7 @@ module.exports = (app, path) => {
       .exists({ checkFalsy: true })
       .isLength({ min: 1, max: 100 })
       .withMessage('Password is required.'),
-  ], (req, res, next) => {
+  ], loginThrottle.guard, (req, res, next) => {
     const errors = validationResult(req);
     if (!errors.isEmpty()) {
       return res.status(422).json({ errors: errors.array() });
@@ -49,8 +50,10 @@ module.exports = (app, path) => {
     return passport.authenticate('local', (err, user, info) => {
       if (err) return next(err);
       if (!user) {
+        loginThrottle.recordFailure(req.ip, req.body.username);
         return res.status(401).json({ error: info.message || 'Login failed' });
       }
+      loginThrottle.recordSuccess(req.ip, req.body.username);
       return req.logIn(user, (loginErr) => {
         if (loginErr) return next(loginErr);
         return res.json({
@@ -95,7 +98,7 @@ module.exports = (app, path) => {
       .exists({ checkFalsy: true })
       .isLength({ min: 1, max: 200 })
       .withMessage('Password is required.'),
-  ], ssoGuard, (req, res, next) => {
+  ], loginThrottle.guard, ssoGuard, (req, res, next) => {
     const errors = validationResult(req);
     if (!errors.isEmpty()) {
       return res.status(422).json({ errors: errors.array() });
@@ -112,8 +115,10 @@ module.exports = (app, path) => {
         return res.status(503).json({ error: 'The directory could not be reached.' });
       }
       if (!user) {
+        loginThrottle.recordFailure(req.ip, req.body.username);
         return res.status(401).json({ error: (info && info.message) || 'Login failed' });
       }
+      loginThrottle.recordSuccess(req.ip, req.body.username);
       return req.logIn(user, (loginErr) => {
         if (loginErr) return next(loginErr);
         return res.json({

app/utils/login-throttle.js

@@ -0,0 +1,118 @@
+const debug = require('debug');
+
+const log = debug('auth:throttle');
+
+/*
+ * Slows down password guessing against the credential logins (local and LDAP).
+ *
+ * Two limits, both counting failed sign-ins within a sliding window:
+ *
+ *  - per client IP + username (default 5): stops one client guessing one account's
+ *    password. Reached, that pair is refused until the window passes.
+ *  - per client IP (default 50): stops one client spraying a few guesses at many
+ *    accounts.
+ *
+ * There is deliberately no limit on a username alone. It would also stop a distributed
+ * guess at one account, but it would let anyone lock any user (an admin, say) out simply
+ * by failing to sign in as them from a few addresses.
+ *
+ * A successful sign-in clears that client's count for the username. Counts are kept in
+ * memory, so each API instance limits on its own and a restart clears them.
+ *
+ * Behind a reverse proxy, set TRUST_PROXY=true so the client's address (from
+ * X-Forwarded-For) is used; otherwise every request appears to come from the proxy and
+ * the per-IP limit applies to everyone at once.
+ */
+
+const positiveInt = (value, fallback) => {
+  const parsed = parseInt(value, 10);
+  return Number.isFinite(parsed) && parsed > 0 ? parsed : fallback;
+};
+
+const defaults = () => ({
+  maxFailuresPerAccount: positiveInt(process.env.ANGLES_LOGIN_MAX_FAILURES, 5),
+  maxFailuresPerIp: positiveInt(process.env.ANGLES_LOGIN_MAX_FAILURES_PER_IP, 50),
+  windowMs: positiveInt(process.env.ANGLES_LOGIN_LOCKOUT_MINUTES, 15) * 60 * 1000,
+});
+
+let settings = defaults();
+
+// key -> timestamps (ms) of failures still inside the window
+const failures = new Map();
+// Bound the memory a flood of distinct addresses/usernames can take.
+const MAX_KEYS = 10000;
+
+const accountKey = (ip, username) => `account:${ip}:${String(username || '').toLowerCase().trim()}`;
+const ipKey = (ip) => `ip:${ip}`;
+
+const recent = (key, now) => {
+  const timestamps = (failures.get(key) || []).filter((t) => now - t < settings.windowMs);
+  if (timestamps.length) failures.set(key, timestamps);
+  else failures.delete(key);
+  return timestamps;
+};
+
+const prune = (now) => {
+  if (failures.size < MAX_KEYS) return;
+  [...failures.keys()].forEach((key) => recent(key, now));
+  // Still full of live entries: drop the oldest keys (Map keeps insertion order).
+  const excess = failures.size - MAX_KEYS + 1;
+  [...failures.keys()].slice(0, Math.max(0, excess)).forEach((key) => failures.delete(key));
+};
+
+// Seconds until the oldest counted failure leaves the window.
+const retryAfterSeconds = (timestamps, now) => Math.max(
+  1,
+  Math.ceil((timestamps[0] + settings.windowMs - now) / 1000),
+);
+
+/**
+ * Whether this client may attempt to sign in as `username` now. Returns
+ * `{ allowed: true }` or `{ allowed: false, retryAfter }` (seconds).
+ */
+const check = (ip, username, now = Date.now()) => {
+  const perAccount = recent(accountKey(ip, username), now);
+  if (perAccount.length >= settings.maxFailuresPerAccount) {
+    return { allowed: false, retryAfter: retryAfterSeconds(perAccount, now) };
+  }
+  const perIp = recent(ipKey(ip), now);
+  if (perIp.length >= settings.maxFailuresPerIp) {
+    return { allowed: false, retryAfter: retryAfterSeconds(perIp, now) };
+  }
+  return { allowed: true };
+};
+
+const recordFailure = (ip, username, now = Date.now()) => {
+  prune(now);
+  [accountKey(ip, username), ipKey(ip)].forEach((key) => {
+    failures.set(key, [...recent(key, now), now]);
+  });
+  log('Failed sign-in for %s from %s', username, ip);
+};
+
+const recordSuccess = (ip, username) => {
+  failures.delete(accountKey(ip, username));
+};
+
+/**
+ * Express middleware for a credential login route: refuses with 429 (and Retry-After)
+ * while the client is over a limit for the posted username.
+ */
+const guard = (req, res, next) => {
+  const result = check(req.ip, req.body && req.body.username);
+  if (result.allowed) return next();
+  res.set('Retry-After', String(result.retryAfter));
+  return res.status(429).json({
+    error: `Too many failed sign-in attempts. Try again in ${Math.ceil(result.retryAfter / 60)} minute(s).`,
+  });
+};
+
+module.exports = {
+  guard,
+  check,
+  recordFailure,
+  recordSuccess,
+  // Tests only: replace the limits, and forget every recorded failure.
+  configure: (overrides) => { settings = { ...defaults(), ...overrides }; },
+  reset: () => { failures.clear(); settings = defaults(); },
+};

app/utils/security-headers.js

@@ -0,0 +1,37 @@
+/*
+ * Security headers for every API response.
+ *
+ * The API answers with JSON and files, so the default policy forbids loading anything at
+ * all (`default-src 'none'`): if a response is ever rendered as a page - an error echoing
+ * input, a file opened directly - nothing in it can run or load. `frame-ancestors 'none'`
+ * and X-Frame-Options stop any API page being framed.
+ *
+ * Two kinds of response render real pages and set their own policy instead:
+ *  - the Swagger UI under /api-docs, which needs its own scripts and styles;
+ *  - the HTML build report (see buildController.getReport), which uses a nonce.
+ * Attachment files set a sandbox policy of their own as well.
+ */
+const API_POLICY = "default-src 'none'; frame-ancestors 'none'";
+const SWAGGER_POLICY = "frame-ancestors 'none'";
+
+const securityHeaders = (req, res, next) => {
+  res.set('X-Content-Type-Options', 'nosniff');
+  res.set('X-Frame-Options', 'DENY');
+  res.set('Referrer-Policy', 'no-referrer');
+  res.set('Content-Security-Policy', req.path.startsWith('/api-docs') ? SWAGGER_POLICY : API_POLICY);
+  next();
+};
+
+/*
+ * The policy for the HTML build report: its own inline styles, its one inline script (by
+ * nonce) and its embedded data: screenshots, and nothing else - no network requests.
+ */
+const reportPolicy = (nonce) => [
+  "default-src 'none'",
+  "style-src 'unsafe-inline'",
+  `script-src 'nonce-${nonce}'`,
+  'img-src data:',
+  "frame-ancestors 'none'",
+].join('; ');
+
+module.exports = { securityHeaders, reportPolicy };

server.js

@@ -18,6 +18,7 @@ const { configureProviders } = require('./app/utils/passport-setup.js');
 const authSettingsService = require('./app/utils/auth-settings-service.js');
 const featureSettingsService = require('./app/utils/feature-settings-service.js');
 const adminSeedService = require('./app/utils/admin-seed-service.js');
+const { securityHeaders } = require('./app/utils/security-headers.js');
 // mongo db config
 const dbConfig = require('./config/database.config.js');
 
@@ -28,6 +29,8 @@ const mongoURL = process.env.MONGO_URL || dbConfig.url;
 // create express app
 const PORT = process.env.PORT || 3000;
 const app = express();
+// Don't advertise the framework.
+app.disable('x-powered-by');
 
 const corsOptionsDelegate = (req, callback) => {
   const origin = req.header('Origin');
@@ -60,6 +63,7 @@ const corsOptionsDelegate = (req, callback) => {
 };
 
 app.use(cors(corsOptionsDelegate));
+app.use(securityHeaders);
 app.use(compression());
 
 // Request instrumentation for the Prometheus endpoint. Registered before the routes so it