From 61a85c1e75e11daffb8c3e0e49812aa70451c857 Mon Sep 17 00:00:00 2001 From: Emanuele Stoppa Date: Thu, 4 Apr 2024 17:40:27 +0100 Subject: [PATCH 01/12] feat: add origin check for CSRF protection --- packages/astro/src/@types/astro.ts | 66 +++++++++++++++++++ packages/astro/src/core/app/index.ts | 45 +++++++++++++ packages/astro/src/core/app/types.ts | 5 ++ packages/astro/src/core/build/generate.ts | 3 + .../src/core/build/plugins/plugin-manifest.ts | 3 + packages/astro/src/core/config/schema.ts | 14 ++++ .../src/vite-plugin-astro-server/plugin.ts | 3 + 7 files changed, 139 insertions(+) diff --git a/packages/astro/src/@types/astro.ts b/packages/astro/src/@types/astro.ts index e39689a25b75..eb447d90721b 100644 --- a/packages/astro/src/@types/astro.ts +++ b/packages/astro/src/@types/astro.ts @@ -1610,6 +1610,49 @@ export interface AstroUserConfig { */ legacy?: object; + /** + * @name security + * @type {object} + * @version 4.6.0 + * @type {object} + * @description + * + * It allows to opt-in various security measures for Astro applications. + */ + security?: { + /** + * @name security.csrfProtection + * @type {object} + * @default '{}' + * @version 4.6.0 + * @description + */ + + csrfProtection?: { + /** + * @name security.csrfProtection.origin + * @type {boolean} + * @default 'false' + * @version 4.6.0 + * @description + * + * Something + */ + origin?: boolean; + + /** + * @name security.csrfProtection.token + * @type {boolean} + * @default 'false' + * @version 4.6.0 + * @description + * + * Something + */ + token?: boolean | string; + }; + }; + /** * @docs * @kind heading @@ -1821,6 +1864,29 @@ export interface AstroUserConfig { * See the [Internationalization Guide](https://docs.astro.build/en/guides/internationalization/#domains-experimental) for more details, including the limitations of this experimental feature. */ i18nDomains?: boolean; + + /** + * @docs + * @name experimental.csrfProtection + * @type {boolean} + * @default `false` + * @version 4.6.0 + * @description + * + * It enables the CSRF protection for Astro websites. + * + * The CSRF protection works only for on-demand pages (SSR) and hybrid pages where prerendering is opted out. + * + * ```js + * // astro.config.mjs + * export default defineConfig({ + * experimental: { + * csrfProtection: true, + * }, + * }) + * ``` + */ + csrfProtection?: boolean; }; } diff --git a/packages/astro/src/core/app/index.ts b/packages/astro/src/core/app/index.ts index 7c1480bd7662..aced1dfb77bd 100644 --- a/packages/astro/src/core/app/index.ts +++ b/packages/astro/src/core/app/index.ts @@ -1,6 +1,7 @@ import type { ComponentInstance, ManifestData, + MiddlewareHandler, RouteData, SSRManifest, } from '../../@types/astro.js'; @@ -31,6 +32,7 @@ import { createAssetLink } from '../render/ssr-element.js'; import { ensure404Route } from '../routing/astro-designed-error-pages.js'; import { matchRoute } from '../routing/match.js'; import { AppPipeline } from './pipeline.js'; +import { defineMiddleware, sequence } from '../middleware/index.js'; export { deserializeManifest } from './common.js'; export interface RenderOptions { @@ -112,6 +114,13 @@ export class App { * @private */ #createPipeline(streaming = false) { + if (this.#manifest.csrfProtection) { + this.#manifest.middleware = sequence( + this.#createOriginCheckMiddleware(), + this.#manifest.middleware + ); + } + return AppPipeline.create({ logger: this.#logger, manifest: this.#manifest, @@ -137,6 +146,42 @@ export class App { }); } + /** + * Content types that can be passed when sending a request via a form + * + * https://developer.mozilla.org/en-US/docs/Web/API/HTMLFormElement/enctype + * @private + */ + #formContentTypes = ['application/x-www-form-urlencoded', 'multipart/form-data', 'text/plain']; + + /** + * Returns a middleware function in charge to check the `origin` header. + * + * @private + */ + #createOriginCheckMiddleware(): MiddlewareHandler { + return defineMiddleware((context, next) => { + const { request, url } = context; + const contentType = request.headers.get('content-type'); + if (contentType) { + if (this.#formContentTypes.includes(contentType)) { + const forbidden = + (request.method === 'POST' || + request.method === 'PUT' || + request.method === 'PATCH' || + request.method === 'DELETE') && + request.headers.get('origin') !== url.origin; + if (forbidden) { + return new Response(`Cross-site ${request.method} form submissions are forbidden`, { + status: 403, + }); + } + } + } + return next(); + }); + } + set setManifestData(newManifestData: ManifestData) { this.#manifestData = newManifestData; } diff --git a/packages/astro/src/core/app/types.ts b/packages/astro/src/core/app/types.ts index 2596ab3a69f2..f36d39e81112 100644 --- a/packages/astro/src/core/app/types.ts +++ b/packages/astro/src/core/app/types.ts @@ -64,6 +64,7 @@ export type SSRManifest = { pageMap?: Map; i18n: SSRManifestI18n | undefined; middleware: MiddlewareHandler; + csrfProtection: SSRCsrfProtection | undefined; }; export type SSRManifestI18n = { @@ -74,6 +75,10 @@ export type SSRManifestI18n = { domainLookupTable: Record; }; +export type SSRCsrfProtection = { + origin: boolean; +}; + export type SerializedSSRManifest = Omit< SSRManifest, 'middleware' | 'routes' | 'assets' | 'componentMetadata' | 'inlinedScripts' | 'clientDirectives' diff --git a/packages/astro/src/core/build/generate.ts b/packages/astro/src/core/build/generate.ts index 7dc00073fc5b..aaff8924f5fd 100644 --- a/packages/astro/src/core/build/generate.ts +++ b/packages/astro/src/core/build/generate.ts @@ -615,5 +615,8 @@ function createBuildManifest( i18n: i18nManifest, buildFormat: settings.config.build.format, middleware, + csrfProtection: settings.config.experimental.csrfProtection + ? settings.config.security?.csrfProtection + : undefined, }; } diff --git a/packages/astro/src/core/build/plugins/plugin-manifest.ts b/packages/astro/src/core/build/plugins/plugin-manifest.ts index 24437d4e5765..5e23f42f8cb8 100644 --- a/packages/astro/src/core/build/plugins/plugin-manifest.ts +++ b/packages/astro/src/core/build/plugins/plugin-manifest.ts @@ -276,5 +276,8 @@ function buildManifest( assets: staticFiles.map(prefixAssetPath), i18n: i18nManifest, buildFormat: settings.config.build.format, + csrfProtection: settings.config.experimental.csrfProtection + ? settings.config.security?.csrfProtection + : undefined, }; } diff --git a/packages/astro/src/core/config/schema.ts b/packages/astro/src/core/config/schema.ts index ef1a6ec85d67..5d67073bf82c 100644 --- a/packages/astro/src/core/config/schema.ts +++ b/packages/astro/src/core/config/schema.ts @@ -86,6 +86,7 @@ const ASTRO_CONFIG_DEFAULTS = { clientPrerender: false, globalRoutePriority: false, i18nDomains: false, + csrfProtection: false, }, } satisfies AstroUserConfig & { server: { open: boolean } }; @@ -139,6 +140,15 @@ export const AstroConfigSchema = z.object({ .array(z.object({ name: z.string(), hooks: z.object({}).passthrough().default({}) })) .default(ASTRO_CONFIG_DEFAULTS.integrations) ), + security: z + .object({ + csrfProtection: z + .object({ + origin: z.boolean().default(false), + }) + .optional(), + }) + .optional(), build: z .object({ format: z @@ -508,6 +518,10 @@ export const AstroConfigSchema = z.object({ .boolean() .optional() .default(ASTRO_CONFIG_DEFAULTS.experimental.globalRoutePriority), + csrfProtection: z + .boolean() + .optional() + .default(ASTRO_CONFIG_DEFAULTS.experimental.csrfProtection), i18nDomains: z.boolean().optional().default(ASTRO_CONFIG_DEFAULTS.experimental.i18nDomains), }) .strict( diff --git a/packages/astro/src/vite-plugin-astro-server/plugin.ts b/packages/astro/src/vite-plugin-astro-server/plugin.ts index b08bcb4ebd57..8484483a3f05 100644 --- a/packages/astro/src/vite-plugin-astro-server/plugin.ts +++ b/packages/astro/src/vite-plugin-astro-server/plugin.ts @@ -143,6 +143,9 @@ export function createDevelopmentManifest(settings: AstroSettings): SSRManifest componentMetadata: new Map(), inlinedScripts: new Map(), i18n: i18nManifest, + csrfProtection: settings.config.experimental.csrfProtection + ? settings.config.security?.csrfProtection + : undefined, middleware(_, next) { return next(); }, From 01b39bedc9947080923b8786d5db886fcb5b869e Mon Sep 17 00:00:00 2001 From: Emanuele Stoppa Date: Fri, 5 Apr 2024 11:36:51 +0100 Subject: [PATCH 02/12] add tests --- packages/astro/src/core/app/index.ts | 3 +- packages/astro/test/csrf-protection.test.js | 188 ++++++++++++++++++ .../csrf-check-origin/astro.config.mjs | 15 ++ .../fixtures/csrf-check-origin/package.json | 8 + .../csrf-check-origin/src/pages/api.ts | 29 +++ 5 files changed, 242 insertions(+), 1 deletion(-) create mode 100644 packages/astro/test/csrf-protection.test.js create mode 100644 packages/astro/test/fixtures/csrf-check-origin/astro.config.mjs create mode 100644 packages/astro/test/fixtures/csrf-check-origin/package.json create mode 100644 packages/astro/test/fixtures/csrf-check-origin/src/pages/api.ts diff --git a/packages/astro/src/core/app/index.ts b/packages/astro/src/core/app/index.ts index aced1dfb77bd..b96a8f7c84fa 100644 --- a/packages/astro/src/core/app/index.ts +++ b/packages/astro/src/core/app/index.ts @@ -114,7 +114,8 @@ export class App { * @private */ #createPipeline(streaming = false) { - if (this.#manifest.csrfProtection) { + console.log(this.#manifest); + if (this.#manifest.csrfProtection?.origin === true) { this.#manifest.middleware = sequence( this.#createOriginCheckMiddleware(), this.#manifest.middleware diff --git a/packages/astro/test/csrf-protection.test.js b/packages/astro/test/csrf-protection.test.js new file mode 100644 index 000000000000..d5c5577e37ae --- /dev/null +++ b/packages/astro/test/csrf-protection.test.js @@ -0,0 +1,188 @@ +import { before, describe, it } from 'node:test'; +import { loadFixture } from './test-utils.js'; +import testAdapter from './test-adapter.js'; +import assert from 'node:assert/strict'; + +describe('CSRF origin check', () => { + let app; + + before(async () => { + const fixture = await loadFixture({ + root: './fixtures/csrf-check-origin/', + adapter: testAdapter(), + }); + await fixture.build(); + app = await fixture.loadTestAdapterApp(); + }); + + it("return 403 when the origin doesn't match and calling a POST", async () => { + let request; + let response; + request = new Request('http://example.com/api/', { + headers: { origin: 'http://loreum.com', 'content-type': 'multipart/form-data' }, + method: 'POST', + }); + response = await app.render(request); + assert.equal(response.status, 403); + + request = new Request('http://example.com/api/', { + headers: { origin: 'http://loreum.com', 'content-type': 'application/x-www-form-urlencoded' }, + method: 'POST', + }); + response = await app.render(request); + assert.equal(response.status, 403); + + request = new Request('http://example.com/api/', { + headers: { origin: 'http://loreum.com', 'content-type': 'text/plain' }, + method: 'POST', + }); + response = await app.render(request); + assert.equal(response.status, 403); + }); + + it("return 403 when the origin doesn't match and calling a PUT", async () => { + let request; + let response; + request = new Request('http://example.com/api/', { + headers: { origin: 'http://loreum.com', 'content-type': 'multipart/form-data' }, + method: 'PUT', + }); + response = await app.render(request); + assert.equal(response.status, 403); + + request = new Request('http://example.com/api/', { + headers: { origin: 'http://loreum.com', 'content-type': 'application/x-www-form-urlencoded' }, + method: 'PUT', + }); + response = await app.render(request); + assert.equal(response.status, 403); + + request = new Request('http://example.com/api/', { + headers: { origin: 'http://loreum.com', 'content-type': 'text/plain' }, + method: 'PUT', + }); + response = await app.render(request); + assert.equal(response.status, 403); + }); + + it("return 403 when the origin doesn't match and calling a DELETE", async () => { + let request; + let response; + request = new Request('http://example.com/api/', { + headers: { origin: 'http://loreum.com', 'content-type': 'multipart/form-data' }, + method: 'DELETE', + }); + response = await app.render(request); + assert.equal(response.status, 403); + + request = new Request('http://example.com/api/', { + headers: { origin: 'http://loreum.com', 'content-type': 'application/x-www-form-urlencoded' }, + method: 'DELETE', + }); + response = await app.render(request); + assert.equal(response.status, 403); + + request = new Request('http://example.com/api/', { + headers: { origin: 'http://loreum.com', 'content-type': 'text/plain' }, + method: 'DELETE', + }); + response = await app.render(request); + assert.equal(response.status, 403); + }); + + it("return 403 when the origin doesn't match and calling a PATCH", async () => { + let request; + let response; + request = new Request('http://example.com/api/', { + headers: { origin: 'http://loreum.com', 'content-type': 'multipart/form-data' }, + method: 'PATCH', + }); + response = await app.render(request); + assert.equal(response.status, 403); + + request = new Request('http://example.com/api/', { + headers: { origin: 'http://loreum.com', 'content-type': 'application/x-www-form-urlencoded' }, + method: 'PATCH', + }); + response = await app.render(request); + assert.equal(response.status, 403); + + request = new Request('http://example.com/api/', { + headers: { origin: 'http://loreum.com', 'content-type': 'text/plain' }, + method: 'PATCH', + }); + response = await app.render(request); + assert.equal(response.status, 403); + }); + + it("return a 200 when the origin doesn't match but calling a GET", async () => { + let request; + let response; + request = new Request('http://example.com/api/', { + headers: { origin: 'http://loreum.com', 'content-type': 'multipart/form-data' }, + method: 'GET', + }); + response = await app.render(request); + assert.equal(response.status, 200); + assert.deepEqual(await response.json(), { + something: 'true', + }); + + request = new Request('http://example.com/api/', { + headers: { origin: 'http://loreum.com', 'content-type': 'application/x-www-form-urlencoded' }, + method: 'GET', + }); + response = await app.render(request); + assert.equal(response.status, 200); + assert.deepEqual(await response.json(), { + something: 'true', + }); + + request = new Request('http://example.com/api/', { + headers: { origin: 'http://loreum.com', 'content-type': 'text/plain' }, + method: 'GET', + }); + response = await app.render(request); + assert.equal(response.status, 200); + assert.deepEqual(await response.json(), { + something: 'true', + }); + }); + + it('return 200 when calling POST/PUT/DELETE/PATCH with the correct origin', async () => { + let request; + let response; + request = new Request('http://example.com/api/', { + headers: { origin: 'http://example.com', 'content-type': 'multipart/form-data' }, + method: 'POST', + }); + response = await app.render(request); + assert.equal(response.status, 200); + assert.deepEqual(await response.json(), { + something: 'true', + }); + + request = new Request('http://example.com/api/', { + headers: { + origin: 'http://example.com', + 'content-type': 'application/x-www-form-urlencoded', + }, + method: 'PUT', + }); + response = await app.render(request); + assert.equal(response.status, 200); + assert.deepEqual(await response.json(), { + something: 'true', + }); + + request = new Request('http://example.com/api/', { + headers: { origin: 'http://example.com', 'content-type': 'text/plain' }, + method: 'PATCH', + }); + response = await app.render(request); + assert.equal(response.status, 200); + assert.deepEqual(await response.json(), { + something: 'true', + }); + }); +}); diff --git a/packages/astro/test/fixtures/csrf-check-origin/astro.config.mjs b/packages/astro/test/fixtures/csrf-check-origin/astro.config.mjs new file mode 100644 index 000000000000..62f024593cac --- /dev/null +++ b/packages/astro/test/fixtures/csrf-check-origin/astro.config.mjs @@ -0,0 +1,15 @@ +import { defineConfig } from 'astro/config'; + +// https://astro.build/config +export default defineConfig({ + output: "server", + security: { + csrfProtection: { + origin: true + } + }, + experimental: { + csrfProtection: true + } +}); + diff --git a/packages/astro/test/fixtures/csrf-check-origin/package.json b/packages/astro/test/fixtures/csrf-check-origin/package.json new file mode 100644 index 000000000000..1573627d8d4e --- /dev/null +++ b/packages/astro/test/fixtures/csrf-check-origin/package.json @@ -0,0 +1,8 @@ +{ + "name": "@test/csrf", + "version": "0.0.0", + "private": true, + "dependencies": { + "astro": "workspace:*" + } +} diff --git a/packages/astro/test/fixtures/csrf-check-origin/src/pages/api.ts b/packages/astro/test/fixtures/csrf-check-origin/src/pages/api.ts new file mode 100644 index 000000000000..8aa35cc25859 --- /dev/null +++ b/packages/astro/test/fixtures/csrf-check-origin/src/pages/api.ts @@ -0,0 +1,29 @@ +export const GET = () => { + return Response.json({ + something: 'true', + }); +}; + +export const POST = () => { + return Response.json({ + something: 'true', + }); +}; + +export const PUT = () => { + return Response.json({ + something: 'true', + }); +}; + +export const DELETE = () => { + return Response.json({ + something: 'true', + }); +}; + +export const PATCH = () => { + return Response.json({ + something: 'true', + }); +}; From 6e55a5958305c5163ae93b56afde95626a1f57c1 Mon Sep 17 00:00:00 2001 From: Emanuele Stoppa Date: Mon, 8 Apr 2024 10:33:51 +0100 Subject: [PATCH 03/12] chore: documentation --- packages/astro/src/@types/astro.ts | 22 +++++++++------------- packages/astro/src/core/app/index.ts | 1 - pnpm-lock.yaml | 6 ++++++ 3 files changed, 15 insertions(+), 14 deletions(-) diff --git a/packages/astro/src/@types/astro.ts b/packages/astro/src/@types/astro.ts index eb447d90721b..672a30a62bdc 100644 --- a/packages/astro/src/@types/astro.ts +++ b/packages/astro/src/@types/astro.ts @@ -1617,7 +1617,7 @@ export interface AstroUserConfig { * @type {object} * @description * - * It allows to opt-in various security measures for Astro applications. + * It allows to opt-in various security features. */ security?: { /** @@ -1626,6 +1626,8 @@ export interface AstroUserConfig { * @default '{}' * @version 4.6.0 * @description + * + * It enables some security measures to prevent CSRF attacks: https://owasp.org/www-community/attacks/csrf */ csrfProtection?: { @@ -1636,20 +1638,14 @@ export interface AstroUserConfig { * @version 4.6.0 * @description * - * Something - */ - origin?: boolean; - - /** - * @name security.csrfProtection.token - * @type {boolean} - * @default 'false' - * @version 4.6.0 - * @description + * When enabled, it enables a strict check on the "origin" header. This is header it that is passed by the browsers on each request. * - * Something + * The "origin" check is executed only on-demand pages, and only for the requests `POST, `PATCH`, `DELETE` and `PUT`, only for those requests that + * the followin `content-type` header: 'application/x-www-form-urlencoded', 'multipart/form-data', 'text/plain'. + * + * It the "origin" header doesn't match the pathname of the request, Astro will return a 403 status code and won't render the page. */ - token?: boolean | string; + origin?: boolean; }; }; diff --git a/packages/astro/src/core/app/index.ts b/packages/astro/src/core/app/index.ts index b96a8f7c84fa..3cc55ae099d9 100644 --- a/packages/astro/src/core/app/index.ts +++ b/packages/astro/src/core/app/index.ts @@ -114,7 +114,6 @@ export class App { * @private */ #createPipeline(streaming = false) { - console.log(this.#manifest); if (this.#manifest.csrfProtection?.origin === true) { this.#manifest.middleware = sequence( this.#createOriginCheckMiddleware(), diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index b97f52944eeb..dc75d4c94635 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -2559,6 +2559,12 @@ importers: specifier: workspace:* version: link:../../.. + packages/astro/test/fixtures/csrf-check-origin: + dependencies: + astro: + specifier: workspace:* + version: link:../../.. + packages/astro/test/fixtures/css-assets: dependencies: '@test/astro-font-awesome-package': From 90c4284d7c2920c8de8049efdbb35d7208d4e18b Mon Sep 17 00:00:00 2001 From: Emanuele Stoppa Date: Mon, 8 Apr 2024 10:47:10 +0100 Subject: [PATCH 04/12] changeset and grammar --- .changeset/fair-jars-behave.md | 26 ++++++++++++++++++++++++++ packages/astro/src/@types/astro.ts | 2 +- 2 files changed, 27 insertions(+), 1 deletion(-) create mode 100644 .changeset/fair-jars-behave.md diff --git a/.changeset/fair-jars-behave.md b/.changeset/fair-jars-behave.md new file mode 100644 index 000000000000..d453d3141b53 --- /dev/null +++ b/.changeset/fair-jars-behave.md @@ -0,0 +1,26 @@ +--- +"astro": minor +--- + +Adds a new security - and experimental - option to prevent CSRF attacks. This feature is available only for on-demand pages: + +```js +import { defineConfig } from "astro/config" +export default defineConfig({ + security: { + csrfProtection: { + origin: true + } + }, + experimental: { + csrfProtection: true + } +}) +``` + +When enabled, it checks that the "origin" header, automatically passed by all modern browsers, matches the URL sent by each `Request`. + +The "origin" check is executed only on-demand pages, and only for the requests `POST, `PATCH`, `DELETE` and `PUT`, only for those requests that +the followin `content-type` header: 'application/x-www-form-urlencoded', 'multipart/form-data', 'text/plain'. + +It the "origin" header doesn't match the pathname of the request, Astro will return a 403 status code and won't render the page. diff --git a/packages/astro/src/@types/astro.ts b/packages/astro/src/@types/astro.ts index 672a30a62bdc..2a37c04db3eb 100644 --- a/packages/astro/src/@types/astro.ts +++ b/packages/astro/src/@types/astro.ts @@ -1638,7 +1638,7 @@ export interface AstroUserConfig { * @version 4.6.0 * @description * - * When enabled, it enables a strict check on the "origin" header. This is header it that is passed by the browsers on each request. + * When enabled, it checks that the "origin" header, automatically passed by all modern browsers, matches the URL sent by each `Request`. * * The "origin" check is executed only on-demand pages, and only for the requests `POST, `PATCH`, `DELETE` and `PUT`, only for those requests that * the followin `content-type` header: 'application/x-www-form-urlencoded', 'multipart/form-data', 'text/plain'. From d052811c5fa503f7c66f994d793582cf396d6730 Mon Sep 17 00:00:00 2001 From: Emanuele Stoppa Date: Mon, 8 Apr 2024 16:19:00 +0100 Subject: [PATCH 05/12] chore: add casing check --- packages/astro/src/core/app/index.ts | 2 +- packages/astro/test/csrf-protection.test.js | 8 ++++++++ 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/packages/astro/src/core/app/index.ts b/packages/astro/src/core/app/index.ts index 3cc55ae099d9..751680f9aba6 100644 --- a/packages/astro/src/core/app/index.ts +++ b/packages/astro/src/core/app/index.ts @@ -164,7 +164,7 @@ export class App { const { request, url } = context; const contentType = request.headers.get('content-type'); if (contentType) { - if (this.#formContentTypes.includes(contentType)) { + if (this.#formContentTypes.includes(contentType.toLowerCase())) { const forbidden = (request.method === 'POST' || request.method === 'PUT' || diff --git a/packages/astro/test/csrf-protection.test.js b/packages/astro/test/csrf-protection.test.js index d5c5577e37ae..ab76a18f5b81 100644 --- a/packages/astro/test/csrf-protection.test.js +++ b/packages/astro/test/csrf-protection.test.js @@ -25,6 +25,14 @@ describe('CSRF origin check', () => { response = await app.render(request); assert.equal(response.status, 403); + // case where content-type has different casing + request = new Request('http://example.com/api/', { + headers: { origin: 'http://loreum.com', 'content-type': 'MULTIPART/FORM-DATA' }, + method: 'POST', + }); + response = await app.render(request); + assert.equal(response.status, 403); + request = new Request('http://example.com/api/', { headers: { origin: 'http://loreum.com', 'content-type': 'application/x-www-form-urlencoded' }, method: 'POST', From d482c8d16c5e86013bc058a442e5e48a34766040 Mon Sep 17 00:00:00 2001 From: Emanuele Stoppa Date: Mon, 8 Apr 2024 17:37:45 +0100 Subject: [PATCH 06/12] split function --- packages/astro/src/core/app/index.ts | 42 ++-------------------- packages/astro/src/core/app/middlewares.ts | 42 ++++++++++++++++++++++ 2 files changed, 45 insertions(+), 39 deletions(-) create mode 100644 packages/astro/src/core/app/middlewares.ts diff --git a/packages/astro/src/core/app/index.ts b/packages/astro/src/core/app/index.ts index 751680f9aba6..5e7a0dae2f43 100644 --- a/packages/astro/src/core/app/index.ts +++ b/packages/astro/src/core/app/index.ts @@ -1,7 +1,6 @@ import type { ComponentInstance, ManifestData, - MiddlewareHandler, RouteData, SSRManifest, } from '../../@types/astro.js'; @@ -32,7 +31,8 @@ import { createAssetLink } from '../render/ssr-element.js'; import { ensure404Route } from '../routing/astro-designed-error-pages.js'; import { matchRoute } from '../routing/match.js'; import { AppPipeline } from './pipeline.js'; -import { defineMiddleware, sequence } from '../middleware/index.js'; +import { sequence } from '../middleware/index.js'; +import { createOriginCheckMiddleware } from './middlewares.js'; export { deserializeManifest } from './common.js'; export interface RenderOptions { @@ -116,7 +116,7 @@ export class App { #createPipeline(streaming = false) { if (this.#manifest.csrfProtection?.origin === true) { this.#manifest.middleware = sequence( - this.#createOriginCheckMiddleware(), + createOriginCheckMiddleware(), this.#manifest.middleware ); } @@ -146,42 +146,6 @@ export class App { }); } - /** - * Content types that can be passed when sending a request via a form - * - * https://developer.mozilla.org/en-US/docs/Web/API/HTMLFormElement/enctype - * @private - */ - #formContentTypes = ['application/x-www-form-urlencoded', 'multipart/form-data', 'text/plain']; - - /** - * Returns a middleware function in charge to check the `origin` header. - * - * @private - */ - #createOriginCheckMiddleware(): MiddlewareHandler { - return defineMiddleware((context, next) => { - const { request, url } = context; - const contentType = request.headers.get('content-type'); - if (contentType) { - if (this.#formContentTypes.includes(contentType.toLowerCase())) { - const forbidden = - (request.method === 'POST' || - request.method === 'PUT' || - request.method === 'PATCH' || - request.method === 'DELETE') && - request.headers.get('origin') !== url.origin; - if (forbidden) { - return new Response(`Cross-site ${request.method} form submissions are forbidden`, { - status: 403, - }); - } - } - } - return next(); - }); - } - set setManifestData(newManifestData: ManifestData) { this.#manifestData = newManifestData; } diff --git a/packages/astro/src/core/app/middlewares.ts b/packages/astro/src/core/app/middlewares.ts new file mode 100644 index 000000000000..095158b42ba3 --- /dev/null +++ b/packages/astro/src/core/app/middlewares.ts @@ -0,0 +1,42 @@ +import type { MiddlewareHandler } from '../../@types/astro.js'; +import { defineMiddleware } from '../middleware/index.js'; + +/** + * Content types that can be passed when sending a request via a form + * + * https://developer.mozilla.org/en-US/docs/Web/API/HTMLFormElement/enctype + * @private + */ +const FORM_CONTENT_TYPES = [ + 'application/x-www-form-urlencoded', + 'multipart/form-data', + 'text/plain', +]; + +/** + * Returns a middleware function in charge to check the `origin` header. + * + * @private + */ +export function createOriginCheckMiddleware(): MiddlewareHandler { + return defineMiddleware((context, next) => { + const { request, url } = context; + const contentType = request.headers.get('content-type'); + if (contentType) { + if (FORM_CONTENT_TYPES.includes(contentType.toLowerCase())) { + const forbidden = + (request.method === 'POST' || + request.method === 'PUT' || + request.method === 'PATCH' || + request.method === 'DELETE') && + request.headers.get('origin') !== url.origin; + if (forbidden) { + return new Response(`Cross-site ${request.method} form submissions are forbidden`, { + status: 403, + }); + } + } + } + return next(); + }); +} From 1239432faf5f4ca8fd70896c0ff91bceaa20f9db Mon Sep 17 00:00:00 2001 From: Emanuele Stoppa Date: Tue, 9 Apr 2024 13:32:04 +0100 Subject: [PATCH 07/12] better naming --- packages/astro/src/@types/astro.ts | 4 ++-- packages/astro/src/core/app/index.ts | 2 +- packages/astro/src/core/app/types.ts | 2 +- packages/astro/src/core/build/generate.ts | 6 +++--- packages/astro/src/core/build/plugins/plugin-manifest.ts | 6 +++--- packages/astro/src/core/config/schema.ts | 7 ++----- packages/astro/src/vite-plugin-astro-server/plugin.ts | 6 +++--- 7 files changed, 15 insertions(+), 18 deletions(-) diff --git a/packages/astro/src/@types/astro.ts b/packages/astro/src/@types/astro.ts index 2a37c04db3eb..87f87b9d65bf 100644 --- a/packages/astro/src/@types/astro.ts +++ b/packages/astro/src/@types/astro.ts @@ -1877,12 +1877,12 @@ export interface AstroUserConfig { * // astro.config.mjs * export default defineConfig({ * experimental: { - * csrfProtection: true, + * security: true, * }, * }) * ``` */ - csrfProtection?: boolean; + security?: boolean; }; } diff --git a/packages/astro/src/core/app/index.ts b/packages/astro/src/core/app/index.ts index 5e7a0dae2f43..cb7a8d9db188 100644 --- a/packages/astro/src/core/app/index.ts +++ b/packages/astro/src/core/app/index.ts @@ -114,7 +114,7 @@ export class App { * @private */ #createPipeline(streaming = false) { - if (this.#manifest.csrfProtection?.origin === true) { + if (this.#manifest.checkOrigin) { this.#manifest.middleware = sequence( createOriginCheckMiddleware(), this.#manifest.middleware diff --git a/packages/astro/src/core/app/types.ts b/packages/astro/src/core/app/types.ts index f36d39e81112..415eb475a82f 100644 --- a/packages/astro/src/core/app/types.ts +++ b/packages/astro/src/core/app/types.ts @@ -64,7 +64,7 @@ export type SSRManifest = { pageMap?: Map; i18n: SSRManifestI18n | undefined; middleware: MiddlewareHandler; - csrfProtection: SSRCsrfProtection | undefined; + checkOrigin: boolean; }; export type SSRManifestI18n = { diff --git a/packages/astro/src/core/build/generate.ts b/packages/astro/src/core/build/generate.ts index aaff8924f5fd..979d1bbb1915 100644 --- a/packages/astro/src/core/build/generate.ts +++ b/packages/astro/src/core/build/generate.ts @@ -615,8 +615,8 @@ function createBuildManifest( i18n: i18nManifest, buildFormat: settings.config.build.format, middleware, - csrfProtection: settings.config.experimental.csrfProtection - ? settings.config.security?.csrfProtection - : undefined, + checkOrigin: settings.config.experimental.security + ? settings.config.security?.csrfProtection?.origin ?? false + : false, }; } diff --git a/packages/astro/src/core/build/plugins/plugin-manifest.ts b/packages/astro/src/core/build/plugins/plugin-manifest.ts index 5e23f42f8cb8..5d2d08eff9dd 100644 --- a/packages/astro/src/core/build/plugins/plugin-manifest.ts +++ b/packages/astro/src/core/build/plugins/plugin-manifest.ts @@ -276,8 +276,8 @@ function buildManifest( assets: staticFiles.map(prefixAssetPath), i18n: i18nManifest, buildFormat: settings.config.build.format, - csrfProtection: settings.config.experimental.csrfProtection - ? settings.config.security?.csrfProtection - : undefined, + checkOrigin: settings.config.experimental.security + ? settings.config.security?.csrfProtection?.origin ?? false + : false, }; } diff --git a/packages/astro/src/core/config/schema.ts b/packages/astro/src/core/config/schema.ts index 5d67073bf82c..d97c22540c2b 100644 --- a/packages/astro/src/core/config/schema.ts +++ b/packages/astro/src/core/config/schema.ts @@ -86,7 +86,7 @@ const ASTRO_CONFIG_DEFAULTS = { clientPrerender: false, globalRoutePriority: false, i18nDomains: false, - csrfProtection: false, + security: false, }, } satisfies AstroUserConfig & { server: { open: boolean } }; @@ -518,10 +518,7 @@ export const AstroConfigSchema = z.object({ .boolean() .optional() .default(ASTRO_CONFIG_DEFAULTS.experimental.globalRoutePriority), - csrfProtection: z - .boolean() - .optional() - .default(ASTRO_CONFIG_DEFAULTS.experimental.csrfProtection), + security: z.boolean().optional().default(ASTRO_CONFIG_DEFAULTS.experimental.security), i18nDomains: z.boolean().optional().default(ASTRO_CONFIG_DEFAULTS.experimental.i18nDomains), }) .strict( diff --git a/packages/astro/src/vite-plugin-astro-server/plugin.ts b/packages/astro/src/vite-plugin-astro-server/plugin.ts index 8484483a3f05..c1eca9bd484b 100644 --- a/packages/astro/src/vite-plugin-astro-server/plugin.ts +++ b/packages/astro/src/vite-plugin-astro-server/plugin.ts @@ -143,9 +143,9 @@ export function createDevelopmentManifest(settings: AstroSettings): SSRManifest componentMetadata: new Map(), inlinedScripts: new Map(), i18n: i18nManifest, - csrfProtection: settings.config.experimental.csrfProtection - ? settings.config.security?.csrfProtection - : undefined, + checkOrigin: settings.config.experimental.security + ? settings.config.security?.csrfProtection?.origin ?? false + : false, middleware(_, next) { return next(); }, From f5e101b8a51325f1258620b149af76dd79807431 Mon Sep 17 00:00:00 2001 From: Emanuele Stoppa Date: Tue, 9 Apr 2024 13:42:38 +0100 Subject: [PATCH 08/12] make the whole object experimental --- packages/astro/src/@types/astro.ts | 80 +++++++++---------- packages/astro/src/core/build/generate.ts | 4 +- .../src/core/build/plugins/plugin-manifest.ts | 4 +- packages/astro/src/core/config/schema.ts | 23 +++--- .../src/vite-plugin-astro-server/plugin.ts | 4 +- .../csrf-check-origin/astro.config.mjs | 11 ++- 6 files changed, 57 insertions(+), 69 deletions(-) diff --git a/packages/astro/src/@types/astro.ts b/packages/astro/src/@types/astro.ts index 87f87b9d65bf..5ac22025ab7d 100644 --- a/packages/astro/src/@types/astro.ts +++ b/packages/astro/src/@types/astro.ts @@ -1610,45 +1610,6 @@ export interface AstroUserConfig { */ legacy?: object; - /** - * @name security - * @type {object} - * @version 4.6.0 - * @type {object} - * @description - * - * It allows to opt-in various security features. - */ - security?: { - /** - * @name security.csrfProtection - * @type {object} - * @default '{}' - * @version 4.6.0 - * @description - * - * It enables some security measures to prevent CSRF attacks: https://owasp.org/www-community/attacks/csrf - */ - - csrfProtection?: { - /** - * @name security.csrfProtection.origin - * @type {boolean} - * @default 'false' - * @version 4.6.0 - * @description - * - * When enabled, it checks that the "origin" header, automatically passed by all modern browsers, matches the URL sent by each `Request`. - * - * The "origin" check is executed only on-demand pages, and only for the requests `POST, `PATCH`, `DELETE` and `PUT`, only for those requests that - * the followin `content-type` header: 'application/x-www-form-urlencoded', 'multipart/form-data', 'text/plain'. - * - * It the "origin" header doesn't match the pathname of the request, Astro will return a 403 status code and won't render the page. - */ - origin?: boolean; - }; - }; - /** * @docs * @kind heading @@ -1863,7 +1824,7 @@ export interface AstroUserConfig { /** * @docs - * @name experimental.csrfProtection + * @name experimental.security * @type {boolean} * @default `false` * @version 4.6.0 @@ -1876,13 +1837,46 @@ export interface AstroUserConfig { * ```js * // astro.config.mjs * export default defineConfig({ + * output: "server", * experimental: { - * security: true, - * }, + * security: { + * csrfProtection: { + * origin: true + * } + * } + * } * }) * ``` */ - security?: boolean; + security?: { + /** + * @name security.csrfProtection + * @type {object} + * @default '{}' + * @version 4.6.0 + * @description + * + * It enables some security measures to prevent CSRF attacks: https://owasp.org/www-community/attacks/csrf + */ + + csrfProtection?: { + /** + * @name security.csrfProtection.origin + * @type {boolean} + * @default 'false' + * @version 4.6.0 + * @description + * + * When enabled, it checks that the "origin" header, automatically passed by all modern browsers, matches the URL sent by each `Request`. + * + * The "origin" check is executed only on-demand pages, and only for the requests `POST, `PATCH`, `DELETE` and `PUT`, only for those requests that + * the followin `content-type` header: 'application/x-www-form-urlencoded', 'multipart/form-data', 'text/plain'. + * + * It the "origin" header doesn't match the pathname of the request, Astro will return a 403 status code and won't render the page. + */ + origin?: boolean; + }; + }; }; } diff --git a/packages/astro/src/core/build/generate.ts b/packages/astro/src/core/build/generate.ts index 979d1bbb1915..c407de024ff5 100644 --- a/packages/astro/src/core/build/generate.ts +++ b/packages/astro/src/core/build/generate.ts @@ -615,8 +615,6 @@ function createBuildManifest( i18n: i18nManifest, buildFormat: settings.config.build.format, middleware, - checkOrigin: settings.config.experimental.security - ? settings.config.security?.csrfProtection?.origin ?? false - : false, + checkOrigin: settings.config.experimental.security?.csrfProtection?.origin ?? false, }; } diff --git a/packages/astro/src/core/build/plugins/plugin-manifest.ts b/packages/astro/src/core/build/plugins/plugin-manifest.ts index 5d2d08eff9dd..393442861d35 100644 --- a/packages/astro/src/core/build/plugins/plugin-manifest.ts +++ b/packages/astro/src/core/build/plugins/plugin-manifest.ts @@ -276,8 +276,6 @@ function buildManifest( assets: staticFiles.map(prefixAssetPath), i18n: i18nManifest, buildFormat: settings.config.build.format, - checkOrigin: settings.config.experimental.security - ? settings.config.security?.csrfProtection?.origin ?? false - : false, + checkOrigin: settings.config.experimental.security?.csrfProtection?.origin ?? false, }; } diff --git a/packages/astro/src/core/config/schema.ts b/packages/astro/src/core/config/schema.ts index d97c22540c2b..58bea2f2b8f7 100644 --- a/packages/astro/src/core/config/schema.ts +++ b/packages/astro/src/core/config/schema.ts @@ -86,7 +86,7 @@ const ASTRO_CONFIG_DEFAULTS = { clientPrerender: false, globalRoutePriority: false, i18nDomains: false, - security: false, + security: {}, }, } satisfies AstroUserConfig & { server: { open: boolean } }; @@ -140,15 +140,6 @@ export const AstroConfigSchema = z.object({ .array(z.object({ name: z.string(), hooks: z.object({}).passthrough().default({}) })) .default(ASTRO_CONFIG_DEFAULTS.integrations) ), - security: z - .object({ - csrfProtection: z - .object({ - origin: z.boolean().default(false), - }) - .optional(), - }) - .optional(), build: z .object({ format: z @@ -518,7 +509,17 @@ export const AstroConfigSchema = z.object({ .boolean() .optional() .default(ASTRO_CONFIG_DEFAULTS.experimental.globalRoutePriority), - security: z.boolean().optional().default(ASTRO_CONFIG_DEFAULTS.experimental.security), + security: z + .object({ + csrfProtection: z + .object({ + origin: z.boolean().default(false), + }) + .optional() + .default({}), + }) + .optional() + .default(ASTRO_CONFIG_DEFAULTS.experimental.security), i18nDomains: z.boolean().optional().default(ASTRO_CONFIG_DEFAULTS.experimental.i18nDomains), }) .strict( diff --git a/packages/astro/src/vite-plugin-astro-server/plugin.ts b/packages/astro/src/vite-plugin-astro-server/plugin.ts index c1eca9bd484b..3d2889735b0d 100644 --- a/packages/astro/src/vite-plugin-astro-server/plugin.ts +++ b/packages/astro/src/vite-plugin-astro-server/plugin.ts @@ -143,9 +143,7 @@ export function createDevelopmentManifest(settings: AstroSettings): SSRManifest componentMetadata: new Map(), inlinedScripts: new Map(), i18n: i18nManifest, - checkOrigin: settings.config.experimental.security - ? settings.config.security?.csrfProtection?.origin ?? false - : false, + checkOrigin: settings.config.experimental.security?.csrfProtection?.origin ?? false, middleware(_, next) { return next(); }, diff --git a/packages/astro/test/fixtures/csrf-check-origin/astro.config.mjs b/packages/astro/test/fixtures/csrf-check-origin/astro.config.mjs index 62f024593cac..af516bcd9736 100644 --- a/packages/astro/test/fixtures/csrf-check-origin/astro.config.mjs +++ b/packages/astro/test/fixtures/csrf-check-origin/astro.config.mjs @@ -3,13 +3,12 @@ import { defineConfig } from 'astro/config'; // https://astro.build/config export default defineConfig({ output: "server", - security: { - csrfProtection: { - origin: true - } - }, experimental: { - csrfProtection: true + security: { + csrfProtection: { + origin: true + } + } } }); From bf2dfb409743a285966ff74fb1c928b9a39e867b Mon Sep 17 00:00:00 2001 From: Emanuele Stoppa Date: Tue, 9 Apr 2024 13:46:37 +0100 Subject: [PATCH 09/12] remove unused type --- packages/astro/src/core/app/types.ts | 4 ---- 1 file changed, 4 deletions(-) diff --git a/packages/astro/src/core/app/types.ts b/packages/astro/src/core/app/types.ts index 415eb475a82f..e919e80e456f 100644 --- a/packages/astro/src/core/app/types.ts +++ b/packages/astro/src/core/app/types.ts @@ -75,10 +75,6 @@ export type SSRManifestI18n = { domainLookupTable: Record; }; -export type SSRCsrfProtection = { - origin: boolean; -}; - export type SerializedSSRManifest = Omit< SSRManifest, 'middleware' | 'routes' | 'assets' | 'componentMetadata' | 'inlinedScripts' | 'clientDirectives' From 4d5db9fbee8ca7403bde600585e52c24a18981dc Mon Sep 17 00:00:00 2001 From: Emanuele Stoppa Date: Tue, 9 Apr 2024 13:47:20 +0100 Subject: [PATCH 10/12] update changeset --- .changeset/fair-jars-behave.md | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/.changeset/fair-jars-behave.md b/.changeset/fair-jars-behave.md index d453d3141b53..9c59a75b31b1 100644 --- a/.changeset/fair-jars-behave.md +++ b/.changeset/fair-jars-behave.md @@ -7,13 +7,12 @@ Adds a new security - and experimental - option to prevent CSRF attacks. This fe ```js import { defineConfig } from "astro/config" export default defineConfig({ - security: { - csrfProtection: { - origin: true - } - }, experimental: { - csrfProtection: true + security: { + csrfProtection: { + origin: true + } + } } }) ``` From 135380159862c4ee8f41b7e02e8468a9554fe2b3 Mon Sep 17 00:00:00 2001 From: Emanuele Stoppa Date: Tue, 9 Apr 2024 13:52:39 +0100 Subject: [PATCH 11/12] manually apply Sarah's suggestions --- packages/astro/src/@types/astro.ts | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/packages/astro/src/@types/astro.ts b/packages/astro/src/@types/astro.ts index 5ac22025ab7d..a2b7cfba01ac 100644 --- a/packages/astro/src/@types/astro.ts +++ b/packages/astro/src/@types/astro.ts @@ -1830,9 +1830,9 @@ export interface AstroUserConfig { * @version 4.6.0 * @description * - * It enables the CSRF protection for Astro websites. + * Enables CSRF protection for Astro websites. * - * The CSRF protection works only for on-demand pages (SSR) and hybrid pages where prerendering is opted out. + * The CSRF protection works only for pages rendered on demand (SSR) using `server` or `hybrid` mode. The pages must opt out of prerendering in `hybrid` mode. * * ```js * // astro.config.mjs @@ -1867,12 +1867,12 @@ export interface AstroUserConfig { * @version 4.6.0 * @description * - * When enabled, it checks that the "origin" header, automatically passed by all modern browsers, matches the URL sent by each `Request`. + * When enabled, performs a check that the "origin" header, automatically passed by all modern browsers, matches the URL sent by each `Request`. * - * The "origin" check is executed only on-demand pages, and only for the requests `POST, `PATCH`, `DELETE` and `PUT`, only for those requests that - * the followin `content-type` header: 'application/x-www-form-urlencoded', 'multipart/form-data', 'text/plain'. + * The "origin" check is executed only for pages rendered on demand, and only for the requests `POST, `PATCH`, `DELETE` and `PUT` with + * the following `content-type` header: 'application/x-www-form-urlencoded', 'multipart/form-data', 'text/plain'. * - * It the "origin" header doesn't match the pathname of the request, Astro will return a 403 status code and won't render the page. + * If the "origin" header doesn't match the `pathname` of the request, Astro will return a 403 status code and will not render the page. */ origin?: boolean; }; From 51b6dd9fe97b2347e35a439f8d6ea043239deb7f Mon Sep 17 00:00:00 2001 From: Emanuele Stoppa Date: Tue, 9 Apr 2024 15:47:16 +0100 Subject: [PATCH 12/12] Apply suggestions from code review Co-authored-by: Sarah Rainsberger --- .changeset/fair-jars-behave.md | 7 +++---- packages/astro/src/@types/astro.ts | 2 +- 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/.changeset/fair-jars-behave.md b/.changeset/fair-jars-behave.md index 9c59a75b31b1..700b1b883021 100644 --- a/.changeset/fair-jars-behave.md +++ b/.changeset/fair-jars-behave.md @@ -2,7 +2,7 @@ "astro": minor --- -Adds a new security - and experimental - option to prevent CSRF attacks. This feature is available only for on-demand pages: +Adds a new experimental security option to prevent [Cross-Site Request Forgery (CSRF) attacks](https://owasp.org/www-community/attacks/csrf). This feature is available only for pages rendered on demand: ```js import { defineConfig } from "astro/config" @@ -17,9 +17,8 @@ export default defineConfig({ }) ``` -When enabled, it checks that the "origin" header, automatically passed by all modern browsers, matches the URL sent by each `Request`. +Enabling this setting performs a check that the "origin" header, automatically passed by all modern browsers, matches the URL sent by each `Request`. -The "origin" check is executed only on-demand pages, and only for the requests `POST, `PATCH`, `DELETE` and `PUT`, only for those requests that -the followin `content-type` header: 'application/x-www-form-urlencoded', 'multipart/form-data', 'text/plain'. +This experimental "origin" check is executed only for pages rendered on demand, and only for the requests `POST, `PATCH`, `DELETE` and `PUT` with one of the following `content-type` headers: 'application/x-www-form-urlencoded', 'multipart/form-data', 'text/plain'. It the "origin" header doesn't match the pathname of the request, Astro will return a 403 status code and won't render the page. diff --git a/packages/astro/src/@types/astro.ts b/packages/astro/src/@types/astro.ts index a2b7cfba01ac..9d75bd84e1b6 100644 --- a/packages/astro/src/@types/astro.ts +++ b/packages/astro/src/@types/astro.ts @@ -1856,7 +1856,7 @@ export interface AstroUserConfig { * @version 4.6.0 * @description * - * It enables some security measures to prevent CSRF attacks: https://owasp.org/www-community/attacks/csrf + * Allows you to enable security measures to prevent CSRF attacks: https://owasp.org/www-community/attacks/csrf */ csrfProtection?: {