Feature Request: IP Whitelisting

[microadam]

Apologies if this is mentioned elsewhere, I couldn't see anything.

It would be great if we could whitelist specific hostnames to specific IP ranges. Not sure if something like that is planned or outside of the scope of this project?

Thanks!

[magiconair]

No this is t supported right now but shouldn’t be too hard to add. How often do these ip ranges change?

Also, no need to apologize :)

[microadam]

Good to know is relatively easy! I wouldn't have thought they would change TOO often. Being easily updatable is always nice though. Possibly another urlprefix- option?

[aaronhurt]

I was working on a PR for this exact thing or very similar. A few options to allow simple ACL type behavior.

Example:

only allow traffic from the specified source blocks (default deny)
urlprefix-/foobar/ allow=172.16.0.0/12,10.0.0.0/8,192.168.0.0/16

allow all traffic except traffic from the specified source blocks (default allow)
urlprefix-/foobar/ deny=1.0.1.0/24,1.0.2.0/23

Specifying an allow and deny option on the same route would result in an error.

[aaronhurt]

@microadam would that address your need? @magiconair have any suggestions around that approach?

[magiconair]

That looks good. The only question I had what to do with the PROXY connections? You'd have to use the RemoteAddr field from the http.Request to make the decision but that gets overwritten by go-proxyproto. I'm not sure whether this is an issue. Just something to think about.

[aaronhurt]

That's true, it wouldn't be the actual source at that point but the "declared" source from proxyproto. I think that's addressable via documentation unless you feel otherwise. The other option would be to preserve the original remote address separate from proxyproto but I haven't looked how much structural change that would entail.

[magiconair]

@leprechau IPv6 addresses, hostnames (dns reverse lookup timeout comes to mind...)

I also thought about more complex use cases like testing for cookies and so forth but I think that should live in the app layer.

While thinking about this: Is this something that should be configured on a per-route level or on a global level? What is the underlying use-case?

[aaronhurt]

In our use case we have some services that need to be exposed via the load balancer but shouldn't necessarily have complete unrestricted access. We could use firewall rules on the load balancer itself but that would require dedicated IPs or bind ports for those specific services. Having a light ACL option on the route would allow this to happen without needing dynamic listeners.

[aaronhurt]

To the IPv6 part the net.ParseCIDR and the IPNet.Contains function support IPv6 blocks and addresses.

[magiconair]

Just in case we change our minds and allow more complex rules should there be an indication that the value after allow is actually a list of ips?

Wild thinking:

urlprefix-/foo allow=method:GET
urlprefix-/foo allow=ip:1.2.3.4/5

Could the first argument be something that's considered a filter function and the remainder the input values?

Can you have more than one allow option on a single urlprefix-/foo?

I'm not saying we need all this right now or ever but if we can design the syntax in a way that allows us to extend the use-case later then that might help.

Also, if you think this would be overkill and we can solve that differently then I'm fine with that, too.

[aaronhurt]

Those are all valid points. I agree that having a single allow with a filter function is preferred over having allowCIDR=, allowMethod=, etc.. that could messy pretty quick.

[microadam]

The proposed approach definitely fulfils my use case! @leprechau awesome that you are working on this!

[microadam]

On an implementation point, it would probably need to look at any x-forwarded-for headers, in case of upstream proxys

[aaronhurt]

Yes, I was going to isolate that into a separate getRemoteIP function or something similar. I have the CIDR processing done but I need to work on putting the option processing into it's own function to check for subgroups (allow=ip:1.2.3.4,method:GET) processing. Hope to have a PR ready early next week, possibly Monday, depending on the weekend schedule. I need to write some test cases too :)

[magiconair]

@leprechau please keep it to ip matching for now unless you're confident that you thought the other option through.

[aaronhurt]

@magiconair It currently only supports the ip: prefix for the allow and deny tags but I think I made if flexible enough to be expanded in the future if anyone so desires.

[aaronhurt]

@microadam would appreciate your feedback as well ... especially if you can build the code in the PR and let me know how it works for you.