From 5f3c188c59a5689bf28381a8107c08fe5a33e773 Mon Sep 17 00:00:00 2001 From: Sergii Kabashniuk Date: Mon, 15 Feb 2021 11:54:19 +0200 Subject: [PATCH] Do not mount ssh keys as secrets if the name is not a valid Kubernetes config map key name and not a valid host name (#19031) * Do not mount ssh keys as secrets if the name is not a valid Kubernetes config map key name and not a valid hostname Signed-off-by: Sergii Kabashniuk --- .../infrastructure/kubernetes/Warnings.java | 4 + .../namespace/KubernetesObjectUtil.java | 9 +++ .../provision/SshKeysProvisioner.java | 52 ++++++++++-- .../namespace/KubernetesObjectUtilTest.java | 37 +++++++++ .../SshKeySecretProvisionerTest.java | 79 +++++++++++++++++-- 5 files changed, 168 insertions(+), 13 deletions(-) create mode 100644 infrastructures/kubernetes/src/test/java/org/eclipse/che/workspace/infrastructure/kubernetes/namespace/KubernetesObjectUtilTest.java diff --git a/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/Warnings.java b/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/Warnings.java index eb7557989a..33239733a5 100644 --- a/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/Warnings.java +++ b/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/Warnings.java @@ -68,5 +68,9 @@ public final class Warnings { NOT_ABLE_TO_PROVISION_WORKSPACE_DEPLOYMENT_LABELS_OR_ANNOTATIONS_MESSAGE = "Not able to provision workspace %s deployment labels or annotations because of invalid configuration. Reason: '%s'"; + public static final int SSH_KEYS_WILL_NOT_BE_MOUNTED = 4300; + public static final String SSH_KEYS_WILL_NOT_BE_MOUNTED_MESSAGE = + "Ssh keys %s have invalid names and can't be mounted to workspace %s."; + private Warnings() {} } diff --git a/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/namespace/KubernetesObjectUtil.java b/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/namespace/KubernetesObjectUtil.java index fb72a140f8..ed103a0119 100644 --- a/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/namespace/KubernetesObjectUtil.java +++ b/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/namespace/KubernetesObjectUtil.java @@ -36,6 +36,7 @@ import io.fabric8.kubernetes.api.model.apps.Deployment; import io.fabric8.kubernetes.api.model.apps.DeploymentSpec; import java.util.HashMap; import java.util.Map; +import java.util.regex.Pattern; import org.eclipse.che.workspace.infrastructure.kubernetes.Annotations; /** @@ -46,6 +47,9 @@ import org.eclipse.che.workspace.infrastructure.kubernetes.Annotations; public class KubernetesObjectUtil { private static final String STORAGE_PARAM = "storage"; + private static final String VALID_CONFIG_MAP_KEY_NAME = "[-._a-zA-Z0-9]+"; + private static final Pattern VALID_CONFIG_MAP_KEY_NAME_PATTERN = + Pattern.compile(VALID_CONFIG_MAP_KEY_NAME); /** Checks if the specified object has the specified label. */ public static boolean isLabeled(HasMetadata source, String key, String value) { @@ -253,4 +257,9 @@ public class KubernetesObjectUtil { .getOrDefault(CREATE_IN_CHE_INSTALLATION_NAMESPACE, FALSE.toString()) .equals(TRUE.toString()); } + + /** Checks the provided name is a valid kubernetes config map key name */ + public static boolean isValidConfigMapKeyName(String configMapKeyName) { + return VALID_CONFIG_MAP_KEY_NAME_PATTERN.matcher(configMapKeyName).matches(); + } } diff --git a/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/provision/SshKeysProvisioner.java b/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/provision/SshKeysProvisioner.java index 452ba62c45..28ccc4f615 100644 --- a/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/provision/SshKeysProvisioner.java +++ b/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/provision/SshKeysProvisioner.java @@ -14,10 +14,13 @@ package org.eclipse.che.workspace.infrastructure.kubernetes.provision; import static com.google.common.base.Strings.isNullOrEmpty; import static java.lang.String.format; import static java.util.Collections.singletonList; +import static java.util.stream.Collectors.toList; import static java.util.stream.Collectors.toMap; import static org.eclipse.che.workspace.infrastructure.kubernetes.Warnings.NOT_ABLE_TO_PROVISION_SSH_KEYS; import static org.eclipse.che.workspace.infrastructure.kubernetes.Warnings.NOT_ABLE_TO_PROVISION_SSH_KEYS_MESSAGE; +import static org.eclipse.che.workspace.infrastructure.kubernetes.namespace.KubernetesObjectUtil.isValidConfigMapKeyName; +import com.google.common.annotations.VisibleForTesting; import io.fabric8.kubernetes.api.model.ConfigMap; import io.fabric8.kubernetes.api.model.ConfigMapBuilder; import io.fabric8.kubernetes.api.model.ConfigMapVolumeSourceBuilder; @@ -29,12 +32,14 @@ import io.fabric8.kubernetes.api.model.SecretVolumeSourceBuilder; import io.fabric8.kubernetes.api.model.VolumeBuilder; import io.fabric8.kubernetes.api.model.VolumeMount; import io.fabric8.kubernetes.api.model.VolumeMountBuilder; +import java.time.ZonedDateTime; import java.util.ArrayList; import java.util.Base64; import java.util.Date; import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.regex.Pattern; import javax.inject.Inject; import javax.validation.constraints.NotNull; import org.eclipse.che.api.core.ConflictException; @@ -47,8 +52,10 @@ import org.eclipse.che.api.workspace.server.model.impl.WarningImpl; import org.eclipse.che.api.workspace.server.spi.InfrastructureException; import org.eclipse.che.commons.annotation.Traced; import org.eclipse.che.commons.tracing.TracingTags; +import org.eclipse.che.workspace.infrastructure.kubernetes.Warnings; import org.eclipse.che.workspace.infrastructure.kubernetes.environment.KubernetesEnvironment; import org.eclipse.che.workspace.infrastructure.kubernetes.environment.KubernetesEnvironment.PodRole; +import org.eclipse.che.workspace.infrastructure.kubernetes.util.RuntimeEventsPublisher; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -89,13 +96,18 @@ public class SshKeysProvisioner implements ConfigurationProvisioner vcsSshPairs = getVcsSshPairs(k8sEnv, identity); List systemSshPairs = getSystemSshPairs(k8sEnv, identity); - List all = new ArrayList<>(vcsSshPairs); - all.addAll(systemSshPairs); + List allSshPairs = new ArrayList<>(vcsSshPairs); + allSshPairs.addAll(systemSshPairs); - doProvisionSshKeys(all, k8sEnv, workspaceId); - doProvisionVcsSshConfig(vcsSshPairs, k8sEnv, workspaceId); + List invalidSshKeyNames = + allSshPairs + .stream() + .filter(keyPair -> !isValidSshKeyPair(keyPair)) + .map(SshPairImpl::getName) + .collect(toList()); + + if (!invalidSshKeyNames.isEmpty()) { + String message = + format( + Warnings.SSH_KEYS_WILL_NOT_BE_MOUNTED_MESSAGE, + invalidSshKeyNames.toString(), + identity.getWorkspaceId()); + LOG.warn(message); + k8sEnv.addWarning(new WarningImpl(Warnings.SSH_KEYS_WILL_NOT_BE_MOUNTED, message)); + runtimeEventsPublisher.sendRuntimeLogEvent(message, ZonedDateTime.now().toString(), identity); + } + + doProvisionSshKeys( + allSshPairs.stream().filter(this::isValidSshKeyPair).collect(toList()), + k8sEnv, + workspaceId); + doProvisionVcsSshConfig( + vcsSshPairs.stream().filter(this::isValidSshKeyPair).collect(toList()), + k8sEnv, + workspaceId); } /** @@ -298,6 +334,12 @@ public class SshKeysProvisioner implements ConfigurationProvisioner()); - when(podSpec.getContainers()).thenReturn(Collections.singletonList(container)); - when(container.getVolumeMounts()).thenReturn(new ArrayList<>()); + lenient().when(pod.getMetadata()).thenReturn(podMeta); + lenient().when(pod.getSpec()).thenReturn(podSpec); + lenient().when(podSpec.getVolumes()).thenReturn(new ArrayList<>()); + lenient().when(podSpec.getContainers()).thenReturn(Collections.singletonList(container)); + lenient().when(container.getVolumeMounts()).thenReturn(new ArrayList<>()); k8sEnv.addPod(pod); - sshKeysProvisioner = new SshKeysProvisioner(sshManager); + sshKeysProvisioner = new SshKeysProvisioner(sshManager, runtimeEventsPublisher); } @Test @@ -98,6 +103,64 @@ public class SshKeySecretProvisionerTest { assertEquals(k8sEnv.getSecrets().size(), 1); } + @Test(dataProvider = "invalidNames") + public void shouldNotMountKeysWithInvalidKeyNames(String invalidName) throws Exception { + // given + when(sshManager.getPairs(someUser, "vcs")) + .thenReturn( + ImmutableList.of( + new SshPairImpl(someUser, "vcs", invalidName, "public", "private"), + new SshPairImpl(someUser, "vcs", "gothub.mk", "public", "private"), + new SshPairImpl(someUser, "vcs", "boombaket.barabaket.com", "public", "private"))); + // when + sshKeysProvisioner.provision(k8sEnv, runtimeIdentity); + // then + Secret secret = k8sEnv.getSecrets().get("wksp-sshprivatekeys"); + assertNotNull(secret); + assertEquals(secret.getData().size(), 2); + assertTrue(secret.getData().containsKey("gothub.mk")); + assertTrue(secret.getData().containsKey("boombaket.barabaket.com")); + assertFalse(secret.getData().containsKey(invalidName)); + verify(runtimeEventsPublisher, times(1)) + .sendRuntimeLogEvent(anyString(), anyString(), eq(runtimeIdentity)); + } + + @Test(dataProvider = "invalidNames") + public void shouldIgnoreInvalidKeyNames(String invalidName) throws Exception { + assertFalse( + sshKeysProvisioner.isValidSshKeyPair( + new SshPairImpl(someUser, "vcs", invalidName, "public", "private"))); + } + + @Test(dataProvider = "validNames") + public void shouldAcceptValidKeyNames(String validName) throws Exception { + assertTrue( + sshKeysProvisioner.isValidSshKeyPair( + new SshPairImpl(someUser, "vcs", validName, "public", "private"))); + } + + @DataProvider + public static Object[][] invalidNames() { + return new Object[][] { + new Object[] {"https://foo.bar"}, + new Object[] {"_fef_123-ah_*zz**"}, + new Object[] {"_fef_123-ah_z.z"}, + new Object[] {"a-b#-hello"}, + new Object[] {"--ab--"} + }; + } + + @DataProvider + public static Object[][] validNames() { + return new Object[][] { + new Object[] {"foo.bar"}, + new Object[] {"fef-123-ahzz"}, + new Object[] {"fef0123-ahz.z"}, + new Object[] {"a-b.hello"}, + new Object[] {"a--a----------b"} + }; + } + @Test public void addSshKeysConfigInPod() throws Exception { String keyName1 = UUID.randomUUID().toString();