From fe84833ea22c30b68022203132706ebb1e526852 Mon Sep 17 00:00:00 2001
From: Kevin Risden <krisden@apache.org>
Date: Thu, 12 Apr 2018 21:08:15 -0500
Subject: [PATCH] HBase Thrift HTTP - Shouldn't handle TRACE/OPTIONS methods

Signed-off-by: Josh Elser <elserj@apache.org>
Signed-off-by: Ted Yu <yuzhihong@gmail.com>
Signed-off-by: Sean Busbey <busbey@apache.org>

 Conflicts:
	hbase-server/src/test/java/org/apache/hadoop/hbase/http/TestHttpServer.java
	hbase-thrift/src/main/java/org/apache/hadoop/hbase/thrift/ThriftServerRunner.java
---
 .../hadoop/hbase/http/TestHttpServer.java     | 13 ++++++++++--
 .../hbase/thrift/ThriftServerRunner.java      |  2 ++
 .../hbase/thrift/TestThriftHttpServer.java    | 21 +++++++++++++++----
 3 files changed, 30 insertions(+), 6 deletions(-)

diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/http/TestHttpServer.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/http/TestHttpServer.java
index cec3fd1c9be..2cb6cb473f2 100644
--- a/hbase-server/src/test/java/org/apache/hadoop/hbase/http/TestHttpServer.java
+++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/http/TestHttpServer.java
@@ -608,8 +608,6 @@ public class TestHttpServer extends HttpServerFunctionalTest {
     myServer.stop();
   }
 
-
-
   @Test
   public void testNoCacheHeader() throws Exception {
     URL url = new URL(baseUrl, "/echo?a=b&c=d");
@@ -634,4 +632,15 @@ public class TestHttpServer extends HttpServerFunctionalTest {
         .build();
     s.stop();
   }
+
+  @Test
+  public void testHttpMethods() throws Exception {
+    // HTTP TRACE method should be disabled for security
+    // See https://www.owasp.org/index.php/Cross_Site_Tracing
+    URL url = new URL(baseUrl, "/echo?a=b");
+    HttpURLConnection conn = (HttpURLConnection) url.openConnection();
+    conn.setRequestMethod("TRACE");
+    conn.connect();
+    assertEquals(HttpURLConnection.HTTP_FORBIDDEN, conn.getResponseCode());
+  }
 }
diff --git a/hbase-thrift/src/main/java/org/apache/hadoop/hbase/thrift/ThriftServerRunner.java b/hbase-thrift/src/main/java/org/apache/hadoop/hbase/thrift/ThriftServerRunner.java
index 07c18a7ead8..8292e913b9a 100644
--- a/hbase-thrift/src/main/java/org/apache/hadoop/hbase/thrift/ThriftServerRunner.java
+++ b/hbase-thrift/src/main/java/org/apache/hadoop/hbase/thrift/ThriftServerRunner.java
@@ -100,6 +100,7 @@ import org.apache.hadoop.hbase.thrift.generated.TScan;
 import org.apache.hadoop.hbase.util.Bytes;
 import org.apache.hadoop.hbase.util.ConnectionCache;
 import org.apache.hadoop.hbase.util.DNS;
+import org.apache.hadoop.hbase.util.HttpServerUtil;
 import org.apache.hadoop.hbase.util.JvmPauseMonitor;
 import org.apache.hadoop.hbase.util.Strings;
 import org.apache.hadoop.security.SaslRpcServer.SaslGssCallbackHandler;
@@ -422,6 +423,7 @@ public class ThriftServerRunner implements Runnable {
     String httpPath = "/*";
     httpServer.setHandler(context);
     context.addServlet(new ServletHolder(thriftHttpServlet), httpPath);
+    HttpServerUtil.constrainHttpMethods(context);
 
     // set up Jetty and run the embedded server
     Connector connector = new SelectChannelConnector();
diff --git a/hbase-thrift/src/test/java/org/apache/hadoop/hbase/thrift/TestThriftHttpServer.java b/hbase-thrift/src/test/java/org/apache/hadoop/hbase/thrift/TestThriftHttpServer.java
index cf14e8731ea..ed91a29aaaa 100644
--- a/hbase-thrift/src/test/java/org/apache/hadoop/hbase/thrift/TestThriftHttpServer.java
+++ b/hbase-thrift/src/test/java/org/apache/hadoop/hbase/thrift/TestThriftHttpServer.java
@@ -22,6 +22,8 @@ import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertNull;
 import static org.junit.Assert.fail;
 
+import java.net.HttpURLConnection;
+import java.net.URL;
 import java.util.ArrayList;
 import java.util.List;
 
@@ -40,6 +42,7 @@ import org.apache.thrift.protocol.TProtocol;
 import org.apache.thrift.transport.THttpClient;
 import org.apache.thrift.transport.TTransportException;
 import org.junit.AfterClass;
+import org.junit.Assert;
 import org.junit.BeforeClass;
 import org.junit.Test;
 import org.junit.experimental.categories.Category;
@@ -166,8 +169,10 @@ public class TestThriftHttpServer {
       Thread.sleep(100);
     }
 
+    String url = "http://"+ HConstants.LOCALHOST + ":" + port;
     try {
-      talkToThriftServer(customHeaderSize);
+      checkHttpMethods(url);
+      talkToThriftServer(url, customHeaderSize);
     } catch (Exception ex) {
       clientSideException = ex;
     } finally {
@@ -184,11 +189,19 @@ public class TestThriftHttpServer {
     }
   }
 
+  private void checkHttpMethods(String url) throws Exception {
+    // HTTP TRACE method should be disabled for security
+    // See https://www.owasp.org/index.php/Cross_Site_Tracing
+    HttpURLConnection conn = (HttpURLConnection) new URL(url).openConnection();
+    conn.setRequestMethod("TRACE");
+    conn.connect();
+    Assert.assertEquals(HttpURLConnection.HTTP_FORBIDDEN, conn.getResponseCode());
+  }
+
   private static volatile boolean tableCreated = false;
 
-  private void talkToThriftServer(int customHeaderSize) throws Exception {
-    THttpClient httpClient = new THttpClient(
-        "http://"+ HConstants.LOCALHOST + ":" + port);
+  private void talkToThriftServer(String url, int customHeaderSize) throws Exception {
+    THttpClient httpClient = new THttpClient(url);
     httpClient.open();
 
     if (customHeaderSize > 0) {