search

projectcontour / contour

Contour is a Kubernetes ingress controller using Envoy proxy.
https://projectcontour.io
Apache License 2.0
3.71k stars 673 forks source link

Unable to use contour for typical wildcard TLS setup: `IngressSpec.tls.secretName` seems to be required, proxy protocol support is not working #4556

Open artiommocrenco opened 2 years ago

artiommocrenco commented 2 years ago

What steps did you take and what happened: Not willing to use contour CRDs.

Using the same networking.k8s.io/v1 Ingress resources that worked with nginx-ingress. Expected them to work with contour.

I am testing out bitnami/contour chart with the following helm values:

contour:
  extraArgs:
    - --use-proxy-protocol
configInline:
  network:
    num-trusted-hops: 1
  tls:
    fallback-certificate:
      name: wildcard-tls
      namespace: contour

where wildcard-tls secret is created by cert-manager for *.example.com

I have Ingress with similar IngressSpec:

rules:
- host: app.example.com
  http:
    paths:
    - backend:
        service:
          name: app-ui
          port:
            name: http
      path: /
      pathType: Prefix
tls:
- hosts:
  - app.example.com

This produces the following:

  1. unresolved secret reference contour error
time="2022-06-06T12:06:34Z" level=error msg="unresolved secret reference" context=IngressProcessor error="Secret not found" name=app-ui namespace=app secret=app/
  1. envoy.http.original_ip_detection.xff envoy error
[2022-06-06 11:51:46.111][1][warning][config] [source/common/config/grpc_subscription_impl.cc:126] gRPC config for type.googleapis.com/envoy.config.listener.v3.Listener rejected: Error adding/updating listener(s) ingress_http: Didn't find a registered implementation for name: 'envoy.http.original_ip_detection.xff'
  1. Envoy HTTPS endpoints are not starting at all, only HTTP endpoints do (and only if I remove configInline.network.num-trusted-hops setting)

What did you expect to happen:

  1. Not specifying IngressSpec.tls.secretName should not produce a log message of type error. IngressSpec.tls.secretName is optional as per Kubernetes API Reference.
  2. There should not be any envoy.http.original_ip_detection.xff envoy-related errors
  3. Envoy HTTPS endpoints must be functional and use the configured wildcard certificate as per fallback-certificate
  4. App must then be available both via http://app.example.com/ and https://app.example.com/

Environment:

sunjayBhatia commented 2 years ago

envoy.http.original_ip_detection.xff envoy error

This was fixed in the most recent version of Contour so will require an upgrade: https://github.com/projectcontour/contour/releases/tag/v1.21.0

Fix improper use of OriginalIPDetectionFilter in HTTPConnectionManager. Reverts back to XffNumTrustedHops setting which was un-deprecated in Envoy 1.20.
sunjayBhatia commented 2 years ago

Unfortunately we never prioritized/implemented fallback integration with Ingress: https://github.com/projectcontour/contour/issues/2453

if this is a critical feature for you, we can prioritize this or take a PR if you are willing to implement it!

sunjayBhatia commented 2 years ago

@artiommocrenco hey there, @tsaarni brought up an interesting point rereading the Ingress spec in this comment

The description for leaving field Ingress.spec.tls.secretName empty says "Field is left optional to allow TLS routing based on SNI hostname alone" so I wonder if that option is really meant for TLS passthrough?

However from your example it looks like you would rather expect to match the nginx behavior and have Contour/Envoy serve the fallback cert when an Ingress does not specify a TLS secret? Just wanted to clarify what ux people would expect here

tsaarni commented 2 years ago

I searched some historical discussion from Kubernetes github about empty secretName and found these:

This seems bit under specified API, but in my opinion the little evidence there is points towards this being meant to be used for TLS passthrough.

youngnick commented 2 years ago

One of the biggest problems with the Ingress spec has always been how under-specced it is, so I'm not surprised to find that this behavior is the same.

It seems like the issue for @artiommocrenco is that Contour doesn't work like ingress-nginx here, and that having it serve the fallback when the secret is missing is the request?

I think that, in the absence of a clear spec "Do what ingress-nginx does for Ingress" is probably the right play, since it is the defacto default Ingress controller.

tl;dr If we're going to support empty secret references in Ingress, I think it should supply the fallback certificate if one is configured, and not generate any configuration if one is not. That's the best we can do.

(Also note that this type of problem is why we are trying so hard to specify everything fully in Gateway API.)

