Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: add origin check for CSRF protection #10678

Merged
merged 12 commits into from
Apr 10, 2024
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
26 changes: 26 additions & 0 deletions .changeset/fair-jars-behave.md
Original file line number Diff line number Diff line change
@@ -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:
ematipico marked this conversation as resolved.
Show resolved Hide resolved

```js
import { defineConfig } from "astro/config"
export default defineConfig({
security: {
csrfProtection: {
origin: true
}
},
experimental: {
csrfProtection: true
matthewp marked this conversation as resolved.
Show resolved Hide resolved
}
})
```

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.
ematipico marked this conversation as resolved.
Show resolved Hide resolved
62 changes: 62 additions & 0 deletions packages/astro/src/@types/astro.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1610,6 +1610,45 @@ 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
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
* It enables some security measures to prevent CSRF attacks: https://owasp.org/www-community/attacks/csrf
* It enables some security measures to prevent CSRF attacks: https://owasp.org/www-community/attacks/csrf

I think we'll need to provide some more detail here. Not heavy implementation details, but like a sentence or so on HOW are you doing this or WHAT MEASURES are you taking, in lay speak. (cookies, tokens, other random words Sarah knows?) This doesn't exactly inspire trust. 😅

Ideally, think about what people will find on web pages as advice of "things they should do" in their project to protect against this, and be able to say, "Astro enables security measures recommended to protect against CSRF attacks, like [thing that people will recognize as stuff they're supposed to want/need/do] -- if this is at all possible!

*/

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`.
ematipico marked this conversation as resolved.
Show resolved Hide resolved
*
* 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.
ematipico marked this conversation as resolved.
Show resolved Hide resolved
*/
origin?: boolean;
};
};

/**
* @docs
* @kind heading
Expand Down Expand Up @@ -1821,6 +1860,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.
ematipico marked this conversation as resolved.
Show resolved Hide resolved
*
* The CSRF protection works only for on-demand pages (SSR) and hybrid pages where prerendering is opted out.
ematipico marked this conversation as resolved.
Show resolved Hide resolved
*
* ```js
ematipico marked this conversation as resolved.
Show resolved Hide resolved
* // astro.config.mjs
* export default defineConfig({
* experimental: {
* csrfProtection: true,
* },
* })
* ```
*/
csrfProtection?: boolean;
};
}

Expand Down
9 changes: 9 additions & 0 deletions packages/astro/src/core/app/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +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 { sequence } from '../middleware/index.js';
import { createOriginCheckMiddleware } from './middlewares.js';
export { deserializeManifest } from './common.js';

export interface RenderOptions {
Expand Down Expand Up @@ -112,6 +114,13 @@ export class App {
* @private
*/
#createPipeline(streaming = false) {
if (this.#manifest.csrfProtection?.origin === true) {
this.#manifest.middleware = sequence(
createOriginCheckMiddleware(),
this.#manifest.middleware
);
}

return AppPipeline.create({
logger: this.#logger,
manifest: this.#manifest,
Expand Down
42 changes: 42 additions & 0 deletions packages/astro/src/core/app/middlewares.ts
Original file line number Diff line number Diff line change
@@ -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();
});
}
5 changes: 5 additions & 0 deletions packages/astro/src/core/app/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@ export type SSRManifest = {
pageMap?: Map<ComponentPath, ImportComponentInstance>;
i18n: SSRManifestI18n | undefined;
middleware: MiddlewareHandler;
csrfProtection: SSRCsrfProtection | undefined;
};

export type SSRManifestI18n = {
Expand All @@ -74,6 +75,10 @@ export type SSRManifestI18n = {
domainLookupTable: Record<string, string>;
};

export type SSRCsrfProtection = {
origin: boolean;
};

export type SerializedSSRManifest = Omit<
SSRManifest,
'middleware' | 'routes' | 'assets' | 'componentMetadata' | 'inlinedScripts' | 'clientDirectives'
Expand Down
3 changes: 3 additions & 0 deletions packages/astro/src/core/build/generate.ts
Original file line number Diff line number Diff line change
Expand Up @@ -615,5 +615,8 @@ function createBuildManifest(
i18n: i18nManifest,
buildFormat: settings.config.build.format,
middleware,
csrfProtection: settings.config.experimental.csrfProtection
? settings.config.security?.csrfProtection
: undefined,
};
}
3 changes: 3 additions & 0 deletions packages/astro/src/core/build/plugins/plugin-manifest.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
};
}
14 changes: 14 additions & 0 deletions packages/astro/src/core/config/schema.ts
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,7 @@ const ASTRO_CONFIG_DEFAULTS = {
clientPrerender: false,
globalRoutePriority: false,
i18nDomains: false,
csrfProtection: false,
},
} satisfies AstroUserConfig & { server: { open: boolean } };

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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(
Expand Down
3 changes: 3 additions & 0 deletions packages/astro/src/vite-plugin-astro-server/plugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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();
},
Expand Down