enabling annotation validation prevents deployment of previously allowed permanent redirect annotations

[joshsleeper]

What happened:

after adding the --enable-annotation-validation option to our ingress-nginx controller deployment to mitigate recent CVEs including #10572 we encountered some deploy-time issues with historically fine deployments.

specifically, helm deployments containing an Ingress resource with the nginx.ingress.kubernetes.io/permanent-redirect annotation containing a capture group failed to deploy with the following (sanitized) error:

Upgrade "SNIPPED" failed: cannot patch "SNIPPED" with kind Ingress: admission webhook "validate.nginx.ingress.kubernetes.io" denied the request: annotation nginx.ingress.kubernetes.io/permanent-redirect contains invalid value

What you expected to happen:

I expected the helm deployments to succeed as they historically had prior to setting the --enable-annotation-validation option

NGINX Ingress controller version (exec into the pod and run nginx-ingress-controller --version.):

nginx-ingress-controller-7fc847c578-6jkbj:/etc/nginx$ /nginx-ingress-controller --version
-------------------------------------------------------------------------------
NGINX Ingress controller
  Release:       v1.9.4
  Build:         846d251814a09d8a5d8d28e2e604bfc7749bcb49
  Repository:    https://github.com/kubernetes/ingress-nginx
  nginx version: nginx/1.21.6

-------------------------------------------------------------------------------

Kubernetes version (use kubectl version):

$ kubectl version
Client Version: v1.28.3
Kustomize Version: v5.0.4-0.20230601165947-6ce0bf390ce3
Server Version: v1.25.10-gke.2700
WARNING: version difference between client (1.28) and server (1.25) exceeds the supported minor version skew of +/-1

Environment:

• Cloud provider or hardware configuration:

  GKE on Google Cloud, but that shouldn't be relevant to the issue

• OS (e.g. from /etc/os-release):

  N/A

• Kernel (e.g. uname -a):

  N/A

• Install tools:

  • Please mention how/where was the cluster created like kubeadm/kops/minikube/kind etc.

  N/A

• Basic cluster related info:

  • kubectl version
  • kubectl get nodes -o wide

  N/A

• How was the ingress-nginx-controller installed:

  • If helm was used then please show output of helm ls -A | grep -i ingress
  • If helm was used then please show output of helm -n <ingresscontrollernamepspace> get values <helmreleasename>
  • If helm was not used, then copy/paste the complete precise command used to install the controller, along with the flags and options used
  • if you have more than one instance of the ingress-nginx-controller installed in the same cluster, please provide details for all the instances

  helm 3.12+, but pretty much positive this isn't an issue with how we installed/configured anything.

• Current State of the controller:

  • kubectl describe ingressclasses
  • kubectl -n <ingresscontrollernamespace> get all -A -o wide
  • kubectl -n <ingresscontrollernamespace> describe po <ingresscontrollerpodname>
  • kubectl -n <ingresscontrollernamespace> describe svc <ingresscontrollerservicename>

  N/A

• Current state of ingress object, if applicable:

  • kubectl -n <appnnamespace> get all,ing -o wide
  • kubectl -n <appnamespace> describe ing <ingressname>
  • If applicable, then, your complete and exact curl/grpcurl command (redacted if required) and the reponse to the curl/grpcurl command with the -v flag

  N/A

• Others:

  • Any other related information like ;
  • copy/paste of the snippet (if applicable)
  • kubectl describe ... of any custom configmap(s) created and in use
  • Any other related information that may help

  N/A

How to reproduce this issue:

given any ingress-nginx deployment with the --enable-annotation-validation option provided to the controller container.

given the official ingress-nginx helm chart this can be done by setting controller.enableAnnotationValidations: true in the values.

pared-down example of an affected Ingress resource:

apiVersion: networking.k8s.io/v1
kind: Ingress
metadata:
  annotations:
    nginx.ingress.kubernetes.io/permanent-redirect: https://new-example.com/$2
    nginx.ingress.kubernetes.io/use-regex: "true"
  name: SNIPPED
  namespace: SNIPPED
spec:
  ingressClassName: nginx
  rules:
  - host: old-example.com
    http:
      paths:
      - backend:
          service:
            name: old-example
            port:
              number: 8080
        path: /demo($|/)(.*)
        pathType: ImplementationSpecific

attempting to kubectl apply a resource like above should trigger a validation error and reject the resource.

Anything else we need to know:

• issue that motivated the change: #10572
• MR that implemented the affected feature: #9673
• relevant sections of code to the issue at hand
  • where the validation is happening: https://github.com/kubernetes/ingress-nginx/blob/da51393cac818545de796d3c3486c40127aa51f4/internal/ingress/annotations/redirect/redirect.go#L64
  • the validation pattern currently used: https://github.com/kubernetes/ingress-nginx/blob/da51393cac818545de796d3c3486c40127aa51f4/internal/ingress/annotations/parser/validators.go#L62

this could just be a documentation issue if the maintainer's opinion is that supporting capture groups in the redirect annotation isn't something they wish to do, in which case just documenting that is probably the desired outcome.

if that is supposed to be supported, then I believe the desired outcome is to correct the validation pattern used when the annotation validation is enabled.

[k8s-ci-robot]

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/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository.

[channaneq]

I am currently running into a similar issue with the enable-annotation-validation flag just not with the same annotation.

I am trying to get around this issue by running nginx in chroot mode, meaning you don't have to use the enable-annotation-validation flag, assuming that you are making use of this flag in response to the recent nginx vulnerabilities CVE-2023-5043 and CVE-2023-5044. This explains that chroot mode mitigates the vulnerability from a secrets exploitation perspective https://github.com/kubernetes/ingress-nginx/issues/10571#issue-1961738338

I am testing this out at the moment myself, but if you don't want to bother with it yourself I think a redesign of how that annotation is setup (ie the dollar sign being present) should get around your issue

[bmv126]

@channaneq Can you share details on which annotations caused issue for you

[channaneq]

nginx.ingress.kubernetes.io/auth-signin

annotation auth-signin contains invalid value

[eaterm]

We are also running into the same issue with annotation:nginx.ingress.kubernetes.io/session-cookie-samesite: None

We always see the error: validators.go:221] validation error on ingress <namespace/<ingress>: annotation session-cookie-samesite contains invalid value None

Tested this with all 3 values and all gave the same result.

[siegenthalerroger]

Concerning the permanent-redirect I think I've narrowed down the regression in behaviour down to an issue with the Validator not being given capture groups (unlike the rewrite-target annotation which does get these, hence why it works). I've opened another issue for this here: https://github.com/kubernetes/ingress-nginx/issues/10734

Hopefully we hear back

[sdickhoven]

@channaneq wrote:

nginx.ingress.kubernetes.io/auth-signin

annotation auth-signin contains invalid value

...specifically:

nginx.ingress.kubernetes.io/auth-signin: "https://$host/oauth2/start?rd=$escaped_request_uri"

which is what is used in the example here:

https://kubernetes.github.io/ingress-nginx/examples/auth/oauth-external-auth/

i think the current incarnation of annotation validation simply rejects anything with a $ in it. that's not a very useful validation imo.

there are variables that should absolutely be considered "safe" by this validation. e.g.

• $host
• $request_uri
• $service_name
• $service_port
• $namespace
• and many more