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

Changing nginx.ingress.kubernetes.io/whitelist-source-range error code #13105

Closed
DerekTBrown opened this issue Mar 28, 2025 · 5 comments
Closed
Labels
kind/feature Categorizes issue or PR as related to a new feature. needs-priority needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one.

Comments

@DerekTBrown
Copy link

Context

  • ingress-nginx supports an annotation, nginx.ingress.kubernetes.io/whitelist-source-range (docs), which allows the operator to require requests to come from a specific set of IP addresses.
  • If a request comes from outside this range, ingress-nginx returns a 403 status code (ref).
    • Under the hood, this is implemented via the ngx_http_access_module (ref).

Why is this problematic?

  • A 403 error code communicates that the client lacks permissions for a "particular resource, or action" (ref). However, clients will continue to operate under the assumption that requests for a different resource or action may succeed.
  • This may lead to unexpected behavior:
    1. Clients might continually retry their requests, mistakenly believing that with additional credentials or different parameters they might eventually gain access, even though the underlying IP block will persist.
      2. Clients may mishandle/misreport these error messages. For example, a multi-tenant service may treat 4xx errors as being caused by its clients, rather than a misconfiguration of the service itself.
      3. Operators may have a difficult time investigating such issues, since it is ambiguous whether the error is caused by the proxy layer or the service itself.

What is the proper status code?

I believe the correct behavior is for nginx to return a 511 code, which is tailored for this case:

The HTTP 511 Network Authentication Required server error response status code indicates that the client needs to authenticate to gain network access. This status is not generated by origin servers, but by intercepting proxies that control access to a network.

Ref: https://developer.mozilla.org/en-US/docs/Web/HTTP/Reference/Status/511

How could this fix be applied?

Changing from 403 -> 511 is doubtless a breaking change for clients. Thus, I propose allowing this to be user-configurable, defaulting to existing behavior:

  • Step 1: Extend ngx_http_access_module to support custom code:
deny all;              # defaults to 403 Forbidden
deny 192.168.1.0/24 code=511;  # returns 511 Unauthorized for this range
  • Step 2: Make this option configurable within ingress-nginx:
annotations:
       nginx.ingress.kubernetes.io/whitelist-source-range-deny-code: 511
@DerekTBrown DerekTBrown added the kind/bug Categorizes issue or PR as related to a bug. label Mar 28, 2025
@k8s-ci-robot k8s-ci-robot added the needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. label Mar 28, 2025
@k8s-ci-robot
Copy link
Contributor

This issue is currently awaiting triage.

If Ingress contributors determines this is a relevant issue, they will accept it by applying the triage/accepted label and provide further guidance.

The triage/accepted label can be added by org members by writing /triage accepted in a comment.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

@longwuyuan
Copy link
Contributor

/remove-kind bug
/kind feature

@k8s-ci-robot k8s-ci-robot added kind/feature Categorizes issue or PR as related to a new feature. and removed kind/bug Categorizes issue or PR as related to a bug. labels Mar 29, 2025
@Gacko
Copy link
Member

Gacko commented Mar 30, 2025

The ngx_http_access_module is maintained by NGINX project, not here. We only use NGINX as the underlying webserver and do not change its functionality. I'd recommend discussing this with their community first.

@DerekTBrown
Copy link
Author

Apparently this is supported on their end, just not in ingress-nginx:

nginx/nginx#610 (comment)

@DerekTBrown
Copy link
Author

@Gacko can you re-open?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/feature Categorizes issue or PR as related to a new feature. needs-priority needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one.
Projects
Development

No branches or pull requests

4 participants