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/26 10:47:16 UTC

[hive] branch master updated: HIVE-22533: Fix possible LLAP daemon web UI vulnerabilities (Adam Szita, reviewed by Marta Kuczora)

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


The following commit(s) were added to refs/heads/master by this push:
     new 209096a  HIVE-22533: Fix possible LLAP daemon web UI vulnerabilities (Adam Szita, reviewed by Marta Kuczora)
209096a is described below

commit 209096a71b25a3d75dca1930f825c8c10b99d2b9
Author: Adam Szita <sz...@cloudera.com>
AuthorDate: Tue Nov 26 11:38:06 2019 +0100

    HIVE-22533: Fix possible LLAP daemon web UI vulnerabilities (Adam Szita, reviewed by Marta Kuczora)
---
 .../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, 81 insertions(+), 11 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 cfc9091..4393a28 100644
--- a/common/src/java/org/apache/hadoop/hive/conf/HiveConf.java
+++ b/common/src/java/org/apache/hadoop/hive/conf/HiveConf.java
@@ -4517,6 +4517,10 @@ 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 b3ce8da..52253f9 100644
--- a/common/src/java/org/apache/hive/http/HttpServer.java
+++ b/common/src/java/org/apache/hive/http/HttpServer.java
@@ -169,6 +169,7 @@ 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");
@@ -304,6 +305,10 @@ public class HttpServer {
       this.xFrameOption = XFrameOption.getEnum(option);
       return this;
     }
+
+    public void setDisableDirListing(boolean disableDirListing) {
+      this.disableDirListing = disableDirListing;
+    }
   }
 
   public void start() throws Exception {
@@ -577,10 +582,14 @@ 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());
   }
 
@@ -611,7 +620,7 @@ public class HttpServer {
     webServer.setHandler(contexts);
 
 
-    if(b.usePAM){
+    if (b.usePAM) {
       setupPam(b, contexts);
     }
 
@@ -646,6 +655,7 @@ public class HttpServer {
     staticCtx.setResourceBase(appDir + "/static");
     staticCtx.addServlet(DefaultServlet.class, "/*");
     staticCtx.setDisplayName("static");
+    disableDirectoryListingOnServlet(staticCtx);
 
     String logDir = getLogDir(b.conf);
     if (logDir != null) {
@@ -749,6 +759,11 @@ 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 3c124f9..59bdf53 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,6 +83,13 @@ 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 698a56e..5df6ea8 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,6 +27,12 @@ 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;
@@ -45,18 +51,43 @@ public class TestLlapWebServices {
   @Test
   public void testContextRootUrlRewrite() throws Exception {
     String contextRootURL = "http://localhost:" + llapWSPort + "/";
-    String contextRootContent = getURLResponseAsString(contextRootURL);
+    String contextRootContent = getURLResponseAsString(contextRootURL, HTTP_OK);
 
     String indexHtmlUrl = "http://localhost:" + llapWSPort + "/index.html";
-    String indexHtmlContent = getURLResponseAsString(indexHtmlUrl);
+    String indexHtmlContent = getURLResponseAsString(indexHtmlUrl, HTTP_OK);
 
     Assert.assertEquals(contextRootContent, indexHtmlContent);
   }
 
-  private String getURLResponseAsString(String baseURL) throws IOException {
+  @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 {
     URL url = new URL(baseURL);
     HttpURLConnection conn = (HttpURLConnection) url.openConnection();
-    Assert.assertEquals(HttpURLConnection.HTTP_OK, conn.getResponseCode());
+    Assert.assertEquals(expectedStatus, conn.getResponseCode());
+    if (expectedStatus != HTTP_OK) {
+      return null;
+    }
     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 3047443..6c50e81 100644
--- a/service/src/test/org/apache/hive/service/server/TestHS2HttpServer.java
+++ b/service/src/test/org/apache/hive/service/server/TestHS2HttpServer.java
@@ -20,6 +20,7 @@ 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;
@@ -50,6 +51,8 @@ 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;
 
@@ -110,7 +113,7 @@ public class TestHS2HttpServer {
     String baseURL = "http://localhost:" + webUIPort + "/stacks";
     URL url = new URL(baseURL);
     HttpURLConnection conn = (HttpURLConnection) url.openConnection();
-    Assert.assertEquals(HttpURLConnection.HTTP_OK, conn.getResponseCode());
+    Assert.assertEquals(HTTP_OK, conn.getResponseCode());
     BufferedReader reader =
         new BufferedReader(new InputStreamReader(conn.getInputStream()));
     boolean contents = false;
@@ -136,17 +139,27 @@ public class TestHS2HttpServer {
     assertNotNull(xContentTypeHeader);
   }
 
-  private BufferedReader getReaderForUrl(String urlString) throws Exception {
+  @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 {
     URL url = new URL(urlString);
     HttpURLConnection conn = (HttpURLConnection) url.openConnection();
-    Assert.assertEquals(HttpURLConnection.HTTP_OK, conn.getResponseCode());
+    Assert.assertEquals(expectedStatus, conn.getResponseCode());
+    if (expectedStatus != HTTP_OK) {
+      return null;
+    }
+
     BufferedReader reader =
         new BufferedReader(new InputStreamReader(conn.getInputStream()));
     return reader;
   }
 
   private String readFromUrl(String urlString) throws Exception {
-    BufferedReader reader = getReaderForUrl(urlString);
+    BufferedReader reader = getReaderForUrl(urlString, HTTP_OK);
     StringBuilder response = new StringBuilder();
     String inputLine;
 
@@ -306,7 +319,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.", HttpURLConnection.HTTP_OK, conn.getResponseCode());
+    Assert.assertEquals("Got an HTTP response code other thank OK.", HTTP_OK, conn.getResponseCode());
     StringWriter writer = new StringWriter();
     IOUtils.copy(conn.getInputStream(), writer, "UTF-8");
     return writer.toString();