From 344b4d0fa0c9a12f8bd7c9f701006929b608d6e2 Mon Sep 17 00:00:00 2001 From: Brian Gough Date: Thu, 23 May 2024 15:51:27 +0100 Subject: [PATCH] Merge pull request #18088 from overleaf/ab-session-secret-rotation [web/realtime/history-v1] Support session secret rotation GitOrigin-RevId: 3c2fa27b1b3e0a8e0c9d1af2e616ce873d54aedf --- package-lock.json | 60 ++----------------- services/history-v1/app.js | 4 -- services/history-v1/package.json | 2 - services/real-time/app.js | 12 +++- .../real-time/config/settings.defaults.js | 4 +- services/real-time/config/settings.test.js | 5 ++ services/real-time/package.json | 2 +- services/web/app/src/infrastructure/Server.js | 12 +++- services/web/config/settings.defaults.js | 6 +- services/web/package.json | 2 +- .../config/settings.test.defaults.js | 2 + 11 files changed, 41 insertions(+), 70 deletions(-) diff --git a/package-lock.json b/package-lock.json index 746e24b081..ea5a75d61e 100644 --- a/package-lock.json +++ b/package-lock.json @@ -18079,6 +18079,7 @@ "version": "2.8.5", "resolved": "https://registry.npmjs.org/cors/-/cors-2.8.5.tgz", "integrity": "sha512-KIHbLJqu73RGr/hnbrO9uBeixNGuvSQjul/jdFvS/KFSIH1hWVd1ng7zOHx+YrEfInLG7q4n6GHQ9cDtxv/P6g==", + "dev": true, "dependencies": { "object-assign": "^4", "vary": "^1" @@ -42416,8 +42417,6 @@ "check-types": "^11.1.2", "command-line-args": "^3.0.3", "config": "^1.19.0", - "cookie-parser": "~1.4.5", - "cors": "^2.8.5", "express": "^4.19.2", "fs-extra": "^9.0.1", "generic-pool": "^2.1.1", @@ -43962,7 +43961,7 @@ "body-parser": "^1.19.0", "bunyan": "^1.8.15", "connect-redis": "^6.1.3", - "cookie-parser": "^1.4.5", + "cookie-parser": "^1.4.6", "express": "^4.19.2", "express-session": "^1.17.1", "joi": "^17.12.0", @@ -44457,7 +44456,7 @@ "content-disposition": "^0.5.0", "contentful": "^10.8.5", "cookie": "^0.2.3", - "cookie-parser": "1.3.5", + "cookie-parser": "1.4.6", "core-js": "^3.30.2", "crc-32": "^1.2.2", "csurf": "^1.11.0", @@ -45466,31 +45465,6 @@ "url": "https://github.com/chalk/chalk?sponsor=1" } }, - "services/web/node_modules/cookie-parser": { - "version": "1.3.5", - "resolved": "https://registry.npmjs.org/cookie-parser/-/cookie-parser-1.3.5.tgz", - "integrity": "sha1-nXVVcPtdF4kHcSJ6AjFNm+fPg1Y=", - "dependencies": { - "cookie": "0.1.3", - "cookie-signature": "1.0.6" - }, - "engines": { - "node": ">= 0.8.0" - } - }, - "services/web/node_modules/cookie-parser/node_modules/cookie": { - "version": "0.1.3", - "resolved": "https://registry.npmjs.org/cookie/-/cookie-0.1.3.tgz", - "integrity": "sha1-5zSlwUF/zkctWu+Cw4HKu2TRpDU=", - "engines": { - "node": "*" - } - }, - "services/web/node_modules/cookie-signature": { - "version": "1.0.6", - "resolved": "https://registry.npmjs.org/cookie-signature/-/cookie-signature-1.0.6.tgz", - "integrity": "sha1-4wOogrNCzD7oylE6eZmXNNqzriw=" - }, "services/web/node_modules/csv": { "version": "6.2.5", "resolved": "https://registry.npmjs.org/csv/-/csv-6.2.5.tgz", @@ -52554,7 +52528,7 @@ "chai": "^4.3.6", "chai-as-promised": "^7.1.1", "connect-redis": "^6.1.3", - "cookie-parser": "^1.4.5", + "cookie-parser": "^1.4.6", "cookie-signature": "^1.1.0", "express": "^4.19.2", "express-session": "^1.17.1", @@ -53060,7 +53034,7 @@ "content-disposition": "^0.5.0", "contentful": "^10.8.5", "cookie": "^0.2.3", - "cookie-parser": "1.3.5", + "cookie-parser": "1.4.6", "copy-webpack-plugin": "^11.0.0", "core-js": "^3.30.2", "crc-32": "^1.2.2", @@ -53732,27 +53706,6 @@ "supports-color": "^7.1.0" } }, - "cookie-parser": { - "version": "1.3.5", - "resolved": "https://registry.npmjs.org/cookie-parser/-/cookie-parser-1.3.5.tgz", - "integrity": "sha1-nXVVcPtdF4kHcSJ6AjFNm+fPg1Y=", - "requires": { - "cookie": "0.1.3", - "cookie-signature": "1.0.6" - }, - "dependencies": { - "cookie": { - "version": "0.1.3", - "resolved": "https://registry.npmjs.org/cookie/-/cookie-0.1.3.tgz", - "integrity": "sha1-5zSlwUF/zkctWu+Cw4HKu2TRpDU=" - } - } - }, - "cookie-signature": { - "version": "1.0.6", - "resolved": "https://registry.npmjs.org/cookie-signature/-/cookie-signature-1.0.6.tgz", - "integrity": "sha1-4wOogrNCzD7oylE6eZmXNNqzriw=" - }, "csv": { "version": "6.2.5", "resolved": "https://registry.npmjs.org/csv/-/csv-6.2.5.tgz", @@ -61672,6 +61625,7 @@ "version": "2.8.5", "resolved": "https://registry.npmjs.org/cors/-/cors-2.8.5.tgz", "integrity": "sha512-KIHbLJqu73RGr/hnbrO9uBeixNGuvSQjul/jdFvS/KFSIH1hWVd1ng7zOHx+YrEfInLG7q4n6GHQ9cDtxv/P6g==", + "dev": true, "requires": { "object-assign": "^4", "vary": "^1" @@ -72400,8 +72354,6 @@ "check-types": "^11.1.2", "command-line-args": "^3.0.3", "config": "^1.19.0", - "cookie-parser": "~1.4.5", - "cors": "^2.8.5", "express": "^4.19.2", "fs-extra": "^9.0.1", "generic-pool": "^2.1.1", diff --git a/services/history-v1/app.js b/services/history-v1/app.js index 2ad490fb6b..17b5937e97 100644 --- a/services/history-v1/app.js +++ b/services/history-v1/app.js @@ -9,12 +9,10 @@ const config = require('config') const Events = require('events') const BPromise = require('bluebird') const express = require('express') -const cors = require('cors') const helmet = require('helmet') const HTTPStatus = require('http-status') const logger = require('@overleaf/logger') const Metrics = require('@overleaf/metrics') -const cookieParser = require('cookie-parser') const bodyParser = require('body-parser') const swaggerTools = require('swagger-tools') const swaggerDoc = require('./api/swagger') @@ -42,8 +40,6 @@ app.use( extended: false, }) ) -app.use(cookieParser()) -app.use(cors()) security.setupSSL(app) security.setupBasicHttpAuthForSwaggerDocs(app) diff --git a/services/history-v1/package.json b/services/history-v1/package.json index 8170642f18..16e949a108 100644 --- a/services/history-v1/package.json +++ b/services/history-v1/package.json @@ -21,8 +21,6 @@ "check-types": "^11.1.2", "command-line-args": "^3.0.3", "config": "^1.19.0", - "cookie-parser": "~1.4.5", - "cors": "^2.8.5", "express": "^4.19.2", "fs-extra": "^9.0.1", "generic-pool": "^2.1.1", diff --git a/services/real-time/app.js b/services/real-time/app.js index 2373abb613..d1c043bdd8 100644 --- a/services/real-time/app.js +++ b/services/real-time/app.js @@ -56,7 +56,17 @@ const io = require('socket.io').listen(server, { // Bind to sessions const sessionStore = new RedisStore({ client: sessionRedisClient }) -const cookieParser = CookieParser(Settings.security.sessionSecret) + +if (!Settings.security.sessionSecret) { + throw new Error('No SESSION_SECRET provided.') +} + +const sessionSecrets = [ + Settings.security.sessionSecret, + Settings.security.sessionSecretUpcoming, + Settings.security.sessionSecretFallback, +].filter(Boolean) +const cookieParser = CookieParser(sessionSecrets) const sessionSockets = new SessionSockets( io, diff --git a/services/real-time/config/settings.defaults.js b/services/real-time/config/settings.defaults.js index 2065bc7799..1f4f3a9cad 100644 --- a/services/real-time/config/settings.defaults.js +++ b/services/real-time/config/settings.defaults.js @@ -105,7 +105,9 @@ const settings = { }, security: { - sessionSecret: process.env.SESSION_SECRET || 'secret-please-change', + sessionSecret: process.env.SESSION_SECRET, + sessionSecretUpcoming: process.env.SESSION_SECRET_UPCOMING, + sessionSecretFallback: process.env.SESSION_SECRET_FALLBACK, }, cookieName: process.env.COOKIE_NAME || 'overleaf.sid', diff --git a/services/real-time/config/settings.test.js b/services/real-time/config/settings.test.js index 9b426631b9..cb02982be8 100644 --- a/services/real-time/config/settings.test.js +++ b/services/real-time/config/settings.test.js @@ -2,4 +2,9 @@ module.exports = { errors: { catchUncaughtErrors: false, }, + + security: { + sessionSecret: 'static-secret-for-tests', + sessionSecretFallback: 'static-secret-fallback-for-tests', + }, } diff --git a/services/real-time/package.json b/services/real-time/package.json index d6f496bd86..838810fbe1 100644 --- a/services/real-time/package.json +++ b/services/real-time/package.json @@ -27,7 +27,7 @@ "body-parser": "^1.19.0", "bunyan": "^1.8.15", "connect-redis": "^6.1.3", - "cookie-parser": "^1.4.5", + "cookie-parser": "^1.4.6", "express": "^4.19.2", "express-session": "^1.17.1", "joi": "^17.12.0", diff --git a/services/web/app/src/infrastructure/Server.js b/services/web/app/src/infrastructure/Server.js index 4738593229..02a403f843 100644 --- a/services/web/app/src/infrastructure/Server.js +++ b/services/web/app/src/infrastructure/Server.js @@ -155,10 +155,16 @@ if (Settings.useHttpPermissionsPolicy) { RedirectManager.apply(webRouter) if (!Settings.security.sessionSecret) { - throw new Error('Session secret is not set - refusing to start server') + throw new Error('No SESSION_SECRET provided.') } -webRouter.use(cookieParser(Settings.security.sessionSecret)) +const sessionSecrets = [ + Settings.security.sessionSecret, + Settings.security.sessionSecretUpcoming, + Settings.security.sessionSecretFallback, +].filter(Boolean) + +webRouter.use(cookieParser(sessionSecrets)) SessionAutostartMiddleware.applyInitialMiddleware(webRouter) Modules.registerMiddleware(webRouter, 'sessionMiddleware', { store: sessionStore, @@ -167,7 +173,7 @@ webRouter.use( session({ resave: false, saveUninitialized: false, - secret: Settings.security.sessionSecret, + secret: sessionSecrets, proxy: Settings.behindProxy, cookie: { domain: Settings.cookieDomain, diff --git a/services/web/config/settings.defaults.js b/services/web/config/settings.defaults.js index d18a09155b..0c6b456264 100644 --- a/services/web/config/settings.defaults.js +++ b/services/web/config/settings.defaults.js @@ -43,8 +43,6 @@ if (httpAuthUser && httpAuthPass) { httpAuthUsers[httpAuthUser] = httpAuthPass } -const sessionSecret = process.env.SESSION_SECRET - const intFromEnv = function (name, defaultValue) { if ( [null, undefined].includes(defaultValue) || @@ -386,7 +384,9 @@ module.exports = { // Security // -------- security: { - sessionSecret, + sessionSecret: process.env.SESSION_SECRET, + sessionSecretUpcoming: process.env.SESSION_SECRET_UPCOMING, + sessionSecretFallback: process.env.SESSION_SECRET_FALLBACK, bcryptRounds: parseInt(process.env.BCRYPT_ROUNDS, 10) || 12, }, // number of rounds used to hash user passwords (raised to power 2) diff --git a/services/web/package.json b/services/web/package.json index 3e6a976820..bfc0f075d0 100644 --- a/services/web/package.json +++ b/services/web/package.json @@ -100,7 +100,7 @@ "content-disposition": "^0.5.0", "contentful": "^10.8.5", "cookie": "^0.2.3", - "cookie-parser": "1.3.5", + "cookie-parser": "1.4.6", "core-js": "^3.30.2", "crc-32": "^1.2.2", "csurf": "^1.11.0", diff --git a/services/web/test/acceptance/config/settings.test.defaults.js b/services/web/test/acceptance/config/settings.test.defaults.js index 7f8ed923d4..17a3361d51 100644 --- a/services/web/test/acceptance/config/settings.test.defaults.js +++ b/services/web/test/acceptance/config/settings.test.defaults.js @@ -17,6 +17,8 @@ module.exports = { secureCookie: false, security: { sessionSecret: 'static-secret-for-tests', + sessionSecretUpcoming: 'static-secret-upcoming-for-tests', + sessionSecretFallback: 'static-secret-fallback-for-tests', }, adminDomains: process.env.ADMIN_DOMAINS ? JSON.parse(process.env.ADMIN_DOMAINS)