Add handling for a default TLS certificate

[mattalberts]

Feature Request: Add handling for a default TLS certificate

• Search Terms: "is:issue is:open TLS"

Add handling to allow use of a default TLS certificate, following these guidelines

• The certificate is specified as a <namespace>/<name> reference to a secret.
• The default secret is applied to Ingress specs that specify tls without providing spec.tls.secretName
• Existing matching rules against spec.tls.hosts are still satisfied.
• An Ingress spec.tls.secretName overrides the default secret.
• The default secret does not apply to the default host "*"

Hopefully the guidelines don't sound to dogmatic. 😄

All I'm try to capture is a loosening of validTLSSpecforVhost to only reject a TLS spec if both the default secret and spec.tls.secretName are not set.

• https://github.com/heptio/contour/blob/master/internal/contour/virtualhost.go#L234-L237

Example Depolyment|Container Definition

        - name: contour
          image: gcr.io/heptio-images/contour:latest
          imagePullPolicy: Always
          command: ["contour"]
          args:
            - serve
            - --incluster
            - --ingress-class-name=contour
            - --default-tls-secret=$(POD_NAMESPACE)/contour-ingress
          env:
            - name: POD_NAME
              valueFrom:
                fieldRef:
                  fieldPath: metadata.name
            - name: POD_NAMESPACE
              valueFrom:
                fieldRef:
                  fieldPath: metadata.namespace

Example Ingress Definition

---
apiVersion: extensions/v1beta1
kind: Ingress
metadata:
  name: contour-echo
  labels:
    ingress: contour
  annotations:
    kubernetes.io/ingress.class: contour
spec:
  tls:
  - hosts:
    - echo.127.0.0.1.xip.io
  rules:
  - host: echo.127.0.0.1.xip.io
    http:
      paths:
      - path: /
        backend:
          serviceName: echo
          servicePort: 80
---

• we specify tls with a host but leave the secretName empty to use the default secret.
• the behavior is similar to both the nginx and haproxy ingress controllers.

[davecheney]

Thank you for raising this issue. As it stands now I don’t want to add this because it may violate some parts of the ingress spec. Can you please explain the use case for this feature.

…

On 26 May 2018, at 06:34, mattalberts ***@***.***> wrote: Feature Request: Add handling for a default TLS certificate Search Terms: "is:issue is:open TLS" Add handling to allow use of a default TLS certificate, following these guidelines The certificate is specified as a <namespace>/<name> reference to a secret. The default secret is applied to Ingress specs that specify tls without providing spec.tls.secretName Existing matching rules against spec.tls.hosts are still satisfied. An Ingress spec.tls.secretName overrides the default secret. The default secret does not apply to the default host "*" Hopefully the guidelines don't sound to dogmatic. 😄 All I'm try to capture is a loosening of validTLSSpecforVhost to only reject a TLS spec if both the default secret and spec.tls.secretName are not set. https://github.com/heptio/contour/blob/master/internal/contour/virtualhost.go#L234-L237 Example Depolyment|Container Definition - name: contour image: gcr.io/heptio-images/contour:latest imagePullPolicy: Always command: ["contour"] args: - serve - --incluster - --ingress-class-name=contour - --default-tls-secret=$(POD_NAMESPACE)/contour-ingress env: - name: POD_NAME valueFrom: fieldRef: fieldPath: metadata.name - name: POD_NAMESPACE valueFrom: fieldRef: fieldPath: metadata.namespace Example Ingress Definition --- apiVersion: extensions/v1beta1 kind: Ingress metadata: name: contour-echo labels: ingress: contour annotations: kubernetes.io/ingress.class: contour spec: tls: - hosts: - echo.127.0.0.1.xip.io rules: - host: echo.127.0.0.1.xip.io http: paths: - path: / backend: serviceName: echo servicePort: 80 --- we specify tls with a host but leave the secretName empty to use the default secret. the behavior is similar to both the nginx and haproxy ingress controllers. — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub, or mute the thread.

[mattalberts]

@davecheney

I was curious about that too, especially given that the cross-section of istio, nginx, and haproxy ingress controllers are not all consistent (istio is the outlier, both nginx and haproxy provide a --default-ssl-certificate option).

// IngressTLS describes the transport layer security associated with an Ingress.
type IngressTLS struct {
	// Hosts are a list of hosts included in the TLS certificate. The values in
	// this list must match the name/s used in the tlsSecret. Defaults to the
	// wildcard host setting for the loadbalancer controller fulfilling this
	// Ingress, if left unspecified.
	// +optional
	Hosts []string `json:"hosts,omitempty" protobuf:"bytes,1,rep,name=hosts"`
	// SecretName is the name of the secret used to terminate SSL traffic on 443.
	// Field is left optional to allow SSL routing based on SNI hostname alone.
	// If the SNI host in a listener conflicts with the "Host" header field used
	// by an IngressRule, the SNI host is used for termination and value of the
	// Host header is used for routing.
	// +optional
	SecretName string `json:"secretName,omitempty" protobuf:"bytes,2,opt,name=secretName"`
	// TODO: Consider specifying different modes of termination, protocols etc.
}

• Given the wording, I wasn't certain if this constituted a violation ... or expected behavior.. Feels like it falls into the final portion of the wording for SecretName (the qualification around it being optional).
• GitHub Spec Reference

Spec aside (😄 cause I'm sure you're more familiar than I am 😄 ), let's talk use-case. I'll try to set the stage a little.

Assumptions,

