Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CHE-14398: Copy SSH keys with proper permissions #15167

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -12,13 +12,17 @@
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.Arrays.asList;
import static java.util.Collections.singletonList;
import static java.util.stream.Collectors.toMap;

import io.fabric8.kubernetes.api.model.ConfigMap;
import io.fabric8.kubernetes.api.model.ConfigMapBuilder;
import io.fabric8.kubernetes.api.model.ConfigMapVolumeSourceBuilder;
import io.fabric8.kubernetes.api.model.Container;
import io.fabric8.kubernetes.api.model.ContainerBuilder;
import io.fabric8.kubernetes.api.model.PodSecurityContext;
import io.fabric8.kubernetes.api.model.PodSpec;
import io.fabric8.kubernetes.api.model.Secret;
import io.fabric8.kubernetes.api.model.SecretBuilder;
Expand All @@ -32,6 +36,7 @@
import java.util.List;
import java.util.Map;
import javax.inject.Inject;
import javax.inject.Named;
import javax.validation.constraints.NotNull;
import org.eclipse.che.api.core.ConflictException;
import org.eclipse.che.api.core.ServerException;
Expand Down Expand Up @@ -86,10 +91,13 @@ public class VcsSshKeysProvisioner implements ConfigurationProvisioner<Kubernete
private static final Logger LOG = LoggerFactory.getLogger(VcsSshKeysProvisioner.class);

private final SshManager sshManager;
private String jobImage;

@Inject
public VcsSshKeysProvisioner(SshManager sshManager) {
public VcsSshKeysProvisioner(
SshManager sshManager, @Named("che.infra.kubernetes.pvc.jobs.image") String jobImage) {
this.sshManager = sshManager;
this.jobImage = jobImage;
}

@Override
Expand Down Expand Up @@ -121,7 +129,21 @@ public void provision(KubernetesEnvironment k8sEnv, RuntimeIdentity identity)

StringBuilder sshConfigData = new StringBuilder();
for (SshPair sshPair : sshPairs) {
sshConfigData.append(buildConfig(sshPair.getName()));
sshConfigData.append(
buildConfig(
sshPair.getName(),
k8sEnv
.getPodsData()
.values()
.stream()
.anyMatch(
p -> {
PodSecurityContext securityContext = p.getSpec().getSecurityContext();
return p.getMetadata().getName().startsWith(identity.getWorkspaceId())
&& securityContext != null
&& securityContext.getRunAsUser() == 0
&& securityContext.getFsGroup() == 0;
})));
}

String sshConfigMapName = identity.getWorkspaceId() + SSH_CONFIG_MAP_NAME_SUFFIX;
Expand Down Expand Up @@ -171,17 +193,64 @@ private void mountSshKeySecret(String secretName, PodSpec podSpec) {
.build())
.build());
List<Container> containers = podSpec.getContainers();
containers.forEach(
container -> {
VolumeMount volumeMount =
new VolumeMountBuilder()
.withName(secretName)
.withNewReadOnly(true)
.withReadOnly(true)
.withMountPath(SSH_PRIVATE_KEYS_PATH)
.build();
container.getVolumeMounts().add(volumeMount);
});
VolumeMount sshKeysVolumeMount =
new VolumeMountBuilder()
.withName(secretName)
.withNewReadOnly(true)
.withReadOnly(true)
.withMountPath(SSH_PRIVATE_KEYS_PATH)
.build();
containers.forEach(container -> container.getVolumeMounts().add(sshKeysVolumeMount));

PodSecurityContext securityContext = podSpec.getSecurityContext();

/* The OpenSSH daemon requires 600 file mode access permissions for private keys.
Mounted keys have 640 permissions despite setting 'DefaultMode' to 600.
As a workaround copy the SSH keys and manually set them needed permissions.
The copy operation can be applied, if the primary group ID of the containers will be root (0).
TODO rework this when https://github.com/kubernetes/kubernetes/issues/81089 will be resolved */
if (securityContext != null
&& securityContext.getFsGroup() == 0
&& securityContext.getRunAsUser() == 0) {
copyPrivateKeysWithPermissions(podSpec, sshKeysVolumeMount);
}
}

