You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@phoenix.apache.org by el...@apache.org on 2017/05/31 21:38:48 UTC

[2/4] phoenix git commit: PHOENIX-3891 Compare current user to JDBC url principal with optional realm

PHOENIX-3891 Compare current user to JDBC url principal with optional realm

Kerberos realms can be implied (based on configuration), but our
comparison did not take this into mind. This adds some slightly more
intelligent parsing to lessen the burden on what the user provides.


Project: http://git-wip-us.apache.org/repos/asf/phoenix/repo
Commit: http://git-wip-us.apache.org/repos/asf/phoenix/commit/8e73d299
Tree: http://git-wip-us.apache.org/repos/asf/phoenix/tree/8e73d299
Diff: http://git-wip-us.apache.org/repos/asf/phoenix/diff/8e73d299

Branch: refs/heads/4.x-HBase-1.2
Commit: 8e73d299e2441b7d23e1c30a43ed99e4429ab8ee
Parents: 518a973
Author: Josh Elser <el...@apache.org>
Authored: Fri May 26 14:59:41 2017 -0400
Committer: Josh Elser <el...@apache.org>
Committed: Wed May 31 17:06:37 2017 -0400

----------------------------------------------------------------------
 .../phoenix/jdbc/PhoenixEmbeddedDriver.java     | 46 ++++++++++++++++++--
 .../phoenix/jdbc/PhoenixEmbeddedDriverTest.java | 19 ++++++++
 2 files changed, 62 insertions(+), 3 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/phoenix/blob/8e73d299/phoenix-core/src/main/java/org/apache/phoenix/jdbc/PhoenixEmbeddedDriver.java
----------------------------------------------------------------------
diff --git a/phoenix-core/src/main/java/org/apache/phoenix/jdbc/PhoenixEmbeddedDriver.java b/phoenix-core/src/main/java/org/apache/phoenix/jdbc/PhoenixEmbeddedDriver.java
index 2a496c6..00dfb5a 100644
--- a/phoenix-core/src/main/java/org/apache/phoenix/jdbc/PhoenixEmbeddedDriver.java
+++ b/phoenix-core/src/main/java/org/apache/phoenix/jdbc/PhoenixEmbeddedDriver.java
@@ -41,6 +41,7 @@ import org.apache.hadoop.fs.Path;
 import org.apache.hadoop.hbase.HConstants;
 import org.apache.hadoop.hbase.security.User;
 import org.apache.hadoop.security.UserGroupInformation;
+import org.apache.hadoop.security.authentication.util.KerberosUtil;
 import org.apache.phoenix.coprocessor.MetaDataProtocol;
 import org.apache.phoenix.exception.SQLExceptionCode;
 import org.apache.phoenix.exception.SQLExceptionInfo;
