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 IP allowlist and blocklist functionality #3693

Closed
youngnick opened this issue May 19, 2021 · 18 comments · Fixed by #5008
Closed

Add IP allowlist and blocklist functionality #3693

youngnick opened this issue May 19, 2021 · 18 comments · Fixed by #5008
Labels
area/httpproxy Issues or PRs related to the HTTPProxy API. kind/feature Categorizes issue or PR as related to a new feature. priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release.
Milestone

Comments

@youngnick
Copy link
Member

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?

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 youngnick added kind/feature Categorizes issue or PR as related to a new feature. lifecycle/needs-triage Indicates that an issue needs to be triaged by a project contributor. labels May 19, 2021
@youngnick youngnick added area/operational Issues or PRs about making Contour easier to operate as a production service. blocked/needs-info Categorizes the issue or PR as blocked because there is insufficient information to advance it. blocked/needs-product Categorizes the issue or PR as blocked because it needs a decision from product management. labels May 19, 2021
@xaleeks
Copy link

xaleeks commented Jul 1, 2021

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
Copy link

xaleeks commented Jul 1, 2021

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

@youngnick
Copy link
Member Author

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.

@stevesloka stevesloka added blocked/needs-design Categorizes the issue or PR as blocked because it needs a design document. and removed lifecycle/needs-triage Indicates that an issue needs to be triaged by a project contributor. labels Jul 9, 2021
@xaleeks
Copy link

xaleeks commented Sep 20, 2021

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.

@youngnick
Copy link
Member Author

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.

@skriss
Copy link
Member

skriss commented Apr 5, 2022

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 skriss modified the milestones: 1.21.0, 1.22.0 May 3, 2022
@josh-ferrell
Copy link

+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.

@skriss skriss modified the milestones: 1.22.0, 1.23.0 Jul 21, 2022
@m-yosefpor
Copy link
Contributor

m-yosefpor commented Jul 22, 2022

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.

@youngnick
Copy link
Member Author

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

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

@sunjayBhatia
Copy link
Member

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

@izturn
Copy link
Member

izturn commented Jan 13, 2023

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

@sunjayBhatia
Copy link
Member

@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

@izturn
Copy link
Member

izturn commented Jan 14, 2023

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

@skriss skriss added this to the 1.25.0 milestone Jan 24, 2023
@skriss skriss moved this to In Progress in Contour Jan 24, 2023
ecordell added a commit to ecordell/contour that referenced this issue Jan 25, 2023
Configures Envoy's envoy.filters.http.rbac per
route via HTTPProxy.

See API docs in this commit for details.

Fixes projectcontour#3693

Signed-off-by: Evan Cordell <[email protected]>
ecordell added a commit to ecordell/contour that referenced this issue Jan 25, 2023
Configures Envoy's envoy.filters.http.rbac per
route via HTTPProxy.

See API docs in this commit for details.

Fixes projectcontour#3693

Signed-off-by: Evan Cordell <[email protected]>
ecordell added a commit to ecordell/contour that referenced this issue Jan 25, 2023
Configures Envoy's envoy.filters.http.rbac per
route via HTTPProxy.

See API docs in this commit for details.

Fixes projectcontour#3693

Signed-off-by: Evan Cordell <[email protected]>
ecordell added a commit to ecordell/contour that referenced this issue Jan 25, 2023
Configures Envoy's envoy.filters.http.rbac per
route via HTTPProxy.

See API docs in this commit for details.

Fixes projectcontour#3693

Signed-off-by: Evan Cordell <[email protected]>
ecordell added a commit to ecordell/contour that referenced this issue Jan 26, 2023
Configures Envoy's envoy.filters.http.rbac per
route via HTTPProxy.

See API docs in this commit for details.

Fixes projectcontour#3693

Signed-off-by: Evan Cordell <[email protected]>
ecordell added a commit to ecordell/contour that referenced this issue Jan 26, 2023
Configures Envoy's envoy.filters.http.rbac per
route via HTTPProxy.

See API docs in this commit for details.

Fixes projectcontour#3693

Signed-off-by: Evan Cordell <[email protected]>
ecordell added a commit to ecordell/contour that referenced this issue Jan 26, 2023
Configures Envoy's envoy.filters.http.rbac per
route via HTTPProxy.

See API docs in this commit for details.

Fixes projectcontour#3693

Signed-off-by: Evan Cordell <[email protected]>
ecordell added a commit to ecordell/contour that referenced this issue Jan 31, 2023
Configures Envoy's envoy.filters.http.rbac per
route via HTTPProxy.

See API docs in this commit for details.

Fixes projectcontour#3693

Signed-off-by: Evan Cordell <[email protected]>
@adityasundaramurthy-maersk

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

@bison
Copy link

bison commented Mar 9, 2023

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

ecordell added a commit to ecordell/contour that referenced this issue Mar 20, 2023
Configures Envoy's envoy.filters.http.rbac per
route via HTTPProxy.

See API docs in this commit for details.

Fixes projectcontour#3693

Signed-off-by: Evan Cordell <[email protected]>
@skriss skriss removed blocked/needs-info Categorizes the issue or PR as blocked because there is insufficient information to advance it. blocked/needs-product Categorizes the issue or PR as blocked because it needs a decision from product management. blocked/needs-design Categorizes the issue or PR as blocked because it needs a design document. labels Mar 21, 2023
ecordell added a commit to ecordell/contour that referenced this issue Mar 31, 2023
Configures Envoy's envoy.filters.http.rbac per
route via HTTPProxy.

See API docs in this commit for details.

Fixes projectcontour#3693

Signed-off-by: Evan Cordell <[email protected]>
@skriss skriss added area/httpproxy Issues or PRs related to the HTTPProxy API. and removed area/operational Issues or PRs about making Contour easier to operate as a production service. labels Apr 12, 2023
ecordell added a commit to ecordell/contour that referenced this issue Apr 17, 2023
Configures Envoy's envoy.filters.http.rbac per
route via HTTPProxy.

See API docs in this commit for details.

Fixes projectcontour#3693

Signed-off-by: Evan Cordell <[email protected]>
ecordell added a commit to ecordell/contour that referenced this issue Apr 21, 2023
Configures Envoy's envoy.filters.http.rbac per
route via HTTPProxy.

See API docs in this commit for details.

Fixes projectcontour#3693

Signed-off-by: Evan Cordell <[email protected]>
skriss pushed a commit that referenced this issue Apr 25, 2023
Configures Envoy's envoy.filters.http.rbac per
route via HTTPProxy.

See API docs in this commit for details.

Fixes #3693

Signed-off-by: Evan Cordell <[email protected]>
@github-project-automation github-project-automation bot moved this from In Progress to Done in Contour Apr 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/httpproxy Issues or PRs related to the HTTPProxy API. kind/feature Categorizes issue or PR as related to a new feature. priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release.
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

10 participants