Regex in proxy-redirect-from and proxy-redirect-to

[suvl]

I'm using a complex proxy_redirect instruction directly in nginx and it works, it changes the Location header so that clients are redirected to an ingress for direct connection to a specific statefulset instance. The instruction is this:

proxy_redirect ~^.*\/\/pod-(?<pod_nr>\d+?)(\..*\.svc\.cluster\.local)?(:\d*)?(?<pod_path>\/.*)?$ https://p-$pod_nr.project.example.com$pod_path;

This worked for years but we're moving to using ingress-nginx instead of managing nginx configs. I'm now trying to create the same thing using annotations, so tried the following:

metadata:
  annotations:
    nginx.ingress.kubernetes.io/proxy-redirect-from: '~*^.*\/\/pod-(?<pod_nr>\d+?)(\..*\.svc\.cluster\.local)?(:\d*)?(?<pod_path>\/.*)?$'
    nginx.ingress.kubernetes.io/proxy-redirect-to: "https://p-$pod_nr.project.example.com$pod_path"

However I get these two errors in the logs:

ingress-nginx-controller-59d9446fd6-fv265 controller W1129 16:43:09.772362       7 validators.go:221] validation error on ingress ns/pod-ingress: annotation proxy-redirect-from contains invalid value ~*^.*\/\/pod-(?<pod_nr>\d+?)(\..*\.svc\.cluster\.local)?(:\d*)?(?<pod_path>\/.*)?$
ingress-nginx-controller-59d9446fd6-fv265 controller W1129 16:43:09.773233       7 validators.go:221] validation error on ingress ns/pod-ingress: annotation proxy-redirect-to contains invalid value https://p-$pod_nr.project.example.com$pod_path

Now, I need guidance on why this is being refused while working perfectly in nginx and how to fix it. Tried to use a nginx.ingress.kubernetes.io/configuration-snippet directly but then I got:

nginx: [emerg] "proxy_redirect" directive is duplicate in /tmp/nginx/nginx-cfg3481804691:726

as the ingress controller always inserts a proxy_redirect off; instruction and that is not possible to disable.

Without editing the ingress template directly, how can I implement this?

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

[suvl]

I can see from https://github.com/kubernetes/ingress-nginx/blob/main/internal/ingress/annotations/parser/validators.go#L62 and https://github.com/kubernetes/ingress-nginx/blob/main/internal/ingress/annotations/parser/validators.go#L44 , https://github.com/kubernetes/ingress-nginx/blob/main/internal/ingress/annotations/parser/validators.go#L47 that

only -._~a-zA-Z0-9/:?&= chars are allowed in this snippet. Why is that? The nginx configuration has no such restriction.

The nginx documentation for the ngx_http_proxy_module states in here that

The directive can be specified (1.1.11) using regular expressions. In this case, redirect should either start with the “\~” symbol for a case-sensitive matching, or with the “\~*” symbols for case-insensitive matching. The regular expression can contain named and positional captures, and replacement can reference them

So, is there a reason we're expecting a URL in here? Can we also accept a full regex with capture groups?

[longwuyuan]

• Please try to answer the questions asked in a new issue template so that the use-case details are clear to a reader
• Try to see if this is related to your use case https://kubernetes.github.io/ingress-nginx/user-guide/nginx-configuration/configmap/#allow-snippet-annotations

[longwuyuan]

/triage needs-information

[longwuyuan]

/remove-kind bug

[suvl]

@longwuyuan Sorry, not sure what you meant. If I understood correctly, you want me to close this issue and open a new one using a template?

Also, do not agree that this is not a bug. I still believe it is a bug, as the nginx configuration allows for something that is not being allowed via these annotations.

The "allow snippet annotations" is not helpful, as one cannot disable via configuration that a proxy_redirect off; instruction always gets printed for every location, so inserting a configuration snippet will not work for this purpose.

[opencmit2]

Hi @suvl I can't reproduce the problem you stated using your method.

  annotations:
    nginx.ingress.kubernetes.io/proxy-redirect-from: '~*^.*\/\/pod-(?<pod_nr>\d+?)(\..*\.svc\.cluster\.local
)?(:\d*)?(?<pod_path>\/.*)?$'
    nginx.ingress.kubernetes.io/proxy-redirect-to: "https://p-$pod_nr.project.example.com$pod_path"

nginx.config

image version: ingress-nginx/controller:v1.6.4

[suvl]

Hey there @opencmit2, do you have

controller:
  extraArgs:
    enable-annotation-validation: "true"
  config:
    strict-validate-path-type: "true"

in your install values? As required to fix the latest CVEs, right?

[suvl]

@opencmit2 just tested it with version 4.8.3, this is what I get in the logs:

public-nginx-ingress-nginx-controller-799f9bcd97-8695t controller W0429 16:08:10.821757       6 validators.go:221] validation error on ingress webpa-staging/petasos-external: annotation proxy-redirect-from contains invalid value ~.*talaria-(?<talaria_host>\d*)(\.talaria.webpa-staging)?(\:\d+)?(?<talaria_path>\/.*)?$
public-nginx-ingress-nginx-controller-799f9bcd97-8695t controller W0429 16:08:10.821790       6 validators.go:221] validation error on ingress webpa-staging/petasos-external: annotation proxy-redirect-to contains invalid value https://talaria-$talaria_host.webpa-staging.oizys.clg.nos.pt$talaria_path

Only when I set enable-annotation-validation: "false" does this work.

So I call this a bug. The annotation has a valid value, the regex compiles and is somewhat sane. I cannot feel safe setting this to false in this, multi-tenant, cluster, so I really think this validation should be fixed.

edit: latest 4.10.1 version also has this problem.

[suvl]

Just checked https://github.com/kubernetes/ingress-nginx/blob/main/internal/ingress/annotations/parser/validators.go#L64

It now accepts a regex like

^[\-\.\_\~a-zA-Z0-9\/:?&=]*$

For this to work none of the regexes in the file would do.

The closest is the one from IsValidRegex but not quite close. For instance, it fails to have the ~ character for specifying case sensitive or insensitive matching. But we could maybe do without it, probably. For named capture groups (that are reused later in proxy-redirect-to) it is still missing the <> characters, though.

So the working regex would be:

^[\-\.\_\~a-zA-Z0-9\/:^$\[\]\(\)\{\}*+?|&=\\<>]+$

[longwuyuan]

cc @rikatz @tao12345666333 corner-case combo of regex in proxy-redirect-from and proxy-redirect-ro annotations