This is an automated email from the ASF dual-hosted git repository. szita pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/hive.git
commit baaf082b1f3974a5f49c0cb7187ce9d2ab038e89 Author: Adam Szita <[email protected]> AuthorDate: Mon Nov 25 15:00:38 2019 +0100 Revert "HIVE-22533: Fix possible LLAP daemon web UI vulnerabilities (WIP)" (unintentional commit) This reverts commit 1e09f07afc421b3afa3a921a870d69c8b470a356. --- .../java/org/apache/hadoop/hive/conf/HiveConf.java | 4 --- .../src/java/org/apache/hive/http/HttpServer.java | 19 ++--------- .../llap/daemon/services/impl/LlapWebServices.java | 7 ---- .../daemon/services/impl/TestLlapWebServices.java | 39 +++------------------- .../hive/service/server/TestHS2HttpServer.java | 23 +++---------- 5 files changed, 11 insertions(+), 81 deletions(-) diff --git a/common/src/java/org/apache/hadoop/hive/conf/HiveConf.java b/common/src/java/org/apache/hadoop/hive/conf/HiveConf.java index 4393a28..cfc9091 100644 --- a/common/src/java/org/apache/hadoop/hive/conf/HiveConf.java +++ b/common/src/java/org/apache/hadoop/hive/conf/HiveConf.java @@ -4517,10 +4517,6 @@ public class HiveConf extends Configuration { "llap.daemon.service.port"), LLAP_DAEMON_WEB_SSL("hive.llap.daemon.web.ssl", false, "Whether LLAP daemon web UI should use SSL.", "llap.daemon.service.ssl"), - LLAP_DAEMON_WEB_XFRAME_ENABLED("hive.llap.daemon.web.xframe.enabled", true, - "Whether to enable xframe on LLAP daemon webUI\n"), - LLAP_DAEMON_WEB_XFRAME_VALUE("hive.llap.daemon.web.xframe.value", "SAMEORIGIN", - "Configuration to allow the user to set the x_frame-options value\n"), LLAP_CLIENT_CONSISTENT_SPLITS("hive.llap.client.consistent.splits", true, "Whether to setup split locations to match nodes on which llap daemons are running, " + "instead of using the locations provided by the split itself. If there is no llap daemon " + diff --git a/common/src/java/org/apache/hive/http/HttpServer.java b/common/src/java/org/apache/hive/http/HttpServer.java index 52253f9..b3ce8da 100644 --- a/common/src/java/org/apache/hive/http/HttpServer.java +++ b/common/src/java/org/apache/hive/http/HttpServer.java @@ -169,7 +169,6 @@ public class HttpServer { private XFrameOption xFrameOption = XFrameOption.SAMEORIGIN; private final List<Pair<String, Class<? extends HttpServlet>>> servlets = new LinkedList<Pair<String, Class<? extends HttpServlet>>>(); - private boolean disableDirListing = false; public Builder(String name) { Preconditions.checkArgument(name != null && !name.isEmpty(), "Name must be specified"); @@ -305,10 +304,6 @@ public class HttpServer { this.xFrameOption = XFrameOption.getEnum(option); return this; } - - public void setDisableDirListing(boolean disableDirListing) { - this.disableDirListing = disableDirListing; - } } public void start() throws Exception { @@ -582,14 +577,10 @@ public class HttpServer { } Map<String, String> xFrameParams = setHeaders(); - if (b.xFrameEnabled) { + if(b.xFrameEnabled){ setupXframeFilter(b,xFrameParams); } - if (b.disableDirListing) { - disableDirectoryListingOnServlet(webAppContext); - } - initializeWebServer(b, threadPool.getMaxThreads()); } @@ -620,7 +611,7 @@ public class HttpServer { webServer.setHandler(contexts); - if (b.usePAM) { + if(b.usePAM){ setupPam(b, contexts); } @@ -655,7 +646,6 @@ public class HttpServer { staticCtx.setResourceBase(appDir + "/static"); staticCtx.addServlet(DefaultServlet.class, "/*"); staticCtx.setDisplayName("static"); - disableDirectoryListingOnServlet(staticCtx); String logDir = getLogDir(b.conf); if (logDir != null) { @@ -759,11 +749,6 @@ public class HttpServer { webAppContext.addServlet(holder, pathSpec); } - - private static void disableDirectoryListingOnServlet(ServletContextHandler contextHandler) { - contextHandler.setInitParameter("org.eclipse.jetty.servlet.Default.dirAllowed", "false"); - } - /** * The X-FRAME-OPTIONS header in HTTP response to mitigate clickjacking * attack. diff --git a/llap-server/src/java/org/apache/hadoop/hive/llap/daemon/services/impl/LlapWebServices.java b/llap-server/src/java/org/apache/hadoop/hive/llap/daemon/services/impl/LlapWebServices.java index 59bdf53..3c124f9 100644 --- a/llap-server/src/java/org/apache/hadoop/hive/llap/daemon/services/impl/LlapWebServices.java +++ b/llap-server/src/java/org/apache/hadoop/hive/llap/daemon/services/impl/LlapWebServices.java @@ -83,13 +83,6 @@ public class LlapWebServices extends AbstractService { HttpServer.Builder builder = new HttpServer.Builder("llap").setPort(this.port).setHost(bindAddress); builder.setConf(new HiveConf(conf, HiveConf.class)); - builder.setDisableDirListing(true); - if (conf.getBoolean(ConfVars.LLAP_DAEMON_WEB_XFRAME_ENABLED.varname, - ConfVars.LLAP_DAEMON_WEB_XFRAME_ENABLED.defaultBoolVal)) { - builder.configureXFrame(true).setXFrameOption( - conf.get(ConfVars.LLAP_DAEMON_WEB_XFRAME_VALUE.varname, - ConfVars.LLAP_DAEMON_WEB_XFRAME_VALUE.defaultStrVal)); - } if (UserGroupInformation.isSecurityEnabled()) { LOG.info("LLAP UI useSSL=" + this.useSSL + ", auto-auth/SPNEGO=" + this.useSPNEGO + ", port=" + this.port); diff --git a/llap-server/src/test/org/apache/hadoop/hive/llap/daemon/services/impl/TestLlapWebServices.java b/llap-server/src/test/org/apache/hadoop/hive/llap/daemon/services/impl/TestLlapWebServices.java index 5df6ea8..698a56e 100644 --- a/llap-server/src/test/org/apache/hadoop/hive/llap/daemon/services/impl/TestLlapWebServices.java +++ b/llap-server/src/test/org/apache/hadoop/hive/llap/daemon/services/impl/TestLlapWebServices.java @@ -27,12 +27,6 @@ import java.io.StringWriter; import java.net.HttpURLConnection; import java.net.URL; -import com.google.common.collect.ImmutableSet; - -import static java.net.HttpURLConnection.HTTP_FORBIDDEN; -import static java.net.HttpURLConnection.HTTP_OK; -import static org.junit.Assert.assertNotNull; - public class TestLlapWebServices { private static LlapWebServices llapWS = null; @@ -51,43 +45,18 @@ public class TestLlapWebServices { @Test public void testContextRootUrlRewrite() throws Exception { String contextRootURL = "http://localhost:"; + llapWSPort + "/"; - String contextRootContent = getURLResponseAsString(contextRootURL, HTTP_OK); + String contextRootContent = getURLResponseAsString(contextRootURL); String indexHtmlUrl = "http://localhost:"; + llapWSPort + "/index.html"; - String indexHtmlContent = getURLResponseAsString(indexHtmlUrl, HTTP_OK); + String indexHtmlContent = getURLResponseAsString(indexHtmlUrl); Assert.assertEquals(contextRootContent, indexHtmlContent); } - @Test - public void testDirListingDisabled() throws Exception { - for (String folder : ImmutableSet.of("images", "js", "css")) { - String url = "http://localhost:"; + llapWSPort + "/" + folder; - getURLResponseAsString(url, HTTP_FORBIDDEN); - } - } - - @Test - public void testBaseUrlResponseHeader() throws Exception{ - String baseURL = "http://localhost:"; + llapWSPort + "/"; - URL url = new URL(baseURL); - HttpURLConnection conn = (HttpURLConnection) url.openConnection(); - String xfoHeader = conn.getHeaderField("X-FRAME-OPTIONS"); - String xXSSProtectionHeader = conn.getHeaderField("X-XSS-Protection"); - String xContentTypeHeader = conn.getHeaderField("X-Content-Type-Options"); - assertNotNull(xfoHeader); - assertNotNull(xXSSProtectionHeader); - assertNotNull(xContentTypeHeader); - } - - private static String getURLResponseAsString(String baseURL, int expectedStatus) - throws IOException { + private String getURLResponseAsString(String baseURL) throws IOException { URL url = new URL(baseURL); HttpURLConnection conn = (HttpURLConnection) url.openConnection(); - Assert.assertEquals(expectedStatus, conn.getResponseCode()); - if (expectedStatus != HTTP_OK) { - return null; - } + Assert.assertEquals(HttpURLConnection.HTTP_OK, conn.getResponseCode()); StringWriter writer = new StringWriter(); IOUtils.copy(conn.getInputStream(), writer, "UTF-8"); return writer.toString(); diff --git a/service/src/test/org/apache/hive/service/server/TestHS2HttpServer.java b/service/src/test/org/apache/hive/service/server/TestHS2HttpServer.java index 6c50e81..3047443 100644 --- a/service/src/test/org/apache/hive/service/server/TestHS2HttpServer.java +++ b/service/src/test/org/apache/hive/service/server/TestHS2HttpServer.java @@ -20,7 +20,6 @@ package org.apache.hive.service.server; import com.fasterxml.jackson.databind.JsonNode; import com.fasterxml.jackson.databind.ObjectMapper; - import org.apache.commons.io.IOUtils; import org.apache.hadoop.hive.conf.HiveConf; import org.apache.hadoop.hive.conf.HiveConf.ConfVars; @@ -51,8 +50,6 @@ import java.util.ArrayList; import java.util.HashMap; import java.util.List; -import static java.net.HttpURLConnection.HTTP_FORBIDDEN; -import static java.net.HttpURLConnection.HTTP_OK; import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertNull; @@ -113,7 +110,7 @@ public class TestHS2HttpServer { String baseURL = "http://localhost:"; + webUIPort + "/stacks"; URL url = new URL(baseURL); HttpURLConnection conn = (HttpURLConnection) url.openConnection(); - Assert.assertEquals(HTTP_OK, conn.getResponseCode()); + Assert.assertEquals(HttpURLConnection.HTTP_OK, conn.getResponseCode()); BufferedReader reader = new BufferedReader(new InputStreamReader(conn.getInputStream())); boolean contents = false; @@ -139,27 +136,17 @@ public class TestHS2HttpServer { assertNotNull(xContentTypeHeader); } - @Test - public void testDirListingDisabledOnStaticServlet() throws Exception { - String url = "http://localhost:"; + webUIPort + "/static"; - getReaderForUrl(url, HTTP_FORBIDDEN); - } - - private BufferedReader getReaderForUrl(String urlString, int expectedStatus) throws Exception { + private BufferedReader getReaderForUrl(String urlString) throws Exception { URL url = new URL(urlString); HttpURLConnection conn = (HttpURLConnection) url.openConnection(); - Assert.assertEquals(expectedStatus, conn.getResponseCode()); - if (expectedStatus != HTTP_OK) { - return null; - } - + Assert.assertEquals(HttpURLConnection.HTTP_OK, conn.getResponseCode()); BufferedReader reader = new BufferedReader(new InputStreamReader(conn.getInputStream())); return reader; } private String readFromUrl(String urlString) throws Exception { - BufferedReader reader = getReaderForUrl(urlString, HTTP_OK); + BufferedReader reader = getReaderForUrl(urlString); StringBuilder response = new StringBuilder(); String inputLine; @@ -319,7 +306,7 @@ public class TestHS2HttpServer { private String getURLResponseAsString(String baseURL) throws IOException { URL url = new URL(baseURL); HttpURLConnection conn = (HttpURLConnection) url.openConnection(); - Assert.assertEquals("Got an HTTP response code other thank OK.", HTTP_OK, conn.getResponseCode()); + Assert.assertEquals("Got an HTTP response code other thank OK.", HttpURLConnection.HTTP_OK, conn.getResponseCode()); StringWriter writer = new StringWriter(); IOUtils.copy(conn.getInputStream(), writer, "UTF-8"); return writer.toString();
