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

Cannot log in when OpenCRVS is hosted behind a reverse proxy #7878

Open
manitra opened this issue Oct 30, 2024 · 2 comments
Open

Cannot log in when OpenCRVS is hosted behind a reverse proxy #7878

manitra opened this issue Oct 30, 2024 · 2 comments
Labels
Milestone

Comments

@manitra
Copy link

manitra commented Oct 30, 2024

Describe the bug
When OpenCRVS is hosted on an environment where it is not directly exposed to internet but a reverse proxy (or load balancer) is used to forward Internet requests to it, end users cannot log in as soon as there 3 or 4 end users trying to log in in the same minute.

Which feature of OpenCRVS your bug concern?

Login
Security

To Reproduce
Steps to reproduce the behaviour:

  1. Host opencrvs on a server which does not have a public IP with the host name crvs.domain
  2. Configure a reverse proxy which has a public (ex: nginx or aws loadbalancer) to forward all HTTP request to crvs.domain and *.crvs.domain to the opencrvs server
  3. Configure the DNS to point crvs.domain and *.crvs.domain to the public IP of the reverse proxy or load balancer
  4. Ensure the app is working for yourself (login, do one or two tasks)
  5. Ask 3 endusers to try to login in and log out and log in by yourself
  6. See error (Red thing below the login button)

Expected behaviour
Every 4 users including yourself should be able to login.

Actual behaviour
Most likely, at list one of the 4 users won't be able to log in and will have a weird red small popup below the login button instead.

Opening the Devtools, the /authenticate HTTP request will have a status code of 402 with the message :
To many requests within a minute. Please throttle your requests.

Screenshots

Image

OpenCRVS Core Version:

  • v1.6 (Git branch: release-v1.6.0)

Country Configuration Version:

  • v1.1.0 (Git branch: release-v1.1.0)

Desktop (please complete the following information):

  • OS: MacOS
  • Browser: Chrome
  • Version: 130.0.6723.70

Smartphone (please complete the following information):

N/A

Additional context
Madagascar did its first training of trainers with 60 persons in the same rooms trying to use the software at the same time.
We didn't spot this issue before.

Possible fixes
After investigation there few workaround for staging environments : define the DISABLE_RATE_LIMIT env variable in the staging environement so that the rate limiter is completely switched off

For production, we cannot do this because there need to be a rate limiter for any critical endpoints like /authenticate .
What could be done is to

  • create an utility function to get the current user's IP and implement it this way
  • use that utility function everywhere needed instead of the built in one (eg. rate limiting, logging, black/whitelisting ...)
string getUserAddress(Request request) {
  if (isLocalAddress(request.sourceAddress))
    return request.headers["X-Forwarded-For"] ?? request.sourceAddress;
  else
    return request.sourceAddress;
}

The trick here is that most correctly configured reverse proxy and load balancers, pass the real user IP to the upstream server using an HTTP header. There seem to be a convention that this HTTP header is called X-Forwarded-For .
But to make sure an attacker don't just add this header to trick our rate limiting system, we also ensure that the source IP is local.

Reproducible demo

A bit complicated to create a demo reproduction because I'm short on docker things.
But I'm sure a docker compose lover would easily add a new service with nginx which would be reverse proxying requests to the current traeffick service and would manage to reproduce the issue locally.

I will try to submit a PR for this because we'll need it before going to production in few days 😉

@manitra manitra added the Bug label Oct 30, 2024
@naftis
Copy link
Collaborator

naftis commented Oct 30, 2024

A fix proposal in #7877

We don't use IP addresses with rate-limiting, but we used a wrong key for bucketing the rate-limiting requests. For now, like you described, you can disable rate limiting with the environment variable DISABLE_RATE_LIMIT.

We will QA this with @SyedaAfrida and amend it to v1.6

@naftis naftis added this to the v1.6.0 milestone Oct 30, 2024
@manitra
Copy link
Author

manitra commented Oct 30, 2024

Wonderful, thank you for your lightning fast support on this guys !
It took you the same time to spot and fix the issue than for me to create the bug report 😂

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Status: Backlog
Development

No branches or pull requests

2 participants