Skip to content
Permalink
Browse files Browse the repository at this point in the history
Fix the docker sock mount security vulnerability
  • Loading branch information
robinshine committed May 30, 2022
1 parent 8aa94e0 commit 0052047
Show file tree
Hide file tree
Showing 6 changed files with 103 additions and 17 deletions.
2 changes: 1 addition & 1 deletion pom.xml
Expand Up @@ -589,7 +589,7 @@
</repositories>
<properties>
<commons.version>2.1.2</commons.version>
<agent.version>1.3.7</agent.version>
<agent.version>1.3.8</agent.version>
<slf4j.version>1.7.30</slf4j.version>
<logback.version>1.2.9</logback.version>
<antlr.version>4.7.2</antlr.version>
Expand Down
Expand Up @@ -4078,4 +4078,27 @@ private void migrate85(File dataDir, Stack<Integer> versions) {
private void migrate86(File dataDir, Stack<Integer> versions) {
}

private void migrate87(File dataDir, Stack<Integer> versions) {
for (File file: dataDir.listFiles()) {
if (file.getName().startsWith("Settings.xml")) {
VersionedXmlDoc dom = VersionedXmlDoc.fromFile(file);
for (Element element: dom.getRootElement().elements()) {
String key = element.elementTextTrim("key");
if (key.equals("JOB_EXECUTORS")) {
Element valueElement = element.element("value");
if (valueElement != null) {
for (Element executorElement: valueElement.elements()) {
if (executorElement.getName().contains("DockerExecutor"))
executorElement.addElement("mountDockerSock").setText("false");
else if (executorElement.getName().contains("KubernetesExecutor"))
executorElement.addElement("mountContainerSock").setText("false");
}
}
}
}
dom.writeToFile(file, false);
}
}
}

}
Expand Up @@ -117,6 +117,8 @@ public class KubernetesExecutor extends JobExecutor implements Testable<TestData

private String kubeCtlPath;

private boolean mountContainerSock;

