diff --git a/.circleci/config.yml b/.circleci/config.yml index 6e09be4c9..779bcfe93 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -29,9 +29,14 @@ jobs: - run: name: Update NPM command: sudo npm update -g npm - - run: - name: Audit dependencies - command: npm audit + # Disabled until there's capabilityies to ignore a specific vun. Related: + # https://npm.community/t/please-provide-option-to-ignore-packages-in-npm-audit/403/4 + # https://github.com/npm/cli/pull/10 + # https://github.com/npm/rfcs/pull/18 + # + # - run: + # name: Audit dependencies + # command: npm audit - run: name: Install dependencies command: npm ci diff --git a/package-lock.json b/package-lock.json index 6dde64c72..e27e4d857 100644 --- a/package-lock.json +++ b/package-lock.json @@ -6725,6 +6725,11 @@ "resolved": "https://registry.npmjs.org/content-disposition/-/content-disposition-0.5.2.tgz", "integrity": "sha1-DPaLud318r55YcOoUXjLhdunjLQ=" }, + "content-security-policy-builder": { + "version": "2.0.0", + "resolved": "https://registry.npmjs.org/content-security-policy-builder/-/content-security-policy-builder-2.0.0.tgz", + "integrity": "sha512-j+Nhmj1yfZAikJLImCvPJFE29x/UuBi+/MWqggGGc515JKaZrjuei2RhULJmy0MsstW3E3htl002bwmBNMKr7w==" + }, "content-type": { "version": "1.0.4", "resolved": "https://registry.npmjs.org/content-type/-/content-type-1.0.4.tgz", diff --git a/package.json b/package.json index 357751253..2c0fb14ed 100644 --- a/package.json +++ b/package.json @@ -53,6 +53,7 @@ "bunyan-prettystream": "^0.1.3", "cheerio": "^1.0.0-rc.2", "consolidate": "0.14.0", + "content-security-policy-builder": "^2.0.0", "convict": "^4.3.1", "cors": "^2.8.4", "dataloader": "^1.4.0", diff --git a/src/core/server/app/middleware/csp/tenant.spec.ts b/src/core/server/app/middleware/csp/tenant.spec.ts new file mode 100644 index 000000000..3ece24218 --- /dev/null +++ b/src/core/server/app/middleware/csp/tenant.spec.ts @@ -0,0 +1,95 @@ +import { generateFrameOptions } from "talk-server/app/middleware/csp/tenant"; +import { Request } from "talk-server/types/express"; + +it("denies when the tenant has no specified domains", () => { + const tenant = { domains: [] }; + const req = {} as Request; + + expect(generateFrameOptions(req, tenant)).toEqual("deny"); +}); + +it("allow-from single domain when there is one domain", () => { + const tenant = { domains: ["https://coralproject.net"] }; + const req = {} as Request; + + expect(generateFrameOptions(req, tenant)).toEqual( + "allow-from https://coralproject.net" + ); +}); + +it("deny from the domain when it does not provide and there are multiple tenants domains", () => { + const tenant = { + domains: ["https://coralproject.net", "https://news.coralproject.net"], + }; + const req = { headers: {}, query: { parentUrl: "" } } as Request; + + expect(generateFrameOptions(req, tenant)).toEqual("deny"); +}); + +it("allows from the domain when it does not provide a match and there are multiple tenants domains", () => { + const tenant = { + domains: ["https://coralproject.net", "https://news.coralproject.net"], + }; + const req = { + headers: {}, + query: { parentUrl: "https://blog.coralproject.net/a/page" }, + } as Request; + + expect(generateFrameOptions(req, tenant)).toEqual("deny"); +}); + +it("allows from the domain when it does provide a match and there are multiple tenants domains", () => { + const tenant = { + domains: ["https://coralproject.net", "https://news.coralproject.net"], + }; + const req = { + headers: {}, + query: { parentUrl: "https://news.coralproject.net/a/page" }, + } as Request; + + expect(generateFrameOptions(req, tenant)).toEqual( + "allow-from https://news.coralproject.net" + ); +}); + +it("it prefixes domains of the tenant when generating the frame option", () => { + const tenant = { + domains: ["coralproject.net", "news.coralproject.net"], + }; + const req = { + headers: {}, + query: { parentUrl: "http://news.coralproject.net/a/page" }, + } as Request; + + expect(generateFrameOptions(req, tenant)).toEqual( + "allow-from http://news.coralproject.net" + ); +}); + +it("it prefixes domains of the tenant when generating the frame option", () => { + const tenant = { + domains: ["coralproject.net", "news.coralproject.net"], + }; + const req = { + headers: {}, + query: { parentUrl: "https://news.coralproject.net/a/page" }, + } as Request; + + expect(generateFrameOptions(req, tenant)).toEqual( + "allow-from https://news.coralproject.net" + ); +}); + +it("it prefixes domains of the tenant when generating the frame option and denies based on it", () => { + const tenant = { + domains: ["coralproject.net", "news.coralproject.net"], + }; + const req = { + headers: {}, + query: { parentUrl: "http://news.coralproject.net/a/page" }, + } as Request; + + expect(generateFrameOptions(req, tenant)).toEqual( + "allow-from http://news.coralproject.net" + ); +}); diff --git a/src/core/server/app/middleware/csp/tenant.ts b/src/core/server/app/middleware/csp/tenant.ts new file mode 100644 index 000000000..2db349699 --- /dev/null +++ b/src/core/server/app/middleware/csp/tenant.ts @@ -0,0 +1,103 @@ +import builder from "content-security-policy-builder"; +import { + doesRequireSchemePrefixing, + extractParentsOrigin, + getOrigin, + isURLSecure, + prefixSchemeIfRequired, +} from "talk-server/app/url"; +import { Tenant } from "talk-server/models/tenant"; +import { Request, RequestHandler } from "talk-server/types/express"; + +/** + * cspMiddleware handles adding the CSP middleware to each outgoing request. + */ +export const cspTenantMiddleware: RequestHandler = (req, res, next) => { + const tenant = req.talk!.tenant; + if (!tenant) { + // There is no tenant for the request, don't add any headers. + return next(); + } + + res.setHeader( + "Content-Security-Policy", + generateContentSecurityPolicy(req, tenant) + ); + + // Add some fallbacks for IE. + res.setHeader("X-Frame-Options", generateFrameOptions(req, tenant)); + res.setHeader("X-XSS-Protection", "1; mode=block"); + + next(); +}; + +function generateContentSecurityPolicy( + req: Request, + tenant: Pick +) { + const directives: Record = {}; + + // Only the domains that are allowed by the tenant may embed Talk. + directives.frameAncestors = + tenant.domains.length > 0 ? tenant.domains : ["'none'"]; + + // Build the directive. + const directive = builder({ directives }); + + return directive; +} + +export function generateFrameOptions( + req: Request, + tenant: Pick +) { + // If there aren't any domains, then we reject it. + if (tenant.domains.length === 0) { + return "deny"; + } + + // If there is only one domain on the tenant, and we don't require + // prefixing, then return it! + if ( + tenant.domains.length === 1 && + !doesRequireSchemePrefixing(tenant.domains[0]) + ) { + return `allow-from ${getOrigin(tenant.domains[0])}`; + } + + // Grab the parent's hostname. + const parentsOrigin = extractParentsOrigin(req); + if (!parentsOrigin) { + return "deny"; + } + + // Grab the status if the parent url is secure. + const parentSecure = isURLSecure(parentsOrigin); + if (parentSecure === null) { + return "deny"; + } + + // If there is only one domain on the tenant, and we require prefixing, then + // return it with prefixing! + if ( + tenant.domains.length === 1 && + !doesRequireSchemePrefixing(tenant.domains[0]) + ) { + return `allow-from ${getOrigin( + prefixSchemeIfRequired(parentSecure, tenant.domains[0]) + )}`; + } + + // As we can only return a single domain in the `allow-from` directive as per + // https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/X-Frame-Options + // We need to find the domain that is asking so we can respond with the right + // result, sort of like CORS! + const allowFrom = tenant.domains + .map(domain => getOrigin(prefixSchemeIfRequired(parentSecure, domain))) + .find(origin => origin === parentsOrigin); + if (!allowFrom) { + return "deny"; + } + + return `allow-from ${allowFrom}`; +} diff --git a/src/core/server/app/middleware/tenant.ts b/src/core/server/app/middleware/tenant.ts index d04e1e097..4acc6fba0 100644 --- a/src/core/server/app/middleware/tenant.ts +++ b/src/core/server/app/middleware/tenant.ts @@ -1,17 +1,13 @@ -import { RequestHandler } from "express"; - import TenantCache from "talk-server/services/tenant/cache"; -import { Request } from "talk-server/types/express"; +import { RequestHandler } from "talk-server/types/express"; export interface MiddlewareOptions { cache: TenantCache; } -export default ({ cache }: MiddlewareOptions): RequestHandler => async ( - req: Request, - res, - next -) => { +export const tenantMiddleware = ({ + cache, +}: MiddlewareOptions): RequestHandler => async (req, res, next) => { try { // Attach the tenant to the request. const tenant = await cache.retrieveByDomain(req.hostname); diff --git a/src/core/server/app/router/api/tenant.ts b/src/core/server/app/router/api/tenant.ts index 63bbe121d..23567d770 100644 --- a/src/core/server/app/router/api/tenant.ts +++ b/src/core/server/app/router/api/tenant.ts @@ -2,7 +2,7 @@ import express from "express"; import { AppOptions } from "talk-server/app"; import { tenantInstallHandler } from "talk-server/app/handlers/api/tenant/install"; -import tenantMiddleware from "talk-server/app/middleware/tenant"; +import { tenantMiddleware } from "talk-server/app/middleware/tenant"; import { RouterOptions } from "talk-server/app/router/types"; import tenantGraphMiddleware from "talk-server/graph/tenant/middleware"; diff --git a/src/core/server/app/router/client.ts b/src/core/server/app/router/client.ts index 9b7cab8c2..067b0d4c2 100644 --- a/src/core/server/app/router/client.ts +++ b/src/core/server/app/router/client.ts @@ -28,9 +28,9 @@ export function createClientTargetRouter({ // Create a router. const router = express.Router(); - router.get("/", cacheHeadersMiddleware(cacheDuration), (req, res) => - res.render(view, { staticURI }) - ); + router.use(cacheHeadersMiddleware(cacheDuration)); + + router.get("/", (req, res) => res.render(view, { staticURI })); return router; } diff --git a/src/core/server/app/router/index.ts b/src/core/server/app/router/index.ts index 27444b3c5..3814ada32 100644 --- a/src/core/server/app/router/index.ts +++ b/src/core/server/app/router/index.ts @@ -7,6 +7,8 @@ import playground from "talk-server/app/middleware/playground"; import { RouterOptions } from "talk-server/app/router/types"; import logger from "talk-server/logger"; +import { cspTenantMiddleware } from "talk-server/app/middleware/csp/tenant"; +import { tenantMiddleware } from "talk-server/app/middleware/tenant"; import { createAPIRouter } from "./api"; import { createClientTargetRouter } from "./client"; @@ -21,16 +23,26 @@ export async function createRouter(app: AppOptions, options: RouterOptions) { attachGraphiQL(router, app); } + router.use(tenantMiddleware({ cache: app.tenantCache })); + router.use(cspTenantMiddleware); + const staticURI = app.config.get("static_uri"); // Add the embed targets. router.use( "/embed/stream", - createClientTargetRouter({ staticURI, view: "stream" }) + createClientTargetRouter({ + staticURI, + view: "stream", + }) ); router.use( "/embed/auth", - createClientTargetRouter({ staticURI, view: "auth", cacheDuration: false }) + createClientTargetRouter({ + staticURI, + view: "auth", + cacheDuration: false, + }) ); // Add the standalone targets. @@ -40,15 +52,19 @@ export async function createRouter(app: AppOptions, options: RouterOptions) { installedMiddleware({ tenantCache: app.tenantCache, }), - createClientTargetRouter({ staticURI, view: "admin", cacheDuration: false }) + createClientTargetRouter({ + staticURI, + view: "admin", + cacheDuration: false, + }) ); router.use( "/install", // If we're already installed, redirect the user to the admin page. installedMiddleware({ - tenantCache: app.tenantCache, redirectIfInstalled: true, redirectURL: "/admin", + tenantCache: app.tenantCache, }), createClientTargetRouter({ staticURI, diff --git a/src/core/server/app/url.spec.ts b/src/core/server/app/url.spec.ts new file mode 100644 index 000000000..8928b264c --- /dev/null +++ b/src/core/server/app/url.spec.ts @@ -0,0 +1,80 @@ +import { Request } from "express"; +import { + doesRequireSchemePrefixing, + extractParentsOrigin, + prefixSchemeIfRequired, +} from "talk-server/app/url"; + +it("extracts the url when the parentUrl is not provided", () => { + const req = { headers: {}, query: {} } as Request; + + expect(extractParentsOrigin(req)).toEqual(null); +}); + +it("extracts the url when the parentUrl is provided but is empty", () => { + const req = { headers: {}, query: { parentUrl: "" } } as Request; + + expect(extractParentsOrigin(req)).toEqual(null); +}); + +it("extracts the url when the parentUrl is provided", () => { + const req = { + headers: {}, + query: { parentUrl: "https://coralproject.net/" }, + } as Request; + + expect(extractParentsOrigin(req)).toEqual("https://coralproject.net"); +}); + +it("extracts the url when the referer header is provided but is empty", () => { + const req = { headers: {}, query: {} } as Request; + + req.headers.referer = ""; + + expect(extractParentsOrigin(req)).toEqual(null); +}); + +it("extracts the url when the referer header is provided", () => { + const req = { + headers: {}, + query: {}, + } as Request; + + req.headers.referer = "https://coralproject.net/"; + + expect(extractParentsOrigin(req)).toEqual("https://coralproject.net"); +}); + +it("does not do any prefixing", () => { + expect(prefixSchemeIfRequired(true, "https://coralproject.net")).toEqual( + "https://coralproject.net" + ); +}); + +it("prefixes the url with https://", () => { + expect(prefixSchemeIfRequired(true, "coralproject.net")).toEqual( + "https://coralproject.net" + ); +}); + +it("prefixes the url with http://", () => { + expect(prefixSchemeIfRequired(false, "coralproject.net")).toEqual( + "http://coralproject.net" + ); +}); + +it("prefixes the url with http://", () => { + expect(prefixSchemeIfRequired(false, "//coralproject.net")).toEqual( + "http://coralproject.net" + ); +}); + +it("determines prefixing requirements correctly", () => { + expect(doesRequireSchemePrefixing("")).toEqual(true); + expect(doesRequireSchemePrefixing("coralproject.net")).toEqual(true); + expect(doesRequireSchemePrefixing("localhost:8080")).toEqual(true); + expect(doesRequireSchemePrefixing("http://coralproject.net")).toEqual(false); + expect(doesRequireSchemePrefixing("https://coralproject.net")).toEqual(false); + expect(doesRequireSchemePrefixing("http://localhost:8080")).toEqual(false); + expect(doesRequireSchemePrefixing("https://localhost:8080")).toEqual(false); +}); diff --git a/src/core/server/app/url.ts b/src/core/server/app/url.ts index b656e4421..3654c4c46 100644 --- a/src/core/server/app/url.ts +++ b/src/core/server/app/url.ts @@ -10,3 +10,75 @@ export function reconstructURL(req: Request, path: string = "/"): string { return url.href; } + +export function doesRequireSchemePrefixing(url: string) { + return !url.startsWith("http"); +} + +export function isURLSecure(url: string) { + if (doesRequireSchemePrefixing(url)) { + return null; + } + + return url.startsWith("https://"); +} + +export function getHostname(url: string) { + try { + return new URL(url).hostname; + } catch (err) { + return null; + } +} + +export function getOrigin(url: string) { + try { + return new URL(url).origin; + } catch (err) { + return null; + } +} + +export function prefixSchemeIfRequired(secure: boolean, url: string) { + if (doesRequireSchemePrefixing(url)) { + return ( + "http" + + (secure ? "s" : "") + + (url.indexOf("//") === -1 ? "://" : ":") + + url + ); + } + + return url; +} + +/** + * extractParentsOrigin will pull the parent's origin out. + * + * @param req the request where we want to extract the parent's hostname from. + */ +export function extractParentsOrigin(req: Request) { + // The only two places this could be is in the referer header or the parentUrl + // query parameter (injected by pym.js). If both of these are empty, then we + // can't find anything. + if (!req.headers.referer && !req.query.parentUrl) { + return null; + } + + // If the referer header is defined, then use it. + if (req.headers.referer && req.headers.referer.length > 0) { + // If the header contains multiple values, return the first one. + if (Array.isArray(req.headers.referer)) { + return getOrigin(req.headers.referer[0]); + } + + return getOrigin(req.headers.referer); + } + + // If the parentUrl query parameter is provided, then try to parse it. + if (req.query.parentUrl) { + return getOrigin(req.query.parentUrl); + } + + return null; +} diff --git a/src/core/server/types/express.ts b/src/core/server/types/express.ts index 264adb165..bfab9acee 100644 --- a/src/core/server/types/express.ts +++ b/src/core/server/types/express.ts @@ -1,4 +1,4 @@ -import { Request } from "express"; +import { NextFunction, Request as ExpressRequest, Response } from "express"; import TenantContext from "talk-server/graph/tenant/context"; import { Tenant } from "talk-server/models/tenant"; @@ -15,7 +15,13 @@ export interface TalkRequest { }; } -export interface Request extends Request { +export interface Request extends ExpressRequest { talk?: TalkRequest; user?: User; } + +export type RequestHandler = ( + req: Request, + res: Response, + next: NextFunction +) => void; diff --git a/src/types/content-security-policy-builder.d.ts b/src/types/content-security-policy-builder.d.ts new file mode 100644 index 000000000..7326b560b --- /dev/null +++ b/src/types/content-security-policy-builder.d.ts @@ -0,0 +1,7 @@ +declare module "content-security-policy-builder" { + export default function builder({ + directives, + }: { + directives: Record; + }): string; +} diff --git a/src/types/fluent.d.ts b/src/types/fluent.d.ts index b53ae7ecc..8f247aaf0 100644 --- a/src/types/fluent.d.ts +++ b/src/types/fluent.d.ts @@ -54,8 +54,8 @@ declare module "fluent/compat" { protected value: any; protected opts: any; constructor(value: any, opts?: any); - valueOf(): any; - toString(bundle: FluentBundle): string; + public valueOf(): any; + public toString(bundle: FluentBundle): string; } export class FluentNumber extends FluentType {}