From e61606231f6d1da39c1d5d0fd078e6fb8c9ac626 Mon Sep 17 00:00:00 2001
From: Reid Chan <reidddchan@outlook.com>
Date: Mon, 30 Oct 2017 17:25:59 +0800
Subject: [PATCH] HBASE-19118 Use SaslUtil to set Sasl.QOP in 'Thrift'

Signed-off-by: Josh Elser <elserj@apache.org>
---
 .../hbase/thrift/ThriftServerRunner.java      | 25 ++++++++++++-------
 .../hadoop/hbase/thrift2/ThriftServer.java    |  5 +---
 2 files changed, 17 insertions(+), 13 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 a6c59c0a623..599713f7936 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
@@ -23,7 +23,6 @@ import static org.apache.hadoop.hbase.util.Bytes.getBytes;
 import javax.security.auth.callback.Callback;
 import javax.security.auth.callback.UnsupportedCallbackException;
 import javax.security.sasl.AuthorizeCallback;
-import javax.security.sasl.Sasl;
 import javax.security.sasl.SaslServer;
 import java.io.IOException;
 import java.net.InetAddress;
@@ -80,6 +79,8 @@ import org.apache.hadoop.hbase.filter.Filter;
 import org.apache.hadoop.hbase.filter.ParseFilter;
 import org.apache.hadoop.hbase.filter.PrefixFilter;
 import org.apache.hadoop.hbase.filter.WhileMatchFilter;
+import org.apache.hadoop.hbase.security.SaslUtil;
+import org.apache.hadoop.hbase.security.SaslUtil.QualityOfProtection;
 import org.apache.hadoop.hbase.security.SecurityUtil;
 import org.apache.hadoop.hbase.security.UserProvider;
 import org.apache.hadoop.hbase.thrift.generated.AlreadyExists;
@@ -207,7 +208,7 @@ public class ThriftServerRunner implements Runnable {
   private final HBaseHandler hbaseHandler;
   private final UserGroupInformation realUser;
 
-  private final String qop;
+  private SaslUtil.QualityOfProtection qop;
   private String host;
 
   private final boolean securityEnabled;
@@ -334,7 +335,10 @@ public class ThriftServerRunner implements Runnable {
     this.handler = HbaseHandlerMetricsProxy.newInstance(
       hbaseHandler, metrics, conf);
     this.realUser = userProvider.getCurrent().getUGI();
-    qop = conf.get(THRIFT_QOP_KEY);
+    String strQop = conf.get(THRIFT_QOP_KEY);
+    if (strQop != null) {
+      this.qop = SaslUtil.getQop(strQop);
+    }
     doAsEnabled = conf.getBoolean(THRIFT_SUPPORT_PROXYUSER, false);
     if (doAsEnabled) {
       if (!conf.getBoolean(USE_HTTP_CONF_KEY, false)) {
@@ -342,10 +346,14 @@ public class ThriftServerRunner implements Runnable {
       }
     }
     if (qop != null) {
-      if (!qop.equals("auth") && !qop.equals("auth-int")
-          && !qop.equals("auth-conf")) {
-        throw new IOException("Invalid " + THRIFT_QOP_KEY + ": " + qop
-          + ", it must be 'auth', 'auth-int', or 'auth-conf'");
+      if (qop != QualityOfProtection.AUTHENTICATION &&
+          qop != QualityOfProtection.INTEGRITY &&
+          qop != QualityOfProtection.PRIVACY) {
+        throw new IOException(String.format("Invalide %s: It must be one of %s, %s, or %s.",
+                              THRIFT_QOP_KEY,
+                              QualityOfProtection.AUTHENTICATION.name(),
+                              QualityOfProtection.INTEGRITY.name(),
+                              QualityOfProtection.PRIVACY.name()));
       }
       if (!securityEnabled) {
         throw new IOException("Thrift server must"
@@ -524,8 +532,7 @@ public class ThriftServerRunner implements Runnable {
       // Extract the name from the principal
       String name = SecurityUtil.getUserFromPrincipal(
         conf.get("hbase.thrift.kerberos.principal"));
-      Map<String, String> saslProperties = new HashMap<>();
-      saslProperties.put(Sasl.QOP, qop);
+      Map<String, String> saslProperties = SaslUtil.initSaslProperties(qop.name());
       TSaslServerTransport.Factory saslFactory = new TSaslServerTransport.Factory();
       saslFactory.addServerDefinition("GSSAPI", name, host, saslProperties,
         new SaslGssCallbackHandler() {
diff --git a/hbase-thrift/src/main/java/org/apache/hadoop/hbase/thrift2/ThriftServer.java b/hbase-thrift/src/main/java/org/apache/hadoop/hbase/thrift2/ThriftServer.java
index 68a71ffb649..c5903707936 100644
--- a/hbase-thrift/src/main/java/org/apache/hadoop/hbase/thrift2/ThriftServer.java
+++ b/hbase-thrift/src/main/java/org/apache/hadoop/hbase/thrift2/ThriftServer.java
@@ -23,7 +23,6 @@ import java.net.InetAddress;
 import java.net.InetSocketAddress;
 import java.net.UnknownHostException;
 import java.security.PrivilegedAction;
-import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
 import java.util.concurrent.ExecutorService;
@@ -35,7 +34,6 @@ import java.util.concurrent.TimeUnit;
 import javax.security.auth.callback.Callback;
 import javax.security.auth.callback.UnsupportedCallbackException;
 import javax.security.sasl.AuthorizeCallback;
-import javax.security.sasl.Sasl;
 import javax.security.sasl.SaslServer;
 
 import org.apache.commons.cli.CommandLine;
@@ -197,8 +195,7 @@ public class ThriftServer extends Configured implements Tool {
     } else if (qop == null) {
       return new TTransportFactory();
     } else {
-      Map<String, String> saslProperties = new HashMap<>();
-      saslProperties.put(Sasl.QOP, qop.getSaslQop());
+      Map<String, String> saslProperties = SaslUtil.initSaslProperties(qop.name());
       TSaslServerTransport.Factory saslFactory = new TSaslServerTransport.Factory();
       saslFactory.addServerDefinition("GSSAPI", name, host, saslProperties,
         new SaslGssCallbackHandler() {