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/05 16:07:22 UTC

[hive] branch master updated: HIVE-22451: Secure LLAP configurations are still deemed unsecure in Tez AM processes (Adam Szita, reviewed by Peter Vary)

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 65a5935  HIVE-22451: Secure LLAP configurations are still deemed unsecure in Tez AM processes (Adam Szita, reviewed by Peter Vary)
65a5935 is described below

commit 65a593548780351273329be8bcabf524ad43ce3c
Author: Adam Szita <sz...@cloudera.com>
AuthorDate: Mon Nov 4 09:14:32 2019 +0100

    HIVE-22451: Secure LLAP configurations are still deemed unsecure in Tez AM processes (Adam Szita, reviewed by Peter Vary)
---
 llap-client/pom.xml                                | 12 ++++++
 .../hadoop/hive/registry/impl/ZookeeperUtils.java  | 15 ++++---
 .../hive/registry/impl/TestZookeeperUtils.java     | 46 +++++++++++++++++++---
 .../hadoop/hive/ql/exec/tez/TezSessionState.java   |  8 ++++
 .../metastore/security/ZooKeeperTokenStore.java    | 10 ++++-
 5 files changed, 79 insertions(+), 12 deletions(-)

diff --git a/llap-client/pom.xml b/llap-client/pom.xml
index 6923c35..84e87ec 100644
--- a/llap-client/pom.xml
+++ b/llap-client/pom.xml
@@ -156,6 +156,18 @@
         </exclusion>
       </exclusions>
     </dependency>
+    <dependency>
+      <groupId>org.powermock</groupId>
+      <artifactId>powermock-module-junit4</artifactId>
+      <version>${powermock.version}</version>
+      <scope>test</scope>
+    </dependency>
+    <dependency>
+      <groupId>org.powermock</groupId>
+      <artifactId>powermock-api-mockito</artifactId>
+      <version>${powermock.version}</version>
+      <scope>test</scope>
+    </dependency>
   </dependencies>
   <build>
     <sourceDirectory>${basedir}/src/java</sourceDirectory>