sunjayBhatia commented 2 years ago

yeah I agree with the above

sunjayBhatia commented 2 years ago

another option to the implementation in https://github.com/projectcontour/contour/pull/4595 would be to have the value in the annotation projectcontour.io/enable-fallback-certificate be a comma separated list of hostnames that the fallback would be applied to, to give some more granular control over things, rather than enabling fallback support on all hosts configured the Ingress resource (this would be even more similar to HTTPProxy since we have a per-vhost TLS configuration there to enable fallback)

sunjayBhatia commented 2 years ago

@artiommocrenco Hey there, have you had a chance to take a look at the discussion above and do you have any thoughts on how you expect things to work?

Contour's "fallback cert" was not intended to be a catchall/wildcard TLS cert the server presents in general to any request but rather specifically to cover the case of old TLS clients who misconfigure or don't include an SNI in the connection details. We can possibly stretch the meaning of this cert if need be (to basically match nginx ingress since the Ingress spec itself is a little under-specified), but would want to make sure the UX of what we provide (options described in a few comments above) makes sense.

github-actions[bot] commented 2 years ago

The Contour project currently lacks enough contributors to adequately respond to all Issues.

This bot triages Issues according to the following rules:

You can:

Please send feedback to the #contour channel in the Kubernetes Slack

github-actions[bot] commented 1 year ago

The Contour project currently lacks enough contributors to adequately respond to all Issues.

This bot triages Issues according to the following rules:

You can:

Please send feedback to the #contour channel in the Kubernetes Slack

artiommocrenco commented 1 year ago

Sorry for not commenting for a long time here. I thought that I gave enough information and references, but seeing this issue stale/closed, I will try post here again.

@sunjayBhatia

Just wanted to clarify what ux people would expect here

As specified in Kubernetes API reference: https://kubernetes.io/docs/reference/kubernetes-api/service-resources/ingress-v1/#IngressSpec

Field is left optional to allow TLS routing based on SNI hostname alone.

What would that mean? You can omit this field and not specify it, while certificate will be taken from some other source.

Rationale (UX-wise)

Minimize the number of certs when having multiple virtual hosts

When you have many virtual hosts (or ingress rules), it is harder to manage certificates separately for each.

It is easier to use wildcard certificates. You can have one wildcard certificate per entire organization that is uploaded to a central secret storage like hashicorp vault every time it is issued. From there you it can be automatically uploaded to Kubernetes for use both with and without contour, and to non-Kubernetes workloads. Or something like cert-manager could be used for certificate management.

As a user, I want to be able to use one certificate for many different Ingress objects, without using CRDs. It can be called fallback cert, wildcard cert, common cert, - does not really make much difference for me. Aside from wildcard certificates, you can have a certificate that has multiple SAN entries (wildcard or not). These must also be usable even when secretName is left empty)

Avoid using CRDs, support interoperability as designed by Kubernetes

While CRDs are cool and may provide what I tried to achieve here, using them implies coupling with a specific vendor or certain implementation-specific logic.

As a user, I want to use native Kubernetes ingress resources and be able to switch seamlessly to and from different ingress controllers (speaking of basic functionality). Annotations must not be necessary to consume Kubernetes API in an expected (described) way, meaning that basic use-cases like this one should work without annotations. But annotations can be used to enhance functionality that is expected and already functional.

Make information gathering harder (security)

Having separate certificates for each virtual host means each such certificate would end up in certificate transparency logs.

Often wildcard certs are used for multiple environments (dev, stage, prod, ..) and having non-prod or even prod DNS names in certificate transparency logs is undesirable. (Take this as a bad example https://crt.sh/?q=%25.apple.com)

Do not repeat yourself (DRY), single source of truth (SPOT)

Having certificate defined in one place is sometimes more correct than having it defined in each Ingress object.

When you have hundreds+ Ingress objects and you want to rename the secret that holds the certificate, it becomes easier when it is defined as a sort of fallback certificate in the config instead of each Ingress separately.

github-actions[bot] commented 1 year ago

The Contour project currently lacks enough contributors to adequately respond to all Issues.

This bot triages Issues according to the following rules:

You can:

Please send feedback to the #contour channel in the Kubernetes Slack

github-actions[bot] commented 1 year ago

The Contour project currently lacks enough contributors to adequately respond to all Issues.

This bot triages Issues according to the following rules:

You can:

Please send feedback to the #contour channel in the Kubernetes Slack