You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@hive.apache.org by sz...@apache.org on 2019/11/25 14:04:13 UTC

[hive] 02/02: Revert "HIVE-22533: Fix possible LLAP daemon web UI vulnerabilities (WIP)" (unintentional commit)

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 <sz...@cloudera.com>
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();