feat(auth-properties): allow users to specific cryostat permission mappings

[tthvo]

Fixes #439
Related to #438
Depends on https://github.com/cryostatio/cryostat/pull/1042

User can now specify permission mapping instead of Cryostat's defaults by creating a ConfigMap referenced in Cryostat CR. The mapping data will be mount to /app/resources/io/cryostat/net/openshift/OpenShiftAuthManager.properties in Cryostat container.

[tthvo]

Any thoughts @ebaron @andrewazores? I am thinking of referencing the default properties but maybe I need to wait for https://github.com/cryostatio/cryostat/pull/1042.

[tthvo]

Checking /app/resources/io/cryostat/net/openshift/OpenShiftAuthManager.properties in Cryostat core-container shows as expected.

[jboss@cryostat-sample-59c6ff78ff-kz8dg ~]$ cat /app/resources/io/cryostat/net/openshift/OpenShiftAuthManager.properties 
TARGET=pods,deployments.apps
RECORDING=pods,pods/exec
CERTIFICATE=deployments.apps,pods,cryostats.operator.cryostat.io
CREDENTIALS=cryostats.operator.cryostat.io

[ebaron]

Hi @tthvo, thanks for taking care of this so quickly! As a general comment, I would avoid referring to OAuth specifically. The permissions mapping extends to cases beyond OAuth. It's more like authorization properties.

[tthvo]

Hi @tthvo, thanks for taking care of this so quickly! As a general comment, I would avoid referring to OAuth specifically. The permissions mapping extends to cases beyond OAuth. It's more like authorization properties.

Ohh right! I think we could call it just AuthProperties then.

[tthvo]

From k8s docs:

A container using a ConfigMap as a subPath volume mount will not receive ConfigMap updates.

I think this is our current auth volume behaviour. I wonder how likely this Auth ConfigMap got changed at runtime? If so, would it be of any security risk?

[andrewazores]

There's a narrow potential for a security risk if the end deploying user changes some resource type mappings in a way that should lock some user accounts out of certain resource actions. Cryostat only reads the .properties file once at startup, so it would never apply the updated config map contents. In this scenario, the end deploying user should also restart the Cryostat instance. If this config map is part of our CRD then I think the Operator can simply automatically restart the Cryostat instance if/when the config map is modified.

[ebaron]

There's a narrow potential for a security risk if the end deploying user changes some resource type mappings in a way that should lock some user accounts out of certain resource actions. Cryostat only reads the .properties file once at startup, so it would never apply the updated config map contents. In this scenario, the end deploying user should also restart the Cryostat instance. If this config map is part of our CRD then I think the Operator can simply automatically restart the Cryostat instance if/when the config map is modified.

Ideally, yes, but that unfortunately doesn't happen automatically. We would need something like a checksum of the config map added to the pod template that triggers redeployment when the checksum changes. This would be a similar solution to what's needed for #119. The same applies to the contents of all secrets and config maps used with the operator.

[tthvo]

Ohh yeh make sense. Until #119 is resolved, users currently have to restart Cryostat manually. Should this be blocked by that issue for now?

[ebaron]

Ohh yeh make sense. Until #119 is resolved, users currently have to restart Cryostat manually. Should this be blocked by that issue for now?

I don't think it's worth blocking this over.

[ebaron]

We could add a note in the description indicating that users must manually restart the Cryostat deployment when modifying the Config Map.

[ebaron]

Hey @tthvo, looks good. Just a few more comments.

[tthvo]

@ebaron I just have a question. Since we are using this scoped-token to make requests to cluster API server, what are the purposes of creating roles and role binding for CR in the controller?

[tthvo]

I see two places (i.e. oauth token and role binding in controller) to CR' service account so it was a bit hard to understand for me.

[ebaron]

@ebaron I just have a question. Since we are using this scoped-token to make requests to cluster API server, what are the purposes of creating roles and role binding for CR in the controller?

Cryostat does some Kubernetes API requests using its own Service Account. This would be for things like automated rules for example. There's a Role, Cluster Role and associated Role Binding and Cluster Role Binding that binds those permissions to the Cryostat Service Account. For most user-initiated requests, the user's OAuth token is used instead of the Cryostat Service Account.

We don't bind the OAuth Cluster Role, it exists for us to pass to OpenShift's OAuth server as a list of permissions we want the scoped token to have.

[tthvo]

Also, it seems in our default oauth-client role, we also specify more resource accesses than the configured permission mappings. For example:

cryostat-operator/config/rbac/oauth_client.yaml

