search

kubernetes / ingress-nginx

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

acme-challenge blocked with 403 when auth-tls-match-cn is being used #10185

Closed mbettsteller closed 6 months ago

mbettsteller commented 1 year ago

What happened:

When using cert-manager to get an SSL cert in an ingress that also verifies the client with a client ssl cert and also uses auth-tls-match-cn the acme-challenge is blocked with a 403.

This does NOT happen when you do not use auth-tls-match-cn!

cert-manager(acme challenge ingress) + mTLS client auth = OK cert-manager(acme challenge ingress) + mTLS client auth + auth-tls-match-cn = NOK

What you expected to happen:

The location /.well-known/acme-challenge/redacted/ should not be blocked as 403, so the challenge could be answered even when the auth-tls-match-cn is set in the ingress.

What do you think went wrong?

When looking at the configuration that is being produced I see:

        if ( $ssl_client_s_dn !~ CN=(foo|bar|baz|) ) {
            return 403 "client certificate unauthorized";
        }

pretty early in the server section.

More detail:

server {
        server_name test.domain.tld ;

        listen 80  ;
        listen [::]:80  ;
        listen 443  ssl http2 ;
        listen [::]:443  ssl http2 ;

        set $proxy_upstream_name "-";

        if ( $ssl_client_s_dn !~ CN=(foo|bar|baz|) ) {
            return 403 "client certificate unauthorized";
        }

        ssl_certificate_by_lua_block {
            certificate.call()
        }

        # PEM sha: redacted
        ssl_client_certificate                  /etc/ingress-controller/ssl/ca-something-ca.pem;
        ssl_verify_client                       on;
        ssl_verify_depth                        2;

        # Custom code snippet configured for host test.domain.tld
        ssl_stapling on;
        ssl_stapling_verify on;

        location /.well-known/acme-challenge/redacted/ {

            set $namespace      "test";
            set $ingress_name   "cm-acme-http-solver-xxxxx";
            set $service_name   "cm-acme-http-solver-yyyyy";
            set $service_port   "8089";
            set $location_path  "/.well-known/acme-challenge/redacted";
            set $global_rate_limit_exceeding n;

            rewrite_by_lua_block {
                lua_ingress.rewrite({
                    force_ssl_redirect = false,
                    ssl_redirect = true,
                    force_no_ssl_redirect = true,
                    preserve_trailing_slash = false,
                    use_port_in_redirects = false,
                    global_throttle = { namespace = "", limit = 0, window_size = 0, key = { }, ignored_cidrs = { } },
                })
                balancer.rewrite()
                plugins.run()
            }

            # be careful with `access_by_lua_block` and `satisfy any` directives as satisfy any
            # will always succeed when there's `access_by_lua_block` that does not have any lua code doing `ngx.exit(ngx.DECLINED)`
            # other authentication method such as basic auth or external auth useless - all requests will be allowed.
            #access_by_lua_block {
            #}

            header_filter_by_lua_block {
                lua_ingress.header()
                plugins.run()
            }

            body_filter_by_lua_block {
                plugins.run()
            }

            log_by_lua_block {
                balancer.log()

                monitor.call()

                plugins.run()
            }

            port_in_redirect off;

            set $balancer_ewma_score -1;
            set $proxy_upstream_name "redacted";
            set $proxy_host          $proxy_upstream_name;
            set $pass_access_scheme  $scheme;

            set $pass_server_port    $server_port;

            set $best_http_host      $http_host;
            set $pass_port           $pass_server_port;

            set $proxy_alternative_upstream_name "";

            allow 0.0.0.0/0;
            allow ::/0;
            deny all;

            client_max_body_size                    1m;

            proxy_set_header Host                   $best_http_host;

            # Pass the extracted client certificate to the backend

            proxy_set_header ssl-client-verify      $ssl_client_verify;
            proxy_set_header ssl-client-subject-dn  $ssl_client_s_dn;
            proxy_set_header ssl-client-issuer-dn   $ssl_client_i_dn;

            # Allow websocket connections
            proxy_set_header                        Upgrade           $http_upgrade;

            proxy_set_header                        Connection        $connection_upgrade;

            proxy_set_header X-Request-ID           $req_id;
            proxy_set_header X-Real-IP              $remote_addr;

            proxy_set_header X-Forwarded-For        $remote_addr;

            proxy_set_header X-Forwarded-Host       $best_http_host;
            proxy_set_header X-Forwarded-Port       $pass_port;
            proxy_set_header X-Forwarded-Proto      $pass_access_scheme;
            proxy_set_header X-Forwarded-Scheme     $pass_access_scheme;

            proxy_set_header X-Scheme               $pass_access_scheme;

            # Pass the original X-Forwarded-For
            proxy_set_header X-Original-Forwarded-For $http_x_forwarded_for;

            # mitigate HTTPoxy Vulnerability
            # https://www.nginx.com/blog/mitigating-the-httpoxy-vulnerability-with-nginx/
            proxy_set_header Proxy                  "";

            # Custom headers to proxied server

            proxy_connect_timeout                   5s;
            proxy_send_timeout                      60s;
            proxy_read_timeout                      60s;

            proxy_buffering                         off;
            proxy_buffer_size                       4k;
            proxy_buffers                           4 4k;

            proxy_max_temp_file_size                1024m;

            proxy_request_buffering                 on;
            proxy_http_version                      1.1;

            proxy_cookie_domain                     off;
            proxy_cookie_path                       off;

            # In case of errors try the next upstream server before returning an error
            proxy_next_upstream                     error timeout;
            proxy_next_upstream_timeout             0;
            proxy_next_upstream_tries               3;

            proxy_pass http://upstream_balancer;

            proxy_redirect                          off;

        }

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

NGINX Ingress controller Release: v1.5.1 Build: d003aae913cc25f375deb74f898c7f3c65c06f05 Repository: https://github.com/kubernetes/ingress-nginx nginx version: nginx/1.21.6

Kubernetes version (use kubectl version):

$ kubectl version Client Version: version.Info{Major:"1", Minor:"25", GitVersion:"v1.25.10", GitCommit:"e770bdbb87cccdc2daa790ecd69f40cf4df3cc9d", GitTreeState:"clean", BuildDate:"2023-05-17T14:12:20Z", GoVersion:"go1.19.9", Compiler:"gc", Platform:"linux/amd64"} Kustomize Version: v4.5.7 Server Version: version.Info{Major:"1", Minor:"25+", GitVersion:"v1.25.10-eks-c12679a", GitCommit:"bbfa7e393476eb418f98a8c785721a006ba830cd", GitTreeState:"clean", BuildDate:"2023-05-22T20:31:17Z", GoVersion:"go1.19.9", Compiler:"gc", Platform:"linux/amd64"}

Environment:



**How to reproduce this issue**:

Create an application with an ingress that will get an SSL cert from any ACME provider (lets encrypt staging) and additionally activate client ssl cert authentication with CN checking.

**Anything else we need to know**:

https://github.com/kubernetes/ingress-nginx/blob/helm-chart-4.4.0/rootfs/etc/nginx/template/nginx.tmpl#L961 is producing the CN checking line in the nginx config file. This needs to be smartened to not kill acme challenge pod locations.

As a test take out the CN checking (you will get your cert!).

Caveat: When you need to retest, you might need to use a different URL because once you succeeded, cert-manager will take the cert from the local cache!
strongjz commented 1 year ago

/assign @rikatz /triage accepted /priority backlog

rikatz commented 1 year ago

Ok, so taking a look here, the issue is ssl_verify_client is a server directive, and we cannot disable it per location: https://nginx.org/en/docs/http/ngx_http_ssl_module.html#ssl_verify_client

An alternative you have is to set it as optional and deal with it on your backend.

When cert auth is enabled, the var https://nginx.org/en/docs/http/ngx_http_ssl_module.html#var_ssl_client_verify is passed to the backend (see https://github.com/kubernetes/ingress-nginx/blob/main/rootfs/etc/nginx/template/nginx.tmpl#L1180) so you can turn it optional, but on your backends deny the authentication.

We can set on the template a validation enforcement or a new type of validation called on-without-restrictions that tells us it is "optional", but should enforce denial if the ssl_client_verify is an error except on auth locations, but this would take a bit of time for me to implement right now.

So, my suggestion right now is to turn it optional and validate on backend, if this is possible

mbettsteller commented 1 year ago

Hi, thank you very much for looking into this.

We can set on the template a validation enforcement or a new type of validation called on-without-restrictions that tells us it is "optional", but should enforce denial if the ssl_client_verify is an error except on auth locations, but this would take a bit of time for me to implement right now.

So, my suggestion right now is to turn it optional and validate on backend, if this is possible

I will check back with our backend DEVs and see if that can be easily done. But it is vacation time, so might take a while :-). Will post later how we decided to go on.

