Advice around ingress secret cross-namespace

[Stono]

Hey!
I know this has been briefly discussed before on https://github.com/kubernetes/ingress-nginx/issues/2170 - but it's something that's hit us to so i'm wondering if there's any way we can work around it?

At a high level, we have say 100 applications all deployed onto the same cluster, being handled by ingress-nginx. They all share the same FQDN of: app-name.dev.k8.ourcloud.io, and all need to be terminated with the same *.dev.k8.ourcloud.io cert. However, they're deployed across separate namespaces.

What we're trying not to do is replicate the ingress-cert across all the namespaces... because it's a bit of a logical nightmare.

Proposal Rather than breaking the spec of a kubernetes Ingress, perhaps we could add an annotation which is like nginx.ingress.kubernetes.io/tlsSecretNamespace: ingress-nginx? This would tell the ingress-nginx controller to look for the tls.secretName in the ingress-nginx namespace, regardless of the namespace that the ingress definition is in?

[aledbf]

@Stono the new wildcard support in cert-manager or just event secrets with wildcard certificates has been always an issue.

At a high level, we have say 100 applications all deployed onto the same cluster, being handled by ingress-nginx. They all share the same FQDN of: app-name.dev.k8.ourcloud.io, and all need to be terminated with the same *.dev.k8.ourcloud.io cert. However, they're deployed across separate namespaces.

I know how painful is this becomes.

I am open to suggestion. Reading IngressTLS definition I don't see why we could not implement something like #2382.

Just to be clear, I've always been against this but the cert-manager wildcard support it's here and we need to move forward. If this change is accepted the Ingress rule will become incompatible with any other ingress controller

ping @antoineco @gianrubio @ElvinEfendi for feedback about this change/scenario

[Stono]

I really, really would like this. We have work that essentially blocked due to this complexity :-(

[kfox1111]

Permissions are the main issue I can think of. how do you prevent a user from accessing another users secrets?

[aledbf]

Permissions are the main issue I can think of. how do you prevent a user from accessing another users secrets?

If you can restrict this tuning the RBAC permissions you give to the ingress controller

Edit: you need to adjust the defaults RBAC permissions to your environment.

[antoineco]

@Stono: From what I can see there is no validation performed on the SecretName field, the following spec is perfectly valid:

spec:
  rules:
  - host: tls.example.com
    http:
      paths:
      - backend:
          serviceName: tls
          servicePort: http
        path: /
  tls:
  - hosts:
    - tls.example.com
    secretName: foo/bar

Therefore we could accept the ns/ name form without breaking the spec like @aledbf said.

[Stono]

True, I suppose playing devils advocate though there is an implicit contract between the ingress and the controller.

Ingress-nginx would interpret that as namespace/secret, if the consumer then moved to another controller for the same ingress it would be interpreted as secret, with a / in the name.

I'm all for it though, because this is a mega annoying problem haha

On Fri, 20 Apr 2018, 11:09 am Antoine Cotten, notifications@github.com wrote:

@Stono https://github.com/Stono: From what I can see there is no validation performed on the SecretName field https://github.com/kubernetes/kubernetes/blob/master/pkg/apis/extensions/validation/validation.go#L372-L391, the following spec is perfectly valid:

spec: rules:

• host: tls.example.com http: paths:
  • backend: serviceName: tls servicePort: http path: / tls:
• hosts:
  • tls.example.com secretName: foo/bar

Therefore we could accept the ns/ name form without breaking the spec like @aledbf https://github.com/aledbf said.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/kubernetes/ingress-nginx/issues/2371#issuecomment-383050782, or mute the thread https://github.com/notifications/unsubscribe-auth/ABaviWPOc5t_AywRAPyXLaq_-1IszR2Cks5tqbPCgaJpZM4TaBwS .

[antoineco]

Ingress-nginx would interpret that as namespace/secret, if the consumer then moved to another controller for the same ingress it would be interpreted as secret, with a / in the name.

Right, the absence of validation leaves a lot of room for interpretation. I think it's fine because:

1. the Ingress spec is to date insufficient anyway (the way every controller implements its own set of custom annotations is the direct result of this)
2. the kubernetes.io/ingress.class annotation has been well-accepted as a way to tie Ingress objects to a particular controller implementation by now

To be clear, I don't believe either 1) or 2) excuse that kind of freedom of interpretation, but as long as the compatibility with current versions of ingress-nginx is not broken the change feels reasonable to me.

[Stono]

I'm sold. Let's do it 👍

On Fri, 20 Apr 2018, 11:43 am Antoine Cotten, notifications@github.com wrote:

Ingress-nginx would interpret that as namespace/secret, if the consumer then moved to another controller for the same ingress it would be interpreted as secret, with a / in the name.

Right, the absence of validation leaves a lot of room for interpretation. I think it's fine because:

1. the Ingress spec is to date insufficient anyway (the way every controller implements its own set of custom annotations is the direct result of this)
2. the kubernetes.io/ingress.class annotation https://github.com/kubernetes/ingress-nginx/blob/master/internal/ingress/annotations/class/main.go#L24-L29 has been well-accepted as a way to tie Ingress objects to a particular controller implementation by now

To be clear, I don't believe either 1) or 2) excuse that kind of freedom of interpretation, but as long as the compatibility with current versions of ingress-nginx is not broken the change feels reasonable to me.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/kubernetes/ingress-nginx/issues/2371#issuecomment-383058536, or mute the thread https://github.com/notifications/unsubscribe-auth/ABaviZEgIWpjE2RmIaLak2EWvVM1T4MHks5tqbvrgaJpZM4TaBwS .

[antoineco]

/assign @antoineco

[aledbf]

@antoineco just in case, I want input from more users.

[antoineco]

Sure, I'll just push a quick proposal that we can put on hold.

[aledbf]

A side effect of implementing this is that we need to allow multiple namespaces in the flag --watch-namespace to avoid * (all) or just one namespace to limit the scope

Edit: like https://github.com/containous/traefik/pull/1895

[kfox1111]

@aledbf no, I don't think it is just an rbac issue. if you have an operator setting up an nginx ingress for the whole cluster, but have multiple tenants, nginx-ingress would need access to the secrets for all tenants it will be accessing. but you don't want one tenant accessing secrets for another tenant.

What about this.... An ingress annotation with the name of a secret in the same namespace that can be used to access the tenants secrets?

The tenant would: 1, create a service account that has access to their secrets. 2, give nginx-ingress's service account access to that service account. and nginx-ingress would read the tenant's service account secret, and retrieve the tenants secret using it.

With this method, no permissions are granted to nginx-ingress that it doesn't actually need to do its work, and users cant access secrets across namespaces that they don't have access to.

[Stono]

Perhaps I'm missing something, but granting access to a service account which has access to read the secrets is no different than granting the nginx ingress service account permissions to read the secret in the first place?

On Fri, 20 Apr 2018, 6:21 pm kfox1111, notifications@github.com wrote:

@aledbf https://github.com/aledbf no, I don't think it is just an rbac issue. if you have an operator setting up an nginx ingress for the whole cluster, but have multiple tenants, nginx-ingress would need access to the secrets for all tenants it will be accessing. but you don't want one tenant accessing secrets for another tenant.

What about this.... An ingress annotation with the name of a secret in the same namespace that can be used to access the tenants secrets?

The tenant would: 1, create a service account that has access to their secrets. 2, give nginx-ingress's service account access to that service account. and nginx-ingress would read the tenant's service account secret, and retrieve the tenants secret using it.

With this method, no permissions are granted to nginx-ingress that it doesn't actually need to do its work, and users cant access secrets across namespaces that they don't have access to.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/kubernetes/ingress-nginx/issues/2371#issuecomment-383164867, or mute the thread https://github.com/notifications/unsubscribe-auth/ABavidHfgFU2YfpU4jSQ6Osagtb7A3LRks5tqhkxgaJpZM4TaBwS .

[aledbf]

if you have an operator setting up an nginx ingress for the whole cluster, but have multiple tenants, nginx-ingress would need access to the secrets for all tenants it will be accessing. but you don't want one tenant accessing secrets for another tenant.

Besides the security aspect, you should use multiple ingress controllers deployments if you have multiple tenants because there is no way to limit resources between them (a concept it doesn't mean anything in nginx).

The tenant would: 1, create a service account that has access to their secrets. 2, give nginx-ingress's service account access to that service account. and nginx-ingress would read the tenant's service account secret, and retrieve the tenants secret using it.

You already gave permissions to the ingress controller to access the whole cluster if you don't use the --watch-namespace feature so even a tenant in a namespace can access a secret that is located there because there is no way to restrict that.

[kfox1111]

Thats what I'm saying. give nginx-ingress only access to see ingress objects in every namespace. no secret access at all by default. Then, the end user can create a serviceaccount with access to the secrets they want access to. Then give rbac permission to nginx-ingress's serviceaccount access to use the users's serviceaccount. Then nginx-ingress uses that serviceaccount to access the secrets the user puts in its ingress rules?

[Stono]

Still really doesn't make any sense to me if I'm being honest. You're just proposing privilege escalation via access to a service account. You might as well just give the nginx service account access to those secrets.

On Fri, 20 Apr 2018, 8:09 pm kfox1111, notifications@github.com wrote:

Thats what I'm saying. give nginx-ingress only access to see ingress objects in every namespace. no secret access at all by default. Then, the end user can create a serviceaccount with access to the secrets they want access to. Then give rbac permission to nginx-ingress's serviceaccount access to use the users's serviceaccount. Then nginx-ingress uses that serviceaccount to access the secrets the user puts in its ingress rules?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/kubernetes/ingress-nginx/issues/2371#issuecomment-383193817, or mute the thread https://github.com/notifications/unsubscribe-auth/ABaviVyhf-r9Wl-eUIMqIWshVJZ8Sryjks5tqjJ9gaJpZM4TaBwS .

[kfox1111]

hmm.... yeah, you are right. there still would need to be some kind of check that validated the user updating the ingress had access to the service account.

[kfox1111]

What about a webhook admission controller? It could allow/block ingress annotation creates/updates.

[aledbf]

What about a webhook admission controller? It could allow/block ingress annotation creates/updates.

Sure but this is something you should build for you cluster as an additional controller and not inside this ingress controller because we cannot assume everyone wants the same restrictions and also we cannot assume there is only one ingress controller in the cluster

[kfox1111]

Thats fair. What about about a reference controller though? I think the multitenant use case is pretty common. The code will be pretty specific I think to nginx-ingress (no other ingress has the great annotation support I think).

[yvespp]

Another way to handle this would be to have a default tls cert(s) per ingress controller. Only the ingress operator would have to rights to set this up.

Here is an old pull request that implemented that: https://github.com/kubernetes/ingress-nginx/pull/1099 With that pull you don't have to specify that you want ssl, it is alway enabled and uses the default tls cert. Maybe that needs to be changed so you can select one of the default certs or opt out. In the pull, you can overwrite the default cert with the tls config of an ingress.

[gianrubio]

Another way to handle this would be to have a default tls cert(s) per ingress controller.

This didn't solve the problem, you could have multiple wildcard certificates (for multiple domains) running in the same nginx.

[aledbf]

Closing. This change breaks the currently enforced isolation. The issue with multiple namespaces and secrets could be solved in cert-manager. In any other case is up to the user to copy the secrets between namespaces (this could be an usecase for a custom controller to avoid manual tasks).

ping @munnerz

[Stono]

Interestingly, fyi we have migrated to cert manager and letsencrypt.

On Tue, 1 May 2018, 5:02 pm Manuel Alejandro de Brito Fontes, < notifications@github.com> wrote:

Closing. This change breaks the currently enforced isolation. The issue with multiple namespaces and secrets could be solved in cert-manager. In any other case is up to the user to copy the secrets between namespaces (this could be an usecase for a custom controller to avoid manual tasks).

ping @munnerz https://github.com/munnerz

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/kubernetes/ingress-nginx/issues/2371#issuecomment-385692976, or mute the thread https://github.com/notifications/unsubscribe-auth/ABavia-sRUBz7I36XKGzVswDW0m7CNiqks5tuHjogaJpZM4TaBwS .

[0x53A]

This would be really useful to have, if there are security concerns, could this be off-by-default and behind a switch?

I really don't want to either have to copy secrets around, or use a custom controller just for something as simple as that.