@@ -202,6 +203,7 @@ public abstract class PhoenixEmbeddedDriver implements Driver, SQLCloseable {
         private static final org.slf4j.Logger logger = LoggerFactory.getLogger(ConnectionInfo.class);
         private static final Object KERBEROS_LOGIN_LOCK = new Object();
         private static final char WINDOWS_SEPARATOR_CHAR = '\\';
+        private static final String REALM_EQUIVALENCY_WARNING_MSG = "Provided principal does not contan a realm and the default realm cannot be determined. Ignoring realm equivalency check.";
         private static SQLException getMalFormedUrlException(String url) {
             return new SQLExceptionInfo.Builder(SQLExceptionCode.MALFORMED_CONNECTION_URL)
             .setMessage(url).build().buildException();
@@ -377,7 +379,7 @@ public abstract class PhoenixEmbeddedDriver implements Driver, SQLCloseable {
                                 // Double check the current user, might have changed since we checked last. Don't want
                                 // to re-login if it's the same user.
                                 currentUser = UserGroupInformation.getCurrentUser();
-                                if (!currentUser.hasKerberosCredentials() || !currentUser.getUserName().equals(principal)) {
+                                if (!currentUser.hasKerberosCredentials() || !isSameName(currentUser.getUserName(), principal)) {
                                     final Configuration config = getConfiguration(props, info, principal, keytab);
                                     logger.info("Trying to connect to a secure cluster as {} with keytab {}", config.get(QueryServices.HBASE_CLIENT_PRINCIPAL),
                                             config.get(QueryServices.HBASE_CLIENT_KEYTAB));
@@ -404,14 +406,52 @@ public abstract class PhoenixEmbeddedDriver implements Driver, SQLCloseable {
 
         // Visible for testing
         static boolean isSameName(String currentName, String newName) throws IOException {
-            return isSameName(currentName, newName, null);
+            return isSameName(currentName, newName, null, getDefaultKerberosRealm());
+        }
+
+        /**
+         * Computes the default kerberos realm if one is available. If one cannot be computed, null
+         * is returned.
+         *
+         * @return The default kerberos realm, or null.
+         */
+        static String getDefaultKerberosRealm() {
+            try {
+                return KerberosUtil.getDefaultRealm();
+            } catch (Exception e) {
+                if (LOG.isDebugEnabled()) {
+                    // Include the stacktrace at DEBUG
+                    LOG.debug(REALM_EQUIVALENCY_WARNING_MSG, e);
+                } else {
+                    // Limit the content at WARN
+                    LOG.warn(REALM_EQUIVALENCY_WARNING_MSG);
+                }
+            }
+            return null;
         }
 
         static boolean isSameName(String currentName, String newName, String hostname) throws IOException {
+            return isSameName(currentName, newName, hostname, getDefaultKerberosRealm());
+        }
+
+        static boolean isSameName(String currentName, String newName, String hostname, String defaultRealm) throws IOException {
+            final boolean newNameContainsRealm = newName.indexOf('@') != -1;
             // Make sure to replace "_HOST" if it exists before comparing the principals.
             if (newName.contains(org.apache.hadoop.security.SecurityUtil.HOSTNAME_PATTERN)) {
-                newName = org.apache.hadoop.security.SecurityUtil.getServerPrincipal(newName, hostname);
+                if (newNameContainsRealm) {
+                    newName = org.apache.hadoop.security.SecurityUtil.getServerPrincipal(newName, hostname);
+                } else {
+                    // If the principal ends with "/_HOST", replace "_HOST" with the hostname.
+                    if (newName.endsWith("/_HOST")) {
+                        newName = newName.substring(0, newName.length() - 5) + hostname;
+                    }
+                }
+            }
+            // The new name doesn't contain a realm and we could compute a default realm
+            if (!newNameContainsRealm && defaultRealm != null) {
+                return currentName.equals(newName + "@" + defaultRealm);
             }
+            // We expect both names to contain a realm, so we can do a simple equality check
             return currentName.equals(newName);
         }
 

http://git-wip-us.apache.org/repos/asf/phoenix/blob/8e73d299/phoenix-core/src/test/java/org/apache/phoenix/jdbc/PhoenixEmbeddedDriverTest.java
----------------------------------------------------------------------
diff --git a/phoenix-core/src/test/java/org/apache/phoenix/jdbc/PhoenixEmbeddedDriverTest.java b/phoenix-core/src/test/java/org/apache/phoenix/jdbc/PhoenixEmbeddedDriverTest.java
index 59465e8..ba64892 100644
--- a/phoenix-core/src/test/java/org/apache/phoenix/jdbc/PhoenixEmbeddedDriverTest.java
+++ b/phoenix-core/src/test/java/org/apache/phoenix/jdbc/PhoenixEmbeddedDriverTest.java
@@ -167,5 +167,24 @@ public class PhoenixEmbeddedDriverTest {
         assertTrue(ConnectionInfo.isSameName("user/localhost@EXAMPLE.COM", "user/_HOST@EXAMPLE.COM", "localhost"));
         assertFalse(ConnectionInfo.isSameName("user/foobar@EXAMPLE.COM", "user/_HOST@EXAMPLE.COM", "localhost"));
         assertFalse(ConnectionInfo.isSameName("user@EXAMPLE.COM", "user/_HOST@EXAMPLE.COM", "localhost"));
+        assertFalse(ConnectionInfo.isSameName("user@FOO", "user@BAR"));
+
+        // NB: We _should_ be able to provide our or krb5.conf for this test to use, but this doesn't
+        // seem to want to play nicely with the rest of the tests. Instead, we can just provide a default realm
+        // by hand.
+
+        // For an implied default realm, we should also match that. Users might provide a shortname
+        // whereas UGI would provide the "full" name.
+        assertTrue(ConnectionInfo.isSameName("user@APACHE.ORG", "user", null, "APACHE.ORG"));
+        assertTrue(ConnectionInfo.isSameName("user/localhost@APACHE.ORG", "user/localhost", null, "APACHE.ORG"));
+        assertFalse(ConnectionInfo.isSameName("user@APACHE.NET", "user", null, "APACHE.ORG"));
+        assertFalse(ConnectionInfo.isSameName("user/localhost@APACHE.NET", "user/localhost", null, "APACHE.ORG"));
+        assertTrue(ConnectionInfo.isSameName("user@APACHE.ORG", "user@APACHE.ORG", null, "APACHE.ORG"));
+        assertTrue(ConnectionInfo.isSameName("user/localhost@APACHE.ORG", "user/localhost@APACHE.ORG", null, "APACHE.ORG"));
+
+        assertTrue(ConnectionInfo.isSameName("user/localhost@APACHE.ORG", "user/_HOST", "localhost", "APACHE.ORG"));
+        assertTrue(ConnectionInfo.isSameName("user/foobar@APACHE.ORG", "user/_HOST", "foobar", "APACHE.ORG"));
+        assertFalse(ConnectionInfo.isSameName("user/localhost@APACHE.NET", "user/_HOST", "localhost", "APACHE.ORG"));
+        assertFalse(ConnectionInfo.isSameName("user/foobar@APACHE.NET", "user/_HOST", "foobar", "APACHE.ORG"));
     }
 }