@Editable(order=20, description="Optionally specify node selector of the job pods")
public List<NodeSelectorEntry> getNodeSelector() {
return nodeSelector;
Expand Down Expand Up @@ -146,6 +148,21 @@ public List<RegistryLogin> getRegistryLogins() {
public void setRegistryLogins(List<RegistryLogin> registryLogins) {
this.registryLogins = registryLogins;
}

@Editable(order=300, description="Whether or not to mount docker/containerd sock into job "
+ "container to support container operations in job commands, for instance to build "
+ "container image.<br>"
+ "<b class='text-danger'>WARNING</b>: Malicious jobs can take control of k8s node "
+ "running the job by operating the mounted container sock. You should configure job "
+ "requirement option below to make sure the executor can only be used by trusted "
+ "jobs if this option is enabled")
public boolean isMountContainerSock() {
return mountContainerSock;
}

public void setMountContainerSock(boolean mountContainerSock) {
this.mountContainerSock = mountContainerSock;
}

@Editable(order=25000, group="More Settings", description="Optionally specify where to run service pods "
+ "specified in job. The first matching locator will be used. If no any locators are found, "
Expand Down Expand Up @@ -769,8 +786,11 @@ public void consume(String line) {
commonVolumeMounts.add(authInfoMount2);
if (trustCertsConfigMapName != null)
commonVolumeMounts.add(trustCertsMount);
commonVolumeMounts.add(dockerSockMount);
commonVolumeMounts.add(containerdSockMount);

if (isMountContainerSock()) {
commonVolumeMounts.add(dockerSockMount);
commonVolumeMounts.add(containerdSockMount);
}

CompositeFacade entryFacade;
if (jobContext != null) {
Expand Down Expand Up @@ -975,14 +995,17 @@ public Void visit(LeafFacade facade, List<Integer> position) {
"configMap", CollectionUtils.newLinkedHashMap(
"name", trustCertsConfigMapName)));
}
volumes.add(CollectionUtils.newLinkedHashMap(
"name", "docker-sock",
"hostPath", CollectionUtils.newLinkedHashMap(
"path", dockerSock)));
volumes.add(CollectionUtils.newLinkedHashMap(
"name", "containerd-sock",
"hostPath", CollectionUtils.newLinkedHashMap(
"path", containerdSock)));

if (isMountContainerSock()) {
volumes.add(CollectionUtils.newLinkedHashMap(
"name", "docker-sock",
"hostPath", CollectionUtils.newLinkedHashMap(
"path", dockerSock)));
volumes.add(CollectionUtils.newLinkedHashMap(
"name", "containerd-sock",
"hostPath", CollectionUtils.newLinkedHashMap(
"path", containerdSock)));
}
podSpec.put("volumes", volumes);

String podName = "job";
Expand Down
Expand Up @@ -36,6 +36,8 @@ public class RemoteDockerExecutor extends ServerDockerExecutor {

private String agentQuery;

private boolean mountDockerSock;

@Editable(order=390, name="Agent Selector", placeholder="Any agent",
description="Specify agents applicable for this executor")
@io.onedev.server.web.editable.annotation.AgentQuery(forExecutor=true)
Expand All @@ -47,6 +49,20 @@ public void setAgentQuery(String agentQuery) {
this.agentQuery = agentQuery;
}

@Editable(order=400, description="Whether or not to mount docker sock into job container to "
+ "support docker operations in job commands, for instance to build docker image.<br>"
+ "<b class='text-danger'>WARNING</b>: Malicious jobs can take control of the agent "
+ "running the job by operating the mounted docker sock. You should configure job "
+ "requirement option below to make sure the executor can only be used by trusted "
+ "jobs if this option is enabled")
public boolean isMountDockerSock() {
return mountDockerSock;
}

public void setMountDockerSock(boolean mountDockerSock) {
this.mountDockerSock = mountDockerSock;
}

@Override
public void execute(String jobToken, JobContext jobContext) {
AgentQuery parsedQeury = AgentQuery.parse(agentQuery, true);
Expand All @@ -73,8 +89,8 @@ public void runOn(Long agentId, Session agentSession, AgentData agentData) {
List<String> trustCertContent = getTrustCertContent();
DockerJobData jobData = new DockerJobData(jobToken, getName(), jobContext.getProjectPath(),
jobContext.getProjectId(), jobContext.getCommitId().name(), jobContext.getBuildNumber(),
jobContext.getActions(), jobContext.getRetried(), services, registryLogins,
trustCertContent, getRunOptions());
jobContext.getActions(), jobContext.getRetried(), services, registryLogins,
mountDockerSock, trustCertContent, getRunOptions());

try {
WebsocketUtils.call(agentSession, jobData, 0);
Expand Down
Expand Up @@ -93,6 +93,8 @@ public class ServerDockerExecutor extends JobExecutor implements Testable<TestDa

private String dockerExecutable;

private boolean mountDockerSock;

private static transient volatile String hostInstallPath;

@Editable(order=400, description="Specify login information for docker registries if necessary")
Expand All @@ -104,6 +106,20 @@ public void setRegistryLogins(List<RegistryLogin> registryLogins) {
this.registryLogins = registryLogins;
}

@Editable(order=500, description="Whether or not to mount docker sock into job container to "
+ "support docker operations in job commands, for instance to build docker image.<br>"
+ "<b class='text-danger'>WARNING</b>: Malicious jobs can take control of whole OneDev "
+ "by operating the mounted docker sock. You should configure job requirement "
+ "option below to make sure the executor can only be used by trusted jobs if this "
+ "option is enabled")
public boolean isMountDockerSock() {
return mountDockerSock;
}

public void setMountDockerSock(boolean mountDockerSock) {
this.mountDockerSock = mountDockerSock;
}

@Editable(order=50050, group="More Settings", description="Optionally specify options to run container. For instance, you may use <tt>-m 2g</tt> "
+ "to limit memory of created container to be 2 giga bytes")
public String getRunOptions() {
Expand Down Expand Up @@ -247,10 +263,12 @@ else if (workingDir != null)
docker.addArgs("-v", getHostPath(hostCachePath) + ":" + containerCachePath);
}

if (SystemUtils.IS_OS_WINDOWS)
docker.addArgs("-v", "//./pipe/docker_engine://./pipe/docker_engine");
else
docker.addArgs("-v", "/var/run/docker.sock:/var/run/docker.sock");
if (isMountDockerSock()) {
if (SystemUtils.IS_OS_WINDOWS)
docker.addArgs("-v", "//./pipe/docker_engine://./pipe/docker_engine");
else
docker.addArgs("-v", "/var/run/docker.sock:/var/run/docker.sock");
}

if (hostAuthInfoHome.get() != null) {
String hostPath = getHostPath(hostAuthInfoHome.get().getAbsolutePath());
Expand Down
6 changes: 6 additions & 0 deletions server-product/system/incompatibilities/incompatibilities.md
@@ -1,3 +1,9 @@
# 7.3.0
1. [CI/CD] Docker sock is mounted by default for server docker executor, remote docker executor and Kubernetes
executor for security reasons. If your CI job performs docker operation, the build may fail. You may enable
the mount docker sock option in related executors, but make sure to configure job requirement of the executor
to only allow trusted jobs to use the executors

# 7.0.0

1. [RESTful api] Email addresses of a user should now be retrieved via [UserResource.getEmailAddresses](/help/api/io.onedev.server.rest.UserResource/getEmailAddresses), and should be operated via [EmailAddressResource](/help/api/io.onedev.server.rest.EmailAddressResource)
Expand Down

0 comments on commit 0052047

Please sign in to comment.