From 5cb3ab85ae51c1f5ed2e15ec2578c45f67541775 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    | 27 +++++++++++++++++--
 2 files changed, 35 insertions(+), 2 deletions(-)

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 7208a7bb41c..07c18a7ead8 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
@@ -344,6 +344,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");
@@ -351,6 +352,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 8e8e9f9ba5c..cf14e8731ea 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
@@ -18,11 +18,16 @@
  */
 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.commons.logging.Log;
 import org.apache.commons.logging.LogFactory;
+import org.apache.hadoop.conf.Configuration;
 import org.apache.hadoop.hbase.HBaseTestingUtility;
 import org.apache.hadoop.hbase.HConstants;
 import org.apache.hadoop.hbase.testclassification.LargeTests;
@@ -38,8 +43,6 @@ import org.junit.AfterClass;
 import org.junit.BeforeClass;
 import org.junit.Test;
 import org.junit.experimental.categories.Category;
-import static org.junit.Assert.assertFalse;
-import static org.junit.Assert.assertTrue;
 import org.junit.Rule;
 import org.junit.rules.ExpectedException;
 
@@ -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));