Add path validation rules for `pathType=ImplementationSpecific`

[jkroepke]

While the Exact and Prefix pathTypes are well-covered by strict-validate-path-type, allowing ImplementationSpecific poses a security risk.

Ingress objects with ImplementationSpecific enable potential code injection into the nginx configuration, creating a vulnerability where the service account token of the ingress-controller could be compromised. For example /api/ { return 200; #. To mitigate this, it is strongly advised to deny ImplementationSpecific ingress objects from untrusted users.

However, certain essential features like rewrite-target (as demonstrated in https://kubernetes.github.io/ingress-nginx/examples/rewrite/#rewrite-target) rely on the ImplementationSpecific pathType, leading to a deadlock scenario.

Since ImplementationSpecific typically involves regular expressions, devising a comprehensive validation regex becomes an ongoing challenge.

Instead, when utilizing the ImplementationSpecific pathType, consider validating the path using https://pkg.go.dev/regexp#Compile. This should at least prevent syntax errors.

Secondly, curly braces in regular expressions are primarily used to define a specific number of matches.

Source: Oracle Regular Expressions

A regex pattern like \{([^\d,]|$) could restrict the use of curly braces to specific contexts. Additionally, the hash character #, commonly employed for comments in nginx configurations, should also be prohibited. Restrict any invisible characters, excluding spaces? should be also restrict.

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

[longwuyuan]

• AFAIK, normal regex lib is PCRE but K8S uses RE2 https://kubernetes.github.io/ingress-nginx/user-guide/ingress-path-matching/#regular-expression-support
• An admin has to explicitly allow characters https://kubernetes.github.io/ingress-nginx/faq/#validation-of-path as per spec

[jkroepke]

AFAIK, normal regex lib is PCRE but K8S uses RE2 kubernetes.github.io/ingress-nginx/user-guide/ingress-path-matching/#regular-expression-support

Okay, if Kubernetes already does a regex validation, the idea could be drop.

• An admin has to explicitly allow characters kubernetes.github.io/ingress-nginx/faq/#validation-of-path as per spec

I'm I have a built-in solution in mind. Not everyone has the power or capability to setup Open Policy Agent or Kyverno. Before someone is going deeper with REGO, I would more recommend to take an alternative ingress controller.

I'm coming from https://kubernetes.github.io/ingress-nginx/faq/#validation-of-path, so I'm aware of the bullet points.

[rikatz]

@jkroepke thanks for this issue (and all of the other reports ;) )

Anything we can use to improve security on this case is acceptable. That said, can we work on something with https://kubernetes.io/docs/reference/access-authn-authz/validating-admission-policy/ so we don't need to rely on any external webhook?

[jkroepke]

While I was writing the issue, I had more in my mind to improve the validate inside ingress-nginx:

• https://github.com/kubernetes/ingress-nginx/blob/e78af97ecdf10187b43786df216478df6ea440d1/internal/ingress/inspector/rules.go#L25-L29
• https://github.com/kubernetes/ingress-nginx/blob/e78af97ecdf10187b43786df216478df6ea440d1/internal/ingress/inspector/inspector.go#L45-L65

and toggle them behind validate-strict-path, to have an opt-in unless we are on 2.0.0.

https://kubernetes.io/docs/reference/access-authn-authz/validating-admission-policy/ may fits, too. But its would be a strategy design, we future validation should done by Kubernetes it-self or nginx.

While ValidatingAdmissionPolicy sound great first, it my not work, if ingress-nginx runs on a namespace scope. ValidatingAdmissionWebhook, doesn't work on namespace scope, too. But ingress-nginx will reject invalid ingress objects without ValidatingAdmissionWebhook, too.

[github-actions[bot]]

This is stale, but we won't close it automatically, just bare in mind the maintainers may be busy with other tasks and will reach your issue ASAP. If you have any question or request to prioritize this, please reach #ingress-nginx-dev on Kubernetes Slack.

[jkroepke]

It's still an issue