Line 51 in 56867fb

- daemonsets

I think this could come as a 'surprise' for users when defining their custom mappings.

[tthvo]

@ebaron I just have a question. Since we are using this scoped-token to make requests to cluster API server, what are the purposes of creating roles and role binding for CR in the controller?

Cryostat does some Kubernetes API requests using its own Service Account. This would be for things like automated rules for example. There's a Role, Cluster Role and associated Role Binding and Cluster Role Binding that binds those permissions to the Cryostat Service Account. For most user-initiated requests, the user's OAuth token is used instead of the Cryostat Service Account.

We don't bind the OAuth Cluster Role, it exists for us to pass to OpenShift's OAuth server as a list of permissions we want the scoped token to have.

Ahh right! That makes sense! In a sense, user-initiated requests means actions on webUI then. Thanks for explaining!

[tthvo]

Interestingly, with this default oauth-client cluster role, permission is still denied for secrets resource.

# Permissions for Cryostat to validate tokens and perform
# actions with the web-ui
apiVersion: rbac.authorization.k8s.io/v1
kind: ClusterRole
metadata:
  creationTimestamp: null
  name: oauth-client
rules:
- apiGroups:
  - operator.cryostat.io
  resources:
  - cryostats
  verbs:
  - create
  - delete
  - get
- apiGroups:
  - ""
  resources:
  - pods
  - pods/exec
  - services
  - secrets
  verbs:
  - create
  - get
  - delete

From docs for scoped tokens:

Caveat: This prevents escalating access. Even if the role allows access to resources like secrets, rolebindings, and roles, this scope will deny access to those resources. This helps prevent unexpected escalations. Many people do not think of a role like edit as being an escalating role, but with access to a secret it is.

I found a section describing secret protection from k8s that disallows escalations: https://kubernetes.io/docs/concepts/configuration/secret/#information-security-for-secrets

I think using user:check-access role:cryostat-operator-oauth-client:default:! solves the issue but allows escalations. Any thoughts @ebaron @andrewazores ?

[tthvo]

Logs from cryostat container:

java.util.concurrent.ExecutionException: java.util.concurrent.ExecutionException: io.cryostat.net.PermissionDeniedException: Requesting client in namespace "default" cannot get secrets: scopes [user:check-access role:cryostat-operator-oauth-client:default] prevent this action
	at java.base/java.util.concurrent.CompletableFuture.reportGet(CompletableFuture.java:396)
	at java.base/java.util.concurrent.CompletableFuture.get(CompletableFuture.java:2073)
	at io.cryostat.net.AbstractAuthManager$1.execute(AbstractAuthManager.java:76)
	at io.cryostat.messaging.MessagingServer.lambda$start$5(MessagingServer.java:159)
	at io.vertx.core.impl.ContextImpl.lambda$null$0(ContextImpl.java:159)
	at io.vertx.core.impl.AbstractContext.dispatch(AbstractContext.java:100)
	at io.vertx.core.impl.ContextImpl.lambda$executeBlocking$1(ContextImpl.java:157)
	at io.vertx.core.impl.TaskQueue.run(TaskQueue.java:76)
	at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1136)
	at java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:635)
	at io.netty.util.concurrent.FastThreadLocalRunnable.run(FastThreadLocalRunnable.java:30)
	at java.base/java.lang.Thread.run(Thread.java:833)
Caused by: java.util.concurrent.ExecutionException: io.cryostat.net.PermissionDeniedException: Requesting client in namespace "default" cannot get secrets: scopes [user:check-access role:cryostat-operator-oauth-client:default] prevent this action
	at java.base/java.util.concurrent.CompletableFuture.reportGet(CompletableFuture.java:396)
	at java.base/java.util.concurrent.CompletableFuture.get(CompletableFuture.java:2096)
	at io.cryostat.net.openshift.OpenShiftAuthManager.validateToken(OpenShiftAuthManager.java:263)
	at io.cryostat.net.openshift.OpenShiftAuthManager.validateWebSocketSubProtocol(OpenShiftAuthManager.java:381)
	at io.cryostat.messaging.MessagingServer.lambda$start$2(MessagingServer.java:149)
	... 10 more

[ebaron]

Also, it seems in our default oauth-client role, we also specify more resource accesses than the configured permission mappings. For example:

cryostat-operator/config/rbac/oauth_client.yaml

Line 51 in 56867fb

- daemonsets

I think this could come as a 'surprise' for users when defining their custom mappings.

