TLS namespace isolation privilege escalation

[dhain]

Is this a request for help? (If yes, you should use our troubleshooting guide and community support channels, see https://kubernetes.io/docs/tasks/debug-application-cluster/troubleshooting/.): no

What keywords did you search in NGINX Ingress controller issues before filing this one? (If you have found any duplicates, you should instead reply there.): tls namespace

---

Is this a BUG REPORT or FEATURE REQUEST? (choose one): bug report

NGINX Ingress controller version: 0.16.2

Kubernetes version (use kubectl version): 1.10.4

Environment:

• Cloud provider or hardware configuration: bare metal

What happened: Create an Ingress in one namespace that specifies TLS using a Secret in that namespace. Then create another Ingress in another namespace that uses the same hostname. Due to how the controller builds the configuration model, the TLS definition from the first ingress wins because it has an earlier resourceVersion. The second Ingress effectively hijacks the TLS secret from the other namespace.

What you expected to happen: The current stance (see #2371, #2170) is that cross-namespace secret references are disallowed, so I would expect the observed behavior not to happen. However I can't really imagine what the correct behavior would be in this case, because it seems to point to a disconnect between the definition of the Ingress resource in the API and the real world: The API allows multiple Ingresses for the same hostname, but with different TLS certificates, which is a practical impossibility. This should be fixed upstream, however I still think it represents a bug in the way nginx-ingress constructs the config model. If we stick with picking the oldest resourceVersion, then if we later encounter an Ingress that contradicts what we picked, the controller should refuse to honor the new Ingress.

Some other context: I've actually been taking advantage of the above behavior to work around the cross-namespace issue, as I am one of the (seemingly many) people who has a legitimate use-case: a central wildcard certificate with Ingresses in multiple namespaces -> subdomains, and not wanting to copy the private key to multiple namespaces. The rationale so far for not supporting this is that it constitutes a privilege escalation, but this bug report demonstrates that the privilege escalation currently exists anyway. If I had my way, nginx-ingress would reject contradictory Ingresses, but also would allow cross-namespace references.

[aledbf]

@dhain the real problem here (besides the one you are describing) is that we don't have a wait to determine which Ingress in a particular namespace has the privilege to use a particular hostname.

[dhain]

Yeah, that seems to be a limitation of the API. There are no restrictions at all on which Ingresses can reference which hostnames. As a result, the API allows specifying things that just don't make sense. Any idea how the other ingress controllers behave in this scenario?

[dhain]

Also relevant (for reference): kubernetes/kubernetes#57325

[aledbf]

@dhain the contour team is working in a CRD that solves some of the current Ingress API limitations.
Please check https://github.com/heptio/contour/blob/master/design/ingressroute-design.md

[dhain]

I like the Contour spec. Is nginx-ingress planning on adopting the same CRD?

[aledbf]

I really like the CRD. That said, before thinking in adding support for this we need to do other things like switching to the dynamic mode by default and some of the items here

Edit: keep in mind nobody works in ingress-nginx full-time (most of us work on our spare time)

[kfox1111]

Has there been any effort recently in the api community to enhance the stock Ingress api? One of the really great features about it is in general, it allows writing an app and deploy it in multiple k8s's (helm charts for example). If we fall back to every ingress controller having its own CRD format, then app developers will suffer having to write many different ingress objects. This will fragment the k8s community.

[aledbf]

Has there been any effort recently in the api community to enhance the stock Ingress api?

Yes and no. Please check

• https://groups.google.com/forum/#!searchin/kubernetes-sig-network/ingress|sort:date/kubernetes-sig-network/MsCDmYLlSnM/4OHEJZPZAwAJ
• https://groups.google.com/forum/#!searchin/kubernetes-sig-network/ingress|sort:date/kubernetes-sig-network/FfQ3HzUul3g/-M7IARm2AwAJ

One of the really great features about it is in general, it allows writing an app and deploy it in multiple k8s's (helm charts for example)

@kfox1111 that's something like a contradiction because the spec is not really generic besides the path, service, and port. Everything else depends on the software behind the controller.

[kfox1111]

Thanks for the links. I'll take a look.

The spec didn't cover a lot of things, but a lot of things were exposed as annotations. Most charts started following the convention of allowing ingress annotations to be passed by the end user through to the ingress object so at least the customizations could be done. Less then ideal, but did work. But going to every controller getting its own crd object type abandons the little bit of standardization we had. Feels like a step in the wrong direction.

[aledbf]

The spec didn't cover a lot of things, but a lot of things were exposed as annotations.

And this is the main problem. Annotation should be used just for some things, not everything we need to do to customize the configuration. The main problem with annotation is that only allow strings. This means you always delegate to the controller code how to deal with the content. For instance, we should not be able to specify an annotation with invalid values.

[kfox1111]

Yup. and thats why I was speaking up and asking for the majority of those things to go into the Ingress object api proper, rather then go into a ingress controller specific format. K8s has done so well, because they don't have a CRD for Docker vs Cri-O vs ContainerD, etc. We don't want the same for Nginx,Contour,Haproxy,Istio,etc.

[aledbf]

We don't want the same for Nginx,Contour,Haproxy,Istio,etc.

The problem here is the features that can be supported in the software behind the spec. This has been the main issue. Also, please keep in mind some of the features cannot be provided in some cloud providers.

[kfox1111]

There's got to be a reasonable middle ground though.... get the 80% covered and then switch to vendor specific things as needed.

What about two ingress base standards. One for features that 100% of balancers provide. an Enhanced one for features 80% of controllers provide. And then specific balancer cruds?

[aledbf]

What about two ingress base standards. One for features that 100% of balancers provide. an Enhanced one for features 80% of controllers provide. And then specific balancer cruds?

That's the problem, 80% means, basically, only path, service and port.

Edit: s/80/100/

[kfox1111]

80% of the load balancers out there don't support stripping a path when forwarding on the request for example?

[kfox1111]

path, service, and port I think is the 100% case.

[aledbf]

80% of the load balancers out there don't support stripping a path when forwarding on the request for example?

Please don't take this in a wrong way but this is an unproductive discussion. Any change to the Ingress API means we need to reach consensus about several things; something already proved almost impossible to achieve. Just as an example, check #555. We cannot even agree on what should be supported in a single field.

Also, keep in mind I am just another k8s user, I have no power to start such discussion.

[kfox1111]

The discussion was definitely unproductive in the past. I've seen this in the k8s community in other places as well. Usually some key folks are too resource constrained to consider the problem until later. Looking at one of the links posted above, https://groups.google.com/forum/#!searchin/kubernetes-sig-network/ingress%7Csort:date/kubernetes-sig-network/MsCDmYLlSnM/4OHEJZPZAwAJ it looks like Tim is starting to free up the cognitive resources to potentially allow a productive discussion of it again. Have you looked at that document yet? It may be a different situation now and may be productive again?

I'm just a user too. but thats the great thing about opensource communities. users have influence. :)

[fejta-bot]

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/lifecycle stale

[fejta-bot]

Stale issues rot after 30d of inactivity.
Mark the issue as fresh with /remove-lifecycle rotten.
Rotten issues close after an additional 30d of inactivity.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/lifecycle rotten

[fejta-bot]

Rotten issues close after 30d of inactivity.
Reopen the issue with /reopen.
Mark the issue as fresh with /remove-lifecycle rotten.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/close

[k8s-ci-robot]

@fejta-bot: Closing this issue.

In response to this:

Rotten issues close after 30d of inactivity.
Reopen the issue with /reopen.
Mark the issue as fresh with /remove-lifecycle rotten.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/close

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.