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

Add option to specify allowed IPs #6

Open
ItsDrike opened this issue Apr 3, 2022 · 4 comments
Open

Add option to specify allowed IPs #6

ItsDrike opened this issue Apr 3, 2022 · 4 comments

Comments

@ItsDrike
Copy link
Contributor

ItsDrike commented Apr 3, 2022

There should really be an option to specify allowed IP CIDR ranges. Only if the incomming IP is within this CIDR range should the header be used instaed.

Doing this is pretty important since the service may be exposed in different ways, only one of which is a reverse proxy. This would mean if people could access the service directly, they could make fake requests with one of these headers manually set by them, triggering the IP replacement and allowing them to fake their real address like this. This makes it very important to add a security measure like this into the plugin.

This could be handled simply by a variable holding an array of these ranges (I'm not very familiar with PHP's data types, but I'd assume there is a list/array like structure that could be used for this), in there, ranges of IPs like "10.1.0.0/24" could be defined by the user. By default, if this variable isn't defined, a range of 0.0.0.0/0 could be used to cover all IPs.

Note that the above is an example that only covers IPv4 CIDR ranges, there should probably be support for IPv6 ones too.

@Diftraku
Copy link
Owner

Diftraku commented Apr 3, 2022

That is true, the best practice is to use the remote IP to determine if we should trust the "real" IP header being sent.

The last time I properly did any development for this was back in 2012/2014 and I've since dropped using YOURLS, you are
more than welcome to submit a PR for this.

@ItsDrike
Copy link
Contributor Author

ItsDrike commented Apr 3, 2022

The last time I properly did any development for this was back in 2012/2014 and I've since dropped using YOURLS, you are
more than welcome to submit a PR for this.

I can try and throw something together a bit later, but I have very little experience with coding in PHP, so I can't promise that the code quality of that would be particularly good.

@Diftraku
Copy link
Owner

Diftraku commented Apr 3, 2022

No worries, even if the PR is for a single IP (or checking against a list of IPs) that would be more than enough for this purpose.

I did some googling and it looks like there is not a built-in function in PHP to check if an IP matches a given CIDR range but Symphony does have one in the IpUtils helper class of their HttpFoundation component: https://github.com/symfony/http-foundation/blob/5.4/IpUtils.php

Though a fair warning: the method they use does involve using bit shifting in case that is way out of your comfort zone.

@ItsDrike
Copy link
Contributor Author

ItsDrike commented Apr 3, 2022

Thanks! That should come in handy.

Though a fair warning: the method they use does involve using bit shifting in case that is way out of your comfort zone.

It's fine, I'm just not familiar with the PHP lang, but I do have experience in other languages and I'm familiar with bitshift operations.

When I will have some time, I'll look into implementing this, but it may take me a while since I'm already working on way too many projects.

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

No branches or pull requests

2 participants