contour icon indicating copy to clipboard operation
contour copied to clipboard

Add IP allowlist and blocklist functionality

Open youngnick opened this issue 3 years ago • 9 comments

As either a cluster admin or application developer, I want to be able to restrict access to services behind Contour by source IP address.

This issue covers:

  • identifying the exact use cases
  • designing a solution to cover the use cases
  • implementing that solution.

The original source issue for this was #62 (way back!), and we have a few things to decide, and I can think of a few things for us to consider.

Firstly, where and how could we implement this?

  • As an annotation on Ingress objects (like in #62 )
  • As a field in HTTPProxy
  • In the Gateway API (although this would also include designing something for upstream)

When implementing it, we need to make sure that we call out the interactions with upstream load balancers, X-Forwarded-For, and Client IPs etc, which can complicate this sort of thing a bit for proxies.

I suspect the answer here is that the IP address we care about is whatever Envoy thinks the Client IP is, but we will need to document how to be sure you have the right one a bit more than we do now, I think.

youngnick avatar May 19 '21 06:05 youngnick

Great idea. Just want to add we need to support both IPv4 and v6, specified in both single IPs and CIDR blocks. Another comment is we can either make this available as part of the existing spec.rateLimitPolicy or have this interaction with rate limiting possible. My preference, as with a lot of our features, is to have this configurable globally by default that cascades to all HTTPProxy objects in existence and able to set it per HTTPProxy that overrides global config.

Should we allow setting a deny list instead of an allow list? Or both.

@youngnick Regarding the options you listed, I think we have to think about adding these sorts of capabilities to both HTTPProxy and HTTPRoute in the meantime. The best we can do for anything in gateway API is to land a feature request in their upstream for now.

I moved this to v1.19, does that feel appropriate?

xaleeks avatar Jul 01 '21 16:07 xaleeks

When implementing it, we need to make sure that we call out the interactions with upstream load balancers, X-Forwarded-For, and Client IPs etc, which can complicate this sort of thing a bit for proxies.

I suspect the answer here is that the IP address we care about is whatever Envoy thinks the Client IP is, but we will need to document how to be sure you have the right one a bit more than we do now, I think.

Does the X-Forwarded-For not carry the source IP on it? I think we discussed this before as well, but an alternative is to leverage the HAProxy Proxy Protocol for this, and this would cover any TCP stream.

I also found this extension for retrieving the original IP address. Can we leverage this if the above aren’t sufficient?

https://www.envoyproxy.io/docs/envoy/latest/api-v3/extensions/filters/network/http_connection_manager/v3/http_connection_manager.proto#envoy-v3-api-field-extensions-filters-network-http-connection-manager-v3-httpconnectionmanager-original-ip-detection-extensions

xaleeks avatar Jul 01 '21 16:07 xaleeks

I think 1.19 for this is fine.

Yes, my concern about the source IPs was not that we can't retrieve them, but that there are a few ways to retrieve them, and we need to be specific about how the whole setup works, since there are interactions with upstream LBs as well.

youngnick avatar Jul 01 '21 23:07 youngnick

If we do enable this through a field on the HTTPProxy, how do we apply this globally so we don’t need to specify on every HTTPProxy and add annotation on each Ingress? It would be ideal to have the behavior of applying these settings globally by default when nothing is specified but it almost requires this to be specified in its own CR. Then you can associate this with httpproxies and ingresses as you wish.

xaleeks avatar Sep 20 '21 01:09 xaleeks

I think it depends on the expected use case. I can see use for each, since you may, for example, want to allow only some IPs to some secure service, but not to all ingresses managed by Contour. Or you may want to block all addresses in some set globally, and not allow that to be overridden.

Whatever we start with, we should assume that it's not final, but ideally, choosing what to start with would start with talking to people who actually want this feature about how it should work.

youngnick avatar Sep 20 '21 02:09 youngnick

Posted by @viju2008 on #4450:

Please describe the problem you have [A clear, concise, description of the problem you are facing. What is the problem that feature X would solve for you?]

Feature Request for adding feature to create IP whitelist or Deny list for the services accessed through Contour Ingress

Positive security model by implementing whitelist or negative security model using deny lists for IP Addresses.

We need IP whitelisting or denylisting based on X-Forwarded-For IP address

skriss avatar Apr 05 '22 13:04 skriss

+1 for the use case that requires a global deny and then individual HTTPProxy allow list. I've achieved this with other ingress controllers by setting the allow list in the global config to only 127.0.0.1 and then have the allow list for each Ingress annotated appropriately.

josh-ferrell avatar May 19 '22 17:05 josh-ferrell

I think 1.19 for this is fine.

Yes, my concern about the source IPs was not that we can't retrieve them, but that there are a few ways to retrieve them, and we need to be specific about how the whole setup works, since there are interactions with upstream LBs as well.

@youngnick Some other envoy based controllers such as emissary are using a semantic for it: https://www.getambassador.io/docs/edge-stack/latest/topics/running/ambassador/#ip-allow-and-deny

The keyword peer specifies that the match should happen using the IP address of the other end of the network connection carrying the request: X-Forwarded-For and the PROXY protocol are both ignored. Here, our example specifies that connections originating from the Ambassador Edge Stack pod itself should always be allowed. The keyword remote specifies that the match should happen using the IP address of the HTTP client, taking into account X-Forwarded-For and the PROXY protocol if they are allowed (if they are not allowed, or not present, the peer address will be used instead). This permits matches to behave correctly when, for example, Ambassador Edge Stack is behind a layer 7 load balancer. Here, our example specifies that HTTP clients from the IP address range 99.99.0.0 - 99.99.255.255 will be allowed.

This is also a must have feature for our organization to be able to migrate to contour.

m-yosefpor avatar Jul 22 '22 08:07 m-yosefpor

Thanks for that @m-yosefpor, that semantic does sound useful.

I'll speak to @xaleeks about what the prioritization for this feature looks like.

youngnick avatar Jul 25 '22 06:07 youngnick

Bump! Has this feature been prioritised? We're looking to migrate from Nginx Ingress Controller primarily for the Gateway API support and future direction of Contour towards Envoy Gateway. The lack of feature parity with Nginx Ingress is a major blocker for our migration.

We have not yet prioritized implementing this feature, but are very open to community contributions on design and implementation work to get this moving! @adityasundaramurthy-maersk if you have resources to help with this we can chat about how to get started

sunjayBhatia avatar Jan 05 '23 23:01 sunjayBhatia

Happy to help! How can I reach you? I may need some pointers to get started.

We can discuss on this issue or the #contour channel in the Kubernetes slack workspace

sunjayBhatia avatar Jan 05 '23 23:01 sunjayBhatia

@adityasundaramurthy-maersk @sunjayBhatia, what's the plan about this, we need it too

izturn avatar Jan 13 '23 07:01 izturn

@adityasundaramurthy-maersk @sunjayBhatia, what's the plan about this, we need it too

as of yet no-one has contributed a design for this feature so that would be the first step

sunjayBhatia avatar Jan 13 '23 16:01 sunjayBhatia

hey @adityasundaramurthy-maersk, any progress? if you don't have time, I'd like to give it a try

izturn avatar Jan 14 '23 16:01 izturn

@izturn I haven't done any work here. Please go ahead.

Just a note that this actively being worked on here: #4990 and #5008

bison avatar Mar 09 '23 10:03 bison