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: soft-launch rate-limiting of API endpoints #389

Merged
merged 12 commits into from
Oct 7, 2020
Merged

Conversation

mantariksh
Copy link
Contributor

@mantariksh mantariksh commented Sep 30, 2020

Problem

We want to limit the rate of requests to public endpoints as a security measure against DDoS attacks. However, we need to ensure that the limits do not affect regular application usage. Hence it is necessary to ensure that the limits do not get exceeded on a regular basis, before actually denying requests which exceed the limit.

After some discussion, we decided to apply the limits to three endpoints: /auth/sendotp as well as both submissions endpoints, v2/submissions/email and v2/submissions/encrypt.

Solution

Use express-rate-limit with a custom handler which logs a message if the rate limit is exceeded, then carries on handling the request as normal. Note that the package uses per-IP limits by default.

The log message looks like this:
Screenshot 2020-10-06 at 10 35 24 AM

Determining the limit

It is important to apply a limit which does not affect regular application usage. To determine the limit, I counted the per-minute requests per IP address over the past week to each of the endpoints.

Between both v2/submissions endpoints, the maximum was 46 requests to /v2/submissions/email:
Screenshot 2020-10-05 at 5 27 49 PM

Multiplying this by 4 to get the maximum seemed reasonable, since programmatic attacks are likely to exceed regular usage numbers by far more. Moreover, the limit is applied per-instance rather than across instances (see next section). Hence I chose a per-minute limit of 200.

As for the /auth/sendotp endpoint, the maximum was 4 requests in a minute. To be safe, I set the per-minute maximum at 60.

Per-instance vs instance-wide limits

express-rate-limit uses an in-memory store by default. This means that the rate limits are applied per-instance. Applying them instance-wide would require setting up a store in our database using MongoDB plugin. However, the plugin does not seem to support reusing existing connections, which means we would have to create a new connection to the database. I decided against this to avoid concurrency issues with multiple instances accessing the same documents, so the limits are applied per-instance.

Tests

  • Production environment is updated with the environment variables SUBMISSIONS_RATE_LIMIT=200 and SEND_AUTH_OTP_RATE_LIMIT=60.
  • In production, an alarm has been set for any occurrence of the "Rate limit exceeded" log message.
  • In the staging environment, set SUBMISSIONS_RATE_LIMIT=2 and SEND_AUTH_OTP_RATE_LIMIT=2.
    • On the admin console login page, try to resend your OTP 3 times within a minute. Ensure that you receive all 3 OTPs, and that a "Rate limit exceeded" message gets logged with metadata about the rate limit (IP address, endpoint, current number of requests).
    • Try submitting 3 different forms within 1 minute. Ensure that all 3 forms can be submitted, and that a "Rate limit exceeded" message gets logged with metadata about the rate limit (IP address, endpoint, current number of requests).
    • In the staging environment, set SUBMISSIONS_RATE_LIMIT=200 and SEND_AUTH_OTP_RATE_LIMIT=60.

@mantariksh
Copy link
Contributor Author

Converting to draft while switching to app-wide limits

@mantariksh mantariksh marked this pull request as draft October 1, 2020 07:22
@mantariksh mantariksh changed the title feat: soft-launch rate-limiting on public endpoints feat: soft-launch rate-limiting of API endpoints Oct 6, 2020
@mantariksh mantariksh marked this pull request as ready for review October 6, 2020 02:37
Copy link
Contributor

@liangyuanruo liangyuanruo left a comment

Choose a reason for hiding this comment

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

approved with minor changes requested

@karrui karrui mentioned this pull request Oct 8, 2020
@karrui karrui deleted the feat/rate-limit branch October 8, 2020 06:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants