Cloud-on-k8s: Access control on referenced objects

[barkbay]

As raised in #2203 we should decide if we want to improve access control between objects.
The mechanism could be applied to any relationship between 2 objects of the stack, for example:

• Elasticsearch to Elasticsearch (#2203)
• Kibana or APM Server to Elasticsearch
• APM Server to Kibana (#1264)
• Beats to Kibana

So far 2 options have been identified:

Option 1: An annotation or a new field in the spec

For example:

apiVersion: elasticsearch.k8s.elastic.co/v1
kind: Elasticsearch
metadata:
  name: quickstart
spec:
  # allow ECK to configure resources in other namespaces to access this cluster
  # defaults to Nil (allow all), can be set to `allow: []` (only allow resources in the current namespace)
  crossNamespaceReferences:
      allow: ["namespaceA", "namespaceB"]
  version: 7.5.2
  nodeSets: []

Pro:

• Lightweight and easy to setup

Cons:

• Harder to figure out the permissions in case of an audit (it would need some dedicated tooling)
• ~How to manage granularity (namespace ? how to enforce only a given type ? fully qualified object name ?)~ _Edit: not sure it makes sense_

Option 2: Using Kubernetes built-in RBAC mechanism

See https://github.com/elastic/cloud-on-k8s/compare/master...barkbay:rbac-controlled-references

Pro:

• Security is fully delegated to the API server, _(almost)_ no additional security code to maintain.
• Integrate with existing audit tools, for example:

$ kubectl who-can update elasticsearch -n elasticsearch-ns
ROLEBINDING                         NAMESPACE         SUBJECT      TYPE            SA-NAMESPACE
allow-kibana-from-remote-namespace  elasticsearch-ns  kibana-user  ServiceAccount  kibana-ns

CLUSTERROLEBINDING                                    SUBJECT                             TYPE            SA-NAMESPACE
cluster-admin                                         system:masters                      Group
cluster-admin-binding                                 [email protected]          User
elastic-operator                                      elastic-operator                    ServiceAccount  elastic-system
system:controller:clusterrole-aggregation-controller  clusterrole-aggregation-controller  ServiceAccount  kube-system
system:controller:generic-garbage-collector           generic-garbage-collector           ServiceAccount  kube-system

$ kubectl access-matrix for elasticsearch --verbs 'update' -n elasticsearch-ns
NAME                                KIND            SA-NAMESPACE    UPDATE
clusterrole-aggregation-controller  ServiceAccount  kube-system     ✔
elastic-operator                    ServiceAccount  elastic-system  ✔
generic-garbage-collector           ServiceAccount  kube-system     ✔
kibana-user                         ServiceAccount  kibana-ns       ✔
[email protected]          User                            ✔
system:masters                      Group                           ✔

Cons:

• Less trivial to setup than a annotation or an additional field

We should also decide if we want to allow references in a same "local" namespace by default

[barkbay]

We discussed this topic and we agreed that:

• This feature must be backward compatible to not break existing deployments.
• The association must be removed if the permission to establish an association is removed.

This last point would introduce a lot of complexity in the watches logic if we choose to rely on the RBAC mechanism: the painful point is to infer which associated resources must be reconciled given a list of Subjects: we would need to handle the different types of Subject (ServiceAccount or rbac.authorization.k8s.io) which is not impossible but not trivial to do and to maintain.

The benefit of delegating the permission check to the API Server is, imho, lost.

[pebrc]

I think we should maybe revisit the notion that changes in RBAC need to be reflected immediately in the association.

I would argue that updating/removing the association at the next reconciliation or after 10h which is the default resync interval iirc should be good enough and would remove the requirement for complex watches.

[sebgl]

I think that's fair. 10h sounds a bit long to me, we could maybe change the default to eg. 15 minutes for the association controller?
As Michael pointed out going the full watch setup way (+ the additional RBAC permissions the operator would require) would be a pain.

About the APIServer requests load:

• When spawning a basic Elasticsearch + Kibana association I get about 10 association reconciliations executed in a short amount of time (less than 10 seconds). So probably 10 access review API requests.
• If we then trigger a reconciliation every 15 minutes, that's 1 access review API requests every 15 minutes.

That's to be multiplied by the number of clusters. I don't know how much is too much (100 requests for 100 clusters every 15 minutes does not sound too bad), and if we'd need to introduce some kind of async authorization caching at some point. But let's keep optimization for later.

[sebgl]

There's one thing bugging me a little bit.

Say we have UserA that has access to NamespaceA and UserB that has access to NamespaceB.
To establish an association between KibanaA and ElasticsearchB, UserA would specify its own ServiceAccount in the KibanaA resource. Then probably ask a k8s cluster admin to create the RoleBinding to access Elasticsearches in NamespaceB.

Once that's done, UserA can update any Elasticsearch resource in NamespaceB.

I think I'm fine with the namespace scope ("any Elasticsearch resource in NamespaceB"), but not sure I like the fact UserA can now really update the Elasticsearch resource (eg. number of nodes and configuration). Whereas the only thing we wanted is validate we can establish an association.

I'm wondering if it could be worth creating an symbolic Association subresource to Elasticsearch. So the symbolic Role or ClusterRole would be restricted to that subresource only:

apiVersion: rbac.authorization.k8s.io/v1
kind: ClusterRole
metadata:
  name: elasticsearch-association
rules:
- apiGroups:
  - elasticsearch.k8s.elastic.co
  resources:
  - elasticsearches/association
  verbs:
  - get

[barkbay]

I'm wondering if it could be worth creating an symbolic Association subresource to Elasticsearch

I did a quick test and it seems to work as expected, so I'm 👍 to use a subresource.

[anyasabo]

I like that idea! Though I think to clarify your example we would want to require write permissions on the association resources in both clusters.

[barkbay]

Unfortunately it seems that generic subresources are not supported in custom resources:

Custom resources support /status and /scale subresources. (ref)

Also see the upstream issue.

I think that checking RBAC with SubjectAccessReview is working as expected because it does not involve the CRD by itself. I guess that SubjectAccessReview merely checks if there is something like elasticsearches/association in the (Cluster)Role without checking if association is actually an existing and a valid subresource of elasticsearches

The bad news is that some tools do this check:

$ kubectl who-can update elasticsearch --subresource association -n elasticsearch-ns
Error: resolving resource: the server doesn't have a resource type "elasticsearch/association"

It makes the use of RBAC less relevant and I'm not sure that it is a good idea to use a subresource if it is not supported in a CRD

@sebgl I also share your concern on the "update" verb, so I guess get would be the only choice if we want to use a Role.