Closed Rich-Harris closed 2 years ago
halfnelson
commented
3 years ago Listening to your Svelte Summit (spring 2021) post talk discussion about the "enhance" form support and its integration with SvelteKit. Maybe that same integration is what can provide the CSRF support (considering since a non JS post to a form is where CSRF is most needed)
nbgoodall
commented
3 years ago @Rich-Harris I soon need to draw this owl for an app nearing production, and why duplicate work?
Do you have any thoughts on this or how you'd like to see it implemented? :)
nakleiderer
commented
3 years ago I think, since svelte kit pairs nicely with FaaS, it would likely be best to avoid server side state. The Double Submit Cookie technique seems like a reasonable choice. https://cheatsheetseries.owasp.org/cheatsheets/Cross-Site_Request_Forgery_Prevention_Cheat_Sheet.html#double-submit-cookie
I believe it could be implemented as a POC without modifying svelte kit code. There's a few strategies that could be considered, but I'd consider using the JWT standard to be the simplest.
Proposal using JWT:
mlusetti
commented
3 years ago I think, since svelte kit pairs nicely with FaaS, it would likely be best to avoid server side state. The Double Submit Cookie technique seems like a reasonable choice. https://cheatsheetseries.owasp.org/cheatsheets/Cross-Site_Request_Forgery_Prevention_Cheat_Sheet.html#double-submit-cookie
I agree. A cookie with a SameSite: strict and HttpOnly (Secure too obviously) should do the trick
Karlinator
commented
3 years ago @nbgoodall I'm also needing to draw this owl about now. Did you get anywhere with this?
My first idea was to set the double submit cookie and inject the form field/value though the handle function (now nicely chainable with sequence). The cookie worked, but the input value was of course overwritten by Svelte during hydration. I don't yet have an idea for injecting the token into the form (so it will also appear in the POST request).
I'm trying right now to do this purely as a handler middleware without changing any code in SvelteKit itself.
Karlinator
commented
3 years ago So I've arrived at (I think) a working solution using only a handler.
It's essentially a signed double submit cookie with the added security of submitting it through a custom header (which means the same-origin policy comes to our defence as well).
The handler generates a JWT token with an expiry in a few minutes, then it adds a Set-Cookie header and injects a meta tag with the token in the document head.
On incoming requests, the handler reads the x-csrf-token custom header, compares it to the cookie, and verifies the signature. If everything is good it sets requests.locals.csrfVerified = true.
What I haven't decided is how to deal with getting new tokens. As it stands, tokens are only changed when the user reloads the page (and triggers SSR). The easy option is of course to just disable the router where necessary. That said, I don't think there's a way to disable the router for all navigation to a page, only from a page.
Alternately, you can just accept that the token is the same for however long the user remains on the page without refreshing.
nbgoodall
commented
3 years ago @Karlinator how are you adding the x-csrf-token header? In the fetch requests themselves?
My biggest motivation for this was making forms secure without JavaScript. It is a double submit cookie, storing an encrypted value in the cookie session and exposing a masked version in forms (similar to how Rails does it to protect against SSL breach attacks). I was already using an encrypted cookie to store arbitrary JSON, so the token lives in there.
On each request:
$session.Form element). This works for me, but asking users to do something like this seems a bit much...The session is encrypted using AES as I needed to decrypt the contents, but HMAC would be slightly faster if all you need is to verify the token...
What I haven't decided is how to deal with getting new tokens. As it stands, tokens are only changed when the user reloads the page (and triggers SSR).
I'm not sure this is an issue; it's common for a token to last a whole session, and realistically how long is someone going to be on a page for?
Karlinator
commented
3 years ago @nbgoodall
I'm adding it in the fetch requests, yes. We ended up not supporting non-JS anyway, so at that point there was no reason not to.
I tried to limit myself as far as I could to only doing stuff in the handle function, as I didn't really want to create custom components. That said, I did have to manually pass the token into the fetch calls so I only partly succeeded. That is the only thing that happens outside the handle hook though, which I am pretty happy with.
But I had to trade off non-JS function entirely so it's definitely not a universal solution. I think creating (or importing, if this were a package) a custom form component is probably reasonable given the alternatives right now? Otherwise we'd have to modify Svelte itself probably (and in the process "lie" about what the DOM will look like). I could definitely see a CSRFForm as a component with a slot (although that double F is killing me).
What I haven't decided is how to deal with getting new tokens. As it stands, tokens are only changed when the user reloads the page (and triggers SSR).
I'm not sure this is an issue; it's common for a token to last a whole session, and realistically how long is someone going to be on a page for?
You're probably right. Depending on who you ask (there are some long threads on this on StackOverflow) I'd probably be fine with just checking for the existence of the header and relying on the same-origin policy. It's generated anew every time it's sent out, after all, so it shouldn't really be a problem. I'm probably being a bit paranoid; I have more than enough layers of defence in depth already not to worry about the token living through some client-side routing.
Thanks for outlining your solution though!
nakleiderer
commented
3 years ago Here's more details on the solution I ended up using. It supports JS and non-JS forms and operated in a serverless environment, so seems to be perfectly aligned with svelte-kit's principles. I'm only using this token for CSRF protection and not to store any session info.
To add CSRF to a non-js page, you can hydrate the tokens in the handle hook. You'll need to set your cookie value and inject values into the HTML, like so:
import * as cheerio from "cheerio";
export async function hydrateCsrfProtection(
response: ServerResponse,
csrfToken: string
): Promise<ServerResponse> {
if (response.headers["content-type"] !== "text/html") {
return response;
}
// Add csrf information to meta tags
const $ = cheerio.load(response.body.toString());
$("head").append(
`<meta name="csrf-param" content="${clientConfig.csrfParam}" />`,
`<meta name="csrf-token" content="${csrfToken}" />`
);
/// Add csrf information to forms in case javascript is disabled or not loaded
$("body")
.find("form")
.each(function () {
$(this).append(
`<input type="hidden" name="csrf-token" content="${csrfToken}" />`
);
});
return {
...response,
body: $.html(),
};
}To solve the cookie going away with hydration, you can add a use directive to "enhance" the form. For example:
const csrfTokenMeta = (): HTMLMetaElement =>
document.querySelector('meta[name="csrf-token"]');
const csrfParamMeta = (): HTMLMetaElement =>
document.querySelector('meta[name="csrf-param"]');
export const csrfToken = (): string => csrfTokenMeta().content;
export const csrfParam = (): string => csrfParamMeta().content;
// this action (https://svelte.dev/tutorial/actions) allows us to
// progressively enhance a <form> that already works without JS
export function enhance(form: HTMLFormElement): void {
if (browser) {
const csrfParamInput = document.createElement("input");
csrfParamInput.setAttribute("type", "hidden");
csrfParamInput.setAttribute("name", csrfParam());
csrfParamInput.setAttribute("value", csrfToken());
form.append(csrfParamInput);
}
}You can support client-side fetch like this:
export const clientFetch: typeof fetch = (
input: RequestInfo,
init?: RequestInit
) => {
const initHeader =
typeof input === "string" ? init?.headers : init.headers ?? input.headers;
const headers = new Headers(initHeader);
headers.set("Content-Type", "application/json");
headers.set("x-csrftoken, csrfToken());
if (typeof input === "string") {
return fetch(input, { ...init, headers });
}
return fetch({ ...input, headers }, { ...init, headers });
};To get the token from the form or the header, you can use another handle hook modification
const extractCsrfToken = (request: ServerRequest) => {
if (request.body === undefined) {
return "";
}
return typeof request.body !== "string" && "get" in request.body
? request.body.get(clientConfig.csrfParam)
: request.headers[clientConfig.csrfHeader];
};You'll need to make sure the header/form value matches the cookie. To get the cookie:
const cookies = cookie.parse(headers.cookie ?? "");
const cookieCsrfToken = cookies.csrf_token;Your matching algorithm will differ depending on how you implement your tokens. A good start would be a HS256 JWT for both tokens. Use the same sub value for both tokens with a different cyptographically-random jti. In addition, set an eat (15 minutes is probably a good start). Validate both JWTs to make sure they haven't been tampered with or expired. Then, make sure the sub value in each token matches.
Prinzhorn
commented
3 years ago You don't need to add a CSRF token to a fetch request, when SOP/CORS already solve that (Origin header). An attacker cannot forge a request via fetch without CORS being configured, and then you'd additional need Access-Control-Allow-Credentials for it to work and a CORS misconfiguration that allows any (evil) host. The only reason a CSRF token for fetch is needed in some frameworks is because it already has a CSRF token for classic forms and then fetch requests need to be made compatible to that because the server doesn't know. If you do not use <form> and only fetch, then the Origin header solves CSRF. There is no need to use a token. Turns out all modern browsers also set Origin for <form> POSTs. That means if you only use fetch but an attacker uses <form> on a different origin then Origin header does not match. If it is missing (IE < 11 + some edge cases, Firefox for Android) you can safely discard the request, since you only use fetch, which always includes Origin for non-GET/HEAD. No false-positive in that case.
My suggestion for SvelteKit:
Origin header on every request that is not GET or HEAD against a configured host (this is important, I recommend configuring a strict host and not rely on any X- headers etc.). If Origin is missing, fall back to Referer. If both are missing or do not match, reject the request.POST, or else you can bypass the whole thingSameSite=strict as additional measurementTo my knowledge there is no way to bypass this and this is the recommended approach moving forward. It is also recommended by OWASP. There is one downside: this can lead to false positives (rejecting valid requests) under very specific circumstances:
Origin for <form>Referer disabled via some privacy settingsfetchSources:
Prinzhorn
commented
3 years ago I want to point out that OWASP (in the source I linked above) only recommends this in addition to the other approaches under the "Defense In Depth Techniques" section. Because of the following downsides (I didn't list all of them above):
This mitigation is working properly when origin or referrer headers are present in the requests. Though these headers are included majority of the time, there are few use cases where they are not included (most of them are for legitimate reasons to safeguard users privacy/to tune to browsers ecosystem). The following lists some use cases:
- Internet Explorer 11 does not add the Origin header on a CORS request across sites of a trusted zone. The Referer header will remain the only indication of the UI origin. See the following references in Stack Overflow here and here.
- In an instance following a 302 redirect cross-origin, Origin is not included in the redirected request because that may be considered sensitive information that should not be sent to the other origin.
- There are some privacy contexts where Origin is set to "null" For example, see the following here.
- Origin header is included for all cross origin requests but for same origin requests, in most browsers it is only included in POST/DELETE/PUT Note: Although it is not ideal, many developers use GET requests to do state changing operations.
- Referer header is no exception. There are multiple use cases where referrer header is omitted as well (1, 2, 3, 4 and 5). Load balancers, proxies and embedded network devices are also well known to strip the referrer header due to privacy reasons in logging them.
However, some of these points don't really apply (e.g. if you are using GET to make mutating requests that's a bug in your app). It's up to the maintainers to decide if this is something that they want in the core. I personally think the benefits outweigh the downsides by a lot and I will take this approach, it's trivial to implement. I don't think a token based approach can be added to SvelteKit in a way that works for all use cases and adapters. There are too many edge cases since SvelteKit doesn't follow the classic request/response pattern to serve pages and swapping out tokens will become a pain (will end up in false positives as well because of edge cases and timed out tokens and whatnot, see https://github.com/xing/cross-application-csrf-prevention and read through the issues and confusion on https://github.com/j0lv3r4/next-csrf/issues).
I also want to point out that if your Svelte app uses a non-cookie approach to sessions (e.g. JWT in localStorage) CSRF does not exist. CSRF only exists because of cookies.
benmccann
commented
2 years ago It looks like Firefox for Android has likely added support for the Origin header, so the suggestion above seems pretty safe:
benmccann
commented
2 years ago I notice that Play Framework uses a token and checks under the following conditions:
And it has a warning about needing different settings when using NTLM or client certificate based authentication.
Rails and Laravel use tokens as well. Next.js apparently leaves it to the user :stuck_out_tongue_closed_eyes:
Some additional thoughts about the configuration:
$env/dynamic/public? (should we rename our existing environment variables to fit the new naming scheme?)benmccann
commented
2 years ago It's possible we could just add this to the templates rather than the framework so that users have full control over the logic being used. E.g. something like this in handle:
if (request.headers.get('origin') !== url.origin) {
return new Response('nice try, bad guy', { status: 403 });
}
bato3
commented
2 years ago I don't remember where I found this implementation, but I like it very much.
There are 2 values in the form: "token number" and the token itself. Each time, the user gets a new pair. It may have two tabs open and the submission of the forms will be correct.
I don't like the solution: <input type="hidden" name="csrf-token" content="${csrfToken}" /> because the first submission of the form will consume a token, and the next submission will report an error.
I imagine the use in SK as:
<Form csrf>and:
/** @type {import('./$types').FormAction} */
export function createTodo({ request }) {
if (!request.csrf.validate()) {
return {
errors: {
csrf: 'Oh no!.. You failed'
}
};
}
// another form validations
request.csrf.consume(); //Consider when to refresh the token: How the next message will behave, how errors occurred in the form / database.
// ...
}BTW: With the server, I can easily fake the header origin. :-P
PatrickG
commented
2 years ago BTW: With the server, I can easily fake the header
origin. :-P
With a server, I can easily scrape the csrf-token from the html source and send it along. The purpose of a csrf token is to protect your users which are visiting your site with a browser.
BTT I think checking the origin header is enough. Even Same-Site cookies should be enough.
(draw the rest of the owl)