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

Rate limiter whitelist #517

Open
zguig52 opened this issue Aug 29, 2024 · 6 comments · May be fixed by #573
Open

Rate limiter whitelist #517

zguig52 opened this issue Aug 29, 2024 · 6 comments · May be fixed by #573
Labels
enhancement New feature or request

Comments

@zguig52
Copy link

zguig52 commented Aug 29, 2024

Is your feature request related to a problem? Please describe.

During load/performances tests, it would be nice to allow defining whitelisted IP (traffic injectors), so there is no need to disable this feature during the tests.

Also this could be interesting to be used when other internal trusted servers are calling the application for batch jobs.

Describe the solution you'd like

Add a new parameter, a list of IPs that will be whitelisted and not stored in the rate limiter DB, thus allowing no limits for specific hosts.

Describe alternatives you've considered

Disable the rate limiter during the tests

@zguig52 zguig52 added the enhancement New feature or request label Aug 29, 2024
@Baroshem
Copy link
Collaborator

Baroshem commented Sep 9, 2024

Hey @zguig52

Thanks for this idea, sounds really reasonable. Would you be interested in contributing to the module with this feature? I can provide any help needed :)

@zguig52
Copy link
Author

zguig52 commented Sep 10, 2024

Hi @Baroshem ,

I would be very happy to contribute with this feature. I will try to get some time before the end of the month to work on it. I have first to read all the doc related to Nuxt plugin dev and setup the dev env with your plugin, I never worked on this.

Thanks for this :)

@Baroshem
Copy link
Collaborator

Awesome, please let me know if you have some questions.

@zguig52 zguig52 linked a pull request Nov 28, 2024 that will close this issue
6 tasks
@zguig52
Copy link
Author

zguig52 commented Nov 28, 2024

Hi @Baroshem, just did a first basic version of whitelist management, I was quite busy, sorry for so late reply.
I was amazed how easy it was to setup dev env and start developping...
I let you check if it is fine for you.

In order to support more advance configuration use cases, it would be nice to support IPs range and subnets.
For this, a specific library shall be used. I first found this which seems perfect:
https://github.com/indutny/node-ip
But there are many concerns about its status: indutny/node-ip#150
They refer to this alternative library:
https://github.com/beaugunderson/ip-address

In current implementation (array of string), the challenge to keep same efficient and simple format is related to identifying first if the string is an IP, an IP range or an IP subnet. There is also the IPv4 vs IPv6 format to be detected.

What would be nice, to keep same configuration format, would be to perform configuration checks and store structured data at server startup (for example detect automatically if the configuration element is an IPv4, IPv6, a range, a network, etc).

If possible to do so, we would be able to efficiently make comparisons vs current IP address. I don't want to parse/classify configuration input each time a connection is performed as it would slow down the whole process too much.

For ex, input config could be:

whiteList: [
  '127.0.0.1',
  '192.168.2.1',
  '192.168.0.0/24',
  '192.168.0.1-192.168.0.5',
  '::1',
  '2001:db8::ff00:42:8329',
  '2001:db8:abcd:0012::0/64',
  '2001:db8:abcd:0013::1-2001:db8:abcd:0013::12'
]

From this we could identify:

  • based on the presence of a / : a network
  • based on the presence of a - : a range
  • else: an IP

Then we verify if IPs, networks and ranges are valid IPv4 or IPv6 addresses and store valid configuration elements in a structured array like this (to be further defined based on library used requirements):

parsedWhiteList: {
  'v4': {
    'addresses': [
      '127.0.0.1',
      '192.168.2.1',
    ],
    'networks': [
      '192.168.0.0/24'
    ],
    'ranges': [
      '192.168.0.1-192.168.0.5'
    ]
  },
  'v6': {
    'addresses': [
      '::1',
      '2001:db8::ff00:42:8329',
    ],
    'networks': [
      '2001:db8:abcd:0012::0/64',
    ],
    'ranges': [
      '2001:db8:abcd:0013::1-2001:db8:abcd:0013::12'
    ]
}

Based on this, at execution time, we then have to:

  1. Identify if current IP is v4 or v6
  2. Search for a match VS associated class addresses if defined
  3. If not found, then networks
  4. If not found, then ranges

If this is not possible, then we could make a breaking format input VS basic implementation done (but not yet released so maybe not too late to change before) and directly ask to the user to classify them (but it is not so elegant and easy to use vs config systems I already saw + there is still the address format validity which has to be performed at startup to clear useless infos and warn user about config mistake):

whiteList:  {
  'v4': {
    'addresses': [
      '127.0.0.1',
      '192.168.2.1',
    ],
    'networks': [
      '192.168.0.0/24'
    ],
    'ranges': [
      '192.168.0.1-192.168.0.5'
    ]
  },
  'v6': {
    'addresses': [
      '::1',
      '2001:db8::ff00:42:8329',
    ],
    'networks': [
      '2001:db8:abcd:0012::0/64',
    ],
    'ranges': [
      '2001:db8:abcd:0013::1-2001:db8:abcd:0013::12'
    ]
}

@vejja
Copy link
Collaborator

vejja commented Nov 29, 2024

@zguig52 I think we can easily parse at server startup
Right now the runtime/nitro/plugins/00-routeRules plugin is already used to instantiate the appSecurityOptions singleton at startup.
This would also allow to use the nuxt-security:routeRules hook to dynamically modify the whitelist if needed

@zguig52
Copy link
Author

zguig52 commented Dec 2, 2024

@vejja , thanks for your response. I will have a look to this file to check what can be performed at startup!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants