abstractdog commented on code in PR #536:
URL: https://github.com/apache/tez/pull/536#discussion_r4060747500
##########
tez-plugins/tez-aux-services/src/test/java/org/apache/tez/auxservices/TestShuffleHandler.java:
##########
@@ -1810,6 +1810,123 @@ public FullHttpRequest createHttpRequest() {
return new DefaultFullHttpRequest(HttpVersion.HTTP_1_1, HttpMethod.GET,
uri);
}
+ /**
+ * dag, vertex and map query parameters are concatenated into filesystem
+ * paths inside the shuffle handler. A value carrying a path-traversal
+ * component must be refused so it cannot escape the per-app shuffle
+ * directory and drive an out-of-scope read or delete.
+ */
+ @Test
+ @Timeout(value = 10000, unit = TimeUnit.MILLISECONDS)
+ public void testTraversalInDagVertexMapIsRejected() throws Exception {
+ Configuration conf = getInitialConf();
+ conf.setInt(ShuffleHandler.MAX_SHUFFLE_CONNECTIONS, 3);
+ conf.set(CommonConfigurationKeysPublic.HADOOP_SECURITY_AUTHENTICATION,
+ "simple");
+ UserGroupInformation.setConfiguration(conf);
+ conf.set(YarnConfiguration.NM_LOCAL_DIRS, TEST_DIR.getAbsolutePath());
+ ApplicationId appId = ApplicationId.newInstance(12345, 1);
+ String appAttemptId = "attempt_12345_1_m_1_0";
+ String user = "randomUser";
+ List<File> fileMap = new ArrayList<File>();
+ createShuffleHandlerFiles(TEST_DIR, user, appId.toString(), appAttemptId,
+ conf, fileMap);
+ ShuffleHandler shuffleHandler = new ShuffleHandler() {
+ private AuxiliaryLocalPathHandler pathHandler = new
TestAuxiliaryLocalPathHandler();
+ @Override
+ protected Shuffle getShuffle(Configuration conf) {
+ return new Shuffle(conf) {
+ @Override
+ protected void verifyRequest(String appid, ChannelHandlerContext ctx,
+ HttpRequest request, HttpResponse response, URL requestUri)
+ throws IOException {
+ // Traversal must be rejected regardless of authentication state.
+ }
+ };
+ }
+ @Override
+ public AuxiliaryLocalPathHandler getAuxiliaryLocalPathHandler() {
+ return pathHandler;
+ }
+ };
+ shuffleHandler.init(conf);
+ try {
+ shuffleHandler.start();
+ DataOutputBuffer outputBuffer = new DataOutputBuffer();
+ outputBuffer.reset();
+ Token<JobTokenIdentifier> jt =
+ new Token<JobTokenIdentifier>("identifier".getBytes(),
+ "password".getBytes(), new Text(user), new
Text("shuffleService"));
+ jt.write(outputBuffer);
+ shuffleHandler
+ .initializeApplication(new ApplicationInitializationContext(user,
+ appId, ByteBuffer.wrap(outputBuffer.getData(), 0,
+ outputBuffer.getLength())));
+ String base = "http://127.0.0.1:";
+ +
shuffleHandler.getConfig().get(ShuffleHandler.SHUFFLE_PORT_CONFIG_KEY);
+
+ // File that must survive every attempt below.
+ File outside = new File(TEST_DIR, "outside.txt");
+ try (FileOutputStream out = new FileOutputStream(outside)) {
+ out.write("keep me\n".getBytes());
+ }
+ assertTrue(outside.exists());
+
+ // dagAction=delete with a traversing dag value must not delete anything
+ // and must not return OK.
+ String badDag = URI.create("http:///a";).resolve(
+ "?dagAction=delete&job=job_12345_0001&dag=1/../../..").getRawQuery();
+ HttpURLConnection conn = (HttpURLConnection) URI.create(
+ base + "/mapOutput?" + badDag).toURL().openConnection();
+ conn.setRequestProperty(ShuffleHeader.HTTP_HEADER_NAME,
+ ShuffleHeader.DEFAULT_HTTP_HEADER_NAME);
+ conn.setRequestProperty(ShuffleHeader.HTTP_HEADER_VERSION,
+ ShuffleHeader.DEFAULT_HTTP_HEADER_VERSION);
+ conn.connect();
+ // The server must reject the request; exact status is a bad-request
+ // family response, never a successful delete.
+ int code = conn.getResponseCode();
+ assertTrue(code >= 400 && code < 600,
+ "Expected an error response for traversing dag, got " + code);
+ assertTrue(outside.exists(),
+ "outside.txt must not be deleted by a traversing dag delete");
+
+ // vertexAction=delete with a traversing vertex value must be rejected.
+ conn = (HttpURLConnection) URI.create(
+ base +
"/mapOutput?vertexAction=delete&job=job_12345_0001&dag=1&vertex=00/../"
+ ).toURL().openConnection();
+ conn.setRequestProperty(ShuffleHeader.HTTP_HEADER_NAME,
+ ShuffleHeader.DEFAULT_HTTP_HEADER_NAME);
+ conn.setRequestProperty(ShuffleHeader.HTTP_HEADER_VERSION,
+ ShuffleHeader.DEFAULT_HTTP_HEADER_VERSION);
+ conn.connect();
+ code = conn.getResponseCode();
+ assertTrue(code >= 400 && code < 600,
+ "Expected an error response for traversing vertex, got " + code);
+ assertTrue(outside.exists(),
+ "outside.txt must not be deleted by a traversing vertex delete");
+
+ // A shuffle read with a traversing map= value must not resolve to a
+ // path outside the app's shuffle output directory. The pathCache loader
+ // will refuse to load such an attempt id.
+ conn = (HttpURLConnection) URI.create(
+ base + "/mapOutput?job=job_12345_1&dag=1&reduce=1&map="
Review Comment:
ack
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