Thanks!

github-actions[bot] commented 1 year ago

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.

bossm8 commented 6 months ago

We got the same problem, is there any information if this will be fixed? Maybe by reordering in the nginx template?

When having a nginx.ingress.kubernetes.io/auth-tls-match-cn annotation configured ingress-nginx responds with 403 client certificate unauthorized. When the annotation is commented out it works as @mbettsteller already pointed out.

bossm8 commented 6 months ago

@strongjz Would a change like this be accepted? It would be really great if this issue could be fixed. (tested and verified locally, as the problem is not the ssl_verify_client directive itself but the reject if the client CN does not match when the certificate issuer requests /.well-known/acme-challenge without a certificate obviously)

        {{ if not ( empty $server.CertificateAuth.MatchCN ) }}
        {{ if gt (len $server.CertificateAuth.MatchCN) 0 }}
        location ~ ^/(?!(\.well-known/acme-challenge)) {
            if ( $ssl_client_s_dn !~ {{ $server.CertificateAuth.MatchCN }} ) {
                return 403 "client certificate unauthorized";
            }
        }
        {{ end }}
        {{ end }}

https://github.com/kubernetes/ingress-nginx/blob/dc999d81da6d9258bf448874be5f1f0e2156ec94/rootfs/etc/nginx/template/nginx.tmpl#L986-L992

lkt82 commented 2 weeks ago

Hi

The fix for this issue was reverted in 11082

Is it possible to reopen as the issue is still present in the current release.