Skip to content

Commit

Permalink
[FLINK-3478] [runtime-web] Don't serve files outside of web folder
Browse files Browse the repository at this point in the history 
This closes apache#1697
  • Loading branch information
@uce @rmetzger
uce authored and rmetzger committed Feb 24, 2016
1 parent ac0135a commit eaa9050
Show file tree
Hide file tree
Showing 3 changed files with 207 additions and 12 deletions.
8 changes: 6 additions & 2 deletions flink-runtime-web/src/main/java/org/apache/flink/runtime/webmonitor/WebRuntimeMonitor.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -146,14 +146,14 @@ public WebRuntimeMonitor(

// create an empty directory in temp for the web server
String rootDirFileName = "flink-web-" + UUID.randomUUID();
webRootDir = new File(System.getProperty("java.io.tmpdir"), rootDirFileName);
webRootDir = new File(getBaseDir(), rootDirFileName);
LOG.info("Using directory {} for the web interface files", webRootDir);

final boolean webSubmitAllow = cfg.isProgramSubmitEnabled();
if (webSubmitAllow) {
// create storage for uploads
String uploadDirName = "flink-web-upload-" + UUID.randomUUID();
this.uploadDir = new File(System.getProperty("java.io.tmpdir"), uploadDirName);
this.uploadDir = new File(getBaseDir(), uploadDirName);
if (!uploadDir.mkdir() || !uploadDir.canWrite()) {
throw new IOException("Unable to create temporary directory to support jar uploads.");
}
Expand Down Expand Up @@ -424,4 +424,8 @@ private void cleanup() {
private RuntimeMonitorHandler handler(RequestHandler handler) {
return new RuntimeMonitorHandler(handler, retriever, jobManagerAddressPromise.future(), timeout);
}

File getBaseDir() {
return new File(System.getProperty("java.io.tmpdir"));
}
}
51 changes: 41 additions & 10 deletions ...-web/src/main/java/org/apache/flink/runtime/webmonitor/files/StaticFileServerHandler.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,9 @@
import java.io.IOException;
import java.io.InputStream;
import java.io.RandomAccessFile;
import java.net.URI;
import java.net.URISyntaxException;
import java.net.URL;
import java.nio.file.Files;
import java.text.ParseException;
import java.text.SimpleDateFormat;
Expand Down Expand Up @@ -126,7 +129,7 @@ public StaticFileServerHandler(
JobManagerRetriever retriever,
Future<String> localJobManagerAddressPromise,
FiniteDuration timeout,
File rootPath) {
File rootPath) throws IOException {

this(retriever, localJobManagerAddressPromise, timeout, rootPath, DEFAULT_LOGGER);
}
Expand All @@ -136,12 +139,12 @@ public StaticFileServerHandler(
Future<String> localJobManagerAddressFuture,
FiniteDuration timeout,
File rootPath,
Logger logger) {
Logger logger) throws IOException {

this.retriever = checkNotNull(retriever);
this.localJobManagerAddressFuture = checkNotNull(localJobManagerAddressFuture);
this.timeout = checkNotNull(timeout);
this.rootPath = checkNotNull(rootPath);
this.rootPath = checkNotNull(rootPath).getCanonicalFile();
this.logger = checkNotNull(logger);
}

Expand Down Expand Up @@ -196,24 +199,47 @@ public void channelRead0(ChannelHandlerContext ctx, Routed routed) throws Except
* Response when running with leading JobManager.
*/
private void respondAsLeader(ChannelHandlerContext ctx, HttpRequest request, String requestPath)
throws IOException, ParseException {
throws IOException, ParseException, URISyntaxException {

// convert to absolute path
final File file = new File(rootPath, requestPath);

if(!file.exists()) {
if (!file.exists()) {
// file does not exist. Try to load it with the classloader
ClassLoader cl = StaticFileServerHandler.class.getClassLoader();

try(InputStream resourceStream = cl.getResourceAsStream("web" + requestPath)) {
if (resourceStream == null) {
boolean success = false;
try {
if (resourceStream != null) {
URL root = cl.getResource("web");
URL requested = cl.getResource("web" + requestPath);

if (root != null && requested != null) {
URI rootURI = new URI(root.getPath()).normalize();
URI requestedURI = new URI(requested.getPath()).normalize();

// Check that we don't load anything from outside of the
// expected scope.
if (!rootURI.relativize(requestedURI).equals(requestedURI)) {
logger.debug("Loading missing file from classloader: {}", requestPath);
// ensure that directory to file exists.
file.getParentFile().mkdirs();
Files.copy(resourceStream, file.toPath());

success = true;
}
}
}
} catch (Throwable t) {
logger.error("error while responding", t);
} finally {
if (!success) {
logger.debug("Unable to load requested file {} from classloader", requestPath);
sendError(ctx, NOT_FOUND);
return;
}
}
logger.debug("Loading missing file from classloader: {}", requestPath);
// ensure that directory to file exists.
file.getParentFile().mkdirs();
Files.copy(resourceStream, file.toPath());
}
}

Expand All @@ -222,6 +248,11 @@ private void respondAsLeader(ChannelHandlerContext ctx, HttpRequest request, Str
return;
}

if (!file.getCanonicalFile().toPath().startsWith(rootPath.toPath())) {
sendError(ctx, NOT_FOUND);
return;
}

// cache validation
final String ifModifiedSince = request.headers().get(IF_MODIFIED_SINCE);
if (ifModifiedSince != null && !ifModifiedSince.isEmpty()) {
Expand Down
160 changes: 160 additions & 0 deletions ...untime-web/src/test/java/org/apache/flink/runtime/webmonitor/WebRuntimeMonitorITCase.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -50,9 +50,11 @@
import java.util.ArrayList;
import java.util.List;
import java.util.Scanner;
import java.util.UUID;
import java.util.concurrent.TimeUnit;

import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertTrue;
import static org.mockito.Mockito.mock;

Expand Down Expand Up @@ -328,6 +330,164 @@ public void testLeaderNotAvailable() throws Exception {
}
}

// ------------------------------------------------------------------------
// Tests that access outside of the web root is not allowed
// ------------------------------------------------------------------------

/**
* Files are copied from the flink-dist jar to a temporary directory and
* then served from there. Only allow to access files in this temporary
* directory.
*/
@Test
public void testNoEscape() throws Exception {
final Deadline deadline = TestTimeout.fromNow();

TestingCluster flink = null;
WebRuntimeMonitor webMonitor = null;

try {
flink = new TestingCluster(new Configuration());
flink.start(true);

ActorSystem jmActorSystem = flink.jobManagerActorSystems().get().head();
ActorRef jmActor = flink.jobManagerActors().get().head();

// Needs to match the leader address from the leader retrieval service
String jobManagerAddress = AkkaUtils.getAkkaURL(jmActorSystem, jmActor);

// Web frontend on random port
Configuration config = new Configuration();
config.setInteger(ConfigConstants.JOB_MANAGER_WEB_PORT_KEY, 0);

webMonitor = new WebRuntimeMonitor(
config,
flink.createLeaderRetrievalService(),
jmActorSystem);

webMonitor.start(jobManagerAddress);

try (HttpTestClient client = new HttpTestClient("localhost", webMonitor.getServerPort())) {
String expectedIndex = new Scanner(new File(MAIN_RESOURCES_PATH + "/index.html"))
.useDelimiter("\\A").next();

// 1) Request index.html from web server
client.sendGetRequest("index.html", deadline.timeLeft());

HttpTestClient.SimpleHttpResponse response = client.getNextResponse(deadline.timeLeft());
assertEquals(HttpResponseStatus.OK, response.getStatus());
assertEquals(response.getType(), MimeTypes.getMimeTypeForExtension("html"));
assertEquals(expectedIndex, response.getContent());

// 2) Request file outside of web root
// Create a test file in the web base dir (parent of web root)
File illegalFile = new File(webMonitor.getBaseDir(), "test-file-" + UUID.randomUUID());
illegalFile.deleteOnExit();

assertTrue("Failed to create test file", illegalFile.createNewFile());

// Request the created file from the web server
client.sendGetRequest("../" + illegalFile.getName(), deadline.timeLeft());
response = client.getNextResponse(deadline.timeLeft());
assertEquals(
"Unexpected status code " + response.getStatus() + " for file outside of web root.",
HttpResponseStatus.NOT_FOUND,
response.getStatus());

// 3) Request non-existing file
client.sendGetRequest("not-existing-resource", deadline.timeLeft());
response = client.getNextResponse(deadline.timeLeft());
assertEquals(
"Unexpected status code " + response.getStatus() + " for file outside of web root.",
HttpResponseStatus.NOT_FOUND,
response.getStatus());
}
} finally {
if (flink != null) {
flink.shutdown();
}

if (webMonitor != null) {
webMonitor.stop();
}
}
}

/**
* Files are copied from the flink-dist jar to a temporary directory and
* then served from there. Only allow to copy files from <code>flink-dist.jar:/web</code>
*/
@Test
public void testNoCopyFromJar() throws Exception {
final Deadline deadline = TestTimeout.fromNow();

TestingCluster flink = null;
WebRuntimeMonitor webMonitor = null;

try {
flink = new TestingCluster(new Configuration());
flink.start(true);

ActorSystem jmActorSystem = flink.jobManagerActorSystems().get().head();
ActorRef jmActor = flink.jobManagerActors().get().head();

// Needs to match the leader address from the leader retrieval service
String jobManagerAddress = AkkaUtils.getAkkaURL(jmActorSystem, jmActor);

// Web frontend on random port
Configuration config = new Configuration();
config.setInteger(ConfigConstants.JOB_MANAGER_WEB_PORT_KEY, 0);

webMonitor = new WebRuntimeMonitor(
config,
flink.createLeaderRetrievalService(),
jmActorSystem);

webMonitor.start(jobManagerAddress);

try (HttpTestClient client = new HttpTestClient("localhost", webMonitor.getServerPort())) {
String expectedIndex = new Scanner(new File(MAIN_RESOURCES_PATH + "/index.html"))
.useDelimiter("\\A").next();

// 1) Request index.html from web server
client.sendGetRequest("index.html", deadline.timeLeft());

HttpTestClient.SimpleHttpResponse response = client.getNextResponse(deadline.timeLeft());
assertEquals(HttpResponseStatus.OK, response.getStatus());
assertEquals(response.getType(), MimeTypes.getMimeTypeForExtension("html"));
assertEquals(expectedIndex, response.getContent());

// 2) Request file from class loader
client.sendGetRequest("../log4j-test.properties", deadline.timeLeft());

response = client.getNextResponse(deadline.timeLeft());
assertEquals(
"Returned status code " + response.getStatus() + " for file outside of web root.",
HttpResponseStatus.NOT_FOUND,
response.getStatus());

assertFalse("Did not respond with the file, but still copied it from the JAR.",
new File(webMonitor.getBaseDir(), "log4j-test.properties").exists());

// 3) Request non-existing file
client.sendGetRequest("not-existing-resource", deadline.timeLeft());
response = client.getNextResponse(deadline.timeLeft());
assertEquals(
"Unexpected status code " + response.getStatus() + " for file outside of web root.",
HttpResponseStatus.NOT_FOUND,
response.getStatus());
}
} finally {
if (flink != null) {
flink.shutdown();
}

if (webMonitor != null) {
webMonitor.stop();
}
}
}

// ------------------------------------------------------------------------

private void waitForLeaderNotification(
Expand Down

0 comments on commit eaa9050

Please sign in to comment.