// TODO remove this when https://github.com/kubernetes/kubernetes/issues/81089 will be resolved
private void copyPrivateKeysWithPermissions(PodSpec podSpec, VolumeMount sshKeysVolumeMount) {
String copiedSshKeysVolumeName = "copied-private-keys";
podSpec
.getVolumes()
.add(
new VolumeBuilder()
.withName(copiedSshKeysVolumeName)
.withNewEmptyDir()
.endEmptyDir()
.build());

VolumeMount copiedSshKeysVolumeMount =
new VolumeMountBuilder()
.withName(copiedSshKeysVolumeName)
.withMountPath(SSH_PRIVATE_KEYS_PATH + "-copy")
.build();

Container initContainer =
new ContainerBuilder()
.withName("ssh-private-keys-modifier")
.withImage(jobImage)
.withVolumeMounts(sshKeysVolumeMount, copiedSshKeysVolumeMount)
.withCommand(
asList(
"/bin/sh",
"-c",
format(
"(cp -r %1$s/. %1$s-copy && chmod -R 600 %1$s-copy) || exit 0",
SSH_PRIVATE_KEYS_PATH)))
.build();
podSpec
.getContainers()
.forEach(container -> container.getVolumeMounts().add(copiedSshKeysVolumeMount));
podSpec.getInitContainers().add(initContainer);
}

private void doProvisionSshConfig(
Expand Down Expand Up @@ -253,15 +322,20 @@ private void mountConfigFile(PodSpec podSpec, String sshConfigMapName) {
* provide own name. Details see here:
* https://github.com/eclipse/che/issues/13494#issuecomment-512761661. Note: behavior can be
* improved in 7.x releases after 7.0.0
* @param useCopiedKeys use the copied keys with the proper permissions, see the description in
* the {@link #mountSshKeySecret}
* @return the ssh configuration which include host, identity file location and Host Key checking
* policy
*/
private String buildConfig(@NotNull String name) {
private String buildConfig(@NotNull String name, boolean useCopiedKeys) {
String host = name.startsWith("default-") ? "*" : name;
return "host "
+ host
+ "\nIdentityFile "
+ SSH_PRIVATE_KEYS_PATH
// TODO remove this when https://github.com/kubernetes/kubernetes/issues/81089 will be
// resolved
+ (useCopiedKeys ? "-copy" : "")
+ "/"
+ name
+ "\nStrictHostKeyChecking = no"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
import io.fabric8.kubernetes.api.model.ObjectMeta;
import io.fabric8.kubernetes.api.model.ObjectMetaBuilder;
import io.fabric8.kubernetes.api.model.Pod;
import io.fabric8.kubernetes.api.model.PodSecurityContext;
import io.fabric8.kubernetes.api.model.PodSpec;
import io.fabric8.kubernetes.api.model.Secret;
import java.util.ArrayList;
Expand Down Expand Up @@ -61,10 +62,14 @@ public class VcsSshKeySecretProvisionerTest {

@Mock private PodSpec podSpec;

@Mock private PodSecurityContext podSecurityContext;

@Mock private Container container;

private final String someUser = "someuser";

private final String jobImage = "centos:centos7";

private VcsSshKeysProvisioner vcsSshKeysProvisioner;

@BeforeMethod
Expand All @@ -77,9 +82,12 @@ public void setup() {
when(pod.getSpec()).thenReturn(podSpec);
when(podSpec.getVolumes()).thenReturn(new ArrayList<>());
when(podSpec.getContainers()).thenReturn(Collections.singletonList(container));
when(podSecurityContext.getFsGroup()).thenReturn((long) 999);
when(podSecurityContext.getRunAsUser()).thenReturn((long) 999);
when(podSpec.getSecurityContext()).thenReturn(podSecurityContext);
when(container.getVolumeMounts()).thenReturn(new ArrayList<>());
k8sEnv.addPod(pod);
vcsSshKeysProvisioner = new VcsSshKeysProvisioner(sshManager);
vcsSshKeysProvisioner = new VcsSshKeysProvisioner(sshManager, jobImage);
}

@Test
Expand Down