Ah right, this also has permissions for things like the discovery/GraphQL APIs that operate on deployments/replicasets/etc. I agree it would be confusing for users to add these permissions to their custom cluster role.

How about this? Instead of the user-supplied custom cluster role replacing the built-in one, we include both. I think it's possible to request more than one role scope, but I'm not certain. This would need a change on the Cryostat side to accept more than one cluster role name (maybe comma-delimited). @tthvo @andrewazores what do you think of this solution?

[ebaron]

From docs for scoped tokens:

Caveat: This prevents escalating access. Even if the role allows access to resources like secrets, rolebindings, and roles, this scope will deny access to those resources. This helps prevent unexpected escalations. Many people do not think of a role like edit as being an escalating role, but with access to a secret it is.

I found a section describing secret protection from k8s that disallows escalations: https://kubernetes.io/docs/concepts/configuration/secret/#information-security-for-secrets

I think using user:check-access role:cryostat-operator-oauth-client:default:! solves the issue but allows escalations. Any thoughts @ebaron @andrewazores ?

Ah, that's unfortunate. I think it's probably better to remove Secrets from our permissions mapping than to enable the privilege escalation option. We'll have to decide on some substitute.

[tthvo]

Also, it seems in our default oauth-client role, we also specify more resource accesses than the configured permission mappings. For example:

cryostat-operator/config/rbac/oauth_client.yaml

Line 51 in 56867fb

- daemonsets

I think this could come as a 'surprise' for users when defining their custom mappings.

Ah right, this also has permissions for things like the discovery/GraphQL APIs that operate on deployments/replicasets/etc. I agree it would be confusing for users to add these permissions to their custom cluster role.

How about this? Instead of the user-supplied custom cluster role replacing the built-in one, we include both. I think it's possible to request more than one role scope, but I'm not certain. This would need a change on the Cryostat side to accept more than one cluster role name (maybe comma-delimited). @tthvo @andrewazores what do you think of this solution?

I think it's good idea :D With rbac 'purely additive' manner, this should work out nicely. Though, we will document that their custom Cluster Role is requested on top of this default one.

I think it's possible to request more than one role scope, but I'm not certain.

I will try to check this!

[tthvo]

Ah, that's unfortunate. I think it's probably better to remove Secrets from our permissions mapping than to enable the privilege escalation option. We'll have to decide on some substitute.

Then, this means if users specify secrets as their permission mappings, it should fail and this seems be beyond what we could handle unless allowing escalation by default?

[ebaron]

Ah, that's unfortunate. I think it's probably better to remove Secrets from our permissions mapping than to enable the privilege escalation option. We'll have to decide on some substitute.

Then, this means if users specify secrets as their permission mappings, it should fail and this seems be beyond what we could handle unless allowing escalation by default?

Right. At least we link to the OAuth token scoping documentation that mentions this, but there's not much else we can do I think.

[andrewazores]

Also, it seems in our default oauth-client role, we also specify more resource accesses than the configured permission mappings. For example:

cryostat-operator/config/rbac/oauth_client.yaml

Line 51 in 56867fb

- daemonsets

I think this could come as a 'surprise' for users when defining their custom mappings.

Ah right, this also has permissions for things like the discovery/GraphQL APIs that operate on deployments/replicasets/etc. I agree it would be confusing for users to add these permissions to their custom cluster role.

How about this? Instead of the user-supplied custom cluster role replacing the built-in one, we include both. I think it's possible to request more than one role scope, but I'm not certain. This would need a change on the Cryostat side to accept more than one cluster role name (maybe comma-delimited). @tthvo @andrewazores what do you think of this solution?

Seems feasible from the Cryostat side.

[tthvo]

Using scope=scope=user:check-access role:cryostat-operator-oauth-client:default role:custom-oauth-client:default as below:

https://cryostat-sample-default.apps.ci-ln-3bfz8ct-72292.origin-ci-int-gce.dev.rhcloud.com/#access_token=sha256~zifOtG8Qd3FC0N0Ea-gTsCL8qzMRVjdk0S5z7nfzH9w&expires_in=86400&scope=user%3Acheck-access+role%3Acryostat-operator-oauth-client%3Adefault+role%3Acustom-oauth-client%3Adefault&token_type=Bearer

So, we can specify multiple roles. It looks like the page also mentions about secret escalation issue.

[ebaron]

Seems to work well! Great job @tthvo!

[github-actions]

This PR/issue depends on:

• cryostatio/cryostat#1042
  By Dependent Issues (🤖). Happy coding!