From c01f5514e3219cc32ca10992ac1b9166024f5ca3 Mon Sep 17 00:00:00 2001 From: Igor Vinokur Date: Tue, 20 Jul 2021 11:58:09 +0300 Subject: [PATCH] fix: Revert the kubernetes Secrets role due to the security vulnerability Signed-off-by: Igor Vinokur --- .../AbstractWorkspaceServiceAccount.java | 10 --- .../KubernetesNamespaceFactoryTest.java | 72 ++----------------- 2 files changed, 4 insertions(+), 78 deletions(-) diff --git a/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/namespace/AbstractWorkspaceServiceAccount.java b/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/namespace/AbstractWorkspaceServiceAccount.java index 4093bf6b4a7..9c9b6d2cad9 100644 --- a/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/namespace/AbstractWorkspaceServiceAccount.java +++ b/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/namespace/AbstractWorkspaceServiceAccount.java @@ -47,7 +47,6 @@ public abstract class AbstractWorkspaceServiceAccount< public static final String EXEC_ROLE_NAME = "exec"; public static final String VIEW_ROLE_NAME = "workspace-view"; public static final String METRICS_ROLE_NAME = "workspace-metrics"; - public static final String SECRETS_ROLE_NAME = "workspace-secrets"; protected final String namespace; protected final String serviceAccountName; @@ -134,15 +133,6 @@ private void createImplicitRolesWithBindings(Client k8sClient) { singletonList("metrics.k8s.io"), Arrays.asList("list", "get", "watch"), serviceAccountName + "-metrics"); - - // secrets role - createRoleWithBinding( - k8sClient, - SECRETS_ROLE_NAME, - Arrays.asList("secrets"), - singletonList(""), - Arrays.asList("list", "create", "delete"), - serviceAccountName + "-secrets"); } private void createRoleWithBinding( diff --git a/infrastructures/kubernetes/src/test/java/org/eclipse/che/workspace/infrastructure/kubernetes/namespace/KubernetesNamespaceFactoryTest.java b/infrastructures/kubernetes/src/test/java/org/eclipse/che/workspace/infrastructure/kubernetes/namespace/KubernetesNamespaceFactoryTest.java index 813ba59c849..0cd0e49cef8 100644 --- a/infrastructures/kubernetes/src/test/java/org/eclipse/che/workspace/infrastructure/kubernetes/namespace/KubernetesNamespaceFactoryTest.java +++ b/infrastructures/kubernetes/src/test/java/org/eclipse/che/workspace/infrastructure/kubernetes/namespace/KubernetesNamespaceFactoryTest.java @@ -16,7 +16,6 @@ import static org.eclipse.che.api.workspace.shared.Constants.WORKSPACE_INFRASTRUCTURE_NAMESPACE_ATTRIBUTE; import static org.eclipse.che.workspace.infrastructure.kubernetes.api.shared.KubernetesNamespaceMeta.DEFAULT_ATTRIBUTE; import static org.eclipse.che.workspace.infrastructure.kubernetes.api.shared.KubernetesNamespaceMeta.PHASE_ATTRIBUTE; -import static org.eclipse.che.workspace.infrastructure.kubernetes.namespace.AbstractWorkspaceServiceAccount.SECRETS_ROLE_NAME; import static org.eclipse.che.workspace.infrastructure.kubernetes.namespace.KubernetesNamespaceFactory.NAMESPACE_TEMPLATE_ATTRIBUTE; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.anyMap; @@ -44,7 +43,6 @@ import io.fabric8.kubernetes.api.model.ServiceAccountList; import io.fabric8.kubernetes.api.model.Status; import io.fabric8.kubernetes.api.model.rbac.ClusterRoleBuilder; -import io.fabric8.kubernetes.api.model.rbac.PolicyRule; import io.fabric8.kubernetes.api.model.rbac.Role; import io.fabric8.kubernetes.api.model.rbac.RoleBindingList; import io.fabric8.kubernetes.api.model.rbac.RoleList; @@ -59,7 +57,6 @@ import java.util.HashMap; import java.util.List; import java.util.Map; -import java.util.Optional; import java.util.Set; import java.util.stream.Collectors; import java.util.stream.Stream; @@ -654,7 +651,7 @@ public void shouldBindToAllConfiguredClusterRoles() throws Exception { RoleList roles = k8sClient.rbac().roles().inNamespace("workspace123").list(); assertEquals( - Sets.newHashSet("workspace-view", "workspace-metrics", "exec", "workspace-secrets"), + Sets.newHashSet("workspace-view", "workspace-metrics", "exec"), roles.getItems().stream().map(r -> r.getMetadata().getName()).collect(Collectors.toSet())); RoleBindingList bindings = k8sClient.rbac().roleBindings().inNamespace("workspace123").list(); assertEquals( @@ -668,64 +665,7 @@ public void shouldBindToAllConfiguredClusterRoles() throws Exception { "serviceAccount-cluster0", "serviceAccount-cluster1", "serviceAccount-view", - "serviceAccount-exec", - "serviceAccount-secrets")); - } - - @Test - public void shouldCreateAndBindSecretsRole() throws Exception { - // given - namespaceFactory = - spy( - new KubernetesNamespaceFactory( - "serviceAccount", - "cr2, cr3", - "-che", - true, - true, - NAMESPACE_LABELS, - NAMESPACE_ANNOTATIONS, - clientFactory, - cheClientFactory, - userManager, - preferenceManager, - pool)); - KubernetesNamespace toReturnNamespace = mock(KubernetesNamespace.class); - when(toReturnNamespace.getWorkspaceId()).thenReturn("workspace123"); - when(toReturnNamespace.getName()).thenReturn("workspace123"); - doReturn(toReturnNamespace).when(namespaceFactory).doCreateNamespaceAccess(any(), any()); - when(clientFactory.create(any())).thenReturn(k8sClient); - - // when - RuntimeIdentity identity = - new RuntimeIdentityImpl("workspace123", null, USER_ID, "workspace123"); - namespaceFactory.getOrCreate(identity); - - // then - Optional roleOptional = - k8sClient - .rbac() - .roles() - .inNamespace("workspace123") - .list() - .getItems() - .stream() - .filter(r -> r.getMetadata().getName().equals(SECRETS_ROLE_NAME)) - .findAny(); - assertTrue(roleOptional.isPresent()); - PolicyRule rule = roleOptional.get().getRules().get(0); - assertEquals(rule.getResources(), singletonList("secrets")); - assertEquals(rule.getApiGroups(), singletonList("")); - assertEquals(rule.getVerbs(), Arrays.asList("list", "create", "delete")); - assertTrue( - k8sClient - .rbac() - .roleBindings() - .inNamespace("workspace123") - .list() - .getItems() - .stream() - .anyMatch(rb -> rb.getMetadata().getName().equals("serviceAccount-secrets"))); + "serviceAccount-exec")); } @Test @@ -766,7 +706,7 @@ public void shouldCreateExecAndViewRolesAndBindings() throws Exception { RoleList roles = k8sClient.rbac().roles().inNamespace("workspace123").list(); assertEquals( - Sets.newHashSet("workspace-view", "workspace-secrets", "workspace-metrics", "exec"), + Sets.newHashSet("workspace-view", "workspace-metrics", "exec"), roles.getItems().stream().map(r -> r.getMetadata().getName()).collect(Collectors.toSet())); Role role1 = roles.getItems().get(0); Role role2 = roles.getItems().get(1); @@ -783,11 +723,7 @@ public void shouldCreateExecAndViewRolesAndBindings() throws Exception { .stream() .map(r -> r.getMetadata().getName()) .collect(Collectors.toSet()), - Sets.newHashSet( - "serviceAccount-metrics", - "serviceAccount-view", - "serviceAccount-exec", - "serviceAccount-secrets")); + Sets.newHashSet("serviceAccount-metrics", "serviceAccount-view", "serviceAccount-exec")); } @Test