diff --git a/app/client/ui/AdminPanel.ts b/app/client/ui/AdminPanel.ts index bfd2bd6c..91f6802b 100644 --- a/app/client/ui/AdminPanel.ts +++ b/app/client/ui/AdminPanel.ts @@ -145,6 +145,13 @@ Please log in as an administrator.`)), description: t('Current authentication method'), value: this._buildAuthenticationDisplay(owner), expandedContent: this._buildAuthenticationNotice(owner), + }), + dom.create(AdminSectionItem, { + id: 'session', + name: t('Session Secret'), + description: t('Key to sign sessions with'), + value: this._buildSessionSecretDisplay(owner), + expandedContent: this._buildSessionSecretNotice(owner), }) ]), dom.create(AdminSection, t('Version'), [ @@ -241,6 +248,27 @@ We recommend enabling one of these if Grist is accessible over the network or be to multiple people.'); } + private _buildSessionSecretDisplay(owner: IDisposableOwner) { + return dom.domComputed( + use => { + const req = this._checks.requestCheckById(use, 'session-secret'); + const result = req ? use(req.result) : undefined; + + if (result?.status === 'warning') { + return cssValueLabel(cssDangerText('default')); + } + + return cssValueLabel(cssHappyText('configured')); + } + ); + } + + private _buildSessionSecretNotice(owner: IDisposableOwner) { + return t('Grist signs user session cookies with a secret key. Please set this key via the environment variable \ +GRIST_SESSION_SECRET. Grist falls back to a hard-coded default when it is not set. We may remove this notice \ +in the future as session IDs generated since v1.1.16 are inherently cryptographically secure.'); + } + private _buildUpdates(owner: MultiHolder) { // We can be in those states: enum State { @@ -472,7 +500,11 @@ to multiple people.'); return dom.domComputed( use => [ ...use(this._checks.probes).map(probe => { - const isRedundant = probe.id === 'sandboxing'; + const isRedundant = [ + 'sandboxing', + 'authentication', + 'session-secret' + ].includes(probe.id); const show = isRedundant ? options.showRedundant : options.showNovel; if (!show) { return null; } const req = this._checks.requestCheck(probe); diff --git a/app/common/BootProbe.ts b/app/common/BootProbe.ts index 49fb9911..c73843b1 100644 --- a/app/common/BootProbe.ts +++ b/app/common/BootProbe.ts @@ -8,7 +8,8 @@ export type BootProbeIds = 'sandboxing' | 'system-user' | 'authentication' | - 'websockets' + 'websockets' | + 'session-secret' ; export interface BootProbeResult { diff --git a/app/server/lib/BootProbes.ts b/app/server/lib/BootProbes.ts index 61ac66eb..adef4811 100644 --- a/app/server/lib/BootProbes.ts +++ b/app/server/lib/BootProbes.ts @@ -6,6 +6,7 @@ import { GristServer } from 'app/server/lib/GristServer'; import * as express from 'express'; import WS from 'ws'; import fetch from 'node-fetch'; +import { DEFAULT_SESSION_SECRET } from 'app/server/lib/coreCreator'; /** * Self-diagnostics useful when installing Grist. @@ -61,6 +62,7 @@ export class BootProbes { this._probes.push(_sandboxingProbe); this._probes.push(_authenticationProbe); this._probes.push(_webSocketsProbe); + this._probes.push(_sessionSecretProbe); this._probeById = new Map(this._probes.map(p => [p.id, p])); } } @@ -284,3 +286,17 @@ const _authenticationProbe: Probe = { }; }, }; + +const _sessionSecretProbe: Probe = { + id: 'session-secret', + name: 'Session secret', + apply: async(server, req) => { + const usingDefaultSessionSecret = server.create.sessionSecret() === DEFAULT_SESSION_SECRET; + return { + status: usingDefaultSessionSecret ? 'warning' : 'success', + details: { + "GRIST_SESSION_SECRET": process.env.GRIST_SESSION_SECRET ? "set" : "not set", + } + }; + }, +}; diff --git a/app/server/lib/coreCreator.ts b/app/server/lib/coreCreator.ts index f4536c16..477c970b 100644 --- a/app/server/lib/coreCreator.ts +++ b/app/server/lib/coreCreator.ts @@ -3,11 +3,14 @@ import { checkMinIOBucket, checkMinIOExternalStorage, import { makeSimpleCreator } from 'app/server/lib/ICreate'; import { Telemetry } from 'app/server/lib/Telemetry'; +export const DEFAULT_SESSION_SECRET = + 'Phoo2ag1jaiz6Moo2Iese2xoaphahbai3oNg7diemohlah0ohtae9iengafieS2Hae7quungoCi9iaPh'; + export const makeCoreCreator = () => makeSimpleCreator({ deploymentType: 'core', // This can and should be overridden by GRIST_SESSION_SECRET // (or generated randomly per install, like grist-omnibus does). - sessionSecret: 'Phoo2ag1jaiz6Moo2Iese2xoaphahbai3oNg7diemohlah0ohtae9iengafieS2Hae7quungoCi9iaPh', + sessionSecret: DEFAULT_SESSION_SECRET, storage: [ { name: 'minio', diff --git a/app/server/lib/gristSessions.ts b/app/server/lib/gristSessions.ts index 987fae58..a5555ba1 100644 --- a/app/server/lib/gristSessions.ts +++ b/app/server/lib/gristSessions.ts @@ -6,10 +6,10 @@ import {GristServer} from 'app/server/lib/GristServer'; import {fromCallback} from 'app/server/lib/serverUtils'; import {Sessions} from 'app/server/lib/Sessions'; import {promisifyAll} from 'bluebird'; +import * as crypto from 'crypto'; import * as express from 'express'; import assignIn = require('lodash/assignIn'); import * as path from 'path'; -import * as shortUUID from "short-uuid"; export const cookieName = process.env.GRIST_SESSION_COOKIE || 'grist_sid'; @@ -118,7 +118,10 @@ export function initGristSessions(instanceRoot: string, server: GristServer) { // cookie could be stolen (with some effort) by the custom domain's owner, we limit the damage // by only honoring custom-domain cookies for requests to that domain. const generateId = (req: RequestWithOrg) => { - const uid = shortUUID.generate(); + // Generate 256 bits of cryptographically random data to use as the session ID. + // This ensures security against brute-force session hijacking even without signing the session ID. + const randomNumbers = crypto.getRandomValues(new Uint8Array(32)); + const uid = Buffer.from(randomNumbers).toString("hex"); return req.isCustomHost ? `c-${uid}@${req.org}@${req.get('host')}` : `g-${uid}`; }; const sessionSecret = server.create.sessionSecret(); diff --git a/static/locales/en.client.json b/static/locales/en.client.json index 7a8ee8df..baae4538 100644 --- a/static/locales/en.client.json +++ b/static/locales/en.client.json @@ -1541,6 +1541,7 @@ "Error": "Error", "Error checking for updates": "Error checking for updates", "Grist allows for very powerful formulas, using Python. We recommend setting the environment variable GRIST_SANDBOX_FLAVOR to gvisor if your hardware supports it (most will), to run formulas in each document within a sandbox isolated from other documents and isolated from the network.": "Grist allows for very powerful formulas, using Python. We recommend setting the environment variable GRIST_SANDBOX_FLAVOR to gvisor if your hardware supports it (most will), to run formulas in each document within a sandbox isolated from other documents and isolated from the network.", + "Grist signs user session cookies with a secret key. Please set this key via the environment variable GRIST_SESSION_SECRET. Grist falls back to a hard-coded default when it is not set. We may remove this notice in the future since session IDs have been updated to be inherently cryptographically secure.": "Grist signs user session cookies with a secret key. Please set this key via the environment variable GRIST_SESSION_SECRET. Grist falls back to a hard-coded default when it is not set. We may remove this notice in the future as session IDs generated since v1.1.16 are inherently cryptographically secure.", "Grist is up to date": "Grist is up to date", "Grist releases are at ": "Grist releases are at ", "Last checked {{time}}": "Last checked {{time}}",