• a kubernetes 1.10.x cluster with ~100 namespaces (its early, the number will grow).
• namespaces are used to develop restful apis and other socket/grpc services
• 1 or more ingresses per-namespace (all needing tls support)
• most ingresses want exposure of the form {{{service-name}}].example.com
• a least one wildcard signed certificate

I'm aiming for an environment where TSL setup/management is so easy its free (from a development/operational cost perspective), without removing the ability to self-manage certs where needed (for customization & control).

To get there now, I see a few choices (you might see more 😄).

1. add default handling (controller default with ingress spec override)
2. use Cert Managert
3. reliably replicate the secret into all namespaces

Cert Manager is ear-marked as not ready, which is fine because I'm not ready for it either 😄.

The replication option is workable. It would be fairly easy to write a list-watch to ensure the secret is present (or replicate it if not present). My small cluster would see ~100 copies of the certificate (again, the number will grow). It also leads to exposure of the certificate key. Namespaces are expect to have additional secrets, locking it down via rbac wouldn't be an option (I could explore web-hook options for restrictions, but it starts to feel a little rube goldberg-esch).

[davecheney]

a least one wildcard signed certificate

I think this is the key requirement here, it's come up before with respect to applying the ingress notion of a default ingress to tls. IMO this case was not well thought out in the ingress spec so I want to tread very carefully because any behaviour we adopt now (it's been prohibited since 0.3) will be irreversible once adopted.

Duplicating a secret across multiple namespaces sounds like a pain operationally and will give your security folks hives.

My recommendation is:

a. use cert-manager and lets encrypt, this problem is exactly their target market. AFAIK JetStack consider ready for production. /cc @munnerz
b. wait for the ingress route crd to land. No ETA yet and TBH we haven't thought about how to accomodate wildcard explicitly either, so in that respect we're no better than the current ingress spec.

[mattalberts]

Even vocalizing the suggestion would trigger hives (its truly a voldemort of phrases 😄).

Sadly lets encrypt is also a non-option. The diagram shows integration with vault; vault is a workable option. My light reading suggests cert-manager will produce signed certificates as CRDs. That hints at requiring additional processing to translate those CRDs into Secrets within the namespace for use by Ingress definitions? /cc @munnerz.

I'm likely stuck with a modified option b. for a period of time. The modification being, run with an patched version of contour while I wait for the ingress route crd to land.

Its the least risky of all the options 😄.

[munnerz]

I'm just crunching through my GitHub notifications and have spotted these questions - sorry for not getting back sooner!

@mattalberts cert-manager uses the Certificate CRD to define what a Certificate should look like. It ultimately stores the signed keypair as a Secret resource in the API server (as named by spec.secretName.

Happy to discuss with you further if you are still exploring this. We support Vault now, and would love to hear any feedback on the integration (especially with the sort of setup you have, with many namespaces and many things TLS'd! 😄)

[mauilion]

bumping this issue again. There are a number of folks that are unable to adopt contour until this feature lands.

[floriankoch]

any progress here? i have the same situation / requirement as @mattalberts , we use nginx-ingress atm, but want to switch to contour to use the ingress route

[jcrowthe]

Likewise, similar situation here. In a multi-tenant cluster environment with a wildcard certificate, placing the cert as a secret in every namespace is horrific. Cert-manager can be adopted by some, but not all, and lack of support in Contour is a hindrance to its adoption.

IngressRoute doesn't appear to be well suited to a multi tenant team with a wildcard cert, as you need to delegate each and every fqdn before tenants get access. Most multi-tenant models attempt to be as self-service as possible, making the requirement to of delegation a large road block.

Can we revisit this issue?

[davecheney]

Thanks for your comment. This issue is absolutely on my radar, i hope to get to it next year. I know that’s a really vague promise, but it’s the best I can offer. I see the need, I understand the requirement, it’s just a matter of prioritisation and personnel.

…

On 17 Nov 2018, at 11:31, Jacob Crowther ***@***.***> wrote: Likewise, similar situation here. In a multi-tenant cluster environment with a wildcard certificate, placing the cert as a secret in every namespace is horrific. Cert-manager can be adopted by some, but not all, and lack of support in Contour is a hindrance to its adoption. IngressRoute doesn't appear to be well suited to a multi tenant team with a wildcard cert, as you need to delegate each and every fqdn before tenants get access. Most multi-tenant models attempt to be as self-service as possible, making the requirement to of delegation a large road block. Can we revisit this issue? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.

[jcrowthe]

@davecheney Thanks, it's good to know this is on the radar.

If a dev on my team happened to have some spare cycles, would you accept a PR for this feature? If so, we'd like to implement it right, which means we'd need a spec to build toward.

[davecheney]

If a dev on my team happened to have some spare cycles, would you accept a PR for this feature?

Of course, all help gratefully received, but please before sending any code discuss and agree on a design in this issue, or in a spec PT to the design/ or docs/ directory.

[jcrowthe]

@davecheney Thanks, sounds good. I'm also seeing that an attempt was already made at this feature in PR #411. Would you comment further on why it was rejected initially? Would it be possible to revive that PR? Thanks!

[davecheney]

The contribution process for this repo is to agree on a design, then code. #411 did it the other way around.

[davecheney]

I'm closing this issue as we shipped TLS Certificate Delegation in 0.10. Additions or improvements should be handled in a new issue.

[mattalberts]

I saw that! Thanks for the update. Certificate Delegation looks to be a very flexible resolution (reading about it right now).