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

Question about Denial-of-Service (DoS) #75

Closed
MrHertal opened this issue Oct 14, 2021 · 6 comments
Closed

Question about Denial-of-Service (DoS) #75

MrHertal opened this issue Oct 14, 2021 · 6 comments
Labels
enhancement New feature or request feature/terraform Missing feature from Terraform

Comments

@MrHertal
Copy link

Hi 👋

First of all, thank you for this nice project. Code is very clean and inspiring.

I am considering using it in production and I have a question regarding DoS attack.
I think image optimization could be expensive if a bad person plays with the api endpoint.

My first question is: every time you get a Miss from cloudfront, the lambda is run and (costly) image optimization occurs?

If it's the case, the api should be less permissive than it is. I noticed that with the api gateway configuration: /_next/{proxy+}, you just have to put random strings like /_next/blabla to avoid cache (Miss) and generate a new image.

What are your thoughts about this?

Thanks!

@ofhouse
Copy link
Member

ofhouse commented Oct 14, 2021

Hi, thanks for raising the question.

A Miss from cloudfront indicates that the object is not available at the edge cache and is requested from the origin.
The origin itself is also wrapped in a cache layer (CloudFront Origin Shield), so when two identical requests from different locations (e.g. Frankfurt and Paris) are made, the first request is forwarded to the Lambda while the second is served from the Origin Shield cache layer without hitting the Lambda.
So there is a "soft miss" (served from Origin Shield) and a "hard miss" (processed by Lambda) which are both labeled with Miss from cloudfront.

Yes, the configuration of the API-Gateway with the route /_next/{proxy+} would enable an attack scenario as you described since the path could be altered with each request to prevent a caching in both layers.
To mitigate the issue the routes accepted by API-Gateway should be restricted to /_next/image & /_next/image/.

However the issue would still be present then, since the attack could be modified to target other parts of the cache-key from CloudFront.
The cache-key is currently composed of the path, different request headers "accept", "referer" and the path parameters "url", "w", "q".
So modifying the accept or referer headers in the client request would also result in a Miss from CloudFront.

However CloudFront has several monitoring and security measurements built-in to mitigate DoS attacks and can be further secured with AWS WAF.

So while I agree that we should restrict the path further in API Gateway, I would say that CloudFront is not an easy target for a massive DoS attack.

@ofhouse ofhouse added this to the v12.0.0 milestone Oct 14, 2021
@ofhouse ofhouse added enhancement New feature or request feature/terraform Missing feature from Terraform labels Oct 14, 2021
@MrHertal
Copy link
Author

Hi, thanks for the detailed explanation!

Let's say that I'm in a situation where I want to restrict the api as much as I can before relying on AWS built in protection or WAF.

If I understand correctly, here are the settings that compose the cache-key:

  • accept header

Values could be limited to the browsers default so that an attacker could not set random values.

  • referer header

I don't understand why the referer is part of the cache-key because I could use the same api on multiple next.js website.

  • w and q parameters

Possible values are limited.

  • url parameter

The value is reliable since url domain is white listed. But an attacker could add query params to that url. A solution for that case would be to only allow urls without query params.

Sorry if it's beyond the scope of this library and if it's specific to my use case.

Do you think that such restrictions would effectively protect the api and avoid useless costs ?

@ofhouse
Copy link
Member

ofhouse commented Oct 15, 2021

No problem, the module is designed as an enterprise application and given that it currently has 1000+ weekly installs, improving security is a top priority for this project.

Referer header

The referer header was added to the cache-key as a way to determine the the correct host for relative paths.
For example the request example.com/_next/image?url=/image.png would have the Referer: example.com as header, when an embedded resource is requested by the browser.
So the image optimizer is then able to resolve the absolute URL for the image from it: example.com/image.png.

This was meant as an easy way to integrate, but we should change it in the following way:

  1. Since there is also an option to connect an S3 bucket, we should remove the header from the cache-key when a S3 bucket is connected.
  2. For other use cases we should also offer an option to specify the domain for relative paths via an input parameter to the module so that the referer header can be removed from the cache-key.
  3. If 1 & 2 are not used, the referer header is added to the cache-key.

Accept header

Limiting the header to default values is a risky take since this can change every time.
An alternative approach would be to use a CloudFront function to normalize the header for each request.

  1. Detect Avif support -> replaces Accept header with image/avif,image/webp,*/*
  2. Detect Webp support -> replaces Accept header with image/webp,*/*
  3. Otherwise replaces the header with */*

The advantage of using a CloudFront function for this is that it is relatively cheap ($0.10 per 1 million invocations) and the modified Header is used to serve the request from the cache instead of hitting the origin.
So two identical requests with different headers Accept: image/webp,*/* Accept: image/webp,a/b,*/* would both served from the same cached object.
The disadvantage is that the CloudFront function would be invoked for every request (whether it's in the cache or not).

url parameter

This is a tricky one since adding parameters to the the url is a valid option since it could be used with storage engines that require some form of authentication. E.g. adding a api key through a parameter.

The only way to secure this would probably to introduce another S3 bucket instead of (or together with) CloudFront Origin Shield for the local caching.
This way the CloudFront Function could from the Accept header could determine a cache key (leaving out path params) based on the input and forward the request to S3.
If an object with the key exists in S3 it is served from there otherwise a failover would trigger the Lambda to generate a response or image.
The Lambda would then determine a cache-key based on the input params and store the image back to S3 before returning it to CloudFront.

@MrHertal
Copy link
Author

Thanks for the detailed explanation. I understand the solutions you describe and they seem really suitable.
I'm not sure that I have enough skills to implement them, but I will definitely try on my current project.
I will eventually come back with a PR if you are interested.

@ofhouse
Copy link
Member

ofhouse commented Oct 16, 2021

Absolutely would welcome a PR on this!

My current plan is to start small and implement the changes to the referer header first.
Since it's not that much time until Next.js 12 will be released (Currently scheduled for 26. Oct in time with Next.js Conf) and we plan to release new versions timely after new Next.js releases I would defer the other proposed changes to upcoming releases.

@ofhouse
Copy link
Member

ofhouse commented Nov 22, 2021

Closing this for now since I've splitted the issue in smaller packages:

Thanks for the input!

@ofhouse ofhouse closed this as completed Nov 22, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request feature/terraform Missing feature from Terraform
Projects
None yet
Development

No branches or pull requests

2 participants