diff --git a/llap-client/src/java/org/apache/hadoop/hive/registry/impl/ZookeeperUtils.java b/llap-client/src/java/org/apache/hadoop/hive/registry/impl/ZookeeperUtils.java
index be7657a..c3d34c4 100644
--- a/llap-client/src/java/org/apache/hadoop/hive/registry/impl/ZookeeperUtils.java
+++ b/llap-client/src/java/org/apache/hadoop/hive/registry/impl/ZookeeperUtils.java
@@ -55,14 +55,19 @@ public class ZookeeperUtils {
 
   /**
    * Check if Kerberos authentication is enabled.
+   * This is used by:
+   * - LLAP daemons
+   * - Tez AM
+   * - HS2
+   * - LLAP status service
+   * Among the these Tez AM process has the lowest security setting wrt Kerberos in UGI.
+   * Even in secure scenarios Tez AM will return false for
+   * UGI.getLoginUser().hasKerberosCredentials() as it does not log in using Kerberos.
+   * Hence UGI.isSecurityEnabled() is the tightest setting we can check against.
    */
   public static boolean isKerberosEnabled(Configuration conf) {
-    try {
-      return UserGroupInformation.getLoginUser().hasKerberosCredentials() &&
+      return UserGroupInformation.isSecurityEnabled() &&
           HiveConf.getBoolVar(conf, HiveConf.ConfVars.HIVE_ZOOKEEPER_USE_KERBEROS);
-    } catch (IOException e) {
-      return false;
-    }
   }
 
   /**
diff --git a/llap-client/src/test/org/apache/hadoop/hive/registry/impl/TestZookeeperUtils.java b/llap-client/src/test/org/apache/hadoop/hive/registry/impl/TestZookeeperUtils.java
index 46e7438..0f2cae6 100644
--- a/llap-client/src/test/org/apache/hadoop/hive/registry/impl/TestZookeeperUtils.java
+++ b/llap-client/src/test/org/apache/hadoop/hive/registry/impl/TestZookeeperUtils.java
@@ -24,11 +24,18 @@ import org.apache.hadoop.security.UserGroupInformation;
 import org.junit.Assert;
 import org.junit.Before;
 import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.mockito.BDDMockito;
 import org.mockito.Mockito;
+import org.powermock.api.mockito.PowerMockito;
+import org.powermock.core.classloader.annotations.PrepareForTest;
+import org.powermock.modules.junit4.PowerMockRunner;
 
 /**
  * ZookeeperUtils test suite.
  */
+@RunWith(PowerMockRunner.class)
+@PrepareForTest(UserGroupInformation.class)
 public class TestZookeeperUtils {
 
   private Configuration conf;
@@ -37,18 +44,26 @@ public class TestZookeeperUtils {
   @Before
   public void setup() {
     conf = new Configuration();
+    PowerMockito.mockStatic(UserGroupInformation.class);
+    BDDMockito.given(UserGroupInformation.isSecurityEnabled()).willReturn(true);
     ugi = Mockito.mock(UserGroupInformation.class);
     UserGroupInformation.setLoginUser(ugi);
   }
 
+  /**
+   * Secure scenario, invoked e.g. from within HS2 or LLAP daemon process, kinit'ed inside proc.
+   */
   @Test
-  public void testHadoopAuthKerberosAndZookeeperUseKerberos() {
+  public void testHadoopAuthKerberosFromKeytabAndZookeeperUseKerberos() {
     Mockito.when(ugi.hasKerberosCredentials()).thenReturn(true);
     Mockito.when(ugi.isFromKeytab()).thenReturn(true);
     Assert.assertTrue(HiveConf.getBoolVar(conf, HiveConf.ConfVars.HIVE_ZOOKEEPER_USE_KERBEROS));
     Assert.assertTrue(ZookeeperUtils.isKerberosEnabled(conf));
   }
 
+  /**
+   * Secure scenario, invoked e.g. from within HS2 or LLAP status process, kinit'ed in parent proc.
+   */
   @Test
   public void testHadoopAuthKerberosFromTicketAndZookeeperUseKerberos() {
     Mockito.when(ugi.hasKerberosCredentials()).thenReturn(true);
@@ -57,17 +72,36 @@ public class TestZookeeperUtils {
     Assert.assertTrue(ZookeeperUtils.isKerberosEnabled(conf));
   }
 
+  /**
+   * Secure scenario, invoked e.g. from within Tez AM process.
+   */
   @Test
-  public void testHadoopAuthKerberosAndZookeeperNoKerberos(){
-    Mockito.when(ugi.hasKerberosCredentials()).thenReturn(true);
-    conf.setBoolean(HiveConf.ConfVars.HIVE_ZOOKEEPER_USE_KERBEROS.varname, false);
-    Assert.assertFalse(ZookeeperUtils.isKerberosEnabled(conf));
+  public void testHadoopAuthKerberosNoLoginAndZookeeperUseKerberos() {
+    Mockito.when(ugi.hasKerberosCredentials()).thenReturn(false);
+    Mockito.when(ugi.isFromKeytab()).thenReturn(false);
+    Assert.assertTrue(HiveConf.getBoolVar(conf, HiveConf.ConfVars.HIVE_ZOOKEEPER_USE_KERBEROS));
+    Assert.assertTrue(ZookeeperUtils.isKerberosEnabled(conf));
   }
 
+  /**
+   * Unsecure scenario.
+   */
   @Test
-  public void testHadoopAuthSimpleAndZookeeperKerberos(){
+  public void testHadoopAuthSimpleAndZookeeperUseKerberos() {
+    BDDMockito.given(UserGroupInformation.isSecurityEnabled()).willReturn(false);
     Mockito.when(ugi.hasKerberosCredentials()).thenReturn(false);
+    Mockito.when(ugi.isFromKeytab()).thenReturn(false);
+    Assert.assertTrue(HiveConf.getBoolVar(conf, HiveConf.ConfVars.HIVE_ZOOKEEPER_USE_KERBEROS));
+    Assert.assertFalse(ZookeeperUtils.isKerberosEnabled(conf));
+  }
+
+  /**
+   * Secure scenario with hive.zookeeper.kerberos.enabled=false.
+   */
+  @Test
+  public void testHadoopAuthKerberosAndZookeeperNoKerberos(){
     conf.setBoolean(HiveConf.ConfVars.HIVE_ZOOKEEPER_USE_KERBEROS.varname, false);
     Assert.assertFalse(ZookeeperUtils.isKerberosEnabled(conf));
   }
+
 }
diff --git a/ql/src/java/org/apache/hadoop/hive/ql/exec/tez/TezSessionState.java b/ql/src/java/org/apache/hadoop/hive/ql/exec/tez/TezSessionState.java
index b4cdcb5..8becef1 100644
--- a/ql/src/java/org/apache/hadoop/hive/ql/exec/tez/TezSessionState.java
+++ b/ql/src/java/org/apache/hadoop/hive/ql/exec/tez/TezSessionState.java
@@ -392,6 +392,14 @@ public class TezSessionState {
     }
   }
 
+  /**
+   * Check if Kerberos authentication is enabled.
+   * This is used by:
+   * - HS2 (upon Tez session creation)
+   * In secure scenarios HS2 might either be logged on (by Kerberos) by itself or by a launcher
+   * script it was forked from. In the latter case UGI.getLoginUser().isFromKeytab() returns false,
+   * hence UGI.getLoginUser().hasKerberosCredentials() is a tightest setting we can check against.
+   */
   private boolean isKerberosEnabled(Configuration conf) {
     try {
       return UserGroupInformation.getLoginUser().hasKerberosCredentials() &&
diff --git a/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/security/ZooKeeperTokenStore.java b/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/security/ZooKeeperTokenStore.java
index 785fa02..da28fed 100644
--- a/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/security/ZooKeeperTokenStore.java
+++ b/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/security/ZooKeeperTokenStore.java
@@ -93,9 +93,17 @@ public class ZooKeeperTokenStore implements DelegationTokenStore {
     return nodeAcls;
   }
 
+  /**
+   * Check if Kerberos authentication is enabled.
+   * This is used by:
+   * - HMS
+   * In secure scenarios the HMS is logged in (by itself) using Kerberos keytab, hence
+   * UGI.getLoginUser().isFromKeytab() returns true.
+   * This makes checking against this method the tightest setting we can check against.
+   */
   private boolean isKerberosEnabled(Configuration conf) {
     try {
-      return UserGroupInformation.getLoginUser().hasKerberosCredentials() &&
+      return UserGroupInformation.getLoginUser().isFromKeytab() &&
           MetastoreConf.getBoolVar(conf, MetastoreConf.ConfVars.THRIFT_ZOOKEEPER_USE_KERBEROS);
     } catch (IOException e) {
       return false;