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/02/17 17:45:26 UTC
[7/7] phoenix git commit: PHOENIX-3684 Handle _HOST in principal
specified in JDBC url
PHOENIX-3684 Handle _HOST in principal specified in JDBC url
When the user provides a principal with the _HOST "magic string"
in it, Phoenix was incorrectly re-logging that user in. This results
in an application that creates a new HConnection and ZK connection
for every Phoenix connection which saturates connections to ZK.
Project: http://git-wip-us.apache.org/repos/asf/phoenix/repo
Commit: http://git-wip-us.apache.org/repos/asf/phoenix/commit/46c4cdd7
Tree: http://git-wip-us.apache.org/repos/asf/phoenix/tree/46c4cdd7
Diff: http://git-wip-us.apache.org/repos/asf/phoenix/diff/46c4cdd7
Branch: refs/heads/4.9-HBase-0.98
Commit: 46c4cdd70220d72f4c3db828665131d8573769b4
Parents: aee05ae
Author: Josh Elser <el...@apache.org>
Authored: Thu Feb 16 14:47:35 2017 -0500
Committer: Josh Elser <el...@apache.org>
Committed: Fri Feb 17 12:13:27 2017 -0500
----------------------------------------------------------------------
.../phoenix/jdbc/PhoenixEmbeddedDriver.java | 15 ++++-
.../phoenix/jdbc/PhoenixEmbeddedDriverTest.java | 10 +++
.../phoenix/jdbc/SecureUserConnectionsTest.java | 66 +++++++++++++++++++-
3 files changed, 88 insertions(+), 3 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/phoenix/blob/46c4cdd7/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 272fb22..2eca3d6 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
@@ -350,7 +350,7 @@ public abstract class PhoenixEmbeddedDriver implements Driver, SQLCloseable {
try {
// Check if we need to authenticate with kerberos so that we cache the correct ConnectionInfo
UserGroupInformation currentUser = UserGroupInformation.getCurrentUser();
- if (!currentUser.hasKerberosCredentials() || !currentUser.getUserName().equals(principal)) {
+ if (!currentUser.hasKerberosCredentials() || !isSameName(currentUser.getUserName(), principal)) {
synchronized (KERBEROS_LOGIN_LOCK) {
// Double check the current user, might have changed since we checked last. Don't want
// to re-login if it's the same user.
@@ -380,6 +380,19 @@ public abstract class PhoenixEmbeddedDriver implements Driver, SQLCloseable {
return new ConnectionInfo(zookeeperQuorum, port, rootNode, principal, keytab);
}
+ // Visible for testing
+ static boolean isSameName(String currentName, String newName) throws IOException {
+ return isSameName(currentName, newName, null);
+ }
+
+ static boolean isSameName(String currentName, String newName, String hostname) throws IOException {
+ // 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);
+ }
+ return currentName.equals(newName);
+ }
+
/**
* Constructs a Configuration object to use when performing a Kerberos login.
* @param props QueryServices properties
http://git-wip-us.apache.org/repos/asf/phoenix/blob/46c4cdd7/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 98a028c..475307f 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
@@ -154,4 +154,14 @@ public class PhoenixEmbeddedDriverTest {
assertTrue(driver.acceptsURL("jdbc:phoenix:localhost:123;untest=true;foo=bar"));
DriverManager.deregisterDriver(driver);
}
+
+ @Test
+ public void testPrincipalsMatching() throws Exception {
+ assertTrue(ConnectionInfo.isSameName("user@EXAMPLE.COM", "user@EXAMPLE.COM"));
+ assertTrue(ConnectionInfo.isSameName("user/localhost@EXAMPLE.COM", "user/localhost@EXAMPLE.COM"));
+ // the user provided name might have a _HOST in it, which should be replaced by the hostname
+ 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"));
+ }
}
http://git-wip-us.apache.org/repos/asf/phoenix/blob/46c4cdd7/phoenix-core/src/test/java/org/apache/phoenix/jdbc/SecureUserConnectionsTest.java
----------------------------------------------------------------------
diff --git a/phoenix-core/src/test/java/org/apache/phoenix/jdbc/SecureUserConnectionsTest.java b/phoenix-core/src/test/java/org/apache/phoenix/jdbc/SecureUserConnectionsTest.java
index 4996f94..5a99b69 100644
--- a/phoenix-core/src/test/java/org/apache/phoenix/jdbc/SecureUserConnectionsTest.java
+++ b/phoenix-core/src/test/java/org/apache/phoenix/jdbc/SecureUserConnectionsTest.java
@@ -62,6 +62,7 @@ public class SecureUserConnectionsTest {
private static final File KEYTAB_DIR = new File(TEMP_DIR, "keytabs");
private static final File KDC_DIR = new File(TEMP_DIR, "kdc");
private static final List<File> USER_KEYTAB_FILES = new ArrayList<>();
+ private static final List<File> SERVICE_KEYTAB_FILES = new ArrayList<>();
private static final int NUM_USERS = 3;
private static final Properties EMPTY_PROPERTIES = new Properties();
private static final String BASE_URL = PhoenixRuntime.JDBC_PROTOCOL + ":localhost:2181";
@@ -90,6 +91,7 @@ public class SecureUserConnectionsTest {
+ " attempts.", started);
createUsers(NUM_USERS);
+ createServiceUsers(NUM_USERS);
final Configuration conf = new Configuration(false);
conf.set(CommonConfigurationKeys.HADOOP_SECURITY_AUTHENTICATION, "kerberos");
@@ -163,6 +165,16 @@ public class SecureUserConnectionsTest {
}
}
+ private static void createServiceUsers(int numUsers) throws Exception {
+ assertNotNull("KDC is null, was setup method called?", KDC);
+ for (int i = 1; i <= numUsers; i++) {
+ String principal = "user" + i + "/localhost";
+ File keytabFile = new File(KEYTAB_DIR, "user" + i + ".service.keytab");
+ KDC.createPrincipal(keytabFile, principal);
+ SERVICE_KEYTAB_FILES.add(keytabFile);
+ }
+ }
+
/**
* Returns the principal for a user.
*
@@ -172,6 +184,10 @@ public class SecureUserConnectionsTest {
return "user" + offset + "@" + KDC.getRealm();
}
+ private static String getServicePrincipal(int offset) {
+ return "user" + offset + "/localhost@" + KDC.getRealm();
+ }
+
/**
* Returns the keytab file for the corresponding principal with the same {@code offset}.
* Requires {@link #createUsers(int)} to have been called with a value greater than {@code offset}.
@@ -179,8 +195,16 @@ public class SecureUserConnectionsTest {
* @param offset The "number" for the principal whose keytab should be returned. One-based, not zero-based.
*/
public static File getUserKeytabFile(int offset) {
- assertTrue("Invalid offset: " + offset, (offset - 1) >= 0 && (offset - 1) < USER_KEYTAB_FILES.size());
- return USER_KEYTAB_FILES.get(offset - 1);
+ return getKeytabFile(offset, USER_KEYTAB_FILES);
+ }
+
+ public static File getServiceKeytabFile(int offset) {
+ return getKeytabFile(offset, SERVICE_KEYTAB_FILES);
+ }
+
+ private static File getKeytabFile(int offset, List<File> keytabs) {
+ assertTrue("Invalid offset: " + offset, (offset - 1) >= 0 && (offset - 1) < keytabs.size());
+ return keytabs.get(offset - 1);
}
private String joinUserAuthentication(String origUrl, String principal, File keytab) {
@@ -389,6 +413,44 @@ public class SecureUserConnectionsTest {
verifyAllConnectionsAreKerberosBased(connections);
}
+ @Test
+ public void testHostSubstitutionInUrl() throws Exception {
+ final HashSet<ConnectionInfo> connections = new HashSet<>();
+ final String princ1 = getServicePrincipal(1);
+ final File keytab1 = getServiceKeytabFile(1);
+ final String princ2 = getServicePrincipal(2);
+ final File keytab2 = getServiceKeytabFile(2);
+ final String url1 = joinUserAuthentication(BASE_URL, princ1, keytab1);
+ final String url2 = joinUserAuthentication(BASE_URL, princ2, keytab2);
+
+ // Using the same UGI should result in two equivalent ConnectionInfo objects
+ connections.add(ConnectionInfo.create(url1).normalize(ReadOnlyProps.EMPTY_PROPS, EMPTY_PROPERTIES));
+ assertEquals(1, connections.size());
+ // Sanity check
+ verifyAllConnectionsAreKerberosBased(connections);
+
+ // Logging in as the same user again should not duplicate connections
+ connections.add(ConnectionInfo.create(url1).normalize(ReadOnlyProps.EMPTY_PROPS, EMPTY_PROPERTIES));
+ assertEquals(1, connections.size());
+ // Sanity check
+ verifyAllConnectionsAreKerberosBased(connections);
+
+ // Add a second one.
+ connections.add(ConnectionInfo.create(url2).normalize(ReadOnlyProps.EMPTY_PROPS, EMPTY_PROPERTIES));
+ assertEquals(2, connections.size());
+ verifyAllConnectionsAreKerberosBased(connections);
+
+ // Again, verify this user is not duplicated
+ connections.add(ConnectionInfo.create(url2).normalize(ReadOnlyProps.EMPTY_PROPS, EMPTY_PROPERTIES));
+ assertEquals(2, connections.size());
+ verifyAllConnectionsAreKerberosBased(connections);
+
+ // Because the UGI instances are unique, so are the connections
+ connections.add(ConnectionInfo.create(url1).normalize(ReadOnlyProps.EMPTY_PROPS, EMPTY_PROPERTIES));
+ assertEquals(3, connections.size());
+ verifyAllConnectionsAreKerberosBased(connections);
+ }
+
private void verifyAllConnectionsAreKerberosBased(Collection<ConnectionInfo> connections) {
for (ConnectionInfo cnxnInfo : connections) {
assertTrue("ConnectionInfo does not have kerberos credentials: " + cnxnInfo, cnxnInfo.getUser().getUGI().hasKerberosCredentials());