IMPALA-12552: Fix Kerberos authentication issue that occurs

in python 3 environment when kerberos_host_fqdn option is used

In Pyhton 2, the sasl layer does not accept unicode strings,
so we have to explicitly encode the kerberos_host_fqdn string
to ascii. However, this is not the case in python 3, where
we have to omit the encode, because if we don't do this,
impala-shell wants to use the following service principal
during Kerberos auth:
my_service_name/b'my.kerberos.host.fqdn'@MY.REALM
instead of the correct one, which is:
my_service_name/my.kerberos.host.fqdn@MY.REALM
(This is because the output of the encode function
is a byte array in python 3.)

Tested with new unit tests and with a snapshot build
manually in CDP PVC DS.

Change-Id: I8b157d76824ad67faf531a529256a8afe2ab9d49
Reviewed-on: http://gerrit.cloudera.org:8080/20691
Reviewed-by: Michael Smith <michael.smith@cloudera.com>
Tested-by: Impala Public Jenkins <impala-public-jenkins@cloudera.com>
Reviewed-by: Wenzhe Zhou <wzhou@cloudera.com>

fe/src/test/java/org/apache/impala/customcluster/KerberosKdcEnvironment.java

@@ -84,11 +84,29 @@ class KerberosKdcEnvironment extends ExternalResource {
     return servicePrincipal;
   }
 
+  public String getServicePrincipal(String service, String hostname) {
+    return String.format("%s/%s@%s", service, hostname, realm);
+  }
+
   public String getServiceKeytabFilePath() throws IOException {
     return new File(kerbyServer.getWorkDir().getCanonicalPath() + "/impala.keytab")
             .getCanonicalPath();
   }
 
+  public String getKeytabFilePathWithServicePrincipal(String service, String hostname)
+          throws IOException, KrbException {
+    String servicePrincipal = getServicePrincipal(service, hostname);
+
+    String keytabFileName = String.format("%s.%s.keytab", service, hostname);
+    File keytabFile = new File(kerbyServer.getWorkDir().getCanonicalPath() +
+            "/" + keytabFileName);
+
+    kerbyServer.createPrincipal(servicePrincipal, "password");
+    kerbyServer.exportPrincipal(servicePrincipal, keytabFile);
+
+    return keytabFile.getCanonicalPath();
+  }
+
   public String getKrb5ConfigPath() throws IOException {
     return kerbyServer.getWorkDir().getCanonicalPath() + "/krb5.conf";
   }
@@ -128,6 +146,23 @@ class KerberosKdcEnvironment extends ExternalResource {
     );
   }
 
+  public Map<String, String> getKerberosAuthFlagsWithCustomServicePrincipal(
+          String service, String hostname) throws IOException, KrbException {
+    return ImmutableMap.of(
+            "principal", getServicePrincipal(service, hostname), // enables Kerberos auth
+            "keytab_file", getKeytabFilePathWithServicePrincipal(service, hostname),
+
+            // To use the internal Kerberos authentication, the impala daemons must be
+            // started with a service principal that has a valid hostname,
+            // which in this test environment can be practically only "localhost",
+            // but here we want to specify a unique hostname,
+            // so we need to disable internal Kerberos authentication.
+            // Another, more complicated option would be to put more service principals
+            // in the keytab file above and specify the "be_principal" flag accordingly.
+            "skip_internal_kerberos_auth", "true"
+    );
+  }
+
   private Map<String, String> getClusterEnv() throws IOException {
     String krb5Config = getKrb5ConfigPath();
     return ImmutableMap.of(

fe/src/test/java/org/apache/impala/customcluster/LdapKerberosImpalaShellTest.java

@@ -586,12 +586,47 @@ public class LdapKerberosImpalaShellTest extends LdapKerberosImpalaShellTestBase
             "", "");
   }
 
