yarbsemaj / sveltekit-adapter-lambda

An adapter to build a SvelteKit app into a lambda ready for deployment with lambda proxy via the Serverless Framework or CDK.
https://www.npmjs.com/package/@yarbsemaj/adapter-lambda
MIT License
77 stars 16 forks source link

Serverless/SvelteKit/Lambda@Edge CSRF #32

Closed michaelcuneo closed 1 year ago

michaelcuneo commented 1 year ago

I have a fully working local SvelteKit app that uses form actions, which POST to the server side, and respond accordingly.

This works locally 100% fine, but after deployment using this adapter and serverless-plugin-cloudfront-lambda-edge using any form actions responds with the '403 -Forbidden, 'Cross-site POST form submissions are forbidden'.

It appears as though you can modify SvelteKit to ignore CSRF by adding a configuration options to originCheck: false, which works, but then gives me 404 /login not found, strict-origin-when-cross-origin..

I ned to find a way to add the ORIGIN to the adapter-lambda or configuration somehow I think?

yarbsemaj commented 1 year ago

Hi @michaelcuneo , Thankyou for your interest in my project.

I believe I have a good idea what this issue is . Do you have a repo I can test with?

michaelcuneo commented 1 year ago

For now I've added csrf: { checkOrigin: false } to my svelte.config.ts and also added a response section to my hooks.server.ts handler function to get rid of the CORS issue that occurs when CSRF is off.

    const response = await resolve(event);

    // Apply CORS header for API routes
    if (event.url.pathname.startsWith('/signup')) {
        // Required for CORS to work
        if(event.request.method === 'OPTIONS') {
            return new Response(null, {
                headers: {
                    'Access-Control-Allow-Methods': 'GET, POST, PUT, DELETE, PATCH, OPTIONS',
                    'Access-Control-Allow-Origin': '*',
                    'Access-Control-Allow-Headers': '*',
                }
            });
        }

        response.headers.append('Access-Control-Allow-Origin', `*`);
    }

    return response;

I don't think this will be ideal for security, but it does function.

I'm trying to see a lower level than this in the performRewrite area of the svelte-adaptor but I'm not sure if that'll suit?

yarbsemaj commented 1 year ago

I believe the issue is due to the cloudfrount messing with the host header, this is a well known and documented issue and svelte has a solution here In summery you need to Modify the lambda edge function to forward the x-forward-host header. You can find an example of this here. It may not work with the current origin request function, so you ma need to make a second cloudfrount function (viewer request) to do this forwarding , and then add the header into the aloud headers in the cloudfrount. You then need to set the environment vars when you run your build, as per the 2nd example on the svelte docs, I believe that should fix both your issues.

If you want to raise this as a pull yourself I’d be very happy to review, if not I’ll take a look tonight

michaelcuneo commented 1 year ago

I'm guessing that for this to work, it would require a modification to router.js... a secondary performReWrite, with x-forwarded-host headers added to any POST request?

Do you happen to know where Serverless performs it's node build so that I can add ORIGIN to this step? I can't find it anywhere... From what I can see in the SvelteKit documentation though, it only needs one of the solutions, ORIGIN="" in the build step, or X-forwarded-host in t he headers? Unless I'm reading the docs wrong...

michaelcuneo commented 1 year ago

I'll make up a quick REPO now aside in my sveltekit-aws-boilerplate for testing and push it up shortly.

yarbsemaj commented 1 year ago

@michaelcuneo

From what I can see in the SvelteKit documentation though, it only needs one of the solutions, ORIGIN="" in the build step, or X-forwarded-host in the headers

That is correct

Do you happen to know where Serverless performs it's node build so that I can add ORIGIN to this step

Serverless does not actually perform the build step, that's the job of vite/svelte when you run npm run build

Ill take a look later to see if i can build the setting of the ENV into a build script

michaelcuneo commented 1 year ago

Oh, it has been so long since I ran a node service locally, I forgot how it worked.

ORIGIN="[URL]" node build runs the service after npm run build so the origin can't be added,

So I've removed the csrf: { originCheck: false; } and replaced it with the X-Forwarded-Proto and X-Forwarded-Host headers into the hooks.server.ts

    const response = await resolve(event);

    // Apply CORS & X-Forwarded headers.
    if (event.url.pathname.startsWith('/')) {
        // Required for CORS to work
        if (event.request.method === 'OPTIONS') {
            return new Response(null, {
                headers: {
                    'Access-Control-Allow-Methods': 'GET, POST, PUT, DELETE, PATCH, OPTIONS',
                    'Access-Control-Allow-Origin': '*',
                    'Access-Control-Allow-Headers': '*',
                    'X-Forwarded-Proto': 'https',
                    'X-Forwarded-Host': 'cuzab6i53whrvjdibzpdyobn5u0avvre.lambda-url.us-east-1.on.aws',
                }
            });
        }

        response.headers.append('Access-Control-Allow-Origin', `*`);
    }

    return response;

I thought this would be perfect, curl -k -D - -X OPTIONS https://dev.masamadrepizza.com.au/signup returns all the right header data, so it should be working, but still something weird going on.

yarbsemaj commented 1 year ago

Yer, I’m supprised setting the origin ENV did not work as expected.

you also should not need the x forward headers on your options call, those should be sent by your proxy/cdn to get SvelteKit know what your original origin was, it will probably be this weekend before I get round to adding those to the edge, tuning into a busy week and I haven’t had the mental energy to work on this yet :)

michaelcuneo commented 1 year ago

I've never set the ORIGIN in ENV because ORIGIN is only picked up by adapter-node as a variable to include in the build. ORIGIN, PROTOCOL_HEADER and HOST_HEADER are all specific to adapter-node

/* global ENV_PREFIX */
const origin = env('ORIGIN', undefined);
const address_header = env('ADDRESS_HEADER', '').toLowerCase();
const protocol_header = env('PROTOCOL_HEADER', '').toLowerCase();
const host_header = env('HOST_HEADER', 'host').toLowerCase();

I'll find a way to pull ENV into adapter-lambda, I've tried performing it all in the middle on CloudFront by creating a policy that appends to the forward all http to https by adding a new behaviour with the header policies, but that just gives me access denied straight away, because x-forwarded-proto is denied by Lambda@Edge

yarbsemaj commented 1 year ago

Hummmm, for the proto, we might just be able to hard code it to https, as that’s what 99% of users will be using

michaelcuneo commented 1 year ago

Yeah of course, no one uses http anymore.

yarbsemaj commented 1 year ago

As for the x forward host, you can set its value as the value of the host header (essentially overriding it) when i do the massaging of the lambda event into a request object

zamoshchin commented 1 year ago

Having the same issue. Was a solution merged in?

yarbsemaj commented 1 year ago

@zamoshchin Not yet, im working on one now, give me a few hours

yarbsemaj commented 1 year ago

@zamoshchin @michaelcuneo I have pushed a fix for this issue as v1.1.0. If you update and make the requisite changes to your serverless.yml this should fix your issue.

zamoshchin commented 1 year ago

Thanks for the quick turnaround--I believe that fixed that issue but am not 100% sure as I'm facing a further issue, which I believe is related to needing to set my environment vars. According to the Sveltekit docs I need to run using the command node -r dotenv/config build but I'm not sure where that command is being run. All I do is sls deploy

yarbsemaj commented 1 year ago

@zamoshchin If those ENVs are in relation to the ones discussed above, there not needed, that was just me misreading some docs. If you want to set ENVs on your svelte lambda, this should be done in your serverless.yml