kubernetes / ingress-nginx

Ingress NGINX Controller for Kubernetes
https://kubernetes.github.io/ingress-nginx/
Apache License 2.0
17.57k stars 8.27k forks source link

Predefined custom snippets #12222

Open tylermichael opened 1 month ago

tylermichael commented 1 month ago

What do you want to happen?

A lot of NGINX functionality is locked behind using the custom snippet feature, but it is associated with a CVE. This means that in many environments, you cannot enable it.

To bridge this divide, I think a feature somewhat similar to what's discussed in #11259 should be added that allows the admin operator to predefine custom snippets, and then opt-in an ingress to use them. AFAICT, this is an improvement on the current custom snippets because it limits the surface area where arbitrary code could be introduced to the ingress controller config.

The admin operator could then add the config files that define these snippets to CODEOWNERS to prevent malicious changes.

This comment describes how I think it could work:

apiVersion: v1
kind: ConfigMap
metadata:
  name: ingress-nginx-controller
  namespace: ingress-nginx
data:
  server-snippets: # ...
  configuration-snippets: |
    - name: "add-custom-headers"
      snippet: |
          add_header Foo "Bar";
          add_header X-My-Custom-Header "1234";
    - name: "do-not-proxy-cookies"
      snippet: |
          proxy_set_header Cookie "";
---
apiVersion: networking.k8s.io/v1
kind: Ingress
metadata:
  annotations:
    nginx.ingress.kubernetes.io/enable-configuration-snippets: "add-custom-headers,do-not-proxy-cookies"
  name: foo
  namespace: default
spec:
  ingressClassName: ingress-nginx
  rules:
  - host: foo.example.com
    http:
      paths:
      - path: /
        pathType: Prefix
        backend:
          service:
            name: backend
            port: 
              number: 80

Related issue

11667

k8s-ci-robot commented 1 month ago

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](https://git.k8s.io/community/contributors/guide/pull-requests.md). If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes-sigs/prow](https://github.com/kubernetes-sigs/prow/issues/new?title=Prow%20issue:) repository.
longwuyuan commented 1 month ago

@tylermichael completely agree on the useful of your suggestion. It is actually a demand by many other users as well.

But the flip side is that contributions like your suggestions & other features that are also super useful need maintenance. Relevance being the resource crunch of developers dedicated to supporting & maintaining is so bad that we are actually deprecating useful functioning popular features. So I would not expect any traction on this feature request.

tylermichael commented 1 month ago

@longwuyuan Understood. I wouldn't mind creating a PR to add support for this as it's a bit of a blocker in my org.

longwuyuan commented 1 month ago
tylermichael commented 1 month ago

@longwuyuan That proxy-set-headers config sets the configured headers on all requests for all ingresses. There's currently no way to set proxy headers per ingress.

The way I view this feature is, it could actually reduce dev workload because as mentioned in many other issues, there are a lot of NGINX features that are not configurable via annotations. And now that custom snippets are informally deprecated (there are even talks to formally deprecate and remove them), this provides a more secure escape hatch, removing the need to add explicit support for functionality that can be accomplished via snippets. AFAICT, the CVE exists because arbitrary code can be added on any ingress, breaking isolation principles, but this proposal moves the definition of snippets to a trusted configuration file.

But I will wait for others to comment before starting a PR. I appreciate you and the other maintainers continued support of the project!