+  /**
+   * Tests Kerberos authentication with the Kerberos hostname override option.
+   */
+  @Test
+  public void testShellKerberosAuthWithKerberosHostnameOverride()
+          throws Exception {
+    String kerberosHostFqdn = "any.host";
+
+    Map<String, String> flags = mergeFlags(
+
+            // enable Kerberos authentication
+            kerberosKdcEnvironment.getKerberosAuthFlagsWithCustomServicePrincipal(
+                    "impala", kerberosHostFqdn)
+
+    );
+    int ret = startImpalaCluster(flagsToArgs(flags));
+    assertEquals(ret, 0);
+
+    // Cluster should pass impala-shell Kerberos auth tests:
+    // In this test, the impala daemon is running on localhost, and we connect to it
+    // with impala-shell, but the service principal used in the impala daemon does
+    // not have a "localhost" hostname, so we need to use the kerberos_host_fqdn option.
+    testShellKerberosAuthWithUser( kerberosKdcEnvironment, "user",
+            /* shouldSucceed */ true, kerberosHostFqdn);
+  }
+
   /**
    * Tests Kerberos authentication using impala-shell.
    */
   protected void testShellKerberosAuthWithUser(
           KerberosKdcEnvironment kerberosKdcEnvironment, String username,
           boolean shouldSucceed) throws Exception {
+    testShellKerberosAuthWithUser(kerberosKdcEnvironment, username, shouldSucceed, null);
+  }
+
+  /**
+   * Tests Kerberos authentication using impala-shell.
+   */
+  protected void testShellKerberosAuthWithUser(
+          KerberosKdcEnvironment kerberosKdcEnvironment, String username,
+          boolean shouldSucceed, String kerberosHostFqdn) throws Exception {
 
     List<String> protocolsToTest = Arrays.asList("beeswax", "hs2");
     if (pythonSupportsSSLContext()) {
@@ -605,12 +640,11 @@ public class LdapKerberosImpalaShellTest extends LdapKerberosImpalaShellTestBase
             kerberosKdcEnvironment.createUserPrincipalAndCredentialsCache(username);
 
     for (String protocol : protocolsToTest) {
-      String[] command = {
-              "impala-shell.sh",
-              String.format("--protocol=%s", protocol),
-              "--kerberos",
-              "--query=select logged_in_user()"
-      };
+      String[] command =
+              kerberosHostFqdn == null ?
+                      createCommandForProtocol(protocol) :
+                      createCommandForProtocolAndKerberosHostFqdn(protocol,
+                              kerberosHostFqdn);
       String expectedOut =
               shouldSucceed ? kerberosKdcEnvironment.getUserPrincipal(username) : "";
       String expectedErr =
@@ -622,6 +656,28 @@ public class LdapKerberosImpalaShellTest extends LdapKerberosImpalaShellTestBase
     }
   }
 
+  private String[] createCommandForProtocol(String protocol) {
+    String[] command = {
+            "impala-shell.sh",
+            String.format("--protocol=%s", protocol),
+            "--kerberos",
+            "--query=select logged_in_user()"
+    };
+    return command;
+  }
+
+  private String[] createCommandForProtocolAndKerberosHostFqdn(String protocol,
+                                                               String kerberosHostFqdn) {
+    String[] command = {
+            "impala-shell.sh",
+            String.format("--protocol=%s", protocol),
+            "--kerberos",
+            String.format("--kerberos_host_fqdn=%s", kerberosHostFqdn),
+            "--query=select logged_in_user()"
+    };
+    return command;
+  }
+
   private Map<String, String> getLdapSimpleBindFlags() {
     String ldapUri = String.format("ldap://localhost:%s",
             serverRule.getLdapServer().getPort());

fe/src/test/java/org/apache/impala/customcluster/RunShellCommand.java

@@ -22,7 +22,6 @@ import static org.junit.Assert.assertTrue;
 
 import java.io.BufferedReader;
 import java.io.InputStreamReader;
-import java.io.IOException;
 
 /**
  * Helper class to run a shell command.

infra/python/deps/py2-requirements.txt

@@ -32,3 +32,4 @@ flake8 == 3.9.2
     contextlib2 == 0.6.0
     pathlib2 == 2.3.7.post1
     zipp == 1.2.0
+k5test==0.9.2

infra/python/deps/py3-requirements.txt

@@ -29,3 +29,4 @@ pylint == 2.10.2
   toml == 0.10.2
   platformdirs == 2.4.1
   typing-extensions == 3.10.0.2
+k5test==0.10.3

shell/impala_client.py

@@ -46,6 +46,7 @@ from TCLIService.TCLIService import (TExecuteStatementReq, TOpenSessionReq,
     TOperationState, TFetchResultsReq, TFetchOrientation, TGetLogReq,
     TGetResultSetMetadataReq, TTypeId, TCancelOperationReq, TCloseOperationReq)
 from ImpalaHttpClient import ImpalaHttpClient
+from kerberos_util import get_kerb_host_from_kerberos_host_fqdn
 from thrift.protocol import TBinaryProtocol
 from thrift_sasl import TSaslClientTransport
 from thrift.transport.TSocket import TSocket
@@ -432,7 +433,7 @@ class ImpalaClient(object):
     elif self.use_kerberos or self.kerberos_host_fqdn:
       # Set the Kerberos service
       if self.kerberos_host_fqdn is not None:
-        kerb_host = self.kerberos_host_fqdn.split(':')[0].encode('ascii', 'ignore')
+        kerb_host = get_kerb_host_from_kerberos_host_fqdn(self.kerberos_host_fqdn)
       else:
         kerb_host = self.impalad_host
       kerb_service = "{0}@{1}".format(self.kerberos_service_name, kerb_host)
@@ -463,14 +464,13 @@ class ImpalaClient(object):
       # Systems. Only attempt to import TSSLSocket if the user wants an SSL connection.
       from TSSLSocketWithWildcardSAN import TSSLSocketWithWildcardSAN
 
-    # sasl does not accept unicode strings, explicitly encode the string into ascii.
     # The kerberos_host_fqdn option exposes the SASL client's hostname attribute to
     # the user. impala-shell checks to ensure this host matches the host in the kerberos
     # principal. So when a load balancer is configured to be used, its hostname is
     # expected by impala-shell. Setting this option to the load balancer hostname allows
     # impala-shell to connect directly to an impalad.
     if self.kerberos_host_fqdn is not None:
-      sasl_host = self.kerberos_host_fqdn.split(':')[0].encode('ascii', 'ignore')
+      sasl_host = get_kerb_host_from_kerberos_host_fqdn(self.kerberos_host_fqdn)
     else:
       sasl_host = self.impalad_host
 

shell/kerberos_util.py

@@ -0,0 +1,28 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+#
+
+import sys
+
+
+def get_kerb_host_from_kerberos_host_fqdn(kerberos_host_fqdn):
+  kerb_host = kerberos_host_fqdn.split(':')[0]
+  if sys.version_info.major < 3:
+    # sasl does not accept unicode strings, explicitly encode the string into ascii.
+    kerb_host = kerb_host.encode('ascii', 'ignore')
+  return kerb_host

shell/make_shell_tarball.sh

@@ -167,6 +167,7 @@ cp ${SHELL_HOME}/ImpalaHttpClient.py ${TARBALL_ROOT}/lib
 cp ${SHELL_HOME}/shell_exceptions.py ${TARBALL_ROOT}/lib
 cp ${SHELL_HOME}/shell_output.py ${TARBALL_ROOT}/lib
 cp ${SHELL_HOME}/cookie_util.py ${TARBALL_ROOT}/lib
+cp ${SHELL_HOME}/kerberos_util.py ${TARBALL_ROOT}/lib
 cp ${SHELL_HOME}/value_converter.py ${TARBALL_ROOT}/lib
 cp ${SHELL_HOME}/impala-shell ${TARBALL_ROOT}
 cp ${SHELL_HOME}/impala_shell.py ${TARBALL_ROOT}

shell/packaging/make_python_package.sh

@@ -60,6 +60,7 @@ assemble_package_files() {
   cp "${SHELL_HOME}/ImpalaHttpClient.py" "${MODULE_LIB_DIR}"
   cp "${SHELL_HOME}/shell_exceptions.py" "${MODULE_LIB_DIR}"
   cp "${SHELL_HOME}/cookie_util.py" "${MODULE_LIB_DIR}"
+  cp "${SHELL_HOME}/kerberos_util.py" "${MODULE_LIB_DIR}"
   cp "${SHELL_HOME}/value_converter.py" "${MODULE_LIB_DIR}"
   cp "${SHELL_HOME}/thrift_printer.py" "${MODULE_LIB_DIR}"
 

tests/shell/test_kerberos_util.py

@@ -0,0 +1,32 @@
+#!/usr/bin/env impala-python
+# -*- coding: utf-8 -*-
+#
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+from __future__ import absolute_import, division, print_function
+
+import unittest
+
+from shell.kerberos_util import get_kerb_host_from_kerberos_host_fqdn
+
+
+class TestKerberosUtil(unittest.TestCase):
+  def test_get_kerb_host_from_kerberos_host_fqdn(self):
+    assert isinstance(get_kerb_host_from_kerberos_host_fqdn("any.host:1234"), str)
+    assert get_kerb_host_from_kerberos_host_fqdn("any.host:1234") == "any.host"
+    assert get_kerb_host_from_kerberos_host_fqdn("any.host") == "any.host"

tests/shell/test_shell_commandline_kerberos_auth.py

@@ -0,0 +1,91 @@
+#!/usr/bin/env impala-python
+# -*- coding: utf-8 -*-
+#
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+from __future__ import absolute_import, division, print_function
+import pytest
+
+from tests.common.impala_test_suite import ImpalaTestSuite
+from tests.common.test_dimensions import create_client_protocol_http_transport
+from tests.shell.util import create_impala_shell_executable_dimension
+from tests.shell.util import run_impala_shell_cmd
+from k5test import K5Realm
+
+
+class TestImpalaShellKerberosAuth(ImpalaTestSuite):
+
+  @classmethod
+  def get_workload(self):
+    return 'functional-query'
+
+  @classmethod
+  def add_test_dimensions(cls):
+    """Overrides all other add_dimension methods in super classes up the entire class
+    hierarchy ensuring that each test in this class only get run once
+    on different python versions."""
+    cls.ImpalaTestMatrix.add_dimension(create_client_protocol_http_transport())
+    cls.ImpalaTestMatrix.add_dimension(create_impala_shell_executable_dimension())
+
+  @pytest.mark.execute_serially
+  def test_kerberos_host_fqdn_option(self, vector):
+    """
+    This test checks whether impala-shell uses the hostname specified in
+    the kerberos_host_fqdn option when looking for a service principal
+    for Kerberos authentication.
+
+    Note: Since the Kerberos authentication is not enabled in the python
+    test environment, the connection will fail for sure, but the Kerberos
+    log can be used to check if the correct service principal is used.
+    """
+    realm = None
+    try:
+      realm = self._create_kerberos_realm_and_user("testuser", "password")
+      env = {
+        "KRB5CCNAME": "FILE:" + realm.ccache,  # Ticket cache created by kinit
+        "KRB5_TRACE": "/dev/stderr",           # Krb log to validate the principals
+      }
+      result = run_impala_shell_cmd(vector, ['--kerberos',
+                                    '--connect_max_tries=1',
+                                    '--protocol=hs2-http',
+                                    '--kerberos_host_fqdn=any.host',
+                                    '--quiet'], env=env)
+
+      assert "testuser@KRBTEST.COM" in result.stderr, \
+        "Principal 'testuser@KRBTEST.COM' should be in the Kerberos log"
+      assert "impala/any.host@KRBTEST.COM" in result.stderr, \
+        "Principal 'impala/any.host@KRBTEST.COM' should be in the Kerberos log"
+    finally:
+      realm.stop_kdc()
+
+  def _create_kerberos_realm_and_user(self, principal, password):
+    """
+    Initializes a test Kerberos realm, creates a new user principal,
+    and runs kinit to get a Kerberos ticket.
+
+    Args:
+      principal (str): Name of the new Kerberos user principal.
+      password (str): Password of the new Kerberos user principal.
+
+    Returns:
+      realm (K5Realm): The Kerberos realm.
+    """
+    realm = K5Realm(create_host=False, get_creds=False)
+    realm.addprinc(principal, password)
+    realm.kinit(principal, password)
+    return realm