From 9a45e0a9ded094d18bdcbbcaf4cf3944e7faf6d9 Mon Sep 17 00:00:00 2001
From: Reid Chan <reidddchan@outlook.com>
Date: Mon, 22 Jan 2018 16:18:29 +0800
Subject: [PATCH] HBASE-17513 Thrift Server 1 uses different QOP settings than
 RPC and Thrift Server 2 and can easily be misconfigured so there is no
 encryption when the operator expects it

Signed-off-by: Chia-Ping Tsai <chia7712@gmail.com>
Signed-off-by: Josh Elser <elserj@apache.org>
---
 .../hbase/thrift/ThriftServerRunner.java      | 10 ++++++++
 .../hbase/thrift/TestThriftHttpServer.java    | 23 +++++++++++++++++++
 2 files changed, 33 insertions(+)

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 583a9e9bb86..ef89f254776 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
@@ -362,6 +362,7 @@ public class ThriftServerRunner implements Runnable {
                               QualityOfProtection.INTEGRITY.name(),
                               QualityOfProtection.PRIVACY.name()));
       }
+      checkHttpSecurity(qop, conf);
       if (!securityEnabled) {
         throw new IOException("Thrift server must"
           + " run in secure mode to support authentication");
@@ -369,6 +370,15 @@ public class ThriftServerRunner implements Runnable {
     }
   }
 
+  private void checkHttpSecurity(QualityOfProtection qop, Configuration conf) {
+    if (qop == QualityOfProtection.PRIVACY &&
+        conf.getBoolean(USE_HTTP_CONF_KEY, false) &&
+        !conf.getBoolean(THRIFT_SSL_ENABLED, false)) {
+      throw new IllegalArgumentException("Thrift HTTP Server's QoP is privacy, but " +
+          THRIFT_SSL_ENABLED + " is false");
+    }
+  }
+
   /*
    * Runs the Thrift server
    */
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 d69da6c86f5..991a4cd3b21 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
@@ -19,10 +19,13 @@
 package org.apache.hadoop.hbase.thrift;
 
 import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertNull;
+import static org.junit.Assert.fail;
 
 import java.util.ArrayList;
 import java.util.List;
 
+import org.apache.hadoop.conf.Configuration;
 import org.apache.hadoop.hbase.HBaseTestingUtility;
 import org.apache.hadoop.hbase.HConstants;
 import org.apache.hadoop.hbase.testclassification.ClientTests;
@@ -83,6 +86,26 @@ public class TestThriftHttpServer {
     EnvironmentEdgeManager.reset();
   }
 
+  @Test
+  public void testExceptionThrownWhenMisConfigured() throws Exception {
+    Configuration conf = new Configuration(TEST_UTIL.getConfiguration());
+    conf.set("hbase.thrift.security.qop", "privacy");
+    conf.setBoolean("hbase.thrift.ssl.enabled", false);
+
+    ThriftServerRunner runner = null;
+    ExpectedException thrown = ExpectedException.none();
+    try {
+      thrown.expect(IllegalArgumentException.class);
+      thrown.expectMessage("Thrift HTTP Server's QoP is privacy, " +
+          "but hbase.thrift.ssl.enabled is false");
+      runner = new ThriftServerRunner(conf);
+      fail("Thrift HTTP Server starts up even with wrong security configurations.");
+    } catch (Exception e) {
+    }
+
+    assertNull(runner);
+  }
+
   private void startHttpServerThread(final String[] args) {
     LOG.info("Starting HBase Thrift server with HTTP server: " + Joiner.on(" ").join(args));