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 <els...@apache.org> Authored: Fri May 26 14:59:41 2017 -0400 Committer: Josh Elser <els...@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/localh...@example.com", "user/_h...@example.com", "localhost")); assertFalse(ConnectionInfo.isSameName("user/foo...@example.com", "user/_h...@example.com", "localhost")); assertFalse(ConnectionInfo.isSameName("u...@example.com", "user/_h...@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("u...@apache.org", "user", null, "APACHE.ORG")); + assertTrue(ConnectionInfo.isSameName("user/localh...@apache.org", "user/localhost", null, "APACHE.ORG")); + assertFalse(ConnectionInfo.isSameName("u...@apache.net", "user", null, "APACHE.ORG")); + assertFalse(ConnectionInfo.isSameName("user/localh...@apache.net", "user/localhost", null, "APACHE.ORG")); + assertTrue(ConnectionInfo.isSameName("u...@apache.org", "u...@apache.org", null, "APACHE.ORG")); + assertTrue(ConnectionInfo.isSameName("user/localh...@apache.org", "user/localh...@apache.org", null, "APACHE.ORG")); + + assertTrue(ConnectionInfo.isSameName("user/localh...@apache.org", "user/_HOST", "localhost", "APACHE.ORG")); + assertTrue(ConnectionInfo.isSameName("user/foo...@apache.org", "user/_HOST", "foobar", "APACHE.ORG")); + assertFalse(ConnectionInfo.isSameName("user/localh...@apache.net", "user/_HOST", "localhost", "APACHE.ORG")); + assertFalse(ConnectionInfo.isSameName("user/foo...@apache.net", "user/_HOST", "foobar", "APACHE.ORG")); } }
